[CNV-75960] Add STP for Multi-Arch Boot Image Import and DataSource Lifecycle#22
[CNV-75960] Add STP for Multi-Arch Boot Image Import and DataSource Lifecycle#22geetikakay wants to merge 2 commits intoRedHatQE:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new Software Test Plan "Multi-Architecture Boot Image Import and DataSource Lifecycle - Quality Engineering Plan" at Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
ca56a76 to
2665393
Compare
2665393 to
767aeae
Compare
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@stps/sig-infra/multiarch-automation.md`:
- Line 225: Update the table row under the "Untestable Aspects" column that
reads "Very large scale environment (50+ OS images across multiple
architectures)" to hyphenate the compound adjective: change it to "Very
large-scale environment (50+ OS images across multiple architectures)"; ensure
you only modify that phrase in the multiarch-automation.md table row and keep
the rest of the cell content unchanged.
- Line 202: Fix the typo in the checklist item containing
"enableMultiArchBootImageImport": change the misspelled word "releas" to
"release" so the line reads "[ ] enableMultiArchBootImageImport feature is
**available in target release**"; update the text wherever that exact checkbox
string appears in multiarch-automation.md (search for
"enableMultiArchBootImageImport") to ensure consistency.
- Line 68: The table row has 5 cells but the header defines 4, breaking markdown
rendering; update the row in multiarch-automation.md so it has exactly four
pipe-separated cells by removing the extra cell separator — either merge the two
rightmost cells into one (so the long API Extensions content remains as a single
cell) or escape any literal '|' characters inside the cell content (use |
or wrap the inner content to avoid creating extra columns). Ensure the cell
containing the API Extensions text (referencing HCO: status.nodeInfo,
status.dataImportCronTemplates; SSP: enableMultipleArchitectures, cluster
fields; CDI: platform.architecture, arch-specific DataSource naming) is kept
intact as a single table cell.
767aeae to
8e0ca44
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@stps/sig-infra/multiarch-automation.md`:
- Line 59: Resolve the contradiction between the "Non-Functional Requirements
(NFRs)" summary row and the "Usability Testing" entry by choosing one scope
decision and updating both places: either (A) keep usability in-scope—change the
NFRs row text to exclude "Usability" or reword to explicitly state which NFRs
are out of scope and ensure "Usability Testing" stays marked "Y" with its
validation tasks (reference the table row titled "Non-Functional Requirements
(NFRs)" and the section/header "Usability Testing"), or (B) keep usability
out-of-scope—set "Usability Testing" to "N" and remove the naming-convention
validation tasks under that section; make the two entries consistent and update
any related checklist or validation steps to match.
🧹 Nitpick comments (3)
stps/sig-infra/multiarch-automation.md (3)
5-17: Consider adding VEP issue field to metadata table.The PR description mentions a VEP issue field but it's not present in the metadata table. While the Enhancement field links to the VEP document, consider adding an explicit "VEP Issue" row for consistency with other STPs in the repository.
📋 Suggested addition to metadata table
| **Enhancement(s)** | [dic-on-heterogeneous-cluster](https://github.com/kubevirt/enhancements/blob/main/veps/sig-storage/dic-on-heterogeneous-cluster/dic-on-heterogeneous-cluster.md) | +| **VEP Issue** | TBD or N/A | | **Feature in Jira** | [VIRTSTRAT-494](https://issues.redhat.com/browse/VIRTSTRAT-494) |
177-177: Clarify compute resource requirements for multi-arch testing.Marking "Compute Resources" as "N/A" with "No special compute requirements" may be misleading for a multi-arch test plan that explicitly requires both amd64 and arm64 node architectures (line 174). Consider clarifying that while there are no special CPU/memory quantity requirements, the architecture diversity is itself a compute requirement.
📝 Suggested clarification
-| **Compute Resources** | N/A | No special compute requirements | +| **Compute Resources** | Mixed architecture nodes required | No special CPU/memory quantities beyond standard worker nodes. Architecture diversity (amd64 + arm64) is mandatory per cluster topology. |
244-261: Restructure traceability matrix to link test cases to requirements.The current table uses "Requirement ID" for test case identifiers (TC-01, etc.), but does not trace these test cases back to actual requirements from Jira (e.g., VIRTSTRAT-494 or sub-requirements/user stories). This reduces traceability and makes it difficult to verify that all requirements are covered.
Consider restructuring to either:
- Add a separate "Requirement ID" column that references Jira issues/epics, or
- Rename "Requirement ID" to "Test Case ID" and add requirement traceability in the description
🔗 Example restructured table with requirement traceability
Option 1: Add requirement column (preferred for full traceability):
| Requirement ID | Test Case ID | Test Scenario Summary | Tier | Priority | Known Issues | | :------------- | :----------- | :-------------------- | :--- | :------- | :----------- | | VIRTSTRAT-494 | TC-01 | Golden images on multi-arch cluster | Tier 1 | P0 | | | VIRTSTRAT-494 | TC-02 | Images added post-deployment | Tier 2 | P1 | | ...Option 2: Rename column to clarify:
| Test Case ID | Test Scenario Summary | Tier | Priority | Known Issues | | :----------- | :-------------------- | :--- | :------- | :----------- | | TC-01 (VIRTSTRAT-494) | Golden images on multi-arch cluster | Tier 1 | P0 | |
8e0ca44 to
9f513ef
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@stps/sig-infra/multiarch-automation.md`:
- Line 210: The exit criteria line currently reads "All P0, P1 test scenarios
executed and passing (TC-01, TC-03, TC-06, TC-07, TC-11)" but only lists a
subset; either expand the parenthetical to include every P0 and P1 test case or
change the wording to accurately reflect the list (e.g., "Key P0/P1 scenarios
executed and passing (TC-01, TC-03, TC-06, TC-07, TC-11)"). Update the checklist
text (the line starting with "[ ] All P0, P1 test scenarios executed and
passing") to use one of these two fixes and ensure the phrasing matches the
scope of the listed TC identifiers.
- Around line 170-181: The "Required Operators" row in the environment table
leaves the "Specification Examples" cell empty; update the table so no cells are
blank by adding concrete examples such as operator names and versions (e.g.,
"SSP v1.x, CDI v1.x, HCO v1.x" or the specific operator package names + minimal
version constraints) in the "Specification Examples" column for the "Required
Operators" row to match the rule that "N/A" or explicit values must be present.
RoniKishner
left a comment
There was a problem hiding this comment.
Really Good Job on this STP.
can you add to the PR description a link to https://github.com/geetikakay/openshift-virtualization-tests-design-docs/blob/infra_stp_multiarch/stps/sig-infra/multiarch-automation.md
This will help review the STP in a more human eye way.
9f513ef to
7612d1e
Compare
rnetser
left a comment
There was a problem hiding this comment.
while acceptance criteria covers what needs to be verified, most of it is already covered by regression tests. please make sure you only add (if needed) scenarios that are not already covered. please also make sure that the stp maintains only high level content and does not include implementation details.
your stp are an extension to the main one (and based on size could potentially be incorporated in the main one); please remove any redundant info
9ef457b to
b2bbb5a
Compare
bbcd111 to
ff1d156
Compare
…ifecycle STP for enableMultiArchBootImageImport feature testing on heterogeneous clusters. Covers DataSource lifecycle, architecture labeling, duplicate prevention, and cleanup validation. Related: VIRTSTRAT-494 Parent STP: RedHatQE#12 Signed-off-by: Geetika Kapoor <gkapoor@redhat.com>
2eb707c to
4eb53d3
Compare
for more information, see https://pre-commit.ci
@rnetser thanks for the review. Done with most of the suggested changes. To avoid duplication is child stp for test cases suggested in parent STP, I have added a comment for @hmeir to have links to different sig test cases so it can be called directly in child stp's. |
|
|
||
| **sig-infra — Additive Functional Tests** | ||
|
|
||
| - **[CNV-75960](https://issues.redhat.com/browse/CNV-75960)** — As a cluster admin, I want bootable volumes to be re-imported with correct architecture labels when enabling multi-arch support on an existing cluster |
There was a problem hiding this comment.
IUO can verify that.
Infra should verify that VMs can be created from the available boot sources(there are existing tests that can be modified, so should it be here? it feels like regression
There was a problem hiding this comment.
Infra should verify that VMs can be created from the available boot sources
Before validating that, i would also like to validate labels as well and then start the VM and that's the reason i added here. having it somewhere else will not be a full coverage. i would check it here for test completeness
| **sig-infra — Additive Functional Tests** | ||
|
|
||
| - **[CNV-75960](https://issues.redhat.com/browse/CNV-75960)** — As a cluster admin, I want bootable volumes to be re-imported with correct architecture labels when enabling multi-arch support on an existing cluster | ||
| - *Test Scenario:* [Tier 2] Enable multi-arch boot image import on a heterogeneous cluster that already has existing bootable volumes. Verify volumes are re-imported with correct architecture-specific labels (documented expected behavior per [CNV-75084](https://issues.redhat.com/browse/CNV-75084)). |
There was a problem hiding this comment.
right, so that's why i added documented expected behavior per . I would like to keep it because we as a QE thought of it and decision was to document it.No automation action needed.
| - **[CNV-75960](https://issues.redhat.com/browse/CNV-75960)** — As a cluster admin, I want bootable volumes to be re-imported with correct architecture labels when enabling multi-arch support on an existing cluster | ||
| - *Test Scenario:* [Tier 2] Enable multi-arch boot image import on a heterogeneous cluster that already has existing bootable volumes. Verify volumes are re-imported with correct architecture-specific labels (documented expected behavior per [CNV-75084](https://issues.redhat.com/browse/CNV-75084)). | ||
| - *Priority:* P0 | ||
| - **[CNV-75960](https://issues.redhat.com/browse/CNV-75960)** — As a cluster admin, I want correct architecture labels on all storage resources |
There was a problem hiding this comment.
Isn't it tier1?
And maybe the snapshot is worth mentioning since our SC default is DV.
There was a problem hiding this comment.
Yes, it could go there however I am unsure if i see that n storage but i will check again else add one
| - **[CNV-75960](https://issues.redhat.com/browse/CNV-75960)** — As a cluster admin, I want correct architecture labels on all storage resources | ||
| - *Test Scenario:* [Tier 2] Verify architecture labels are correct on all associated DataVolumes, VolumeSnapshots and PVCs for golden images on a heterogeneous cluster. | ||
| - *Priority:* P1 | ||
| - **[CNV-75960](https://issues.redhat.com/browse/CNV-75960)** — As a cluster admin, I want post-deployment DataImportCron additions to create architecture-specific images |
There was a problem hiding this comment.
post deployment means after cluster installation , adding a new custom dic
STP for enableMultiArchBootImageImport feature testing on heterogeneous clusters. Covers DataSource lifecycle, architecture labeling, duplicate prevention, and cleanup validation.
Related: VIRTSTRAT-494
Parent STP: #12
STP Metadata
VEP issue:
What this PR does
Special notes for your reviewer
Summary by CodeRabbit