Storage: STP for new scratch space SC selection logic#86
Storage: STP for new scratch space SC selection logic#86kshvaika wants to merge 5 commits intoRedHatQE:mainfrom
Conversation
📝 WalkthroughWalkthroughAdded a new Software Test Plan (STP) document for the Scratch Space Storage Class Selection Logic in CDI, capturing feature metadata, acceptance criteria, test strategy, environment assumptions, and mappings to CNV-72238 (import/upload/clone/config propagation). ChangesSTP Documentation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
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
requested review from
Ahmad-Hafe,
acinko-rh,
dalia-frank,
josemacassan and
jpeimer
|
duyanyan 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: 17
🤖 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-storage/scratch_space_sc_selection_logic.md`:
- Around line 58-60: The Testability checkbox under the "Testability" heading is
left unchecked; update the markdown so all Section I.1 checklist items are
marked [x] and include concrete testability content: change "- [ ]
**Testability**" to "- [x] **Testability**" and replace the placeholder line
under it with a short, filled description listing specific, unambiguous tests
(e.g., CDI operations: clone, import, upload) and the storage-class
configurations and expected outcomes to verify (e.g., behavior with default,
reduced-redundancy, and tiered classes), ensuring the "Testability" paragraph is
explicit and non-empty.
- Around line 115-117: Check and update the "Developer Handoff/QE Kickoff" item:
change the unchecked checkbox to a checked one ([ ] -> [x]) and replace the
placeholder "[Summarize key points and concerns]" with real meeting takeaways or
a concise note such as "No specific concerns were raised" so Section I.3
complies with the STP requirement; locate the "Developer Handoff/QE Kickoff"
heading and its accompanying bullet text in the document to make these edits.
- Around line 272-273: Update the "Upgrade Testing" checklist entry for the
Upgrade Testing item (the "- [ ] **Upgrade Testing** — Validates upgrade
paths..." line) to include a concise justification explaining why upgrade
testing is marked "Not applicable" (e.g., upgrade path evaluated and no
migrations/config changes required, or deferred with risk/mitigation and planned
date), and ensure the note explicitly states that the upgrade path was evaluated
and the rationale (or planned follow-up) is documented to satisfy STP Section
II.2 requirements.
- Line 217: The Test Limitations statement in the "STP Section II.1 Test
Limitations" block is missing the required sign-off; update that statement to
append the sign-off line in the exact format "*Sign-off:* [Name/Date]" (e.g.,
"*Sign-off:* Jane Doe/2026-04-16") so reviewers can see who approved it and
when; ensure the sign-off is placed immediately after the limitation text in the
Test Limitations section.
- Around line 340-392: The "Sign-off: TBD" placeholders in the "5. Risks"
section must be replaced with real approver names and dates; update each
occurrence of the "Sign-off" entries under the risks (Timeline/Schedule, Test
Coverage, Test Environment, Untestable Aspects, Resource Constraints,
Dependencies, Other) in the "5. Risks" section so they contain the approver's
full name and approval date (e.g., "Sign-off: Jane Doe, 2026-04-15") instead of
"TBD", ensuring no placeholder text remains before STP approval.
- Around line 395-418: Update the test scenario tiers for the listed
single-operation tests from "[Tier 2]" to "[Tier 1]" — specifically change the
tier label for the scenarios titled "Verify import requiring conversion
allocates scratch space using target DataVolume storage class", "Verify upload
operation allocates scratch space using target DataVolume storage class",
"Verify PVC clone allocates scratch space using source PVC storage class",
"Verify scratchSpaceStorageClass set in HCO CR propagates to CDI CR
configuration", and "Verify import operation uses scratchSpaceStorageClass
configured in HCO" so they reflect Tier 1 per the STP guidelines for isolated
single-operation tests; leave priorities and wording unchanged.
- Around line 62-70: Rewrite the Acceptance Criteria to state observable user
outcomes (not implementation details): replace lines mentioning
"scratchSpaceStorageClass HCO config", "scratchSpaceStorageClass config", and
"CDI functional tests" with clear pass/fail conditions like "When
scratchSpaceStorageClass is unspecified, scratch space uses the same storage
class as the target PVC" and "When an explicit scratchSpaceStorageClass is set,
scratch space uses that storage class", and add a testable criterion such as
"Automated tests verify both default (same-as-target) and explicit
scratchSpaceStorageClass behaviors"; ensure each criterion is a separate bullet
and remove internal test/tool references.
- Around line 39-47: Mark the "Review Requirements" checklist items as completed
([x]) and fill each item with concrete, testable downstream requirements instead
of implementation details: replace bullets referencing internal logic like "CDI
default behavior" and "scratchSpaceStorageClass config" with user-facing
acceptance criteria such as (a) when scratchSpaceStorageClass is unset the
scratch volume must use the target PVC's storage class, (b) when
scratchSpaceStorageClass is explicitly set it must override other defaults, (c)
behavior must remain unchanged for HCO-configured scratchSpaceStorageClass, and
(d) any change must be visible to end users as described; update the section
header "Review Requirements" and the listed items so they are verifiable
downstream requirements rather than implementation notes.
- Around line 16-20: The Document Conventions section currently lists standard
acronyms (SC, HCO, CDI, PVC); remove those entries so the section contains only
feature-specific or novel terms; locate the list under the "Document
Conventions" heading (the bullets containing SC, HCO, CDI, PVC) and delete those
four lines, leaving any genuinely feature-specific definitions intact (add any
necessary new feature-specific terms if needed).
- Around line 176-178: The Testing Goals currently use implementation jargon
(the three bullets referencing "same-as-target storage class",
"scratchSpaceStorageClass HCO config", and "new selection logic"); rewrite them
into end-user focused statements that describe observable behavior and outcomes
instead of internals — e.g., state that scratch space should default to the same
storage characteristics as the target PVC unless the user overrides the setting,
that an explicit user configuration for scratch storage must take precedence
over defaults, and that clone/import/upload operations must transparently use
the appropriate scratch space so users see successful operations without manual
scratch-space selection; remove the internal terms "same-as-target storage
class", "scratchSpaceStorageClass HCO config", and "new selection logic" from
the goals and replace with user-centric phrasing.
- Around line 22-24: The Feature Overview currently contains internal
implementation details (mentions like CDI, scratchSpaceStorageClass HCO
configuration option, virt-default/default storage classes); rewrite this
section to describe only the user-facing change: explain that scratch space will
now be provisioned using the same storage class as the target PVC by default,
and that an explicit configuration can override this behavior, without naming
internal components, CRD/API fields, or default storage class names; update the
"Feature Overview" heading content accordingly to be purely from a
user-experience perspective.
- Around line 7-9: The Enhancement(s) field in
scratch_space_sc_selection_logic.md currently links to the CDI repo and an HCO
PR which are not valid design artifacts; replace those links with a reference to
an appropriate VEP/HLD/design doc if one exists, otherwise explicitly state "No
VEP or HLD exists; see Jira epic CNV-72238" and include that Jira link as the
Enhancement(s) entry (and optionally note that the HCO PR and CDI repo are
implementation pointers, not design documents) so the field conforms to the STP
metadata requirement.
- Around line 72-78: Update the "Non-Functional Requirements (NFRs)" section in
scratch_space_sc_selection_logic.md to explicitly cover Monitoring,
Observability, UI, Documentation, Performance, Security, and Scalability: add
clear, measurable targets or acceptance criteria for each (e.g., what
metrics/logs for Monitoring/Observability, UI behavior or lack thereof,
Documentation updates required, performance targets for scratch space creation),
and for any category not applicable include a short justification explaining why
it's out of scope; modify the existing bullet list under "Non-Functional
Requirements (NFRs)" and the items referencing "Existing CDI metrics" and
"Documentation update" to include the new Security, Scalability, and UI entries
with concrete targets or rationale.
- Around line 49-56: Mark the "Understand Value and Customer Use Cases" checkbox
as completed ([x]) and rewrite the two customer use cases into user-story
format; replace the current bullet "VM owners want consistent storage class
usage for all disk provisioning operations, including scratch space" with a user
story like "As a VM owner, I want scratch space to use the same storage class as
my disk so that provisioning is consistent," and replace "When provisioning a
disk, scratch space should use the same storage class as that disk" with a user
story like "As an operator provisioning a disk, I want scratch space to be
provisioned with the same storage class as the target disk so that storage
behavior is predictable."
- Around line 421-428: Replace the placeholder "TBD" entries in Section IV
"Sign-off and Approval" so that the "Reviewers" and "Approvers" lists contain
actual stakeholder names and GitHub handles (include at minimum the QE Lead, PM,
and Dev Lead); update the entries under the "Reviewers:" and "Approvers:" bullet
points to the real people and their handles and remove all "TBD" text.
- Line 314: The "Platform:" field currently contains a typo "PSIm"; update that
entry to the correct designation "PSI" so the Platform line reads "Platform:
PSI, Bare metal". Locate the "Platform:" line containing the token "PSIm" in
scratch_space_sc_selection_logic.md and replace "PSIm" with "PSI" to match the
repository convention.
- Around line 80-107: Remove the template placeholder HTML comments and example
items from the "#### **2. Known Limitations**" section, replace the two
placeholder limitation bullets with either actual limitations each including a
"*Sign-off:* [Name/Date]" entry, or if there truly are none, change the "Need to
be updated: None identified at this time." line to the required statement
exactly: "None — reviewed and confirmed with [Name/Date] that no feature
limitations apply for this release."; ensure no leftover template comments
remain in the Known Limitations block.
🪄 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: db7d7574-6927-472e-9fd4-e84fcd0b8ef0
📒 Files selected for processing (1)
stps/sig-storage/scratch_space_sc_selection_logic.md
|
duyanyan 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: 2
♻️ Duplicate comments (10)
stps/sig-storage/scratch_space_sc_selection_logic.md (10)
214-214:⚠️ Potential issue | 🟡 MinorMEDIUM: Test Limitations “None” is missing required sign-off with Name/Date
Line 214 lacks the required sign-off format for this section.
As per coding guidelines: "STP Section II.1 Test Limitations: Each limitation must have sign-off in format 'Sign-off: [Name/Date]'".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-storage/scratch_space_sc_selection_logic.md` at line 214, The "Test Limitations" entry currently reads "None — reviewed and confirmed that no test limitations apply for this release." and is missing the required sign-off; update the STP Section II.1 Test Limitations section (the line containing that sentence) to include the sign-off line in the exact format "*Sign-off:* [Name/Date]" (e.g., append a new line "*Sign-off:* Jane Doe/2026-04-17") so the section complies with the guideline.
112-115:⚠️ Potential issue | 🔴 CriticalCRITICAL: Developer Handoff/QE Kickoff is unchecked and still has placeholder text
Line 112 is unchecked and Line 114 still uses template placeholder content. This must be completed with actual handoff takeaways.
As per coding guidelines: "STP Section I.3 Technology and Design Review: All items must use [x] checkboxes — Developer Handoff must describe actual meeting takeaways, not just 'meeting conducted'".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-storage/scratch_space_sc_selection_logic.md` around lines 112 - 115, The "Developer Handoff/QE Kickoff" checklist item is still unchecked and contains placeholder text; update the STP Section I.3 item so the checkbox is marked complete ([x]) and replace the placeholder line "*Key takeaways and concerns:* [Summarize key points and concerns]" with concrete handoff notes from the actual meeting (e.g., decisions made, unresolved risks, testability concerns, action items and owners) ensuring the entry follows the required "[x] checkbox" format and explicitly documents meeting takeaways rather than template text.
97-104:⚠️ Potential issue | 🔴 CriticalCRITICAL: Known Limitations still contains template placeholders and missing sign-off evidence
Line 97-Line 103 still includes placeholder limitations and placeholder sign-off format; this blocks approval.
🛠️ Suggested fix
-- **[Feature Limitation 1]** - - *Sign-off:* [Name/Date — confirms awareness and acceptance of this limitation] - -- **[Feature Limitation 2]** - - *Sign-off:* [Name/Date — confirms awareness and acceptance of this limitation] - -Need to be updated: None identified at this time. +None — reviewed and confirmed with [Name/Date] that no feature limitations apply for this release.As per coding guidelines: "STP Section I.2 Known Limitations: Each limitation must have a sign-off in format 'Sign-off: [Name/Date]' — if none exist, must state 'None — reviewed and confirmed with [Name/Date]'".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-storage/scratch_space_sc_selection_logic.md` around lines 97 - 104, The Known Limitations section still contains template placeholders (“**[Feature Limitation 1]**”, “**[Feature Limitation 2]**” and placeholder sign-off text); replace each placeholder entry with either a concrete limitation and its sign-off in the required format '*Sign-off:* [Name/Date]' or, if there are no limitations, replace the block with the exact statement "None — reviewed and confirmed with [Name/Date]"; update the markdown around the title "Known Limitations" (the lines containing the placeholder limitation headings and the "*Sign-off:*" entries) to reflect the real content and include the reviewer name/date to satisfy STP Section I.2.
396-414:⚠️ Potential issue | 🟡 MinorMEDIUM: These isolated scenarios are mis-tiered as Tier 2
Line 397, 401, 405, 409, and 413 are single-operation validations; they should be Tier 1 unless you intentionally redefine them as multi-feature E2E workflows.
As per coding guidelines: "Tier 1 applies to single feature isolated scenarios ... Tier 2 applies to end-to-end user workflows and multi-feature integration".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-storage/scratch_space_sc_selection_logic.md` around lines 396 - 414, Update the test tier for the five CNV-72238 scenarios listed (the entries titled "import operations", "upload operations", "clone operations", "scratchSpaceStorageClass configuration to propagate from HCO to CDI", and "configure scratch space storage class for import operations") from Tier 2 to Tier 1; locate the bullets by their exact titles in the document (the CNV-72238 blocks) and change the "*Test Scenario:* [Tier 2]" marker to "*Test Scenario:* [Tier 1]" for each, and remove or resolve the duplicate_comment note if present.
7-10:⚠️ Potential issue | 🔴 CriticalCRITICAL: Enhancement(s) links are implementation pointers, not design artifacts
Line 8 and Line 9 currently reference repo/PR links, but this field must point to a VEP/design doc/HLD (or explicitly state none exists and reference Jira as the closest artifact).
As per coding guidelines: "STP Metadata: Enhancement(s) links must reference a VEP, design doc, or HLD (not 'N/A' without justification)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-storage/scratch_space_sc_selection_logic.md` around lines 7 - 10, The Enhancement(s) field currently lists repo/PR links which are implementation pointers; update the "**Enhancement(s):**" section to reference a VEP/design doc/HLD (or explicitly state that no VEP/HLD exists and point to the relevant Jira ticket as the closest artifact) instead of the CDI and HCO PR URLs; modify the entries that currently contain the CDI Enhancement and HCO PR links so they either point to the appropriate design document or are replaced with a single line like "None (see Jira: CNV-72238)" as required by the STP Metadata rule.
16-20:⚠️ Potential issue | 🟠 MajorHIGH: Remove standard acronyms from Document Conventions
Line 17-Line 20 define common terms (SC/HCO/CDI/PVC). This section should contain only feature-specific terms.
As per coding guidelines: "STP Metadata: Document Conventions must define only feature-specific terms, not standard ones (VM, PVC, CDI, etc. are known to all reviewers)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-storage/scratch_space_sc_selection_logic.md` around lines 16 - 20, Remove the generic acronym entries from the "Document Conventions" section: delete the bullets defining SC, HCO, CDI, and PVC and leave only feature-specific terms under the "Document Conventions" header; if these standard acronyms are needed for readers, move them to a separate "Glossary" or omit entirely per guideline that Document Conventions must not define well-known terms (SC, HCO, CDI, PVC).
347-389:⚠️ Potential issue | 🔴 CriticalCRITICAL: Risk section sign-offs are still placeholders
All risk categories use
*Sign-off:* TBD(Line 347, 354, 361, 368, 375, 382, 388). This blocks approval.As per coding guidelines: "STP Section II.5 Risks: Each risk must have Risk description, Mitigation strategy, and Sign-off" and "No empty placeholders in approved STPs".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-storage/scratch_space_sc_selection_logic.md` around lines 347 - 389, Update each risk block that currently uses "*Sign-off:* TBD" so they include a real sign-off and ensure every risk entry contains a Risk description, Mitigation strategy, and Sign-off as required by STP Section II.5; specifically edit the sections titled "Test Coverage", "Test Environment", "Untestable Aspects", "Resource Constraints", "Dependencies", and "Other" (lines showing the Risk/Mitigation/Sign-off trio) to replace "TBD" with the approving person's or team's name and role (or a valid delegated approver), and if any risk is missing a description or mitigation, add a concise description and mitigation before adding the sign-off.
422-425:⚠️ Potential issue | 🔴 CriticalCRITICAL: Sign-off and Approval still has placeholder
TBDentriesLine 423 and Line 425 must be replaced with real names and GitHub handles (QE Lead, PM, Dev Lead at minimum).
As per coding guidelines: "STP Section IV Sign-off and Approval: Reviewers and Approvers must be listed with names and GitHub handles ... 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-storage/scratch_space_sc_selection_logic.md` around lines 422 - 425, Replace the placeholder "TBD" entries under the "* **Reviewers:**" and "* **Approvers:**" list with real people and their GitHub handles (at minimum QE Lead, PM, and Dev Lead); locate the sections marked by the literal lines "* **Reviewers:**" and "* **Approvers:**" and substitute each "TBD" with "Full Name (`@githubhandle`) — Role" entries so there are no remaining placeholders and the document complies with the STP Sign-off and Approval requirement.
69-75:⚠️ Potential issue | 🟠 MajorHIGH: NFR checklist is incomplete and required categories are missing
Line 69 is unchecked, and the NFR section does not explicitly cover all required categories (Monitoring, Observability, UI, Documentation, Performance, Security, Scalability) with targets/justification.
As per coding guidelines: "STP Section I.1 Requirements: NFRs must explicitly address Monitoring, Observability, UI, Documentation, Performance, Security, and Scalability — uncovered NFRs must have justification".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-storage/scratch_space_sc_selection_logic.md` around lines 69 - 75, The NFR checklist under the "Non-Functional Requirements (NFRs)" section is incomplete; update that section to mark the checklist as completed and explicitly list each required NFR (Monitoring, Observability, UI, Documentation, Performance, Security, Scalability) with concrete targets or a short justification for any items not covered; specifically add entries such as Monitoring (metrics/alerts target: e.g., expose CDI metrics for scratch creation and alert on failures), Observability (tracing/logging scope), UI (user-facing behaviors/UX targets), Documentation (where to update and what defaults changed), Performance (latency/throughput targets or test plan missing justification), Security (access controls/encryption requirements), and Scalability (expected scale and testing approach), and ensure the existing bullet "Existing CDI metrics..." and "Documentation update..." are expanded into these targeted NFR lines and the checklist box is checked.
269-270:⚠️ Potential issue | 🟡 MinorMEDIUM: Upgrade Testing marked N/A without evaluated-rationale
Line 270 says only “Not applicable”. Add explicit note that upgrade path was evaluated and why migration/compat checks are not required.
As per coding guidelines: "STP Section II.2 Test Strategy: ... Upgrade Testing must confirm upgrade path was evaluated".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-storage/scratch_space_sc_selection_logic.md` around lines 269 - 270, Replace the ambiguous "Not applicable" under the "Upgrade Testing — Validates upgrade paths from previous versions, data migration, and configuration preservation" checklist item with a concise explicit rationale that the upgrade path was evaluated and why migration/configuration-preservation checks are not required (e.g., no prior compatible releases, no persisted state, or backward-compatible schema changes); reference the "Upgrade Testing" checklist line in the document and include the evaluation outcome and a brief justification sentence stating which specific conditions (migration not required, no stored state, or compatibility guarantees) were verified.
🤖 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-storage/scratch_space_sc_selection_logic.md`:
- Line 194: The "Out of Scope" section currently contains only "None"; replace
that with an explicit reviewed/confirmed statement following the
documented-decision format: list each non-applicable item (or state "No
exclusions") plus a short Rationale and a PM/Lead Agreement entry that includes
sign-off evidence (name, role, date and Jira or PR link). Update the section
header "Out of Scope" in scratch_space_sc_selection_logic.md to include these
fields for each item (Rationale; PM/Lead Agreement: [Name, Role, Date, Link]) so
the STP meets the II.1 requirements and provides verifiable evidence.
- Around line 287-312: The document contains an internal inconsistency: the
"Cluster Topology" section lists a 3-master/3-worker bare-metal topology while
the "Platform" entry lists PSI (a VM-based environment); update the "Cluster
Topology" or "Platform" entries to be consistent by either (A) changing
"Platform" from PSI to Bare metal if the profile targets bare-metal, (B)
changing "Cluster Topology" to a PSI-appropriate VM topology if PSI is in scope,
or (C) split the spec into two clear execution profiles (e.g., "Bare-metal
profile" and "PSI profile") with corresponding "Cluster Topology" and "Platform"
fields; locate and edit the "Cluster Topology" and "Platform" headings in
scratch_space_sc_selection_logic.md to apply the chosen fix.
---
Duplicate comments:
In `@stps/sig-storage/scratch_space_sc_selection_logic.md`:
- Line 214: The "Test Limitations" entry currently reads "None — reviewed and
confirmed that no test limitations apply for this release." and is missing the
required sign-off; update the STP Section II.1 Test Limitations section (the
line containing that sentence) to include the sign-off line in the exact format
"*Sign-off:* [Name/Date]" (e.g., append a new line "*Sign-off:* Jane
Doe/2026-04-17") so the section complies with the guideline.
- Around line 112-115: The "Developer Handoff/QE Kickoff" checklist item is
still unchecked and contains placeholder text; update the STP Section I.3 item
so the checkbox is marked complete ([x]) and replace the placeholder line "*Key
takeaways and concerns:* [Summarize key points and concerns]" with concrete
handoff notes from the actual meeting (e.g., decisions made, unresolved risks,
testability concerns, action items and owners) ensuring the entry follows the
required "[x] checkbox" format and explicitly documents meeting takeaways rather
than template text.
- Around line 97-104: The Known Limitations section still contains template
placeholders (“**[Feature Limitation 1]**”, “**[Feature Limitation 2]**” and
placeholder sign-off text); replace each placeholder entry with either a
concrete limitation and its sign-off in the required format '*Sign-off:*
[Name/Date]' or, if there are no limitations, replace the block with the exact
statement "None — reviewed and confirmed with [Name/Date]"; update the markdown
around the title "Known Limitations" (the lines containing the placeholder
limitation headings and the "*Sign-off:*" entries) to reflect the real content
and include the reviewer name/date to satisfy STP Section I.2.
- Around line 396-414: Update the test tier for the five CNV-72238 scenarios
listed (the entries titled "import operations", "upload operations", "clone
operations", "scratchSpaceStorageClass configuration to propagate from HCO to
CDI", and "configure scratch space storage class for import operations") from
Tier 2 to Tier 1; locate the bullets by their exact titles in the document (the
CNV-72238 blocks) and change the "*Test Scenario:* [Tier 2]" marker to "*Test
Scenario:* [Tier 1]" for each, and remove or resolve the duplicate_comment note
if present.
- Around line 7-10: The Enhancement(s) field currently lists repo/PR links which
are implementation pointers; update the "**Enhancement(s):**" section to
reference a VEP/design doc/HLD (or explicitly state that no VEP/HLD exists and
point to the relevant Jira ticket as the closest artifact) instead of the CDI
and HCO PR URLs; modify the entries that currently contain the CDI Enhancement
and HCO PR links so they either point to the appropriate design document or are
replaced with a single line like "None (see Jira: CNV-72238)" as required by the
STP Metadata rule.
- Around line 16-20: Remove the generic acronym entries from the "Document
Conventions" section: delete the bullets defining SC, HCO, CDI, and PVC and
leave only feature-specific terms under the "Document Conventions" header; if
these standard acronyms are needed for readers, move them to a separate
"Glossary" or omit entirely per guideline that Document Conventions must not
define well-known terms (SC, HCO, CDI, PVC).
- Around line 347-389: Update each risk block that currently uses "*Sign-off:*
TBD" so they include a real sign-off and ensure every risk entry contains a Risk
description, Mitigation strategy, and Sign-off as required by STP Section II.5;
specifically edit the sections titled "Test Coverage", "Test Environment",
"Untestable Aspects", "Resource Constraints", "Dependencies", and "Other" (lines
showing the Risk/Mitigation/Sign-off trio) to replace "TBD" with the approving
person's or team's name and role (or a valid delegated approver), and if any
risk is missing a description or mitigation, add a concise description and
mitigation before adding the sign-off.
- Around line 422-425: Replace the placeholder "TBD" entries under the "*
**Reviewers:**" and "* **Approvers:**" list with real people and their GitHub
handles (at minimum QE Lead, PM, and Dev Lead); locate the sections marked by
the literal lines "* **Reviewers:**" and "* **Approvers:**" and substitute each
"TBD" with "Full Name (`@githubhandle`) — Role" entries so there are no remaining
placeholders and the document complies with the STP Sign-off and Approval
requirement.
- Around line 69-75: The NFR checklist under the "Non-Functional Requirements
(NFRs)" section is incomplete; update that section to mark the checklist as
completed and explicitly list each required NFR (Monitoring, Observability, UI,
Documentation, Performance, Security, Scalability) with concrete targets or a
short justification for any items not covered; specifically add entries such as
Monitoring (metrics/alerts target: e.g., expose CDI metrics for scratch creation
and alert on failures), Observability (tracing/logging scope), UI (user-facing
behaviors/UX targets), Documentation (where to update and what defaults
changed), Performance (latency/throughput targets or test plan missing
justification), Security (access controls/encryption requirements), and
Scalability (expected scale and testing approach), and ensure the existing
bullet "Existing CDI metrics..." and "Documentation update..." are expanded into
these targeted NFR lines and the checklist box is checked.
- Around line 269-270: Replace the ambiguous "Not applicable" under the "Upgrade
Testing — Validates upgrade paths from previous versions, data migration, and
configuration preservation" checklist item with a concise explicit rationale
that the upgrade path was evaluated and why migration/configuration-preservation
checks are not required (e.g., no prior compatible releases, no persisted state,
or backward-compatible schema changes); reference the "Upgrade Testing"
checklist line in the document and include the evaluation outcome and a brief
justification sentence stating which specific conditions (migration not
required, no stored state, or compatibility guarantees) were verified.
🪄 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: ab3ce65f-e029-415c-9de2-295195d0c85f
📒 Files selected for processing (1)
stps/sig-storage/scratch_space_sc_selection_logic.md
kshvaika
force-pushed
the
stp_for_new_scratch_space_storage_class_selection_logic
branch
from
198168b to
0befebf
Compare
|
duyanyan 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"} |
openshift-virtualization-qe-bot-3
removed
the
changes-requested-coderabbitai[bot]
label
Signed-off-by: Kate Shvaika <kshvaika@redhat.com>
Signed-off-by: Kate Shvaika <kshvaika@redhat.com>
Signed-off-by: Kate Shvaika <kshvaika@redhat.com>
kshvaika
force-pushed
the
stp_for_new_scratch_space_storage_class_selection_logic
branch
from
0befebf to
2631df3
Compare
|
duyanyan 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
|
duyanyan 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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@stps/sig-storage/scratch_space_sc_selection_logic.md`:
- Around line 20-22: The Feature Maturity metadata block still contains
placeholder text for DP/TP/GA; update the DP, TP and GA entries (the "DP:",
"TP:", "GA:" lines) to replace `[version or N/A]`/`[version]` with concrete
target version numbers or explicit `N/A` and add brief justification/evidence
(e.g., sign-off note or Jira/PR reference) inline so each field is non-empty and
verifiable before approval.
- Around line 408-425: Update the five STP entries currently labeled "[Tier 2]"
under the CNV-72238 block so they are "[Tier 1]"; specifically change the tier
for the scenarios that verify: "import requiring conversion allocates scratch
space using target DataVolume storage class", "upload operation allocates
scratch space using target DataVolume storage class", "PVC clone allocates
scratch space using source PVC storage class", "scratchSpaceStorageClass set in
HCO CR propagates to CDI CR configuration", and "import operation uses
scratchSpaceStorageClass configured in HCO" (all associated with the CNV-72238
identifier) to reflect Tier 1 per the STP guidelines.
- Around line 194-206: Remove the placeholder/template bullets in the "Out of
Scope" section (the example items like "Core OCP network functionality" and
"Special guest OS coverage") so only the actual statement remains; replace the
current malformed block that mixes examples and the “None — reviewed…” text with
a single properly formatted Out of Scope entry that simply states "None" (or
"None — reviewed and confirmed that all supported product functionality will be
tested this cycle.") and include a Rationale and a real PM/Lead Agreement line
by replacing the placeholder "[Name/Date]" with an actual name and date in the
required format; ensure there are no leftover template markers or HTML comments
and that the heading and bullets conform to the STP Section II.1 formatting
rules.
🪄 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: f4febdf8-d45a-450d-a5eb-2db17a935f83
📒 Files selected for processing (1)
stps/sig-storage/scratch_space_sc_selection_logic.md
Signed-off-by: Kate Shvaika <kshvaika@redhat.com>
kshvaika
force-pushed
the
stp_for_new_scratch_space_storage_class_selection_logic
branch
from
9e08e32 to
cfe77f3
Compare
|
duyanyan 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
|
duyanyan 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"} |
|
/retest all |
Signed-off-by: Kate Shvaika <kshvaika@redhat.com>
kshvaika
force-pushed
the
stp_for_new_scratch_space_storage_class_selection_logic
branch
from
49a0291 to
925abd3
Compare
|
duyanyan 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: 2
♻️ Duplicate comments (2)
stps/sig-storage/scratch_space_sc_selection_logic.md (2)
10-15:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCRITICAL: Feature Maturity block is malformed and currently hidden by an unclosed HTML comment
<!--starts on Line 11 and is never closed, so the maturity content is not validly represented. Also, required explicit entries forDP,TP, andGAare missing from the rendered document.As per coding guidelines: “Feature Maturity in STP metadata must list each phase with its target version using the structured format:
DP: [version or N/A],TP: [version or N/A],GA: [version].”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stps/sig-storage/scratch_space_sc_selection_logic.md` around lines 10 - 15, The Feature Maturity block is malformed because an HTML comment opener "<!--" is not closed, hiding the maturity content and omitting required DP/TP/GA entries; to fix, remove the stray "<!--" or add a matching "-->" to close the comment and then replace the current free-form line with the required structured entries "DP: [version or N/A]", "TP: [version or N/A]", and "GA: [version]" so the Feature Maturity section is valid and displays the DP, TP, and GA values (edit the "Feature Maturity" paragraph in the markdown to correct the comment and add the explicit DP/TP/GA lines).
169-197:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCRITICAL: Out-of-Scope section still contains template examples and unresolved placeholders
The section still includes example bullets (
[e.g., ...]) and a placeholder*PM/Lead Agreement:* [Name/Date]. This blocks approval because template content and placeholders must not remain in submitted STPs.As per coding guidelines: “STP Section II.1 Out of Scope items must have Rationale and PM/Lead Agreement with name and date” and “STP files must remove all template HTML comments and example text before submitting.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stps/sig-storage/scratch_space_sc_selection_logic.md` around lines 169 - 197, The "Out of Scope (Testing Scope Exclusions)" section contains template/example items and unresolved placeholders (e.g., lines with "[e.g., Core OCP network functionality]" and "*PM/Lead Agreement:* [Name/Date]"); remove all example bullets and HTML comment remnants, replace placeholder PM/Lead Agreement entries with the actual approver name and date or, if there are truly no out-of-scope items, replace the examples with a single clear statement "None — reviewed and confirmed that all supported product functionality will be tested this cycle." to satisfy STP Section II.1 requirements.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@stps/sig-storage/scratch_space_sc_selection_logic.md`:
- Around line 35-37: The file contains leftover template instructional HTML
comments (e.g. the "<!-- **How to complete this checklist:** ... -->" block) and
example checklist text that must be removed; search the document for any HTML
comment blocks ("<!-- ... -->") and any example/template checklist items or
guidance text and delete them so only the actual STP content remains, then
re-run a quick scan to ensure no other template fragments remain (the offending
pattern appears multiple times throughout the doc).
- Around line 115-121: The document uses internal implementation names (CDI,
HCO, scratchSpaceStorageClass) in user-facing narrative sections (including the
blocks around the noted ranges); update those passages to describe observable
behavior and admin-facing configuration effects instead (e.g., "the VM's
ephemeral scratch space is stored on the selected storage class" or "operator
configuration option to choose scratch space storage class") while removing
references to internal component or CRD names; ensure each occurrence of CDI,
HCO, or scratchSpaceStorageClass in the narrative is reworded to a user/admin
outcome or setting name and keep any technical/internal names only in a separate
implementation/appendix section if needed.
---
Duplicate comments:
In `@stps/sig-storage/scratch_space_sc_selection_logic.md`:
- Around line 10-15: The Feature Maturity block is malformed because an HTML
comment opener "<!--" is not closed, hiding the maturity content and omitting
required DP/TP/GA entries; to fix, remove the stray "<!--" or add a matching
"-->" to close the comment and then replace the current free-form line with the
required structured entries "DP: [version or N/A]", "TP: [version or N/A]", and
"GA: [version]" so the Feature Maturity section is valid and displays the DP,
TP, and GA values (edit the "Feature Maturity" paragraph in the markdown to
correct the comment and add the explicit DP/TP/GA lines).
- Around line 169-197: The "Out of Scope (Testing Scope Exclusions)" section
contains template/example items and unresolved placeholders (e.g., lines with
"[e.g., Core OCP network functionality]" and "*PM/Lead Agreement:*
[Name/Date]"); remove all example bullets and HTML comment remnants, replace
placeholder PM/Lead Agreement entries with the actual approver name and date or,
if there are truly no out-of-scope items, replace the examples with a single
clear statement "None — reviewed and confirmed that all supported product
functionality will be tested this cycle." to satisfy 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: 697b9361-fcef-4de1-9ce1-4bb5e43b66bf
📒 Files selected for processing (1)
stps/sig-storage/scratch_space_sc_selection_logic.md
| <!-- **How to complete this checklist:** | ||
| 1. **Checkbox**: Mark [x] if the check is complete; if the item cannot be checked - add an explanation why in the `details` section | ||
| 2. Complete the relevant, needed details for the checklist item --> |
There was a problem hiding this comment.
HIGH: Template instructional HTML comments remain throughout the STP
Large template guidance blocks are still present in the final document. These should be removed from submitted STPs to keep the plan concise and reviewable.
As per coding guidelines: “STP files must remove all template HTML comments and example text before submitting.”
Also applies to: 40-42, 87-94, 100-103, 116-118, 141-185, 202-217, 222-227, 321-323, 341-343, 396-397
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@stps/sig-storage/scratch_space_sc_selection_logic.md` around lines 35 - 37,
The file contains leftover template instructional HTML comments (e.g. the "<!--
**How to complete this checklist:** ... -->" block) and example checklist text
that must be removed; search the document for any HTML comment blocks ("<!-- ...
-->") and any example/template checklist items or guidance text and delete them
so only the actual STP content remains, then re-run a quick scan to ensure no
other template fragments remain (the offending pattern appears multiple times
throughout the doc).
| - [x] **API Extensions** | ||
| <!-- Review new or modified APIs and their impact on testing. Covers both new tests for new APIs and updates to existing tests for modified APIs. | ||
| Example: "New VirtualMachineSnapshot API v1beta2 — 3 new endpoints, 1 modified endpoint. Existing snapshot tests need updating." --> | ||
| - *List new or modified APIs:* | ||
| - No API changes - internal CDI logic change only | ||
| - scratchSpaceStorageClass HCO config option unchanged | ||
| - *Testing impact:* |
There was a problem hiding this comment.
HIGH: Several sections still use internal implementation terms instead of user-facing behavior
Content references internal components/fields (e.g., CDI, HCO, scratchSpaceStorageClass) in narrative sections. Rephrase these to observable user/admin outcomes and configuration behavior without internal API/component naming.
As per coding guidelines: “STPs must describe what users experience, not how the system works internally. No API field names, CRD names, internal component references, or implementation mechanisms.”
Also applies to: 233-236, 398-416
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@stps/sig-storage/scratch_space_sc_selection_logic.md` around lines 115 - 121,
The document uses internal implementation names (CDI, HCO,
scratchSpaceStorageClass) in user-facing narrative sections (including the
blocks around the noted ranges); update those passages to describe observable
behavior and admin-facing configuration effects instead (e.g., "the VM's
ephemeral scratch space is stored on the selected storage class" or "operator
configuration option to choose scratch space storage class") while removing
references to internal component or CRD names; ensure each occurrence of CDI,
HCO, or scratchSpaceStorageClass in the narrative is reworded to a user/admin
outcome or setting name and keep any technical/internal names only in a separate
implementation/appendix section if needed.
6 participants
STP Metadata
VEP issue:
What this PR does
This PR adds STP for the new scratch space storage class selection logic feature in CDI.
CDI default behavior has changed to use same-as-target storage class for scratch space instead of falling back to virt-default or default storage class. The
scratchSpaceStorageClassHCO config option continues to override this behavior when explicitly set.https://redhat.atlassian.net/browse/CNV-72238
Special notes for your reviewer
Summary by CodeRabbit