Open
Conversation
Signed-off-by: akri3i <guptaakriti70@gmail.com>
Author
|
/wip |
WalkthroughA new test plan document is added for MIG vGPU functionality in OpenShift Virtualization, containing feature requirements, testing strategy, environment specifications, known limitations, and QE approval tracking. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. |
openshift-virtualization-qe-bot-2
requested review from
SiboWang1997,
dshchedr and
kbidarkar
openshift-virtualization-qe-bot-4
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
Comment on lines
+5
to
+13
| ### **Metadata & Tracking** | ||
|
|
||
| - **Enhancement(s):** [Links to enhancement(s); KubeVirt, OpenShift, etc.] | ||
| - **Feature Tracking:** https://redhat.atlassian.net/browse/VIRTSTRAT-166 | ||
| - **Epic Tracking:** https://redhat.atlassian.net/browse/CNV-13713 | ||
| <!-- Tasks must be created to block the feature --> | ||
| - **QE Owner(s):** [Name(s)] | ||
| - **Owning SIG:** sig-virt | ||
| - **Participating SIGs:** [List of participating SIGs] |
There was a problem hiding this comment.
Address VEP issue field mentioned in PR objectives.
The PR description states "STP Metadata: VEP issue field is present but not filled," but no VEP (Validated Enhancement Proposal) field appears in the Metadata & Tracking section. Please either:
- Add the VEP field with appropriate value/placeholder, or
- Clarify in the PR description if the VEP field is not applicable to this STP
🤖 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` around lines 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.
|
|
||
| ### **Metadata & Tracking** | ||
|
|
||
| - **Enhancement(s):** [Links to enhancement(s); KubeVirt, OpenShift, etc.] |
There was a problem hiding this comment.
Fill in the Enhancement(s) field or remove the placeholder.
The Enhancement(s) field contains placeholder text. If no OpenShift enhancement PR exists for this feature, reference the High-Level Design (HLD) document here. Based on learnings, it's acceptable to reference only the HLD when no enhancement PR exists.
🤖 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 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.
| - **Feature Tracking:** https://redhat.atlassian.net/browse/VIRTSTRAT-166 | ||
| - **Epic Tracking:** https://redhat.atlassian.net/browse/CNV-13713 | ||
| <!-- Tasks must be created to block the feature --> | ||
| - **QE Owner(s):** [Name(s)] |
There was a problem hiding this comment.
Assign QE Owner(s) before approval.
The QE Owner(s) field must be populated with the actual name(s) of the responsible QE engineers before this STP can be approved.
🤖 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 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.
| <!-- Tasks must be created to block the feature --> | ||
| - **QE Owner(s):** [Name(s)] | ||
| - **Owning SIG:** sig-virt | ||
| - **Participating SIGs:** [List of participating SIGs] |
There was a problem hiding this comment.
Specify Participating SIGs or remove if none.
The Participating SIGs field should list any other SIGs involved in this feature, or be removed/marked as "None" if sig-virt is the only participating SIG.
🤖 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 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.
Comment on lines
+70
to
+79
| - *Sign-off:* [Name/Date] | ||
|
|
||
| - **Only RHEL guest OS is validated ** | ||
| - *Sign-off:* [Name/Date] | ||
|
|
||
| - **MIG vGPU for Windows guests is only supported on vGPUs created on RTX Pro 6000 hardware; Windows MIG vGPU is not tested in this cycle as the available hardware is the A30** | ||
| - *Sign-off:* [Name/Date] | ||
|
|
||
| - **MIG vGPU configuration requires pre-configuration of the GPU node (MIG mode enabled, MIG profiles set) before VM scheduling** | ||
| - *Sign-off:* [Name/Date] |
There was a problem hiding this comment.
Complete sign-off fields for Known Limitations.
All Known Limitations require sign-off with name and date before this STP is approved. These sign-offs acknowledge the documented constraints.
🤖 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` around lines 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.
Comment on lines
+116
to
+141
| **Out of Scope (Testing Scope Exclusions)** | ||
|
|
||
| - **Legacy GPUs without MIG support** | ||
| - *Rationale:* Only Ampere and Hopper generation GPUs (e.g., A100, H100/H200) or later that support MIG are targeted; testing on non-MIG GPUs is not planned | ||
| - *PM/Lead Agreement:* [Name/Date] | ||
|
|
||
| - **Advanced multi-tenancy beyond GPU-level isolation** | ||
| - *Rationale:* Deep security isolation beyond MIG's hardware partitioning (e.g., vTPM integration) is not addressed in this feature | ||
| - *PM/Lead Agreement:* [Name/Date] | ||
|
|
||
| - **Custom MIG topologies beyond standard configurations** | ||
| - *Rationale:* Standard MIG slicing profiles recognized by the NVIDIA GPU Operator are assumed; custom or non-standard MIG topologies are not tested | ||
| - *PM/Lead Agreement:* [Name/Date] | ||
|
|
||
| - **Windows guest OS** | ||
| - *Rationale:* MIG vGPU for Windows is only supported on RTX Pro 6000 hardware; the available test hardware is the NVIDIA A30, which does not support Windows MIG vGPU | ||
| - *PM/Lead Agreement:* [Name/Date] | ||
|
|
||
| - **GPU benchmark / performance testing inside VMs** | ||
| - *Rationale:* No standardized GPU benchmark tooling integrated into CI; performance NFRs deferred to a future cycle | ||
| - *PM/Lead Agreement:* [Name/Date] | ||
|
|
||
| - **MIG profile configuration and GPU Operator installation** | ||
| - *Rationale:* Infrastructure pre-configuration is handled outside the test scope; tests assume a correctly configured GPU node | ||
| - *PM/Lead Agreement:* [Name/Date] | ||
|
|
There was a problem hiding this comment.
Complete PM/Lead Agreement sign-offs for out-of-scope items.
All out-of-scope items require PM/Lead sign-off (lines 120, 124, 128, 132, 136, 140) to ensure alignment on testing boundaries. These should be completed before final STP approval.
🤖 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` around lines 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.
Comment on lines
+142
to
+148
| **Test Limitations** | ||
|
|
||
| - **Testing is limited to the NVIDIA A30 GPU — other supported MIG-capable GPUs (e.g., A100, H100/H200) are not available in the test environment** | ||
| - *Sign-off:* [Name/Date] | ||
|
|
||
| - **MIG-capable GPU hardware (e.g., NVIDIA A100) must be available in the test cluster — tests cannot run on standard CI nodes** | ||
| - *Sign-off:* [Name/Date] |
There was a problem hiding this comment.
Complete sign-offs for Test Limitations.
Sign-off fields at lines 145 and 148 must be completed before final STP approval.
🤖 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` around lines 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.
| **Infrastructure** | ||
|
|
||
| - [ ] **Cloud Testing** | ||
| - *Details:* N/A — feature requires bare-metal nodes with MIG supported NVIDIA GPU hardware. |
There was a problem hiding this comment.
Hyphenate compound adjective.
"MIG supported" should be "MIG-supported" when used as a compound adjective modifying "NVIDIA GPU hardware."
📝 Proposed fix
- - *Details:* N/A — feature requires bare-metal nodes with MIG supported NVIDIA GPU hardware.
+ - *Details:* N/A — feature requires bare-metal nodes with MIG-supported NVIDIA GPU hardware.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - *Details:* N/A — feature requires bare-metal nodes with MIG supported NVIDIA GPU hardware. | |
| - *Details:* N/A — feature requires bare-metal nodes with MIG-supported NVIDIA GPU hardware. |
🧰 Tools
🪛 LanguageTool
[grammar] ~197-~197: Use a hyphen to join words.
Context: ...ature requires bare-metal nodes with MIG supported NVIDIA GPU hardware. #### **3...
(QB_NEW_EN_HYPHEN)
🤖 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 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.
|
|
||
| The following conditions must be met before testing can begin: | ||
|
|
||
| - [ ] Requirements and design documents are **approved and merged** |
There was a problem hiding this comment.
Requirements and design approval is a prerequisite.
Entry criterion "Requirements and design documents are approved and merged" is currently unchecked. This must be satisfied before testing begins.
🤖 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 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.
Comment on lines
+266
to
+268
| - **Risk:** Only one NVIDIA A30 GPU node exists in a single cluster; if the node or cluster is unavailable (e.g., hardware failure, cluster maintenance), all MIG vGPU testing is blocked with no fallback environment. | ||
| - **Mitigation:** None — no alternative GPU hardware or cluster is available; testing is fully dependent on this single node's availability. | ||
| - *Sign-off:* [Name/Date] |
There was a problem hiding this comment.
Complete sign-off for Test Environment risk.
The risk acknowledgment at line 268 requires sign-off with name and date before final approval.
🤖 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` around lines 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.
vsibirsk
reviewed
Apr 15, 2026
| - **Mitigation:** None — no alternative GPU hardware or cluster is available; testing is fully dependent on this single node's availability. | ||
| - *Sign-off:* [Name/Date] | ||
|
|
||
| ### **III. Test Scenarios & Traceability** |
There was a problem hiding this comment.
what about these:
- migrate vm?
- restart vm?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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