feat: align the storage performance options with the Rancher machine pool form (#1212)

* feat: align the storage performance options with the Rancher machine pool form

Show the storage performance profile on every volume instead of behind a
collapsible toggle, and move the VM-wide options to a plain Storage
Performance section in the Advanced Options tab. Put the Native I/O tip in
a live region so screen readers announce the cache mode change, use an
integer input for the I/O thread count, and use the same copy as the
Rancher form.

Skip the view-only backup row in the image and container volumes when it
has nothing to show, so it no longer adds an empty gap above the profile.

Signed-off-by: Marcelo Fukumoto <marcelo.fukumoto@suse.com>

* fix: announce the storage performance warnings and name the virtio requirement

- Put the bus tip and the Filesystem warning in an always-rendered status
  region. They appear above the field being changed, so screen readers need
  them announced.
- Say in the bus tip that a dedicated I/O thread needs the virtio bus, because
  KubeVirt rejects it on any other bus.
- Mark I/O Thread Count as required while editing, like the Rancher form.

Signed-off-by: Marcelo Fukumoto <marcelo.fukumoto@suse.com>

* fix: state only the dedicated I/O thread requirement in the bus tip

Neither the Harvester docs nor KubeVirt tie cache mode or I/O mode to the bus,
so stop saying that those options work best on virtio.

Signed-off-by: Marcelo Fukumoto <marcelo.fukumoto@suse.com>

---------

Signed-off-by: Marcelo Fukumoto <marcelo.fukumoto@suse.com>

pkg/harvester/edit/kubevirt.io.virtualmachine/VirtualMachineVolume/__tests__/DiskPerformanceOptions.test.ts

@@ -1,3 +1,6 @@
+import fs from 'fs';
+import path from 'path';
+import { load } from 'js-yaml';
 import { mount } from '@vue/test-utils';
 import { nextTick } from 'vue';
 import { _EDIT } from '@shell/config/query-params';
@@ -24,6 +27,12 @@ const hardDisk = (overrides = {}) => ({
   type: 'disk', bus: 'virtio', cache: '', io: '', dedicatedIOThread: false, ...overrides
 });
 
+const nativeTip = (wrapper) => wrapper.findAllComponents(Banner)
+  .find((b) => b.props('labelKey') === 'harvester.virtualMachine.volume.performance.ioMode.nativeRequiresNoCacheTip');
+
+const filesystemWarning = (wrapper) => wrapper.findAllComponents(Banner)
+  .find((b) => b.props('labelKey') === 'harvester.virtualMachine.volume.performance.cacheMode.filesystemWarning');
+
 describe('component: DiskPerformanceOptions', () => {
   it('is hidden when the highPerformanceStorage feature is disabled', () => {
     const wrapper = mount(DiskPerformanceOptions, mountOptions(hardDisk(), { featureEnabled: false }));
@@ -37,6 +46,102 @@ describe('component: DiskPerformanceOptions', () => {
     expect(wrapper.find('.disk-performance').exists()).toBe(true);
   });
 
+  it('shows the profile without a toggle, and the individual fields only for Custom', () => {
+    const wrapper = mount(DiskPerformanceOptions, mountOptions(hardDisk()));
+
+    expect(wrapper.find('.expand-toggle').exists()).toBe(false);
+    expect(wrapper.find('[data-testid="input-disk-perf-profile"]').exists()).toBe(true);
+    expect(wrapper.find('[data-testid="input-disk-cache"]').exists()).toBe(false);
+    expect(wrapper.find('[data-testid="input-disk-dedicated-iothread"]').exists()).toBe(false);
+  });
+
+  it('shows the individual fields for a disk with custom options', () => {
+    const wrapper = mount(DiskPerformanceOptions, mountOptions(hardDisk({ cache: 'writeback', io: 'threads' })));
+
+    expect(wrapper.find('[data-testid="input-disk-cache"]').exists()).toBe(true);
+    expect(wrapper.find('[data-testid="input-disk-io"]').exists()).toBe(true);
+    expect(wrapper.find('[data-testid="input-disk-dedicated-iothread"]').exists()).toBe(true);
+  });
+
+  it('is hidden for a CD-ROM', () => {
+    const wrapper = mount(DiskPerformanceOptions, mountOptions(hardDisk({ type: 'cd-rom' })));
+
+    expect(wrapper.find('.disk-performance').exists()).toBe(false);
+  });
+
+  it('names the bus in the bus tip once the disk uses an option', () => {
+    const plain = mount(DiskPerformanceOptions, mountOptions(hardDisk({ bus: 'sata' })));
+    const tuned = mount(DiskPerformanceOptions, mountOptions(hardDisk({ bus: 'sata', cache: 'writeback' })));
+
+    expect(plain.find('[data-testid="disk-perf-bus-tip"]').exists()).toBe(false);
+    expect(tuned.find('[data-testid="disk-perf-bus-tip"]').exists()).toBe(true);
+  });
+
+  it('says in the bus tip that a dedicated I/O thread needs the virtio bus', () => {
+    const translations = load(fs.readFileSync(path.join(__dirname, '../../../../l10n/en-us.yaml'), 'utf8'));
+    const { busTip } = translations.harvester.virtualMachine.volume.performance;
+
+    expect(busTip).toContain('{bus}');
+    expect(busTip).toContain('A dedicated I/O thread needs the virtio bus');
+  });
+
+  it('shows the bus tip and the Filesystem warning inside a live region that is there before they appear', async() => {
+    const disk = { bus: 'sata', volumeMode: 'Filesystem' };
+    const wrapper = mount(DiskPerformanceOptions, mountOptions(hardDisk(disk)));
+    const status = wrapper.find('[data-testid="disk-perf-warning-status"]');
+
+    expect(status.attributes('role')).toBe('status');
+    expect(status.find('[data-testid="disk-perf-bus-tip"]').exists()).toBe(false);
+    expect(filesystemWarning(wrapper)).toBeUndefined();
+
+    await wrapper.setProps({ value: hardDisk({ ...disk, cache: 'none' }) });
+
+    expect(wrapper.find('[data-testid="disk-perf-warning-status"]').element).toBe(status.element);
+    expect(status.find('[data-testid="disk-perf-bus-tip"]').exists()).toBe(true);
+    expect(status.element.contains(filesystemWarning(wrapper)?.element)).toBe(true);
+  });
+
+  it('shows the Native I/O tip inside a live region that is there before Native I/O is picked', async() => {
+    const value = hardDisk({ cache: 'writeback', io: 'threads' });
+    const wrapper = mount(DiskPerformanceOptions, mountOptions(value));
+    const status = wrapper.find('[data-testid="disk-perf-native-status"]');
+
+    expect(status.attributes('role')).toBe('status');
+    expect(nativeTip(wrapper)).toBeUndefined();
+
+    wrapper.vm.onIoChange('native');
+    await wrapper.vm.$nextTick();
+
+    expect(wrapper.find('[data-testid="disk-perf-native-status"]').element).toBe(status.element);
+    expect(status.element.contains(nativeTip(wrapper)?.element)).toBe(true);
+  });
+
+  it('keeps the live region and the Native I/O tip when picking Native I/O switches Custom to High Performance', async() => {
+    const value = hardDisk({
+      cache: 'writeback', io: 'threads', dedicatedIOThread: true
+    });
+    const wrapper = mount(DiskPerformanceOptions, mountOptions(value));
+    const status = wrapper.find('[data-testid="disk-perf-native-status"]');
+
+    expect(wrapper.vm.profile).toBe('custom');
+
+    wrapper.vm.onIoChange('native');
+    await wrapper.vm.$nextTick();
+
+    expect(wrapper.vm.profile).toBe('highPerformance');
+    expect(wrapper.find('[data-testid="input-disk-cache"]').exists()).toBe(false);
+    expect(wrapper.find('[data-testid="disk-perf-native-status"]').element).toBe(status.element);
+    expect(status.element.contains(nativeTip(wrapper)?.element)).toBe(true);
+  });
+
+  it('passes the Filesystem warning as a label key so its quotes are not double-escaped', () => {
+    const wrapper = mount(DiskPerformanceOptions, mountOptions(hardDisk({ cache: 'none', volumeMode: 'Filesystem' })));
+    const banner = filesystemWarning(wrapper);
+
+    expect(banner).toBeDefined();
+    expect(banner?.props('label')).toBeFalsy();
+  });
+
   it('applies the High Performance preset and emits update', () => {
     const value = hardDisk();
     const wrapper = mount(DiskPerformanceOptions, mountOptions(value));
@@ -173,8 +278,7 @@ describe('component: DiskPerformanceOptions', () => {
 
   it('passes the Native I/O hint as a label key so its quotes are not double-escaped', () => {
     const wrapper = mount(DiskPerformanceOptions, mountOptions(hardDisk({ io: 'native' })));
-    const banner = wrapper.findAllComponents(Banner)
-      .find((b) => b.props('labelKey') === 'harvester.virtualMachine.volume.performance.ioMode.nativeRequiresNoCacheTip');
+    const banner = nativeTip(wrapper);
 
     expect(banner).toBeDefined();
     expect(banner?.props('label')).toBeFalsy();

pkg/harvester/edit/kubevirt.io.virtualmachine/VirtualMachineVolume/__tests__/VMPerformanceOptions.test.js

@@ -1,19 +1,19 @@
 import { mount } from '@vue/test-utils';
-import { _CREATE } from '@shell/config/query-params';
+import { _CREATE, _VIEW } from '@shell/config/query-params';
 import { Banner } from '@components/Banner';
 import VMPerformanceOptions from '../VMPerformanceOptions.vue';
 
 const AUTO_ENABLED_TIP = 'harvester.virtualMachine.volume.diskPerformance.ioThreadsPolicy.autoEnabledTip';
 const NO_VIRTIO_TIP = 'harvester.virtualMachine.volume.diskPerformance.blockMultiQueue.noVirtioTip';
 
-function mountOptions(props = {}) {
+function mountOptions(props = {}, { featureEnabled = true } = {}) {
   return mount(VMPerformanceOptions, {
     props:  { mode: _CREATE, ...props },
     global: {
       mocks: {
         $store: {
           getters: {
-            'harvester-common/getFeatureEnabled': () => true,
+            'harvester-common/getFeatureEnabled': () => featureEnabled,
             'i18n/t':                             (key) => key,
             'i18n/exists':                        () => true,
           }
@@ -24,35 +24,79 @@ function mountOptions(props = {}) {
   });
 }
 
-async function expand(wrapper) {
-  await wrapper.find('.expand-toggle').trigger('click');
-}
-
 function bannerFor(wrapper, labelKey) {
   return wrapper.findAllComponents(Banner).find((b) => b.props('labelKey') === labelKey);
 }
 
 describe('component: VMPerformanceOptions', () => {
-  it('should pass the I/O threads policy hint as a label key so its quotes are not double-escaped', async() => {
-    const wrapper = mountOptions({ hasDedicatedIothread: true });
+  it('should be hidden when the highPerformanceStorage feature is disabled', () => {
+    const wrapper = mountOptions({}, { featureEnabled: false });
 
-    await expand(wrapper);
+    expect(wrapper.find('.vm-perf').exists()).toBe(false);
+  });
+
+  it('should show the section with its title and fields, without a toggle', () => {
+    const wrapper = mountOptions();
+
+    expect(wrapper.find('h3').text()).toBe('harvester.virtualMachine.volume.diskPerformance.title');
+    expect(wrapper.find('.expand-toggle').exists()).toBe(false);
+    expect(wrapper.find('[data-testid="input-vm-block-multi-queue"]').exists()).toBe(true);
+    expect(wrapper.find('[data-testid="input-vm-iothreads-policy"]').exists()).toBe(true);
+    expect(wrapper.find('[data-testid="input-vm-iothread-count"]').exists()).toBe(false);
+  });
+
+  it('should only accept whole numbers as the supplemental pool thread count', () => {
+    const wrapper = mountOptions({ ioThreadsPolicy: 'supplementalPool', ioThreadCount: 4 });
+    const input = wrapper.find('[data-testid="input-vm-iothread-count"] input');
+
+    expect(input.attributes('inputmode')).toBe('numeric');
+  });
+
+  it.each([
+    [_CREATE, true],
+    [_VIEW, false],
+  ])('in %s mode, should mark the supplemental pool thread count as required: %p', (mode, required) => {
+    const wrapper = mountOptions({
+      mode, ioThreadsPolicy: 'supplementalPool', ioThreadCount: 4
+    });
+
+    expect(wrapper.find('[data-testid="input-vm-iothread-count"] .required').exists()).toBe(required);
+  });
+
+  it.each([
+    [{}, false],
+    [{ blockMultiQueue: true }, true],
+    [{ ioThreadsPolicy: 'auto' }, true],
+  ])('in view mode with %p, should show the section: %p', (props, shown) => {
+    const wrapper = mountOptions({ ...props, mode: _VIEW });
+
+    expect(wrapper.find('.vm-perf').exists()).toBe(shown);
+  });
+
+  it('should clear Block Multi-Queue when the last virtio disk goes away', async() => {
+    const wrapper = mountOptions({ blockMultiQueue: true, hasVirtioDisk: true });
+
+    await wrapper.setProps({ hasVirtioDisk: false });
+
+    expect(wrapper.emitted('update:blockMultiQueue')).toStrictEqual([[false]]);
+  });
+
+  it('should pass the I/O threads policy hint as a label key so its quotes are not double-escaped', () => {
+    const wrapper = mountOptions({ hasDedicatedIothread: true });
     const banner = bannerFor(wrapper, AUTO_ENABLED_TIP);
 
     expect(banner).toBeDefined();
     expect(banner.props('label')).toBeFalsy();
   });
 
-  it('should not show the I/O threads policy hint when a policy is set', async() => {
+  it('should not show the I/O threads policy hint when a policy is set', () => {
     const wrapper = mountOptions({ hasDedicatedIothread: true, ioThreadsPolicy: 'shared' });
 
     expect(bannerFor(wrapper, AUTO_ENABLED_TIP)).toBeUndefined();
   });
 
-  it('should pass the no virtio disk hint as a label key', async() => {
+  it('should pass the no virtio disk hint as a label key', () => {
     const wrapper = mountOptions({ hasVirtioDisk: false });
-
-    await expand(wrapper);
     const banner = bannerFor(wrapper, NO_VIRTIO_TIP);
 
     expect(banner).toBeDefined();