kubevirt
kubevirt copied to clipboard
kubevirt
More control over when DataVolumeTemplates are created
What this PR does
Before this PR:
Currently, virt-controller will create DataVolumeTemplates once the VM is created. Even if the VM is stopped. It also continually ensures that DataVolumeTemplates exist whenever a VM is reconciled. Even if the VM is stopped. This has a couple drawbacks:
- If the VM is never started, the underlying PVC (if on immediate bind StorageClass) was allocated unnecessarily
- It is hard for the user or third party backup software to replace a DataVolume's underlying PVC. See #11637
After this PR:
User may add kubevirt.io/immediate-data-volume-creation: "false" annotation to the VM and it will only reconcile DataVolumeTemplates when the VM is running
Fixes #11637
https://issues.redhat.com/browse/CNV-42076
Why we need it and why it was done in this way
The following tradeoffs were made:
The following alternatives were considered:
Links to places where the discussion took place:
Special notes for your reviewer
Checklist
This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR. Approvers are expected to review this list.
- [ ] Design: A design document was considered and is present (link) or not required
- [ ] PR: The PR description is expressive enough and will help future contributors
- [ ] Code: Write code that humans can understand and Keep it simple
- [ ] Refactor: You have left the code cleaner than you found it (Boy Scout Rule)
- [ ] Upgrade: Impact of this change on upgrade flows was considered and addressed if required
- [ ] Testing: New code requires new unit tests. New features and bug fixes require at least on e2e test
- [ ] Documentation: A user-guide update was considered and is present (link) or not required. You want a user-guide update if it's a user facing feature / API change.
- [ ] Community: Announcement to kubevirt-dev was considered
Release note
VM supports kubevirt.io/immediate-data-volume-creation: "false" which delays creating DataVolumeTemplates until VM is started
@mhenriks the code looks overall good. I agree also with your statement to for now keeping the legacy behavior.
It looks great /approve @fossedihelm can you please take a final look? /hold
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: alicefr
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [alicefr]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
Required labels detected, running phase 2 presubmits: /test pull-kubevirt-e2e-windows2016 /test pull-kubevirt-e2e-kind-1.27-vgpu /test pull-kubevirt-e2e-kind-sriov /test pull-kubevirt-e2e-k8s-1.30-ipv6-sig-network /test pull-kubevirt-e2e-k8s-1.28-sig-network /test pull-kubevirt-e2e-k8s-1.28-sig-storage /test pull-kubevirt-e2e-k8s-1.28-sig-compute /test pull-kubevirt-e2e-k8s-1.28-sig-operator /test pull-kubevirt-e2e-k8s-1.29-sig-network /test pull-kubevirt-e2e-k8s-1.29-sig-storage /test pull-kubevirt-e2e-k8s-1.29-sig-compute /test pull-kubevirt-e2e-k8s-1.29-sig-operator
@fossedihelm
Isn't this simulating a sort of wait for first consumer?
Sort of. If user adds kubevirt.io/immediate-data-volume-creation: "false" the DV (and therfore PVC) will not be created until the VM is started
With kubevirt.io/immediate-data-volume-creation: "true" (current default) the DV+PVC will be created immediately for Immediate` bind storage regardless of VM run strategy. With WFFC storage, only the DV will be created immedietly. PVC will get created when VM is started.
EDIT: so similar in that with WFFC or kubevirt.io/immediate-data-volume-creation: "false" the PVC will not be created until the VM is started
Does it collide with cdi.kubevirt.io/storage.bind.immediate.requested?
No cdi.kubevirt.io/storage.bind.immediate.requested is about overriding WFFC behavior by having CDI immediately create pods to consume WFFC PVCs. KubeVirt apis/annotations always respect WFFC
Are there any tests that check this?
Was thinking unit tests would be sufficient for now. If we change the default to kubevirt.io/immediate-data-volume-creation: "false" many functional tests will have to be updated. But testing kubevirt.io/immediate-data-volume-creation: "false" on it's own is an interesting challenge. How can we prove that the system won't create DVs when the VM is stopped?
/retest-required
This bot automatically retries required jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.
/retest-required
This bot automatically retries required jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.
@mhenriks: #12194 failed to apply on top of branch "release-1.2":
Applying: deepcopy resources from cache
Using index info to reconstruct a base tree...
M pkg/storage/types/dv.go
M pkg/storage/types/pvc.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/storage/types/pvc.go
Auto-merging pkg/storage/types/dv.go
Applying: handleDataVolumes should use cache rather than []*cdiv1.DataVolumes param
Using index info to reconstruct a base tree...
M pkg/virt-controller/watch/vm.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/virt-controller/watch/vm.go
CONFLICT (content): Merge conflict in pkg/virt-controller/watch/vm.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0002 handleDataVolumes should use cache rather than []*cdiv1.DataVolumes param
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
In response to this:
/cherry-pick release-1.2
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.
@mhenriks: new pull request created: #12555
In response to this:
/cherry-pick release-1.3
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.