[Virt] Support MIG vgpu - #71
Conversation
|
/wip |
|
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:
📝 WalkthroughWalkthroughA new MIG vGPU Software Test Plan defines feature scope, QE review criteria, test strategy, environment requirements, limitations, risks, requirement traceability, and approval stakeholders for OpenShift Virtualization. ChangesMIG vGPU Test Plan
Estimated code review effort: 2 (Simple) | ~12 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
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
AI Features
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (1)
stps/sig-virt/mig-vgpu-stp.md (1)
219-219: Consider simplifying wording."prior to" can be simplified to "before" for more concise documentation.
♻️ Proposed simplification
-- **Special Configurations:** GPU node must have MIG mode enabled and appropriate MIG profiles configured prior to test execution +- **Special Configurations:** GPU node must have MIG mode enabled and appropriate MIG profiles configured before test execution🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-virt/mig-vgpu-stp.md` at line 219, Replace the phrase "prior to" with "before" in the sentence "**Special Configurations:** GPU node must have MIG mode enabled and appropriate MIG profiles configured prior to test execution" so it reads "...configured before test execution" to simplify wording and improve conciseness.
🤖 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-virt/mig-vgpu-stp.md`:
- Line 13: Update the "**Participating SIGs:**" field in the mig-vgpu-stp.md
document: either list the other SIGs participating (e.g., "sig-foo, sig-bar") if
there are collaborators, or remove the line entirely or replace it with "None"
when sig-virt is the only SIG; ensure the final text uses the exact
"**Participating SIGs:**" label so reviewers can find it easily.
- Around line 142-148: The "Test Limitations" section contains two unresolved
sign-off placeholders for the NVIDIA A30-specific limitation and the MIG-capable
GPU hardware requirement; complete both sign-off fields by replacing
"[Name/Date]" with the approver's full name and approval date for each bullet
(the bullet mentioning "NVIDIA A30 GPU" and the bullet mentioning "MIG-capable
GPU hardware (e.g., NVIDIA A100)"), ensuring the completed entries are accurate
and authoritative before final STP approval.
- Line 11: Populate the QE Owner(s) field in the STP header by replacing the
placeholder "[Name(s)]" with the actual responsible QE engineer name(s); update
the "QE Owner(s):" entry so it lists one or more real names (e.g., "QE Owner(s):
Jane Doe, John Smith") before approving the document.
- Around line 116-141: Populate the PM/Lead Agreement placeholders for each
out-of-scope bullet so the document records explicit sign-offs: add a named
approver and date in place of each "[Name/Date]" for the items "Legacy GPUs
without MIG support", "Advanced multi-tenancy beyond GPU-level isolation",
"Custom MIG topologies beyond standard configurations", "Windows guest OS", "GPU
benchmark / performance testing inside VMs", and "MIG profile configuration and
GPU Operator installation" to indicate formal acceptance of these exclusions.
- Around line 266-268: The Test Environment risk block currently lacks an
approver signature; update the risk acknowledgement paragraph (the "**Risk:**
Only one NVIDIA A30 GPU node..." / "**Mitigation:** None" block) to include a
completed sign-off by replacing "[Name/Date]" with the approver's full name and
the approval date in YYYY-MM-DD (or the project's standard date format),
ensuring the "*Sign-off:*" line reads e.g. "*Sign-off:* Alice Smith /
2026-04-06" so the document has a clear, traceable approval for the Test
Environment risk.
- Around line 70-79: The Known Limitations section contains placeholder
sign-offs "[Name/Date]" for each bullet (e.g., the lines "Only RHEL guest OS is
validated", "MIG vGPU for Windows guests is only supported on vGPUs created on
RTX Pro 6000 hardware...", and "MIG vGPU configuration requires
pre-configuration of the GPU node..."); replace each placeholder with the actual
reviewer/approver name and date to complete the sign-off fields so the STP can
be approved.
- Around line 5-13: The Metadata & Tracking section currently omits the VEP
field referenced in the PR objectives; either add a "VEP:" (or "VEP issue:")
entry under the "Metadata & Tracking" block with an appropriate value or
placeholder (e.g., "VEP: TBD" or the VEP number) so the text "STP Metadata: VEP
issue field is present but not filled" matches the document, or update the PR
description to explicitly state that a VEP is not applicable; locate and modify
the "Metadata & Tracking" section (the header and the list containing
Enhancement(s), Feature Tracking, Epic Tracking, QE Owner(s), Owning SIG,
Participating SIG) to include the new VEP field or change the PR description
accordingly.
- Line 7: Replace the placeholder line labeled "Enhancement(s):" with either the
link(s) to the relevant enhancement PR(s) (e.g., OpenShift/KubeVirt enhancement
URLs) or, if no enhancement exists, a link and brief citation to the High-Level
Design (HLD) document; update the "Enhancement(s):" field in the document so it
no longer contains placeholder text and includes the actual enhancement or HLD
reference.
- Line 243: Update the entry criterion checklist in mig-vgpu-stp.md by marking
the "Requirements and design documents are **approved and merged**" item as
satisfied: change the unchecked box "[ ] Requirements and design documents are
**approved and merged**" to a checked box "[x] Requirements and design documents
are **approved and merged**" and, if possible, add a brief reference (PR/MR
number or link) to the merged approval artifact so reviewers can verify the
prerequisite is met.
- Around line 81-102: The Technology and Design Review section has unchecked
items and placeholder text; update the checklist by marking the boxes as
completed where content is provided (check "Technology Challenges", "API
Extensions", and "Topology Considerations") and replace the placeholder in
"Developer Handoff/QE Kickoff" with a concise summary of handoff actions and QE
kickoff steps (who, what, and follow-ups), and either populate "Test Environment
Needs" with the referenced environment/tool details from Section II.3/II.3.1 or
add a short note that the item requires stakeholder confirmation; reference the
section headings "Developer Handoff/QE Kickoff", "Technology Challenges", "API
Extensions", "Test Environment Needs", and "Topology Considerations" when making
these edits.
- Line 197: Summary: The phrase "MIG supported NVIDIA GPU hardware" must be
hyphenated as a compound adjective. In the document string "MIG supported NVIDIA
GPU hardware" (found in mig-vgpu-stp.md near the details line), replace it with
"MIG-supported NVIDIA GPU hardware" so the compound adjective correctly modifies
"NVIDIA GPU hardware"; ensure any other occurrences of the exact phrase "MIG
supported" used as a modifier are updated the same way.
- Line 72: Fix the malformed bold markup on the line containing "Only RHEL guest
OS is validated" by removing the trailing `**` so the bold formatting is
balanced; update the line in mig-vgpu-stp.md from "**Only RHEL guest OS is
validated **" to either "**Only RHEL guest OS is validated**" (to keep bold) or
"Only RHEL guest OS is validated" (to remove bold) as appropriate.
---
Nitpick comments:
In `@stps/sig-virt/mig-vgpu-stp.md`:
- Line 219: Replace the phrase "prior to" with "before" in the sentence
"**Special Configurations:** GPU node must have MIG mode enabled and appropriate
MIG profiles configured prior to test execution" so it reads "...configured
before test execution" to simplify wording and improve conciseness.
🪄 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: 2b4c6e69-de0d-4785-9981-c46731afe75b
📒 Files selected for processing (1)
stps/sig-virt/mig-vgpu-stp.md
|
mtessun can not be added as reviewer. Reviews may only be requested from collaborators. One or more of the users or teams you specified is not a collaborator of the RedHatQE/openshift-virtualization-tests-design-docs repository.: 422 {"message": "Reviews may only be requested from collaborators. One or more of the users or teams you specified is not a collaborator of the RedHatQE/openshift-virtualization-tests-design-docs repository.", "documentation_url": "https://docs.github.com/rest/pulls/review-requests#request-reviewers-for-a-pull-request", "status": "422"} |
|
/wip cancel |
mtessun
left a comment
There was a problem hiding this comment.
Just close out the few comments about negative testscenarios and clarification what is NVIDIA and what is us.
|
Clean rebase detected — no code changes compared to previous head ( |
|
mtessun can not be added as reviewer. Reviews may only be requested from collaborators. One or more of the users or teams you specified is not a collaborator of the RedHatQE/openshift-virtualization-tests-design-docs repository.: 422 {"message": "Reviews may only be requested from collaborators. One or more of the users or teams you specified is not a collaborator of the RedHatQE/openshift-virtualization-tests-design-docs repository.", "documentation_url": "https://docs.github.com/rest/pulls/review-requests#request-reviewers-for-a-pull-request", "status": "422"} |
|
Clean rebase detected — no code changes compared to previous head ( |
|
mtessun can not be added as reviewer. Reviews may only be requested from collaborators. One or more of the users or teams you specified is not a collaborator of the RedHatQE/openshift-virtualization-tests-design-docs repository.: 422 {"message": "Reviews may only be requested from collaborators. One or more of the users or teams you specified is not a collaborator of the RedHatQE/openshift-virtualization-tests-design-docs repository.", "documentation_url": "https://docs.github.com/rest/pulls/review-requests#request-reviewers-for-a-pull-request", "status": "422"} |
1 similar comment
|
mtessun can not be added as reviewer. Reviews may only be requested from collaborators. One or more of the users or teams you specified is not a collaborator of the RedHatQE/openshift-virtualization-tests-design-docs repository.: 422 {"message": "Reviews may only be requested from collaborators. One or more of the users or teams you specified is not a collaborator of the RedHatQE/openshift-virtualization-tests-design-docs repository.", "documentation_url": "https://docs.github.com/rest/pulls/review-requests#request-reviewers-for-a-pull-request", "status": "422"} |
|
/approve |
|
/check-can-merge |
|
/lgtm |
|
/check-can-merge |
lyarwood
left a comment
There was a problem hiding this comment.
STP Review — AGENTS.md Checklist
Overall the STP is in solid shape after multiple rounds of review. The major prior issues (constraint categorization, missing PM approver, missing negative scenario, NVIDIA responsibility clarification) have all been addressed.
2 HIGH items remain (template compliance), 2 MEDIUM refinements. See inline comments for details.
Verdict: COMMENT — no blockers, but the Self-Validation Testing type and reviewer role labels should be addressed before merge.
|
mtessun can not be added as reviewer. Reviews may only be requested from collaborators. One or more of the users or teams you specified is not a collaborator of the RedHatQE/openshift-virtualization-tests-design-docs repository.: 422 {"message": "Reviews may only be requested from collaborators. One or more of the users or teams you specified is not a collaborator of the RedHatQE/openshift-virtualization-tests-design-docs repository.", "documentation_url": "https://docs.github.com/rest/pulls/review-requests#request-reviewers-for-a-pull-request", "status": "422"} |
|
Clean rebase detected — no code changes compared to previous head ( |
|
mtessun can not be added as reviewer. Reviews may only be requested from collaborators. One or more of the users or teams you specified is not a collaborator of the RedHatQE/openshift-virtualization-tests-design-docs repository.: 422 {"message": "Reviews may only be requested from collaborators. One or more of the users or teams you specified is not a collaborator of the RedHatQE/openshift-virtualization-tests-design-docs repository.", "documentation_url": "https://docs.github.com/rest/pulls/review-requests#request-reviewers-for-a-pull-request", "status": "422"} |
Signed-off-by: akri3i <guptaakriti70@gmail.com>
|
mtessun can not be added as reviewer. Reviews may only be requested from collaborators. One or more of the users or teams you specified is not a collaborator of the RedHatQE/openshift-virtualization-tests-design-docs repository.: 422 {"message": "Reviews may only be requested from collaborators. One or more of the users or teams you specified is not a collaborator of the RedHatQE/openshift-virtualization-tests-design-docs repository.", "documentation_url": "https://docs.github.com/rest/pulls/review-requests#request-reviewers-for-a-pull-request", "status": "422"} |
|
Clean rebase detected — no code changes compared to previous head ( |
|
mtessun can not be added as reviewer. Reviews may only be requested from collaborators. One or more of the users or teams you specified is not a collaborator of the RedHatQE/openshift-virtualization-tests-design-docs repository.: 422 {"message": "Reviews may only be requested from collaborators. One or more of the users or teams you specified is not a collaborator of the RedHatQE/openshift-virtualization-tests-design-docs repository.", "documentation_url": "https://docs.github.com/rest/pulls/review-requests#request-reviewers-for-a-pull-request", "status": "422"} |
|
/check-can-merge |
|
/lgtm |
|
/approve |
|
/lgtm |
STP Metadata
VEP issue:
What this PR does
Adds the Software Test Plan (STP) for the MIG vGPU feature, planning tests for VMs Running with MIG backed VGPU
Special notes for your reviewer
Summary by CodeRabbit