[Infra] virtual machine template STP#34
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new Software Test Plan document detailing test scope, scenarios, environment requirements, entry criteria, risks, and GA readiness checks for native in-cluster VirtualMachineTemplate and VirtualMachineTemplateRequest functionality (feature-gated Template support). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
6ad3f93 to
ff38850
Compare
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@stps/sig-infra/virtual-machine-template.md`:
- Line 120: Update the table row containing the "Compute Resources" entry so the
environment requirement uses the proper noun capitalization: change the cell
text "Sufficient for windows VM and snapshot/clone operations." to "Sufficient
for Windows VM and snapshot/clone operations." Locate the row that starts with
"**Compute Resources**" in virtual-machine-template.md and replace the lowercase
"windows" with "Windows" to correct the casing.
- Around line 74-84: Ensure priority tiers are consistent between the "Testing
Goals" list and the traceability matrix by picking a single priority for each
scenario and updating both sections to match; specifically reconcile entries
such as "Verify virt-operator deploys virt-template", "Verify cross-namespace:
template in namespace A, VM creation in namespace B", and other duplicated
scenarios (e.g., virtctl template convert/create, golden-image template flow) so
each scenario appears once with the agreed priority in both the Testing Goals
and the traceability matrix; update the priority tags (P0/P1/P2) for the
duplicated items and run a diff to confirm no mismatches remain.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 63370dec-8dd4-40e9-b1da-760b84ee2256
📒 Files selected for processing (1)
stps/sig-infra/virtual-machine-template.md
ff38850 to
a3b2a0e
Compare
|
I have few comments
|
a3b2a0e to
6c89aa2
Compare
f06c1a9 to
8953f3b
Compare
8953f3b to
f801e93
Compare
f801e93 to
84b2f9f
Compare
rnetser
left a comment
There was a problem hiding this comment.
I havne't reviwed the stp but from an overview it contains low level implementation details; an stp is a high level plan
the text should be phrased from end user perspective and not reference CRD names etc
for example
Verify virtctl template create: generates a VirtualMachineTemplateRequest from existing VM; async flow completes and template is ready for process/create.
an end user does not care about these details; As an admin, I can create VM templates from the cli or something similar.
| - *PM/Lead Agreement:* [Dominik Holler] / 1.5.2026 | ||
|
|
||
| - **UI testing** | ||
| - *Rationale:* Good to have at Tech Preview stage. |
There was a problem hiding this comment.
not sure what it means; will there be ui testing? is the ui team involved?
There was a problem hiding this comment.
We already have some, I removed any mention to UI
| - *Priority:* P2 | ||
|
|
||
| - **[CNV-73392]** — As a cluster admin, I want template workflows to recover after I re-enable the Template capability | ||
| - *Test Scenario:* [Tier 2] Enable the capability and create baseline templates and capture requests; disable and re-enable the capability; confirm existing objects are handled predictably and supported workflows work again. |
There was a problem hiding this comment.
confirm existing objects are handled predictably and supported workflows work again. - this is low level
should be something like create a new VM from a template successfully" - btw - why is this tier2?
There was a problem hiding this comment.
This is about disabling and enabling the feature repeatedly
Assisted-by: Cursor Signed-off-by: rkishner <rkishner@redhat.com>
for more information, see https://pre-commit.ci
| #### **1. Requirement & User Story** | ||
|
|
||
| - [x] **Review Requirements** | ||
| - Native data source and existing-VM templating, cross-namespace creation, RBAC for template/VM creation. |
There was a problem hiding this comment.
please rephrase as requirements review summary
e.g
capturing templates from existing VMs
the requirement for each item is not clear
| - A cluster user owner can create a VM from a native in-cluster template. | ||
| - A cluster user can share native in-cluster templates between namespaces they control. | ||
| - A cluster user can still create VMs from OpenShift Templates in the web console, VM templates are available alongside those options and do not remove or replace them. | ||
|
|
There was a problem hiding this comment.
RBAC-related acceptance criteria is missing
|
|
||
| - [x] **Non-Functional Requirements (NFRs)** | ||
| - **Security:** Verify template processing and VM creation flows do not pass Secrets or ConfigMaps unintentionally to created VMs. | ||
| - **Performance/Scalability:** Verify a large quantity of VMs can be created from native VM templates without failures, excessive delays, or service instability. |
There was a problem hiding this comment.
the out of scope includes perf as something that will not be tested. please add a ref here to this section
| - **Performance/Scalability:** Verify a large quantity of VMs can be created from native VM templates without failures, excessive delays, or service instability. | ||
| - **Monitoring:** Verify no unexpected critical alerts are introduced during template processing and bulk VM creation workflows. | ||
| - *Note any NFRs not covered and why:* | ||
| - UI: updated templates display is validated by the UI team. |
There was a problem hiding this comment.
please add a link to the ui epic
| - The feature is deployed by the cluster operator via a feature gate in 4.22.0, and plans to be deployed by default on 4.23.0+. | ||
| - *Impact on testing approach:* All `virtctl template` and API tests run on **CNV 4.22.0+** clusters with the feature gate active. | ||
|
|
||
| - [x] **API Extensions** |
There was a problem hiding this comment.
this comment was not addressed
e.g Feature-toggle tests are required to verify expected behavior when template workflows are enabled, disabled, and re-enabled. is not related to this section and is already mentioned above
| - **OpenShift Virtualization 4.21** | ||
| - `virt-operator` can be installed manually for early integration and developer-level validation. |
There was a problem hiding this comment.
why is this related to the stp? is the team planning on testing 4.21?
| - **Backporting approach for this feature** | ||
| - New feature scope is delivered in new versions (for example, 4.22 -> 5.0), based on the VEP implementation roadmap. | ||
| - Backports to earlier versions are expected to be limited to selected high-priority fixes, not new user-facing capabilities. |
There was a problem hiding this comment.
there's already a section about feature maturity
and this describes a generic backport approach
please keep the stp simple and remove anything that is not directly tied to the feature
| #### **SIG responsibilities** | ||
|
|
||
| - **sig-infra** — VirtualMachineTemplate and VirtualMachineTemplateRequest API behavior, and cluster operator deployment of the template feature when the Template feature gate is enabled. | ||
|
|
There was a problem hiding this comment.
this can be dropped as the stp is only for the infra team (there are no other sigs involved - based on the metadata above)
| - **[P0]** As a cluster admin, when I enable the Template capability on the cluster, I can define templates, process them with parameters, and run the supported create and capture workflows end to end. | ||
| - **[P0]** As a template author, I can save reusable VM designs and supply parameters so new VMs receive the intended CPU, memory, disk, and boot configuration reflected in the resulting workload. | ||
| - **[P0]** As a VM owner, after I ask the cluster to capture a template from a VM I am allowed to manage, I see a clear success or failure outcome with an understandable message; on success I can use the new template in the expected namespace. | ||
| - **[P0]** As a VM owner, I can create a runnable VM from a template using supported automation (declarative apply and CLI-based “render then create” flows), in the intended namespace, with correct boot and storage. | ||
| - **[P0]** As a VM owner, I can create a VM from a template that points at a namespaced boot source and get correct storage and boot behavior. | ||
| - **[P0]** As a cluster admin, I can restrict which users may create VMs from which templates; template owners and VM owners see permissions that match the enhancement’s access model. | ||
| - **[P1]** As a template owner, I can convert an existing OpenShift Template that contains a VM into a native VM template without losing parameters customers rely on. | ||
| - **[P1]** As a VM owner or admin, I can start template-from-VM capture from the CLI for a VM I manage; when the operation finishes, the new template is ready to process and to create VMs from. | ||
| - **[P1]** As a VM owner, templates honor sizing profiles and preferences so VMs created from the template pick up the chosen shapes and defaults. | ||
| - **[P1]** As a Windows VM owner, I can build a Windows-oriented template and create a new guest that boots with the expected disks, network, and Windows-specific settings. | ||
| - **[P1]** As a cluster admin, templates and in-flight capture requests survive cluster upgrade and remain usable afterward. | ||
| - **[P2]** As a VM owner, I can keep a template in one namespace and create a VM in another namespace where cross-namespace creation is supported (for example via rendered manifests applied with standard tooling). | ||
| - **[P2]** As a cluster admin, when required parameters have defaults, the cluster rejects template definitions that would always produce an invalid VM, while still accepting sound templates. | ||
| - **[P2]** As a cluster admin, when the Template capability is turned off, customers cannot start new template workflows until it is turned back on. | ||
| - **[P2]** As a cluster admin, when I disable and re-enable the Template capability, existing templates and capture requests are handled predictably and supported workflows work again after re-enable. |
There was a problem hiding this comment.
testing goals should not be a 1-1 to test scnerios.
proposed consolidation (done by claude, please check):
- **[P0]** As a VM owner, I can create a runnable VM from a template
backed by a boot source, with correct CPU, memory, disk, and boot
configuration — using both declarative and CLI-driven flows.
- **[P0]** As a VM owner, I can capture a template from an existing VM
and see clear success or failure; on success the template is reusable
in the expected namespace.
- **[P0]** As a cluster admin, I can restrict which users may create or
capture templates and which may create VMs from them.
- **[P1]** As a template owner, I can convert an OpenShift Template that
contains a VM into a native VM template without losing customer-facing
parameters.
- **[P1]** As a VM owner, templates honor sizing profiles and preferences
so created VMs pick up the chosen shapes and defaults.
- **[P1]** As a Windows VM owner, I can build a Windows-oriented template
and create a guest that boots with expected disks, network, and
Windows-specific settings.
- **[P1]** As a cluster admin, templates and in-flight capture requests
survive cluster upgrade and remain usable afterward.
- **[P2]** As a VM owner, I can use a template from another namespace to
create a VM where cross-namespace creation is supported.
- **[P2]** As a cluster admin, when I disable and re-enable the Template
capability, workflows are blocked while disabled and recover
predictably after re-enable; the cluster rejects template definitions
that would always produce an invalid VM.
STP Metadata
VEP issue: https://github.com/kubevirt/enhancements/blob/main/veps/sig-compute/76-vmtemplates/vmtemplate-proposal.md
What this PR does
Add STP for new virtual machine template CRD
DCO
Signed-off-by: Roni Kishner rkishner@redhat.com
Summary by CodeRabbit