stp, sig-virt: Introduce the AllowWorkloadDisruption migration STP - #82
stp, sig-virt: Introduce the AllowWorkloadDisruption migration STP#82SamAlber wants to merge 3 commits into
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:
📝 WalkthroughWalkthroughAdded a new Software Test Plan at ChangesAWD Migration Quality Plan
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
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. |
|
SiboWang1997 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"} |
|
jerry7z 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"} |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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/workload-disruption-migration.md`:
- Line 175: Update the PM approver line that currently only lists an email
("Product Manager/Owner: [Ronen Sde-Or](ronen@redhat.com)") to include the
GitHub handle per STP Section IV requirements; locate the "Product
Manager/Owner" entry in workload-disruption-migration.md and replace or augment
the email-only link with the approver's GitHub handle (e.g., include the
`@github-username` or a link to their GitHub profile) so the entry contains both
the name and GitHub handle.
- Around line 46-49: The Known Limitations section under the heading "#### **2.
Known Limitations**" currently lists s390x memory hotplug but lacks the
mandatory sign-off; add an explicit sign-off line for that limitation in the
form "*Sign-off:* [Name/Date]" or replace it with the required phrase "None —
reviewed and confirmed by [Name/Date]" if appropriate, and do the same for the
Test Limitations block referenced around lines 86-88 (ensure each listed
limitation has its own sign-off or the explicit "None — reviewed and confirmed…"
statement) so the STP sections I.2 and II.1 comply with the sign-off guideline.
- Around line 24-30: Summary: The paragraph uses internal API/field names
(allowWorkloadDisruption, MigrationPolicy, allowPostCopy) and implementation
mechanics (precopy, PostCopy, Paused) instead of user-facing behavior; rewrite
to describe only observable user experience and acceptance criteria.
Instructions: Remove all internal identifiers and implementation terms (e.g.,
allowWorkloadDisruption, MigrationPolicy, allowPostCopy, precopy, PostCopy,
Paused) and replace with plain-language descriptions of what users will see
(e.g., "migration may use more disruptive methods to ensure completion", "VM may
be briefly stunned or live state resumed on the destination", "migration will
complete even if memory is rapidly changing"); update acceptance criteria to be
product-observable (e.g., successful node drain completes, VM state visible as
paused or resumed to users, migration completes rather than failing) and apply
the same rewrite style at the other referenced locations (lines ~39, ~57, ~137).
- Around line 41-45: Convert the two items under the "Customer Use Cases"
section into user-story format (e.g., "As an admin, I want to drain a node for
maintenance so that VMs with AWD policy migrate using PostCopy or Paused mode
and resume without process loss" and "As an admin, I want to hotplug CPU/memory
to a running VM so AWD migration reflects new resources without process loss"),
and update the "Non-Functional Requirements (NFRs)" section to explicitly list
and address Monitoring, Observability, UI, Documentation, Performance, Security,
and Scalability (each with a short measurable acceptance criterion or owner),
instead of deferring them to section II.3, so reviewers can clearly verify NFR
coverage.
- Around line 141-149: The Risks table in workload-disruption-migration.md is
missing the required "Other" risk category and lacks explicit sign-off details
for each risk; update the table to include a seventh row labeled "Other" with a
clear risk description and mitigation strategy, and add a "Sign-off" column (or
append sign-off cells) for every risk row containing the approver's name/role
and date (or a placeholder like "TBD by [ROLE] - YYYY-MM-DD") so each risk entry
includes Risk description, Mitigation strategy, and Sign-off; ensure you edit
the existing table header and rows (the risk table block around the Risk
Category/Test Coverage/Test Environment entries) to add these fields
consistently.
- Around line 92-107: The Test Strategy table is missing a "Scale Testing" row
and the "Upgrade Testing" row lacks an explicit statement about upgrade path
evaluation; add a new table row labeled "Scale Testing" with Description
covering scale/volume considerations and a clear Applicable (Y/N or N/A) and
Comments noting planned approach or deferred rationale, and update the existing
"Upgrade Testing" row (the cell under Description or Comments for "Upgrade
Testing") to explicitly state whether the upgrade path was evaluated or deferred
and include any mitigation/plan; locate and edit the rows named "Scale Testing"
and "Upgrade Testing" in the existing markdown table (the entries currently
titled "Upgrade Testing" and the nearby "Performance Testing") to make these
additions/clarifications.
- Around line 79-85: Replace the four TODO PM/Lead Agreement entries in the
Out-of-Scope table for the rows containing "Windows node drain", "Windows memory
hotplug", "s390x memory hotplug", and "Performance / scale testing" with a
proper "PM/Lead Agreement" value including the approver's name and date (e.g.,
"Name / YYYY-MM-DD"), and convert the table into the required itemized format by
ensuring each item has explicit Rationale and PM/Lead Agreement fields (move the
rationale text into the Rationale field and replace each TODO with the name/date
agreement), so the Out-of-Scope section meets the STP Section II.1 requirements.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 090197de-e007-45a6-80c5-dbbc6fba13a7
📒 Files selected for processing (1)
stps/sig-virt/workload-disruption-migration.md
| | **Customer Use Cases** | [x] | 1. Admin drains a node for maintenance; VMs with AWD policy migrate using PostCopy or Paused mode and resume without process loss.<br>2. Admin hotplugs CPU/memory to a running VM; the resulting migration uses AWD mode and the guest reflects the new resources without process loss. | | | ||
| | **Testability** | [x] | Testable by configuring a MigrationPolicy with tight `completionTimeoutPerGiB` and capped bandwidth, then verifying migration mode and guest process PID after migration. | | | ||
| | **Acceptance Criteria** | [x] | 1. Migration completes in the expected mode (PostCopy or Paused) under AWD policy.<br>2. Guest background process PID is preserved after migration.<br>3. Hotplugged CPU/memory is reflected in the guest OS after AWD migration.<br>4. Both RHEL and Windows guests are supported. | | | ||
| | **Non-Functional Requirements (NFRs)** | [x] | See section **II.3 - Test Environment** | | | ||
|
|
There was a problem hiding this comment.
HIGH: Section I.1 does not meet required requirement/NFR structure.
- Line 41 customer use cases are not in user story format (
As a [role], I want...). - Line 44 NFRs are not explicitly covered (Monitoring, Observability, UI, Documentation, Performance, Security, Scalability) and are deferred to another section.
As per coding guidelines, "Customer use cases must be in user story format" and "NFRs must explicitly address Monitoring, Observability, UI, Documentation, Performance, Security, and Scalability".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stps/sig-virt/workload-disruption-migration.md` around lines 41 - 45, Convert
the two items under the "Customer Use Cases" section into user-story format
(e.g., "As an admin, I want to drain a node for maintenance so that VMs with AWD
policy migrate using PostCopy or Paused mode and resume without process loss"
and "As an admin, I want to hotplug CPU/memory to a running VM so AWD migration
reflects new resources without process loss"), and update the "Non-Functional
Requirements (NFRs)" section to explicitly list and address Monitoring,
Observability, UI, Documentation, Performance, Security, and Scalability (each
with a short measurable acceptance criterion or owner), instead of deferring
them to section II.3, so reviewers can clearly verify NFR coverage.
✅ Addressed in commit f77cf90
There was a problem hiding this comment.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="stps/sig-virt/workload-disruption-migration.md"
printf '%s\n' '--- Current relevant sections ---'
rg -n -A14 -B3 \
'Customer Use Cases|Non-Functional Requirements|Monitoring|Observability|Documentation|Performance|Security|Scalability|UI' \
"$file" || true
printf '%s\n' '--- Commit f77cf90 summary and relevant diff ---'
git show --no-ext-diff --format=fuller --stat f77cf90 -- "$file"
git show --no-ext-diff --format= --unified=12 f77cf90 -- "$file"Length of output: 7103
@openshift-virtualization-qe-bot-3, the current STP addresses this finding.
- Lines 51-54 use the required user-story format: “As an admin, I want … so that …”.
- Lines 67-73 explicitly address all required NFR categories: Monitoring, Observability, UI, Documentation, Performance, Security, and Scalability.
- Each NFR states the applicable constraint, inherited coverage, or rationale for no new feature-specific requirement.
The repository snapshot does not contain commit f77cf90, so I could not verify that commit directly. The current document content is sufficient to resolve the original HIGH finding.
🧠 Learnings used
Learnt from: RoniKishner
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 34
File: stps/sig-infra/virtual-machine-template.md:64-69
Timestamp: 2026-05-27T18:26:25.780Z
Learning: When reviewing STP documents in the `stps/` directory, do not require NFR categories like Monitoring, Performance, and Scalability to be re-enumerated in the Section I.1 “Non-Functional Requirements (NFRs)” block if they are already explicitly addressed (with justification) elsewhere in the same STP—specifically in a dedicated “Out of Scope” section. In this case, treating duplication as redundant is acceptable. Also, Observability may be treated as a subset of Monitoring rather than a separate NFR category. Do not flag the I.1 NFR block as incomplete solely because these categories are missing from it when they are covered elsewhere in the document.
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 142
File: stps/sig-virt/dual-stream-cluster-rhcos9-rhcos10/stp.md:82-82
Timestamp: 2026-08-04T10:39:51.886Z
Learning: In STP NFR sections, a UI testing category may be marked N/A without separate PM agreement only when the document explicitly states that the feature introduces no UI changes and that UI testing would provide no customer value. Treat these statements as the required justification for the non-applicable NFR category.
You are interacting with an AI system.
| #### **2. Known Limitations** | ||
|
|
||
| - s390x does not support maxGuest memory, so memory hotplug is not available on this architecture. | ||
|
|
There was a problem hiding this comment.
HIGH: Limitations sections are missing mandatory sign-offs.
Known Limitations and Test Limitations must include explicit sign-off lines (*Sign-off:* [Name/Date]) or the required “None — reviewed and confirmed…” statement.
As per coding guidelines, "STP Section I.2 Known Limitations: Each limitation must have a sign-off..." and "STP Section II.1 Test Limitations: Each limitation must have sign-off...".
Also applies to: 86-88
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stps/sig-virt/workload-disruption-migration.md` around lines 46 - 49, The
Known Limitations section under the heading "#### **2. Known Limitations**"
currently lists s390x memory hotplug but lacks the mandatory sign-off; add an
explicit sign-off line for that limitation in the form "*Sign-off:* [Name/Date]"
or replace it with the required phrase "None — reviewed and confirmed by
[Name/Date]" if appropriate, and do the same for the Test Limitations block
referenced around lines 86-88 (ensure each listed limitation has its own
sign-off or the explicit "None — reviewed and confirmed…" statement) so the STP
sections I.2 and II.1 comply with the sign-off guideline.
✅ Addressed in commit 5e1f80a
There was a problem hiding this comment.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -HI -t f '^(AGENTS\.md|\.markdownlint\.yaml|workload-disruption-migration\.md)$' . | sort
printf '\n--- Markdown heading map ---\n'
ast-grep outline stps/sig-virt/workload-disruption-migration.md --lang markdown --items all 2>/dev/null || true
printf '\n--- Limitation sections ---\n'
rg -n -A8 -B2 'Known Limitations|Test Limitations|Sign-off' stps/sig-virt/workload-disruption-migration.md
printf '\n--- Guidance references ---\n'
rg -n -i -C2 'known limitations|test limitations|sign-off' AGENTS.md .markdownlint.yaml 2>/dev/null || trueLength of output: 13355
@openshift-virtualization-qe-bot-3, HIGH finding addressed.
The current STP now has a dated *Sign-off:* entry for every listed constraint:
- Known Limitations: Lines 80–87 list three product constraints. Each constraint has a named, dated sign-off.
- Test Limitations: Lines 144–145 list the Paused-mode triggering constraint. It has a named, dated sign-off.
This satisfies the sign-off requirements in AGENTS.md for Sections I.2 and II.1. The limitation entries are also distinct from Out of Scope items and Risks.
🧠 Learnings used
Learnt from: kshvaika
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 106
File: stps/sig-virt/dual-stream-cluster-rhcos9-rhcos10/storage.md:139-142
Timestamp: 2026-05-25T09:54:44.096Z
Learning: In STP documents under `stps/`, for Section II.1 “Test Limitations” and similarly named placeholder/known-limitation sections (e.g., “Known Limitations” / “None”), do not require a sign-off line of the form `*Sign-off:* [Name/Date]` when the section is only a bare “None” statement. Require an explicit sign-off only when actual limitations/test constraints are listed that need reviewer acknowledgment; if limitations are present but the sign-off is missing, it should be flagged.
@openshift-virtualization-qe-bot-3 Thanks for confirming the fix. I couldn't resolve this review thread on the repository platform, so it remains open. Please retry or resolve it manually.
You are interacting with an AI system.
| | Item | Description | Applicable (Y/N or N/A) | Comments | | ||
| |:-------------------------------|:-------------------------------------------------------------------------------------------------------------------------------------------------------------|:------------------------|:--------------------------------------------------------------------------------------------------------------------------| | ||
| | Functional Testing | Validates that the feature works according to specified requirements and user stories | Y | | | ||
| | Automation Testing | Ensures test cases are automated for continuous integration and regression coverage | Y | | | ||
| | Performance Testing | Validates feature performance meets requirements (latency, throughput, resource usage) | N | Not in scope for functional AWD validation | | ||
| | Security Testing | Verifies security requirements, RBAC, authentication, authorization, and vulnerability scanning | N/A | MigrationPolicy RBAC is covered by core KubeVirt tests | | ||
| | Usability Testing | Validates user experience, UI/UX consistency, and accessibility requirements. Does the feature require UI? If so, ensure the UI aligns with the requirements | N/A | No UI component; migration mode is reported via standard VMI status fields | | ||
| | Compatibility Testing | Ensures feature works across supported platforms, versions, and configurations | Y | Parametrized across RHEL and Windows guest OSes | | ||
| | Regression Testing | Verifies that new changes do not break existing functionality | Y | Tests validate both migration mode and hotplug functionality | | ||
| | Upgrade Testing | Validates upgrade paths from previous versions, data migration, and configuration preservation | N | Not currently in scope | | ||
| | Backward Compatibility Testing | Ensures feature maintains compatibility with previous API versions and configurations | N/A | No known API changes affecting backward compatibility | | ||
| | Dependencies | Dependent on deliverables from other components/products? Identify what is tested by which team. | Y | Core AWD functionality is implemented in KubeVirt | | ||
| | Cross Integrations | Does the feature affect other features/require testing by other components? Identify what is tested by which team. | Y | AWD interacts with hotplug and node drain (sig-virt); tests cover migration triggered by both | | ||
| | Monitoring | Does the feature require metrics and/or alerts? | N/A | No specific metrics required for AWD | | ||
| | Cloud Testing | Does the feature require multi-cloud platform testing? Consider cloud-specific features. | N/A | Bare metal with RWX storage is required | | ||
|
|
There was a problem hiding this comment.
HIGH: Test Strategy is missing required coverage dimensions.
A dedicated Scale Testing entry is missing, and Upgrade testing does not explicitly state that the upgrade path was evaluated (even if deferred).
As per coding guidelines, "STP Section II.2 Test Strategy: All 13 testing types must be present and addressed..." and "Upgrade Testing must confirm upgrade path was evaluated; Performance/Scale must document consideration and plans even if deferred".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stps/sig-virt/workload-disruption-migration.md` around lines 92 - 107, The
Test Strategy table is missing a "Scale Testing" row and the "Upgrade Testing"
row lacks an explicit statement about upgrade path evaluation; add a new table
row labeled "Scale Testing" with Description covering scale/volume
considerations and a clear Applicable (Y/N or N/A) and Comments noting planned
approach or deferred rationale, and update the existing "Upgrade Testing" row
(the cell under Description or Comments for "Upgrade Testing") to explicitly
state whether the upgrade path was evaluated or deferred and include any
mitigation/plan; locate and edit the rows named "Scale Testing" and "Upgrade
Testing" in the existing markdown table (the entries currently titled "Upgrade
Testing" and the nearby "Performance Testing") to make these
additions/clarifications.
✅ Addressed in commit 5090f82
There was a problem hiding this comment.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="stps/sig-virt/workload-disruption-migration.md"
printf '%s\n' '--- Current Test Strategy section ---'
rg -n -A24 -B2 '^#### \*\*2\. Test Strategy\*\*' "$file" || true
printf '%s\n' '--- Referenced commit summary ---'
git show --no-ext-diff --format=fuller --stat 5090f82 -- "$file" || true
printf '%s\n' '--- Referenced commit patch for the affected rows ---'
git show --no-ext-diff --format= --unified=5 5090f82 -- "$file" | \
rg -n -A8 -B8 'Scale Testing|Upgrade Testing|Performance Testing' || trueLength of output: 2537
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="stps/sig-virt/workload-disruption-migration.md"
sed -n '161,230p' "$file"
printf '%s\n' '--- Upgrade Testing matches ---'
rg -n -i -A3 -B2 'upgrade testing|upgrade path|upgrade' "$file" || trueLength of output: 4962
@openshift-virtualization-qe-bot-3, verification shows that the current STP includes both requested updates:
- Scale Testing is present with a deferred-testing rationale.
- Upgrade Testing explicitly states that the upgrade path was evaluated and that no AWD-specific concerns were identified.
🧠 Learnings used
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 34
File: stps/sig-infra/virtual-machine-template.md:133-168
Timestamp: 2026-06-02T10:06:00.601Z
Learning: When reviewing any STP document under `stps/`, check Section II.2 (“Test Strategy”). It must explicitly enumerate every testing type required by the STP template. No testing type may be omitted: if a testing type is not applicable, it still must be listed and accompanied by a clear justification/rationale. Do not treat Section II.2 as complete if any required testing types are missing—missing entries indicate the template’s testing types were not documented as considered (a review violation).
You are interacting with an AI system.
| * **Approvers:** | ||
| - QE Architect (OCP-V): [Ruth Netser](@rnetser) | ||
| - Principal QE (OCP-V): [Den Shchedrivyi](@dshchedr), [Vasiliy Sibirskiy](@vsibirsk) | ||
| - Product Manager/Owner: [Ronen Sde-Or](ronen@redhat.com) |
There was a problem hiding this comment.
MEDIUM: PM approver should include a GitHub handle, not only email.
Section IV requires approvers with names and GitHub handles.
As per coding guidelines, "STP Section IV Sign-off and Approval: Reviewers and Approvers must be listed with names and GitHub handles...".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stps/sig-virt/workload-disruption-migration.md` at line 175, Update the PM
approver line that currently only lists an email ("Product Manager/Owner: [Ronen
Sde-Or](ronen@redhat.com)") to include the GitHub handle per STP Section IV
requirements; locate the "Product Manager/Owner" entry in
workload-disruption-migration.md and replace or augment the email-only link with
the approver's GitHub handle (e.g., include the `@github-username` or a link to
their GitHub profile) so the entry contains both the name and GitHub handle.
✅ Addressed in commit 7f3a3f4
There was a problem hiding this comment.
There was a problem hiding this comment.
Post @coderabbitai resolve or @coderabbitai approve as a new top-level PR comment. Approve commands are disabled for review-thread replies.
7cb614b to
df64602
Compare
|
jerry7z 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"} |
|
SiboWang1997 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"} |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
stps/sig-virt/workload-disruption-migration.md (7)
175-175:⚠️ Potential issue | 🟡 MinorMEDIUM: PM approver entry needs GitHub handle.
Line 175 includes email only; Section IV requires reviewer/approver identity with GitHub handle.
As per coding guidelines, "STP Sign-off and Approval (Section IV) must list Reviewers with names and GitHub handles and Approvers (QE Lead, PM, Dev Lead at minimum) with no placeholder text remaining".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-virt/workload-disruption-migration.md` at line 175, The PM approver entry currently shows only an email ("Product Manager/Owner: [Ronen Sde-Or](rsdeor@redhat.com)"); update the Section IV approvers/reviewers list to include the PM's GitHub handle alongside their name (for example replace the email-only link with a GitHub-style reference such as "Product Manager/Owner: Ronen Sde‑Or (`@rsdeor`)" or a Markdown link to their GitHub profile), ensuring the Approvers block (QE Lead, PM, Dev Lead) contains real names and GitHub handles with no placeholder text remaining.
18-20:⚠️ Potential issue | 🔴 CriticalCRITICAL: Rewrite internal/API-centric wording into user-observable behavior only.
The STP still relies on API fields/CRD terms and internal mechanics (e.g.,
MigrationPolicy, field names, VMI status internals) across core requirement, strategy, and scenario text. This blocks approval for this repo’s STP standard.As per coding guidelines, "STPs must describe what users experience from the user perspective only — no API field names, CRD names, internal component references, or implementation mechanisms".
Also applies to: 24-30, 39-39, 42-43, 57-57, 66-66, 70-76, 88-88, 98-98, 137-137, 145-148
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-virt/workload-disruption-migration.md` around lines 18 - 20, Rewrite the affected sections to describe only user-observable behavior and outcomes, removing all API/CRD/internal names and implementation mechanics: replace references like "AWD", "MigrationPolicy", "PostCopy", "Paused", and any mentions of VMI status internals with plain-language descriptions of what the user will see (e.g., "the VM may be briefly paused to finish transfer", "remaining memory is fetched on-demand after start on target", "a migration mode that resumes immediately once final data arrives"), and update the core requirement, strategy, and scenario paragraphs accordingly so they describe behaviors, triggers, and user-facing states rather than field names or internal components.
48-48:⚠️ Potential issue | 🟠 MajorHIGH: Known/Test limitations are missing mandatory sign-off lines.
Each limitation item must include explicit sign-off metadata (
*Sign-off:* [Name/Date]) or the required “None — reviewed and confirmed…” statement when applicable.As per coding guidelines, "STP Known Limitations (Section I.2) must include sign-off with
*Sign-off:* [Name/Date]for each limitation" and "STP Test Limitations (Section II.1) must each have sign-off:*Sign-off:* [Name/Date]."Also applies to: 88-88
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-virt/workload-disruption-migration.md` at line 48, The Known Limitations and Test Limitations entries are missing the mandatory sign-off metadata; update each limitation item in "Known Limitations (Section I.2)" and "Test Limitations (Section II.1)" — including the s390x memory hotplug note (the line stating "s390x does not support maxGuest memory...") — to append a sign-off line in the exact format "*Sign-off:* [Name/Date]" or, if there truly are none, replace the absence with the required statement "None — reviewed and confirmed [Name/Date]"; ensure every listed limitation has one of these sign-offs so the document meets the STP sign-off requirements.
92-107:⚠️ Potential issue | 🟠 MajorHIGH: Test Strategy is still incomplete (missing Scale Testing).
The table does not include a dedicated Scale Testing entry, which is mandatory even if deferred/N/A with justification.
As per coding guidelines, "STP Test Strategy (Section II.2) must address all testing types (Functional, Automation, Regression, Performance, Scale, Security, Usability, Monitoring, Compatibility, Upgrade, Dependencies, Cross Integrations, Cloud Testing)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-virt/workload-disruption-migration.md` around lines 92 - 107, Add a missing "Scale Testing" row to the test matrix table: insert a row labeled "Scale Testing" under the other test types with an Applicable (Y/N or N/A) value and a concise comment that either describes the planned scale tests (e.g., node/pod/migration concurrency, VMI counts, storage IO at scale) or justifies why scale testing is deferred/N/A for this STP; ensure the new row follows the existing table formatting and references "Scale Testing" so the STP Test Strategy (Section II.2) explicitly addresses this required testing type.
79-85:⚠️ Potential issue | 🔴 CriticalCRITICAL: Out-of-Scope section still contains TODO approvals and non-compliant structure.
PM/ Lead Agreementcannot remainTODO, and each out-of-scope item must be in the required itemized format with rationale and named/date agreement.As per coding guidelines, "STP Out of Scope items (Section II.1) must have Rationale and PM/Lead Agreement with name and date (format:
- **Item** / *Rationale:* / *PM/Lead Agreement:*)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-virt/workload-disruption-migration.md` around lines 79 - 85, Replace the table under "Out-of-Scope Item" in workload-disruption-migration.md (Section II.1) so that no PM/ Lead Agreement cells contain "TODO": convert each row into the required itemized format "- **Item** / *Rationale:* / *PM/Lead Agreement:*" and fill the PM/Lead Agreement with an actual name and date (e.g., "Jane Doe, 2026-04-19") for each of the four items (Windows node drain, Windows memory hotplug, s390x memory hotplug, Performance / scale testing); ensure the rationale text from the table is preserved and that the header/table is removed or replaced so the section conforms exactly to the prescribed itemized structure.
141-149:⚠️ Potential issue | 🟠 MajorHIGH: Risks section missing required category and sign-off data.
The table omits the Other risk category and does not include explicit per-risk sign-off, which are both required.
As per coding guidelines, "STP Risks (Section II.5) must address ALL 7 risk categories (Timeline/Schedule, Test Coverage, Test Environment, Untestable Aspects, Resource Constraints, Dependencies, Other), even if N/A with justification" and "STP Risks must each have: Risk description, Mitigation strategy, and Sign-off."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-virt/workload-disruption-migration.md` around lines 141 - 149, The Risks table in STP Risks (Section II.5) is missing the required "Other" risk category and per-risk sign-off entries; update the markdown table to include a row for "Other" (even if N/A with justification) and add a Sign-off column entry for every existing row (Timeline/Schedule, Test Coverage, Test Environment, Untestable Aspects, Resource Constraints, Dependencies, and Other) so each risk has Risk description, Mitigation strategy, and Sign-off; ensure the new column values clearly indicate who signs off (or "N/A — justification") and use the same table format/pipe alignment as the existing rows.
41-44:⚠️ Potential issue | 🟠 MajorHIGH: Section I.1 still misses required use-case and NFR structure.
Line 41 is not in user-story format (
As a [role], I want...), and Line 44 does not explicitly and verifiably address all required NFR categories (Monitoring, Observability, UI, Documentation, Performance, Security, Scalability).As per coding guidelines, "STP Customer use cases must be in user story format ("As a [role], I want to [action]")" and "STP NFRs must explicitly address: Monitoring, Observability, UI, Documentation, Performance, Security, Scalability. NFRs not covered must have justification".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-virt/workload-disruption-migration.md` around lines 41 - 44, Convert the "Customer Use Cases" entry into one or more user stories using the required format (e.g., "As an admin, I want to drain a node so that VMs with AWD policy migrate using PostCopy or Paused mode and resume without process loss"; "As an admin, I want to hotplug CPU/memory so that AWD migration preserves processes and reflects new resources in the guest"), and update the "Non-Functional Requirements (NFRs)" row to explicitly list each NFR category (Monitoring, Observability, UI, Documentation, Performance, Security, Scalability) with either a short requirement or a justification why it is not applicable (for example: Monitoring: none required / justification; Observability: covered by X tests; UI: none; Documentation: covered in KubeVirt user-guide; Performance: completionTimeoutPerGiB and bandwidth caps defined for testability; Security: RBAC covered by core KubeVirt tests; Scalability: state impact limited to node-level), referencing the table headings "Customer Use Cases" and "Non-Functional Requirements (NFRs)" and terms like MigrationPolicy and completionTimeoutPerGiB to keep context.
🤖 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/workload-disruption-migration.md`:
- Around line 110-122: The test-environment table contains empty cells; update
the table rows (notably the "Required Operators", "Platform", and "Special
Configurations" columns and any other blank example cells) so every cell
contains either an explicit value or "N/A", and make the "OCP & OpenShift
Virtualization Version(s)" entry explicitly state both the supported OCP version
and the OpenShift Virtualization operator version (e.g., "OCP 4.17+, OVS X.Y.Z"
or "OCP 4.17+, OpenShift Virtualization X.Y.Z"); ensure the "Specification
Examples" column entries are completed for the affected rows (Cluster Topology,
Required Operators, Platform, Special Configurations) so there are no empty
cells remaining.
---
Duplicate comments:
In `@stps/sig-virt/workload-disruption-migration.md`:
- Line 175: The PM approver entry currently shows only an email ("Product
Manager/Owner: [Ronen Sde-Or](rsdeor@redhat.com)"); update the Section IV
approvers/reviewers list to include the PM's GitHub handle alongside their name
(for example replace the email-only link with a GitHub-style reference such as
"Product Manager/Owner: Ronen Sde‑Or (`@rsdeor`)" or a Markdown link to their
GitHub profile), ensuring the Approvers block (QE Lead, PM, Dev Lead) contains
real names and GitHub handles with no placeholder text remaining.
- Around line 18-20: Rewrite the affected sections to describe only
user-observable behavior and outcomes, removing all API/CRD/internal names and
implementation mechanics: replace references like "AWD", "MigrationPolicy",
"PostCopy", "Paused", and any mentions of VMI status internals with
plain-language descriptions of what the user will see (e.g., "the VM may be
briefly paused to finish transfer", "remaining memory is fetched on-demand after
start on target", "a migration mode that resumes immediately once final data
arrives"), and update the core requirement, strategy, and scenario paragraphs
accordingly so they describe behaviors, triggers, and user-facing states rather
than field names or internal components.
- Line 48: The Known Limitations and Test Limitations entries are missing the
mandatory sign-off metadata; update each limitation item in "Known Limitations
(Section I.2)" and "Test Limitations (Section II.1)" — including the s390x
memory hotplug note (the line stating "s390x does not support maxGuest
memory...") — to append a sign-off line in the exact format "*Sign-off:*
[Name/Date]" or, if there truly are none, replace the absence with the required
statement "None — reviewed and confirmed [Name/Date]"; ensure every listed
limitation has one of these sign-offs so the document meets the STP sign-off
requirements.
- Around line 92-107: Add a missing "Scale Testing" row to the test matrix
table: insert a row labeled "Scale Testing" under the other test types with an
Applicable (Y/N or N/A) value and a concise comment that either describes the
planned scale tests (e.g., node/pod/migration concurrency, VMI counts, storage
IO at scale) or justifies why scale testing is deferred/N/A for this STP; ensure
the new row follows the existing table formatting and references "Scale Testing"
so the STP Test Strategy (Section II.2) explicitly addresses this required
testing type.
- Around line 79-85: Replace the table under "Out-of-Scope Item" in
workload-disruption-migration.md (Section II.1) so that no PM/ Lead Agreement
cells contain "TODO": convert each row into the required itemized format "-
**Item** / *Rationale:* / *PM/Lead Agreement:*" and fill the PM/Lead Agreement
with an actual name and date (e.g., "Jane Doe, 2026-04-19") for each of the four
items (Windows node drain, Windows memory hotplug, s390x memory hotplug,
Performance / scale testing); ensure the rationale text from the table is
preserved and that the header/table is removed or replaced so the section
conforms exactly to the prescribed itemized structure.
- Around line 141-149: The Risks table in STP Risks (Section II.5) is missing
the required "Other" risk category and per-risk sign-off entries; update the
markdown table to include a row for "Other" (even if N/A with justification) and
add a Sign-off column entry for every existing row (Timeline/Schedule, Test
Coverage, Test Environment, Untestable Aspects, Resource Constraints,
Dependencies, and Other) so each risk has Risk description, Mitigation strategy,
and Sign-off; ensure the new column values clearly indicate who signs off (or
"N/A — justification") and use the same table format/pipe alignment as the
existing rows.
- Around line 41-44: Convert the "Customer Use Cases" entry into one or more
user stories using the required format (e.g., "As an admin, I want to drain a
node so that VMs with AWD policy migrate using PostCopy or Paused mode and
resume without process loss"; "As an admin, I want to hotplug CPU/memory so that
AWD migration preserves processes and reflects new resources in the guest"), and
update the "Non-Functional Requirements (NFRs)" row to explicitly list each NFR
category (Monitoring, Observability, UI, Documentation, Performance, Security,
Scalability) with either a short requirement or a justification why it is not
applicable (for example: Monitoring: none required / justification;
Observability: covered by X tests; UI: none; Documentation: covered in KubeVirt
user-guide; Performance: completionTimeoutPerGiB and bandwidth caps defined for
testability; Security: RBAC covered by core KubeVirt tests; Scalability: state
impact limited to node-level), referencing the table headings "Customer Use
Cases" and "Non-Functional Requirements (NFRs)" and terms like MigrationPolicy
and completionTimeoutPerGiB to keep context.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ef2e93af-35f0-4f4f-9f48-ea1140c1d9d7
📒 Files selected for processing (1)
stps/sig-virt/workload-disruption-migration.md
| | Environment Component | Configuration | Specification Examples | | ||
| |:----------------------------------------------|:-----------------------------------|:---------------------------------------------------------------------------------------------------------------| | ||
| | **Cluster Topology** | Bare Metal | Multi-worker OCP cluster (minimum 2 workers for migration, additional for Windows special_infra) | | ||
| | **OCP & OpenShift Virtualization Version(s)** | OCP 4.17+ | Feature available from OCP-V 4.17 onward | | ||
| | **CPU Virtualization** | VT-x / AMD-V | Required for VM execution | | ||
| | **Compute Resources** | Standard + high-resource nodes | Standard workers for RHEL; high-resource workers (special_infra) for Windows VMs | | ||
| | **Special Hardware** | N/A | Agnostic | | ||
| | **Storage** | RWX default storage class | Required for live migration (e.g., ocs-storagecluster-ceph-rbd-virtualization) | | ||
| | **Network** | OVN-Kubernetes | Standard cluster networking | | ||
| | **Required Operators** | OpenShift Virtualization | | | ||
| | **Platform** | Bare Metal | | | ||
| | **Special Configurations** | N/A | | | ||
|
|
There was a problem hiding this comment.
HIGH: Test Environment has empty cells; all fields must be explicit or N/A.
Several cells are left blank (for example around Required Operators/Platform/Special Configurations examples). Section II.3 requires no empty fields.
As per coding guidelines, "STP Test Environment (Section II.3) must have all fields filled or marked N/A (no empty fields). OCP and OpenShift Virtualization versions must be explicit".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stps/sig-virt/workload-disruption-migration.md` around lines 110 - 122, The
test-environment table contains empty cells; update the table rows (notably the
"Required Operators", "Platform", and "Special Configurations" columns and any
other blank example cells) so every cell contains either an explicit value or
"N/A", and make the "OCP & OpenShift Virtualization Version(s)" entry explicitly
state both the supported OCP version and the OpenShift Virtualization operator
version (e.g., "OCP 4.17+, OVS X.Y.Z" or "OCP 4.17+, OpenShift Virtualization
X.Y.Z"); ensure the "Specification Examples" column entries are completed for
the affected rows (Cluster Topology, Required Operators, Platform, Special
Configurations) so there are no empty cells remaining.
✅ Addressed in commit 7f3a3f4
There was a problem hiding this comment.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="stps/sig-virt/workload-disruption-migration.md"
printf '%s\n' '--- Section II.3 excerpt ---'
rg -n -A22 -B2 '^#### \*\*3\. Test Environment\*\*' "$file"
printf '%s\n' '--- Empty table-cell check for Section II.3 ---'
python3 - "$file" <<'PY'
from pathlib import Path
import sys
lines = Path(sys.argv[1]).read_text().splitlines()
start = next(i for i, line in enumerate(lines) if line == "#### **3. Test Environment**")
rows = []
for number, line in enumerate(lines[start + 1:], start + 2):
if line.startswith("#### ") or line.startswith("---"):
break
if line.startswith("|") and not set(line.replace("|", "").replace(":", "").replace("-", "").strip()):
continue
if line.startswith("|"):
cells = [cell.strip() for cell in line.strip().strip("|").split("|")]
rows.append((number, cells))
for number, cells in rows[1:]:
empty = [str(index + 1) for index, cell in enumerate(cells) if not cell]
if empty:
print(f"Line {number}: empty cell(s) {', '.join(empty)}")
if not any(not cell for _, cells in rows[1:] for cell in cells):
print("No empty cells found.")
version_row = next((cells for _, cells in rows[1:] if "OCP & OpenShift Virtualization Version(s)" in cells[0]), None)
print("Version row:", " | ".join(version_row) if version_row else "not found")
PYLength of output: 2814
@openshift-virtualization-qe-bot-3, HIGH: The empty-field issue is resolved. Section II.3 now uses explicit bullet entries. All fields have a value or N/A.
However, Line 201 still does not explicitly state both required versions. OCP-V 5.0 identifies OpenShift Virtualization. It does not identify the supported OpenShift Container Platform version.
Update the entry to name both versions. For example:
- **OCP & OpenShift Virtualization Version(s):** OpenShift Container Platform 4.x; OpenShift Virtualization 5.0 (GA). The feature was available as a Technology Preview from OpenShift Virtualization 4.19.This update makes the test environment reproducible and satisfies the Section II.3 version requirement.
🧠 Learnings used
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 65
File: stps/sig-virt/heterogeneous-rhcos9-rhcos10/network.md:174-174
Timestamp: 2026-04-13T10:20:13.766Z
Learning: In this repository’s design-doc markdown files under stps/sig-virt/**, if a feature’s tests are truly storage-agnostic (i.e., the test outcomes are unaffected by the storage type, such as network-focused STPs), then in Section II.3 “Test Environment” use an explicit StorageClass statement like “Any available StorageClass (network-focused tests; storage type does not affect test outcomes)” rather than the bare word “Agnostic”. Also ensure Section II.3 “Platform” is always explicitly populated (e.g., “Bare metal”), not left unspecified.
Learnt from: geetikakay
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 128
File: stps/sig-infra/CNV-82132-pause-guest-agent-probes.md:76-77
Timestamp: 2026-08-12T18:58:37.675Z
Learning: For OpenShift Virtualization STPs that include live-migration scenarios, explicitly require RWX shared storage and Block volume mode in the test environment. State these prerequisites to ensure reproducibility, and do not claim that live migration requires no special storage configuration.
You are interacting with an AI system.
df64602 to
7f3a3f4
Compare
openshift-virtualization-qe-bot-5
removed
the
changes-requested-coderabbitai[bot]
label
|
jerry7z 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"} |
|
SiboWang1997 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"} |
7f3a3f4 to
8af640b
Compare
|
jerry7z 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"} |
|
SiboWang1997 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"} |
8af640b to
3573a3b
Compare
|
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"} |
2 similar comments
|
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"} |
|
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"} |
|
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"} |
rnetser
left a comment
rnetser
left a comment
There was a problem hiding this comment.
Code Review
Found 1 issue(s) in this PR:
💡 Suggestions (1)
| File | Line | Issue |
|---|---|---|
stps/sig-virt/workload-disruption-migration.md |
270 | Cluster-level AWD scenario says "verify the setting is enforced" without definin |
Review generated by pi
Assisted-by: PI (claude-opus-4-6-1m)
|
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"} |
|
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"} |
|
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"} |
2 similar comments
|
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"} |
|
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"} |
rnetser
left a comment
rnetser
left a comment
There was a problem hiding this comment.
Code Review
Found 7 issue(s) in this PR:
💡 Suggestions (7)
| File | Line | Issue |
|---|---|---|
stps/sig-virt/workload-disruption-migration.md |
7 | Question: is there a VEP, design doc, or HLD for this feature? The Enhancement(s |
stps/sig-virt/workload-disruption-migration.md |
61 | AC #1 says migration completes in the expected mode "regardless of migration tri |
stps/sig-virt/workload-disruption-migration.md |
306 | Re-raising a prior point (previously discussed and marked resolved, but the code |
stps/sig-virt/workload-disruption-migration.md |
274 | This was split into separate PostCopy/Paused rows in an earlier revision, but it |
stps/sig-virt/workload-disruption-migration.md |
282 | The CPU hotplug scenario (and the memory hotplug scenario at line 286) say "veri |
stps/sig-virt/workload-disruption-migration.md |
123 | The memory hotplug goal doesn't name the guest-OS dimension, while the CPU hotpl |
stps/sig-virt/workload-disruption-migration.md |
214 | II.3.1 states "CI/CD: N/A", but the Test Environment risk (line 245) says "Tests |
Review generated by pi
Assisted-by: PI (claude-opus-4-8)
| - **[P0]** AWD Migration Mode: Verify AWD migration falls back to PostCopy and Paused modes when pre-copy cannot converge, with process preservation. | ||
| - **[P0]** AWD Cluster Configuration: Verify a VM without an explicit AWD policy inherits the cluster-level AWD setting and migrates in Paused mode when pre-copy cannot converge. | ||
| - **[P1]** AWD CPU Hotplug: Verify CPU hotplug triggers AWD migration and guest reports new CPU count with process preservation (RHEL). | ||
| - **[P1]** AWD Memory Hotplug: Verify memory hotplug triggers AWD migration and guest reports new memory amount with process preservation. |
There was a problem hiding this comment.
The memory hotplug goal doesn't name the guest-OS dimension, while the CPU hotplug goal specifies RHEL-only. Per AGENTS.md II.1, a goal should name all configuration dimensions needed to implement it, so test authors don't have to guess.
Suggest adding the guest OS(es) on which memory hotplug + AWD is validated.
Assisted-by: PI (claude-opus-4-8)
|
PR description vs. STP scope (please update the description): The description says the STP covers "migration, node drain, and CPU/memory hotplug triggers" and "PostCopy and Paused migration modes across RHEL and Windows guests." However, the STP's Out of Scope section explicitly excludes node drain, Windows Paused mode, and Windows CPU hotplug (all with PM/Lead sign-off). Please update the description to drop node drain from the trigger list and clarify that Paused mode is validated on RHEL only — so the description matches the documented scope. Assisted-by: PI (claude-opus-4-8) |
|
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"} |
|
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"} |
|
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"} |
Add Software Test Plan for the AllowWorkloadDisruption (AWD) migration feature, covering PostCopy and Paused migration modes across RHEL and Windows guests with migration, node drain, and CPU/memory hotplug triggers. Signed-off-by: Samuel Albershtein <salbersh@redhat.com> Assisted-by: Claude <noreply@anthropic.com>
Update the AWD migration STP to include HCO-to-KubeVirt configuration propagation testing. The allowWorkloadDisruption field is set via the HCO CR and must propagate to the KubeVirt CR. Signed-off-by: Samuel Albershtein <salbersh@redhat.com>
Remove internal API references (HCO CR, KubeVirt CR, allowWorkloadDisruption) and rewrite in user-facing terms. Expand NFRs into structured list with platform migration parallelism limits acknowledged for scalability. Clarify documentation is upstream-only during TP. Replace generic Developer Handoff with concrete technical takeaways. Update Test Limitations sign-off to QE Lead. Remove empty "Other" risk category and justify no-risk entries per AGENTS.md. Add arm64 known limitation for hotplug and document architecture coverage in scope and test environment. Update Jira links to issues.redhat.com. Move Windows node-drain, Paused-mode, and CPU-hotplug scenarios to Out of Scope. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Samuel Albershtein <salbersh@redhat.com>
|
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"} |
12 participants
Software Test Plan for the AllowWorkloadDisruption (AWD) migration feature (GA in OCP-V 5.0). Covers AWD migration in PostCopy mode (RHEL and Windows) and Paused mode (RHEL only), with CPU hotplug (RHEL) and memory hotplug (RHEL and Windows) as migration triggers. Cluster-level AWD configuration inheritance is validated as a dedicated scenario.
Assisted-by: Claude