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

feat: check usb/pci/gpu resource name (backport #100) #102

Closed
wants to merge 1 commit into from

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented Jan 15, 2025

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 .DeviceName in VM spec while creating or updating. BTW, vGPU device is in the other field, so we need to check it in spec.template.domain.devices.gpus.

.DeviceName means resource name in k8s.

Related Issue:
harvester/harvester#6542

Test plan:

Test PCI Device

  1. Enable pcidevices-controller Add-on
  2. Enable one of PCI devices
  3. Create a VM and attach previous PCI device
  4. Click "Edit as YAML" button to manually edit YAML file, then click "Create" button.
  5. Modify the .DeviceName in 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

  1. Enable pcidevices-controller Add-on
  2. Enable one of USB devices
  3. Create a VM and attach previous USB device
  4. Click "Edit as YAML" button to manually edit YAML file, then click "Create" button.
  5. Modify the .DeviceName in the spec.template.domain.devices.hostDevices.

If you're not sure how to do it, please refer following demo recording.

usbdevice.mov

This is an automatic backport of pull request #100 done by Mergify.

@mergify mergify bot added the conflicts label Jan 15, 2025
Copy link
Author

mergify bot commented Jan 15, 2025

Cherry-pick of da16588 has failed:

On branch mergify/bp/v1.3/pr-100
Your branch is up to date with 'origin/v1.3'.

You are currently cherry-picking commit da16588.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   pkg/util/fakeclients/pcidevices.go

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
	both modified:   pkg/util/common/common.go
	deleted by us:   pkg/util/fakeclients/usbdevice.go
	both modified:   pkg/webhook/indexer.go
	both modified:   pkg/webhook/mutation.go
	both modified:   pkg/webhook/validation.go
	deleted by us:   pkg/webhook/vm_validataion_test.go
	deleted by us:   pkg/webhook/vm_validatation.go

Cherry-pick of 7b7c011 has failed:

On branch mergify/bp/v1.3/pr-100
Your branch is ahead of 'origin/v1.3' by 1 commit.
  (use "git push" to publish your local commits)

You are currently cherry-picking commit 7b7c011.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   pkg/util/fakeclients/vgpudevice.go
	modified:   pkg/util/gpuhelper/gpuhelper.go
	modified:   pkg/webhook/validation.go
	modified:   pkg/webhook/vm_validataion_test.go
	modified:   pkg/webhook/vm_validatation.go

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   pkg/util/common/common.go
	both modified:   pkg/webhook/indexer.go

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@Yu-Jack Yu-Jack force-pushed the mergify/bp/v1.3/pr-100 branch from 1d7029f to eacf768 Compare January 15, 2025 08:07
@Yu-Jack
Copy link
Contributor

Yu-Jack commented Jan 15, 2025

Since v1.3 doesn't have USB device, so I remove most USB device codes.

@Yu-Jack
Copy link
Contributor

Yu-Jack commented Jan 15, 2025

okay ... I thought we need to backport this. It turns out the labels are used to test harvester/bot ...

@Yu-Jack Yu-Jack closed this Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant