NAS-128601 / 25.04 / Fixes the process to assign GPUs to VMs when creating or editing

src/app/helpers/operators/options.operators.ts

@@ -10,7 +10,9 @@ import { MapOption, Option } from 'app/interfaces/option.interface';
  */
 export function choicesToOptions(): OperatorFunction<Choices, Option[]> {
   return map((choices) => {
-    return Object.entries(choices).map(([value, label]) => ({ label, value }));
+    return Object.entries(choices).map(
+      ([value, label]) => ({ label, value }),
+    );
   });
 }
 

src/app/interfaces/api/api-call-directory.interface.ts

@@ -879,6 +879,7 @@ export interface ApiCallDirectory {
   'vm.device.delete': { params: [number, VmDeviceDelete?]; response: boolean };
   'vm.device.disk_choices': { params: void; response: Choices };
   'vm.device.get_pci_ids_for_gpu_isolation': { params: [string]; response: string[] };
+  'system.advanced.get_gpu_pci_choices': { params: void; response: Choices };
   'vm.device.nic_attach_choices': { params: void; response: Choices };
   'vm.device.passthrough_device_choices': { params: void; response: Record<string, VmPassthroughDeviceChoice> };
   'vm.device.query': { params: QueryParams<VmDevice>; response: VmDevice[] };

src/app/pages/system/advanced/isolated-gpus/isolated-gpus-form/isolated-gpus-form.component.spec.ts

@@ -43,6 +43,10 @@ describe('IsolatedGpuPcisFormComponent', () => {
       }),
       mockWebSocket([
         mockCall('system.advanced.update_gpu_pci_ids'),
+        mockCall('system.advanced.get_gpu_pci_choices', {
+          'Fake HD Graphics [0000:00:01.0]': '0000:00:01.0',
+          'Intel Corporation HD Graphics 510 [0000:00:02.0]': '0000:00:02.0',
+        }),
       ]),
       mockProvider(SystemGeneralService),
       mockProvider(IxChainedSlideInService, {
@@ -76,14 +80,14 @@ describe('IsolatedGpuPcisFormComponent', () => {
     const values = await form.getValues();
 
     expect(values).toEqual({
-      GPUs: ['Intel Corporation HD Graphics 510'],
+      GPUs: ['Intel Corporation HD Graphics 510 [0000:00:02.0]'],
     });
   });
 
   it('saves updated settings when Save is pressed', async () => {
     const form = await loader.getHarness(IxFormHarness);
     await form.fillForm({
-      GPUs: 'Fake HD Graphics',
+      GPUs: 'Fake HD Graphics [0000:00:01.0]',
     });
 
     const saveButton = await loader.getHarness(MatButtonHarness.with({ text: 'Save' }));

src/app/pages/system/advanced/isolated-gpus/isolated-gpus-form/isolated-gpus-form.component.ts

@@ -7,7 +7,7 @@ import { MatCard, MatCardContent } from '@angular/material/card';
 import { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy';
 import { Store } from '@ngrx/store';
 import { TranslateService, TranslateModule } from '@ngx-translate/core';
-import { take } from 'rxjs';
+import { map, take } from 'rxjs';
 import { RequiresRolesDirective } from 'app/directives/requires-roles/requires-roles.directive';
 import { Role } from 'app/enums/role.enum';
 import { FormActionsComponent } from 'app/modules/forms/ix-forms/components/form-actions/form-actions.component';
@@ -18,7 +18,6 @@ import { IxModalHeader2Component } from 'app/modules/forms/ix-forms/components/i
 import { FormErrorHandlerService } from 'app/modules/forms/ix-forms/services/form-error-handler.service';
 import { SnackbarService } from 'app/modules/snackbar/services/snackbar.service';
 import { TestDirective } from 'app/modules/test-id/test.directive';
-import { GpuService } from 'app/services/gpu/gpu.service';
 import { IsolatedGpuValidatorService } from 'app/services/gpu/isolated-gpu-validator.service';
 import { WebSocketService } from 'app/services/ws.service';
 import { AppState } from 'app/store';
@@ -55,7 +54,11 @@ export class IsolatedGpusFormComponent implements OnInit {
       asyncValidators: [this.gpuValidator.validateGpu],
     }),
   });
-  readonly options$ = this.gpuService.getGpuOptions();
+  readonly options$ = this.ws.call('system.advanced.get_gpu_pci_choices').pipe(map((choices) => {
+    return Object.entries(choices).map(
+      ([value, label]) => ({ value: label, label: value }),
+    );
+  }));
 
   constructor(
     protected ws: WebSocketService,
@@ -64,7 +67,6 @@ export class IsolatedGpusFormComponent implements OnInit {
     private cdr: ChangeDetectorRef,
     private store$: Store<AppState>,
     private gpuValidator: IsolatedGpuValidatorService,
-    private gpuService: GpuService,
     private snackbar: SnackbarService,
     private chainedRef: ChainedRef<unknown>,
   ) { }

src/app/pages/vm/vm-edit-form/vm-edit-form.component.spec.ts

@@ -79,15 +79,24 @@ describe('VmEditFormComponent', () => {
         }),
         mockCall('vm.update'),
         mockCall('vm.device.get_pci_ids_for_gpu_isolation', ['10DE:1401']),
+        mockCall('system.advanced.update_gpu_pci_ids'),
+        mockCall('system.advanced.get_gpu_pci_choices', {
+          'GeForce [0000:02:00.0]': '0000:02:00.0',
+          'Intel Arc [0000:03:00.0]': '0000:03:00.0',
+        }),
       ]),
       mockAuth(),
       mockProvider(DialogService),
       mockProvider(GpuService, {
-        getGpuOptions: () => of([
-          { label: 'GeForce', value: '0000:02:00.0' },
-          { label: 'Intel Arc', value: '0000:03:00.0' },
-        ]),
-        getAllGpus: () => of([
+        getGpuOptions: jest.fn(() => of([
+          { label: 'GeForce [0000:02:00.0]', value: '0000:02:00.0' },
+          { label: 'Intel Arc [0000:03:00.0]', value: '0000:03:00.0' },
+        ])),
+        addIsolatedGpuPciIds: jest.fn(() => of({})),
+        getIsolatedGpuPciIds: jest.fn(() => of([
+          '0000:02:00.0',
+        ])),
+        getAllGpus: jest.fn(() => of([
           {
             addr: {
               pci_slot: '0000:02:00.0',
@@ -112,8 +121,7 @@ describe('VmEditFormComponent', () => {
               },
             ],
           },
-        ]),
-        addIsolatedGpuPciIds: jest.fn(() => of(undefined)),
+        ])),
       }),
       mockProvider(VmGpuService, {
         updateVmGpus: jest.fn(() => of(undefined)),
@@ -166,7 +174,7 @@ describe('VmEditFormComponent', () => {
 
       'Hide from MSR': false,
       'Ensure Display Device': true,
-      GPUs: ['GeForce'],
+      GPUs: ['GeForce [0000:02:00.0]'],
     });
   });
 
@@ -244,13 +252,15 @@ describe('VmEditFormComponent', () => {
 
   it('updates GPU devices when form is edited and saved', async () => {
     await form.fillForm({
-      GPUs: ['Intel Arc'],
+      GPUs: ['Intel Arc [0000:03:00.0]'],
     });
 
     const saveButton = await loader.getHarness(MatButtonHarness.with({ text: 'Save' }));
     await saveButton.click();
 
-    expect(spectator.inject(VmGpuService).updateVmGpus).toHaveBeenCalledWith(existingVm, ['0000:03:00.0', '10DE:1401']);
-    expect(spectator.inject(GpuService).addIsolatedGpuPciIds).toHaveBeenCalledWith(['0000:03:00.0', '10DE:1401']);
+    expect(spectator.inject(GpuService).addIsolatedGpuPciIds).toHaveBeenCalledWith(
+      ['0000:03:00.0'],
+    );
+    expect(spectator.inject(VmGpuService).updateVmGpus).toHaveBeenCalledWith(existingVm, ['0000:03:00.0']);
   });
 });

src/app/pages/vm/vm-edit-form/vm-edit-form.component.ts

@@ -7,7 +7,7 @@ import { MatCard, MatCardContent } from '@angular/material/card';
 import { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy';
 import { TranslateService, TranslateModule } from '@ngx-translate/core';
 import {
-  Observable, forkJoin, map, of, switchMap,
+  Observable, forkJoin, of, switchMap,
 } from 'rxjs';
 import { MiB } from 'app/constants/bytes.constant';
 import { RequiresRolesDirective } from 'app/directives/requires-roles/requires-roles.directive';
@@ -163,20 +163,13 @@ export class VmEditFormComponent implements OnInit {
     }
 
     const gpusIds = this.form.value.gpus;
-
-    const pciIdsRequests$ = gpusIds.map((gpu) => {
-      return this.ws.call('vm.device.get_pci_ids_for_gpu_isolation', [gpu]);
-    });
-
     let updateVmRequest$: Observable<unknown>;
 
-    if (pciIdsRequests$.length) {
-      updateVmRequest$ = forkJoin(pciIdsRequests$).pipe(
-        map((pciIds) => pciIds.flat()),
-        switchMap((pciIds) => forkJoin([
+    if (gpusIds.length) {
+      updateVmRequest$ = this.gpuService.addIsolatedGpuPciIds(gpusIds).pipe(
+        switchMap(() => forkJoin([
           this.ws.call('vm.update', [this.existingVm.id, vmPayload as VirtualMachineUpdate]),
-          this.vmGpuService.updateVmGpus(this.existingVm, gpusIds.concat(pciIds)),
-          this.gpuService.addIsolatedGpuPciIds(gpusIds.concat(pciIds)),
+          this.vmGpuService.updateVmGpus(this.existingVm, gpusIds),
         ])),
       );
     } else {

src/app/pages/vm/vm-wizard/steps/6-gpu-step/gpu-step.component.spec.ts

@@ -4,6 +4,7 @@ import { TestbedHarnessEnvironment } from '@angular/cdk/testing/testbed';
 import { ReactiveFormsModule } from '@angular/forms';
 import { createComponentFactory, mockProvider, Spectator } from '@ngneat/spectator/jest';
 import { of } from 'rxjs';
+import { mockCall, mockWebSocket } from 'app/core/testing/utils/mock-websocket.utils';
 import { IxFormHarness } from 'app/modules/forms/ix-forms/testing/ix-form.harness';
 import { GpuStepComponent } from 'app/pages/vm/vm-wizard/steps/6-gpu-step/gpu-step.component';
 import { GpuService } from 'app/services/gpu/gpu.service';
@@ -23,13 +24,19 @@ describe('GpuStepComponent', () => {
       CdkStepper,
       mockProvider(GpuService, {
         getGpuOptions: () => of([
-          { label: 'GeForce GTX 1080', value: '0000:03:00.0' },
-          { label: 'GeForce GTX 1080 Ti', value: '0000:04:00.0' },
+          { label: 'GeForce GTX 1080 [0000:03:00.0]', value: '0000:03:00.0' },
+          { label: 'GeForce GTX 1080 Ti [0000:04:00.0]', value: '0000:04:00.0' },
         ]),
       }),
       mockProvider(IsolatedGpuValidatorService, {
         validateGpu: () => of(null),
       }),
+      mockWebSocket([
+        mockCall('system.advanced.get_gpu_pci_choices', {
+          'GeForce GTX 1080 [0000:03:00.0]': '0000:03:00.0',
+          'GeForce GTX 1080 Ti [0000:04:00.0]': '0000:04:00.0',
+        }),
+      ]),
     ],
   });
 
@@ -43,7 +50,7 @@ describe('GpuStepComponent', () => {
     await form.fillForm({
       'Hide from MSR': true,
       'Ensure Display Device': true,
-      GPUs: ['GeForce GTX 1080 Ti'],
+      GPUs: ['GeForce GTX 1080 Ti [0000:04:00.0]'],
     });
   }
 

src/app/pages/vm/vm-wizard/steps/6-gpu-step/gpu-step.component.ts

@@ -46,8 +46,8 @@ export class GpuStepComponent implements SummaryProvider {
   constructor(
     private formBuilder: FormBuilder,
     private gpuValidator: IsolatedGpuValidatorService,
-    private gpuService: GpuService,
     private translate: TranslateService,
+    private gpuService: GpuService,
   ) {}
 
   getSummary(): SummarySection {

src/app/pages/vm/vm-wizard/vm-wizard.component.spec.ts

@@ -94,12 +94,21 @@ describe('VmWizardComponent', () => {
           eno2: 'eno2',
         }),
         mockCall('vm.device.get_pci_ids_for_gpu_isolation', ['10DE:1401']),
+        mockCall('system.advanced.update_gpu_pci_ids'),
+        mockCall('system.advanced.get_gpu_pci_choices', {
+          'GeForce GTX 1080 [0000:03:00.0]': '0000:03:00.0',
+          'GeForce GTX 1070 [0000:02:00.0]': '0000:02:00.0',
+        }),
       ]),
       mockProvider(GpuService, {
         getGpuOptions: () => of([
-          { label: 'GeForce GTX 1080', value: '0000:03:00.0' },
+          { label: 'GeForce GTX 1080 [0000:03:00.0]', value: '0000:03:00.0' },
+          { label: 'GeForce GTX 1070 [0000:02:00.0]', value: '0000:02:00.0' },
         ]),
-        addIsolatedGpuPciIds: jest.fn(() => of(undefined)),
+        addIsolatedGpuPciIds: jest.fn(() => of({})),
+        getIsolatedGpuPciIds: jest.fn(() => of([
+          '0000:02:00.0',
+        ])),
       }),
       mockProvider(FilesystemService, {
         getFilesystemNodeProvider: jest.fn(),
@@ -165,7 +174,7 @@ describe('VmWizardComponent', () => {
     await updateStepHarnesses();
 
     await form.fillForm({
-      GPUs: ['GeForce GTX 1080'],
+      GPUs: ['GeForce GTX 1080 [0000:03:00.0]'],
     });
     await nextButton.click();
   }
@@ -319,8 +328,10 @@ describe('VmWizardComponent', () => {
         web: true,
       },
     }]);
-    expect(spectator.inject(VmGpuService).updateVmGpus).toHaveBeenCalledWith({ id: 4 }, ['0000:03:00.0', '10DE:1401']);
-    expect(spectator.inject(GpuService).addIsolatedGpuPciIds).toHaveBeenCalledWith(['0000:03:00.0', '10DE:1401']);
+    expect(spectator.inject(GpuService).addIsolatedGpuPciIds).toHaveBeenCalledWith(
+      ['0000:03:00.0'],
+    );
+    expect(spectator.inject(VmGpuService).updateVmGpus).toHaveBeenCalledWith({ id: 4 }, ['0000:03:00.0']);
     expect(spectator.inject(IxSlideInRef).close).toHaveBeenCalled();
   });
 });

src/app/pages/vm/vm-wizard/vm-wizard.component.ts

@@ -12,7 +12,7 @@ import { pick } from 'lodash-es';
 import {
   forkJoin, Observable, of, switchMap,
 } from 'rxjs';
-import { catchError, defaultIfEmpty, map } from 'rxjs/operators';
+import { catchError, defaultIfEmpty } from 'rxjs/operators';
 import { GiB, MiB } from 'app/constants/bytes.constant';
 import { RequiresRolesDirective } from 'app/directives/requires-roles/requires-roles.directive';
 import { Role } from 'app/enums/role.enum';
@@ -285,17 +285,9 @@ export class VmWizardComponent implements OnInit {
   private getGpuRequests(vm: VirtualMachine): Observable<unknown> {
     const gpusIds = this.gpuForm.gpus as unknown as string[];
 
-    const pciIdsRequests$ = gpusIds.map((gpu) => {
-      return this.ws.call('vm.device.get_pci_ids_for_gpu_isolation', [gpu]);
-    });
-
-    return forkJoin(pciIdsRequests$).pipe(
+    return this.gpuService.addIsolatedGpuPciIds(gpusIds).pipe(
       defaultIfEmpty([]),
-      map((pciIds) => pciIds.flat()),
-      switchMap((pciIds) => forkJoin([
-        this.vmGpuService.updateVmGpus(vm, gpusIds.concat(pciIds)),
-        this.gpuService.addIsolatedGpuPciIds(gpusIds.concat(pciIds)),
-      ])),
+      switchMap(() => this.vmGpuService.updateVmGpus(vm, gpusIds)),
     );
   }
 

src/app/services/gpu/gpu-service.spec.ts

@@ -1,4 +1,5 @@
 import { createServiceFactory, SpectatorService } from '@ngneat/spectator/jest';
+import { Store } from '@ngrx/store';
 import { provideMockStore } from '@ngrx/store/testing';
 import { firstValueFrom } from 'rxjs';
 import { TestScheduler } from 'rxjs/testing';
@@ -9,6 +10,7 @@ import { AdvancedConfig } from 'app/interfaces/advanced-config.interface';
 import { Device } from 'app/interfaces/device.interface';
 import { GpuService } from 'app/services/gpu/gpu.service';
 import { WebSocketService } from 'app/services/ws.service';
+import { advancedConfigUpdated } from 'app/store/system-config/system-config.actions';
 import { selectAdvancedConfig } from 'app/store/system-config/system-config.selectors';
 
 describe('GpuService', () => {
@@ -35,6 +37,10 @@ describe('GpuService', () => {
       mockWebSocket([
         mockCall('system.advanced.update_gpu_pci_ids'),
         mockCall('device.get_info', allGpus),
+        mockCall('system.advanced.get_gpu_pci_choices', {
+          'GeForce [0000:01:00.0]': '0000:01:00.0',
+          'Radeon [0000:02:00.0]': '0000:02:00.0',
+        }),
       ]),
       provideMockStore({
         selectors: [
@@ -65,10 +71,12 @@ describe('GpuService', () => {
 
   describe('getGpuOptions', () => {
     it('returns an observable with a list of options for GPU select', async () => {
+      const store$ = spectator.inject(Store);
+      jest.spyOn(store$, 'dispatch');
       const options = await firstValueFrom(spectator.service.getGpuOptions());
       expect(options).toEqual([
-        { label: 'GeForce', value: '0000:01:00.0' },
-        { label: 'Radeon', value: '0000:02:00.0' },
+        { label: 'GeForce [0000:01:00.0]', value: '0000:01:00.0' },
+        { label: 'Radeon [0000:02:00.0]', value: '0000:02:00.0' },
       ]);
     });
   });
@@ -94,12 +102,15 @@ describe('GpuService', () => {
 
   describe('addIsolatedGpuPciIds', () => {
     it('adds new ids of new isolated gpu devices in addition to ones that were previously isolated', async () => {
+      const store$ = spectator.inject(Store);
+      jest.spyOn(store$, 'dispatch');
       await firstValueFrom(spectator.service.addIsolatedGpuPciIds(['0000:01:00.0']));
 
       expect(spectator.inject(WebSocketService).call).toHaveBeenCalledWith(
         'system.advanced.update_gpu_pci_ids',
         [['0000:02:00.0', '0000:01:00.0']],
       );
+      expect(spectator.inject(Store).dispatch).toHaveBeenCalledWith(advancedConfigUpdated());
     });
 
     it('does nothing when new gpu has already been isolated', () => {