Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NAS-132593 / 25.04 / Fix how GPU devices are added to containers #11063

Merged
merged 1 commit into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/app/interfaces/api/api-call-directory.interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<string, AvailableGpu>;
response: AvailableGpus;
};
'virt.device.usb_choices': { params: []; response: Record<string, AvailableUsb> };

Expand Down
5 changes: 5 additions & 0 deletions src/app/interfaces/virtualization.interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,16 @@ <h4 class="menu-header">{{ 'USB Devices' | translate }}</h4>
}
}

@if (availableGpuDevices().length) {
@let gpus = availableGpuDevices() | keyvalue;
@if (gpus.length) {
<h4 class="menu-header">{{ 'GPUs' | translate }}</h4>
@for (gpu of availableGpuDevices(); track gpu) {
@for (gpu of gpus; track gpu.key) {
<button
mat-menu-item
[ixTest]="['add-gpu-device', gpu.description]"
(click)="addGpu(gpu)"
[ixTest]="['add-gpu-device', gpu.value.description]"
(click)="addGpu(gpu.key)"
>
{{ gpu.description }}
{{ gpu.value.description }}
</button>
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}),
Expand All @@ -48,6 +48,7 @@ describe('AddDeviceMenuComponent', () => {
},
{
dev_type: VirtualizationDeviceType.Gpu,
pci: 'pci_0000_01_00_0',
description: 'NDIVIA XTR 2000',
},
] as VirtualizationDevice[],
Expand Down Expand Up @@ -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');
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -35,6 +36,7 @@ import { ApiService } from 'app/services/websocket/api.service';
TranslateModule,
MatMenuTrigger,
NgxSkeletonLoaderModule,
KeyValuePipe,
],
})
export class AddDeviceMenuComponent {
Expand All @@ -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(
Expand All @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,9 @@ export class InstanceWizardComponent implements OnInit {
'virt.device.gpu_choices',
[VirtualizationType.Container, VirtualizationGpuType.Physical],
).pipe(
map((choices: Record<string, AvailableGpu>) => Object.values(choices).map((choice) => ({
label: choice.description,
// TODO: Incorrect value – doesn't uniquely identify the GPU
value: choice.vendor,
map((choices: Record<string, AvailableGpu>) => Object.entries(choices).map(([pci, gpu]) => ({
label: gpu.description,
value: pci,
}))),
);

Expand Down Expand Up @@ -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) => ({
Expand Down
Loading