From 0cd2bac2bf1c433372404bc8108822177c585f68 Mon Sep 17 00:00:00 2001 From: undsoft Date: Mon, 18 Nov 2024 15:07:36 +0100 Subject: [PATCH] NAS-132593: Fix how GPU devices are added to containers --- .../api/api-call-directory.interface.ts | 6 +++--- .../interfaces/virtualization.interface.ts | 5 +++++ .../add-device-menu.component.html | 11 +++++----- .../add-device-menu.component.spec.ts | 7 ++++--- .../add-device-menu.component.ts | 20 +++++++++---------- .../instance-wizard.component.spec.ts | 2 +- .../instance-wizard.component.ts | 11 +++++----- 7 files changed, 34 insertions(+), 28 deletions(-) diff --git a/src/app/interfaces/api/api-call-directory.interface.ts b/src/app/interfaces/api/api-call-directory.interface.ts index dd2092fc7f2..d32cef7b6ec 100644 --- a/src/app/interfaces/api/api-call-directory.interface.ts +++ b/src/app/interfaces/api/api-call-directory.interface.ts @@ -244,9 +244,9 @@ import { VmDisplayWebUriParams, VmPortWizardResult, } from 'app/interfaces/virtual-machine.interface'; import { - VirtualizationDevice, VirtualizationGlobalConfig, AvailableGpu, + VirtualizationDevice, VirtualizationGlobalConfig, VirtualizationImage, VirtualizationImageParams, - VirtualizationInstance, VirtualizationNetwork, AvailableUsb, + VirtualizationInstance, VirtualizationNetwork, AvailableUsb, AvailableGpus, } from 'app/interfaces/virtualization.interface'; import { VmDevice, VmDeviceDelete, VmDeviceUpdate, VmDisplayDevice, VmPassthroughDeviceChoice, VmUsbPassthroughDeviceChoice, @@ -840,7 +840,7 @@ export interface ApiCallDirectory { 'virt.device.disk_choices': { params: []; response: Choices }; 'virt.device.gpu_choices': { params: [instanceType: VirtualizationType, gpuType: VirtualizationGpuType]; - response: Record; + response: AvailableGpus; }; 'virt.device.usb_choices': { params: []; response: Record }; diff --git a/src/app/interfaces/virtualization.interface.ts b/src/app/interfaces/virtualization.interface.ts index f2c9755ee6b..b466eda48e6 100644 --- a/src/app/interfaces/virtualization.interface.ts +++ b/src/app/interfaces/virtualization.interface.ts @@ -177,6 +177,11 @@ export interface AvailableGpu { vendor: string | null; } +// eslint-disable-next-line @typescript-eslint/consistent-indexed-object-style +export interface AvailableGpus { + [pci: string]: AvailableGpu; +} + export interface AvailableUsb { vendor_id: string; product_id: string; diff --git a/src/app/pages/virtualization/components/all-instances/instance-details/instance-devices/add-device-menu/add-device-menu.component.html b/src/app/pages/virtualization/components/all-instances/instance-details/instance-devices/add-device-menu/add-device-menu.component.html index 91265aeb0d9..1d0f7f9d94d 100644 --- a/src/app/pages/virtualization/components/all-instances/instance-details/instance-devices/add-device-menu/add-device-menu.component.html +++ b/src/app/pages/virtualization/components/all-instances/instance-details/instance-devices/add-device-menu/add-device-menu.component.html @@ -24,15 +24,16 @@

{{ 'USB Devices' | translate }}

} } - @if (availableGpuDevices().length) { + @let gpus = availableGpuDevices() | keyvalue; + @if (gpus.length) {

{{ 'GPUs' | translate }}

- @for (gpu of availableGpuDevices(); track gpu) { + @for (gpu of gpus; track gpu.key) { } } diff --git a/src/app/pages/virtualization/components/all-instances/instance-details/instance-devices/add-device-menu/add-device-menu.component.spec.ts b/src/app/pages/virtualization/components/all-instances/instance-details/instance-devices/add-device-menu/add-device-menu.component.spec.ts index eabe4382d38..9b7afc7caa7 100644 --- a/src/app/pages/virtualization/components/all-instances/instance-details/instance-devices/add-device-menu/add-device-menu.component.spec.ts +++ b/src/app/pages/virtualization/components/all-instances/instance-details/instance-devices/add-device-menu/add-device-menu.component.spec.ts @@ -30,10 +30,10 @@ describe('AddDeviceMenuComponent', () => { } as AvailableUsb, }), mockCall('virt.device.gpu_choices', { - gpu1: { + pci_0000_01_00_0: { description: 'NDIVIA XTR 2000', } as AvailableGpu, - gpu2: { + pci_0000_01_00_1: { description: 'MAD Galeon 5000', } as AvailableGpu, }), @@ -48,6 +48,7 @@ describe('AddDeviceMenuComponent', () => { }, { dev_type: VirtualizationDeviceType.Gpu, + pci: 'pci_0000_01_00_0', description: 'NDIVIA XTR 2000', }, ] as VirtualizationDevice[], @@ -95,7 +96,7 @@ describe('AddDeviceMenuComponent', () => { expect(spectator.inject(ApiService).call).toHaveBeenCalledWith('virt.instance.device_add', ['my-instance', { dev_type: VirtualizationDeviceType.Gpu, - description: 'MAD Galeon 5000', + pci: 'pci_0000_01_00_1', } as VirtualizationDevice]); expect(spectator.inject(VirtualizationInstancesStore).loadDevices).toHaveBeenCalled(); expect(spectator.inject(SnackbarService).success).toHaveBeenCalledWith('Device was added'); diff --git a/src/app/pages/virtualization/components/all-instances/instance-details/instance-devices/add-device-menu/add-device-menu.component.ts b/src/app/pages/virtualization/components/all-instances/instance-details/instance-devices/add-device-menu/add-device-menu.component.ts index 13833b0142b..870a420e3e7 100644 --- a/src/app/pages/virtualization/components/all-instances/instance-details/instance-devices/add-device-menu/add-device-menu.component.ts +++ b/src/app/pages/virtualization/components/all-instances/instance-details/instance-devices/add-device-menu/add-device-menu.component.ts @@ -1,13 +1,14 @@ +import { KeyValuePipe } from '@angular/common'; import { ChangeDetectionStrategy, Component, computed } from '@angular/core'; import { toSignal } from '@angular/core/rxjs-interop'; import { MatButton } from '@angular/material/button'; import { MatMenu, MatMenuItem, MatMenuTrigger } from '@angular/material/menu'; import { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy'; import { TranslateModule, TranslateService } from '@ngx-translate/core'; +import { pickBy } from 'lodash-es'; import { NgxSkeletonLoaderModule } from 'ngx-skeleton-loader'; import { VirtualizationDeviceType, VirtualizationGpuType, VirtualizationType } from 'app/enums/virtualization.enum'; import { - AvailableGpu, AvailableUsb, VirtualizationDevice, VirtualizationGpu, @@ -35,6 +36,7 @@ import { ApiService } from 'app/services/websocket/api.service'; TranslateModule, MatMenuTrigger, NgxSkeletonLoaderModule, + KeyValuePipe, ], }) export class AddDeviceMenuComponent { @@ -55,18 +57,17 @@ export class AddDeviceMenuComponent { }); protected readonly availableGpuDevices = computed(() => { - const gpuChoices = Object.values(this.gpuChoices()); - const existingGpuDevices = this.instanceStore.selectedInstanceDevices() + const gpuChoices = this.gpuChoices(); + const usedGpus = this.instanceStore.selectedInstanceDevices() .filter((device) => device.dev_type === VirtualizationDeviceType.Gpu); - return gpuChoices.filter((gpu) => { - // TODO: Condition is incorrect. - return !existingGpuDevices.find((device) => device.description === gpu.description); + return pickBy(gpuChoices, (_, pci) => { + return !usedGpus.find((usedGpu) => usedGpu.pci === pci); }); }); protected readonly hasDevicesToAdd = computed(() => { - return this.availableUsbDevices().length > 0 || this.availableGpuDevices().length > 0; + return this.availableUsbDevices().length > 0 || Object.keys(this.availableGpuDevices()).length > 0; }); constructor( @@ -85,11 +86,10 @@ export class AddDeviceMenuComponent { } as VirtualizationUsb); } - protected addGpu(gpu: AvailableGpu): void { + protected addGpu(gpuPci: string): void { this.addDevice({ dev_type: VirtualizationDeviceType.Gpu, - // TODO: Incorrect value. - description: gpu.description, + pci: gpuPci, } as VirtualizationGpu); } diff --git a/src/app/pages/virtualization/components/instance-wizard/instance-wizard.component.spec.ts b/src/app/pages/virtualization/components/instance-wizard/instance-wizard.component.spec.ts index ccc753a9a7b..92ac286daf8 100644 --- a/src/app/pages/virtualization/components/instance-wizard/instance-wizard.component.spec.ts +++ b/src/app/pages/virtualization/components/instance-wizard/instance-wizard.component.spec.ts @@ -167,7 +167,7 @@ describe('InstanceWizardComponent', () => { dest_proto: VirtualizationProxyProtocol.Udp, }, { dev_type: VirtualizationDeviceType.Usb, product_id: '0003' }, - { dev_type: VirtualizationDeviceType.Gpu, gpu_type: 'NVIDIA Corporation' }, + { dev_type: VirtualizationDeviceType.Gpu, pci: 'pci_0000_01_00_0' }, ], image: 'almalinux/8/cloud', memory: GiB, diff --git a/src/app/pages/virtualization/components/instance-wizard/instance-wizard.component.ts b/src/app/pages/virtualization/components/instance-wizard/instance-wizard.component.ts index d5c7c4b0b4a..12fea924464 100644 --- a/src/app/pages/virtualization/components/instance-wizard/instance-wizard.component.ts +++ b/src/app/pages/virtualization/components/instance-wizard/instance-wizard.component.ts @@ -94,10 +94,9 @@ export class InstanceWizardComponent implements OnInit { 'virt.device.gpu_choices', [VirtualizationType.Container, VirtualizationGpuType.Physical], ).pipe( - map((choices: Record) => Object.values(choices).map((choice) => ({ - label: choice.description, - // TODO: Incorrect value – doesn't uniquely identify the GPU - value: choice.vendor, + map((choices: Record) => Object.entries(choices).map(([pci, gpu]) => ({ + label: gpu.description, + value: pci, }))), ); @@ -272,9 +271,9 @@ export class InstanceWizardComponent implements OnInit { const gpuDevices = Object.entries(this.form.controls.gpu_devices.value || {}) .filter(([_, isSelected]) => isSelected) - .map(([gpuType]) => ({ + .map(([pci]) => ({ + pci, dev_type: VirtualizationDeviceType.Gpu, - gpu_type: gpuType, })); const proxies = this.form.controls.proxies.value.map((proxy) => ({