feat: check usb/pci/gpu resource name #100
Conversation
Yu-Jack
changed the title
Yu-Jack
changed the title
Yu-Jack
changed the title
Yu-Jack
force-pushed
the
HARV/6542
branch
5 times, most recently
from
8967558 to
f2fed1f
Compare
Signed-off-by: Jack Yu <jack.yu@suse.com>
| return nil | ||
| } | ||
|
|
||
| func (vmValidator *vmDeviceHostValidator) validatePCIDevice(resourceName string) (found bool, err error) { |
There was a problem hiding this comment.
these validators check if there is a pci resource that exists in the cluster. Do we need to check the resource with a node in any situation?
There was a problem hiding this comment.
I think we could keep this issue simple. So, I'll only check if the resource name exists in the cluster. For your case, if we decide to check node selector, we should consider node affinity. Besides, it's valid that same resource names are used by different VMs. We also need to consider checking if the resource is enough.
pkg/webhook/vm_validatation.go
Outdated
| foundInPCI, err := vmValidator.validatePCIDevice(gpu.DeviceName) | ||
|
|
||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if !foundInPCI { | ||
| return fmt.Errorf("gpu device %s: resource name %s not found in pcidevice cache", gpu.Name, gpu.DeviceName) | ||
| } |
There was a problem hiding this comment.
Hi Jack,
Why do we validate GPUs with PCI devices?
There was a problem hiding this comment.
As I know, the GPU is also bound to PCI device, so we can use PCI device to check it.
There was a problem hiding this comment.
I thought we just mapped the vGPU to PCI device. Actually, It's like USB device. We can pass whole USB controller to VM with PCI device, or just pass one of USB ports to VM with USB passthrough.
Signed-off-by: Jack Yu <jack.yu@suse.com>
| @@ -108,3 +125,80 @@ func (vmValidator *vmDeviceHostValidator) validateDevicesFromSameNodes(vmObj *ku | |||
|
|
|||
| return nil | |||
| } | |||
|
|
|||
| func (vmValidator *vmDeviceHostValidator) validateHostDevices(vmObj *kubevirtv1.VirtualMachine) error { | |||
| for _, hostDevice := range vmObj.Spec.Template.Spec.Domain.Devices.HostDevices { | |||
There was a problem hiding this comment.
The object access path vmObj.Spec.Template.Spec.Domain.Devices.HostDevices is quite long, maybe we can consider extracting it as a utility function.
There was a problem hiding this comment.
We can extract it later if we truly utilize a significant number of these paths.
| } | ||
|
|
||
| func (vmValidator *vmDeviceHostValidator) validateGPUs(vmObj *kubevirtv1.VirtualMachine) error { | ||
| for _, gpu := range vmObj.Spec.Template.Spec.Domain.Devices.GPUs { |
|
@mergify backport v1.3 |
✅ Backports have been created
|
IMPORTANT: Please do not create a Pull Request without creating an issue first.
Problem:
When creating or updating a non-existed pci/usb resource name, it's allowed. Besides, vGPU device is not checked either.
Solution:
We should check
.DeviceNamein VM spec while creating or updating. BTW, vGPU device is in the other field, so we need to check it inspec.template.domain.devices.gpus.Related Issue:
harvester/harvester#6542
Test plan:
Test PCI Device
.DeviceNamein the spec.template.domain.devices.hostDevices.If you're not sure how to do it, please refer following demo recording.
pcidevice.mov
Test USB Device
.DeviceNamein the spec.template.domain.devices.hostDevices.If you're not sure how to do it, please refer following demo recording.
usbdevice.mov