STP for Adding Support for vmiCPUAllocationRatio in AAQ#67
STP for Adding Support for vmiCPUAllocationRatio in AAQ#67akri3i wants to merge 3 commits intoRedHatQE:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds a new software test plan (STP) for AAQ quota accounting when HCO uses Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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)
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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
stps/sig-virt/aaq-vmi-cpu-allocation-ratio-stp.md (1)
7-8: Replace metadata placeholders with concrete tracking linksLine 7 and Line 8 still use
TBD, which leaves STP traceability incomplete. Please link the relevant HLD/feature tracking item (or explicitly state “N/A” with rationale) before merge.Based on learnings: when no enhancement PR exists in this repo, using only the HLD link in Enhancement(s) is acceptable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-virt/aaq-vmi-cpu-allocation-ratio-stp.md` around lines 7 - 8, Replace the placeholder "TBD" under the metadata headers "**Enhancement(s):**" and "**Feature Tracking:**" in stps/sig-virt/aaq-vmi-cpu-allocation-ratio-stp.md with concrete links or values: add the HLD link (or the enhancement PR link) to "**Enhancement(s):**" and the corresponding tracking issue/board URL to "**Feature Tracking:**"; if no enhancement exists, explicitly use "N/A" with a one-sentence rationale under those headers instead of "TBD" to complete STP traceability.
🤖 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/aaq-vmi-cpu-allocation-ratio-stp.md`:
- Line 206: The checklist entry "Compatibility Testing" is inconsistent ("[x]"
plus "N/A"); update that line (the checklist item labeled Compatibility Testing)
to use one status model: either mark it unchecked and keep "N/A" (e.g., "[ ]
**Compatibility Testing** — N/A") or keep it checked and replace "N/A" with a
concrete validation statement summarizing what compatibility was verified (e.g.,
"[x] **Compatibility Testing** — Verified on X kernel/VM config: ...").
- Line 253: Move the checklist item "AAQ controller handles the ratio in
`VirtualResources` quota calculation" out of the Entry Criteria section and into
the Acceptance/Expected Results section so it reads as an outcome to validate
rather than a precondition; update the Entry Criteria to only include
environment/readiness checks and ensure the Acceptance/Expected Results (or
Acceptance Criteria) contains the relocated outcome and any test steps or
pass/fail conditions that verify the AAQ controller behavior.
---
Nitpick comments:
In `@stps/sig-virt/aaq-vmi-cpu-allocation-ratio-stp.md`:
- Around line 7-8: Replace the placeholder "TBD" under the metadata headers
"**Enhancement(s):**" and "**Feature Tracking:**" in
stps/sig-virt/aaq-vmi-cpu-allocation-ratio-stp.md with concrete links or values:
add the HLD link (or the enhancement PR link) to "**Enhancement(s):**" and the
corresponding tracking issue/board URL to "**Feature Tracking:**"; if no
enhancement exists, explicitly use "N/A" with a one-sentence rationale under
those headers instead of "TBD" to complete STP traceability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ddc9f613-2e3f-4cc4-83fd-c0b715a9ccc4
📒 Files selected for processing (1)
stps/sig-virt/aaq-vmi-cpu-allocation-ratio-stp.md
|
/wip |
openshift-virtualization-qe-bot-5
changed the title
|
/wip cancel |
|
Clean rebase detected — no code changes compared to previous head ( |
openshift-virtualization-qe-bot-4
changed the title
|
Clean rebase detected — no code changes compared to previous head ( |
|
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"} |
1 similar comment
|
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"} |
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 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/aaq-vmi-cpu-allocation-ratio-stp.md`:
- Around line 70-75: Convert the procedural customer use case sentence that
begins "Admin sets ratio 1:10 in HCO; a VM with 10 CPU sockets..." into one or
more explicit user stories in the "As a [role], I want [action], so that
[benefit]" format; replace the procedural line with at least one user story such
as "As a cluster administrator, I want AAQ to honor the CPU overcommit ratio
configured in HCO (e.g., 1:10) so that a VM with 10 sockets consumes only 1 CPU
quota unit," and add any additional user stories needed to cover the scenario
where only spec.domain.cpu.sockets and spec.domain.memory.guest are configured.
- Around line 159-173: The document still has PM/Lead agreement placeholders;
for each section header ("Other `vmiCalcConfigName` modes (e.g.,
`VmiPodUsage`)", "Multiple ratio values (e.g., 1:1, 1:4)", "Upgrade testing",
"Windows guest OS") replace the `[ ] Name/Date` placeholder with an explicit
PM/Lead agreement line containing the approver's name and date (e.g., "PM/Lead
Agreement: Name — YYYY-MM-DD"), ensuring each section now has a populated entry
and no template placeholders remain.
- Line 7: Replace the metadata entry "Enhancement(s): N/A" in the document
titled "aaq-vmi-cpu-allocation-ratio-stp.md" with a traceable design artifact:
provide a link to the VEP or HLD (preferred) or, if neither exists, a Jira epic
URL along with an explicit justification note explaining why a Jira epic is used
instead of a VEP/HLD; ensure you update the "Enhancement(s):" field to contain
that link and the one-line rationale so metadata traceability is satisfied.
- Around line 256-293: The Risks section is missing the required two categories
and has an unresolved sign-off ("TBD"); update the "Risks" block to include all
seven categories (Timeline/Schedule, Test Coverage, Test Environment, Untestable
Aspects, Resource Constraints, Dependencies, Other), provide a concrete
mitigation for each (no "N/A" unless truly none, and avoid placeholders), and
replace the "TBD" sign-off under Test Coverage with an actual approver or team
(e.g., QA Team or specific owner) along with a brief rationale; locate and
modify the Risks stanza and the Test Coverage entry (currently containing "TBD")
so each risk entry includes Risk, Mitigation, Estimated impact/areas, and
Sign-off fields populated with actionable content.
- Line 239: The Test Framework entry currently lists specific standard tools
("pytest, openshift-virtualization-tests framework") but Section II.3.1 should
only mention new/non-standard tooling; update the line to use the
standard/default phrasing (e.g., "Test Framework: Standard") or remove the
explicit tool names so only non-standard tooling would be enumerated; locate the
string "**Test Framework:** pytest, openshift-virtualization-tests framework" in
the document (section II.3.1) and replace it accordingly.
- Around line 303-316: Split the combined "Test Scenario 1 — CPU and memory
quota reflects vmiCPUAllocationRatio" into two separate scenarios: one for CPU
quota validation and one for memory quota validation (create distinct entries
for "Test Scenario — CPU quota reflects vmiCPUAllocationRatio" and "Test
Scenario — Memory quota reflects vmiCPUAllocationRatio"), and change their tier
classification from Tier 2 to Tier 1; ensure each new scenario retains its
priority (P0) and update any surrounding text or TOC entries that referenced the
original combined scenario name so they point to the two new scenario headings.
- Around line 324-335: Update the Sign-off section (the "Reviewers:" and
"Approvers:" lists) to include each person's full name followed by their GitHub
handle (e.g., "Dmitry Shchedrov (`@dshchedr`)"), and explicitly add required roles
— QE Lead, PM, and Dev Lead — as named approvers/reviewers (e.g., "QE Lead: Jane
Doe (`@janedoe`)"). Ensure the lists in the existing Reviewers and Approvers
blocks include both name + handle for each entry and that QE Lead, PM, and Dev
Lead are present and labeled.
- Around line 81-93: Update the Acceptance Criteria to state verifiable
user-observable outcomes instead of internal status fields: replace or augment
the ARQ `status.used.requests.cpu` check with user-facing behavior like "VM1 can
be created with N vCPUs and reports X usable CPUs in the VM UI/metrics" and
"When quota is exhausted, attempts to start VM2 show a clear quota-denied error
and automatically start when quota is increased"; also keep the regression check
that ARQ `status.used.requests.memory` equals `spec.domain.memory.guest`. Expand
the Non-Functional Requirements section to explicitly address Monitoring,
Observability, UI, Documentation, Performance, Security, and Scalability—e.g.,
state required metrics/alerts for quota and CPU reporting, expected UI error
messaging for quota denial, documentation updates for behavior and
configuration, performance targets for allocation checks, security
considerations for quota changes, and scalability limits or tests—mark any
domains not applicable with justification.
- Around line 22-50: The overview currently exposes implementation details
(vmiCPUAllocationRatio, vmiCalcConfigName/GuestEffectiveResources,
spec.domain.cpu/spec.domain.memory.guest, VmiPodUsage, virt-launcher, AAQ/ARQ)
and formulas; rewrite the section to a concise user-facing behavior summary
describing what users will observe (how quota is charged for CPU and memory, and
why AAQ enables live migration without requiring extra headroom) and move all
API names, CRD fields, numeric formulas, and lifecycle/scheduling mechanics into
a separate "Scenarios/Notes" or implementation appendix for reviewers and
operators.
- Around line 99-100: The Known Limitation bullet "Only
`GuestEffectiveResources` calc mode is in scope — Other `vmiCalcConfigName`
values are not affected by this change and are not tested here." is missing the
required sign-off metadata; update that limitation in section I.2 Known
Limitations by appending the sign-off line in the format "*Sign-off:*
[Name/Date]" (e.g., add "*Sign-off:* YourName/2026-04-17") so the limitation
includes the required approval metadata.
- Line 177: The "None — reviewed and confirmed that no test limitations apply
for this release." statement in Section II.1 Test Limitations must be replaced
with a signed confirmation including reviewer name and date; update the Section
II.1 line (the existing "None — reviewed and confirmed..." text) to the required
sign-off format such as "None — reviewed and confirmed that no test limitations
apply for this release. Signed: <Reviewer Name>, Date: <YYYY-MM-DD>." Ensure the
actual reviewer name and ISO-style date are inserted.
🪄 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: 63fe6ec1-553e-4dbf-b55a-c348e044ec01
📒 Files selected for processing (1)
stps/sig-virt/aaq-vmi-cpu-allocation-ratio-stp.md
Signed-off-by: akri3i <guptaakriti70@gmail.com>
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"} |
|
Clean rebase detected — no code changes compared to previous head ( |
|
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"} |
|
Clean rebase detected — no code changes compared to previous head ( |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (7)
stps/sig-virt/aaq-vmi-cpu-allocation-ratio-stp.md (7)
140-152:⚠️ Potential issue | 🟠 MajorHIGH: Out-of-scope agreements still contain unresolved placeholders
Lines 140, 144, 148, and 152 retain
[ ] Name/Date. Replace each with explicit PM/Lead agreement name and date.As per coding guidelines: “STP Out of Scope items must follow format…
*PM/Lead Agreement:*with actual name and date, not placeholders.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-virt/aaq-vmi-cpu-allocation-ratio-stp.md` around lines 140 - 152, Replace the unresolved "[ ] Name/Date" placeholders under the out-of-scope items so each "*PM/Lead Agreement:*" entry contains the actual approver name and date; specifically update the entries under the "**Multiple ratio values (e.g., 1:1, 1:4)**", "**Upgrade testing**", and "**Windows guest OS**" headings (and the earlier PM/Lead Agreement line) to a concrete "Name / YYYY-MM-DD" value to meet the STP Out of Scope format.
156-157:⚠️ Potential issue | 🟠 MajorHIGH: “No test limitations” entry still needs signed confirmation
Line 157 is placeholder-only. If none apply, keep the statement and add actual sign-off identity/date.
As per coding guidelines: “STP Test Limitations must each have sign-off:
*Sign-off:* [Name/Date]… and represent constraints imposed on QE.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-virt/aaq-vmi-cpu-allocation-ratio-stp.md` around lines 156 - 157, The "No test limitations" placeholder at the end of the document needs an actual sign-off; update the footer section that currently reads "None — reviewed and confirmed that no test limitations apply for this release." by appending a real signer name and date in the required format `*Sign-off:* [Name/Date]` (do not remove the statement if no limitations apply), e.g., replace the existing placeholder line with the same sentence followed by `*Sign-off:* [Your Name/YYYY-MM-DD]` so the STP Test Limitations section has a recorded approver and date.
306-320:⚠️ Potential issue | 🟠 MajorHIGH: Sign-off section is missing required role-based approvers formatting
Current lists use
Name/handlebut do not explicitly label QE Lead, PM, and Dev Lead roles. Add role labels and keep name + GitHub handle format consistently.As per coding guidelines: “STP Sign-off and Approval (IV) must list reviewers with names and GitHub handles, approvers (QE Lead, PM, Dev Lead 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/aaq-vmi-cpu-allocation-ratio-stp.md` around lines 306 - 320, Update the "IV. Sign-off and Approval" section to follow STP guidelines by replacing the current unlabelled approvers list with explicit role-based approvers (at minimum QE Lead, PM, Dev Lead) and keep all entries in the "Name/GitHubHandle" format; keep the existing Reviewers list but ensure each reviewer entry remains "Full Name/GitHubHandle", and add three approver entries labeled "QE Lead:", "PM:", and "Dev Lead:" with corresponding "Name/GitHubHandle" values (removing any placeholder text).
62-70:⚠️ Potential issue | 🟠 MajorHIGH: NFR coverage is still incomplete and under-justified
Line 64–69 uses
N/Afor key NFR domains without domain-specific justification, and UI lacks PM/UX value reasoning. This section must explicitly justify Monitoring, Observability, UI, Documentation, Performance, Security, and Scalability coverage/non-coverage.As per coding guidelines: “STP NFRs must explicitly address: Monitoring, Observability, UI, Documentation, Performance, Security, Scalability. Uncovered NFRs require justification” and “STP UI NFR must include PM/UX reasoning on whether testing adds customer value.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-virt/aaq-vmi-cpu-allocation-ratio-stp.md` around lines 62 - 70, The NFR section titled "Non-Functional Requirements (NFRs)" currently uses blanket "N/A" entries and lacks PM/UX rationale for UI; update that block to explicitly justify coverage or non-coverage for each required domain (Monitoring/Observability, UI, Documentation, Performance, Security, Scalability) rather than leaving "N/A", include concrete monitoring signals/metrics and alerting needs if applicable, add PM/UX reasoning for the UI NFR explaining whether and why testing the CPU allocation ratio adds customer value, and expand the Documentation entry to reference the specific docs changes (how CPU allocation ratio affects quota calculations and recommended configs) so the reviewer can verify completeness in the "Non-Functional Requirements (NFRs)" section.
79-79:⚠️ Potential issue | 🟠 MajorHIGH: Replace limitation sign-off placeholder with actual name/date
Line 79 still has
*Sign-off:* [Name/Date], which blocks approval readiness.As per coding guidelines: “Known Limitations in STPs must include sign-off:
*Sign-off:* [Name/Date]… 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-virt/aaq-vmi-cpu-allocation-ratio-stp.md` at line 79, Replace the placeholder sign-off text in the STP by updating the line that reads "*Sign-off:* [Name/Date]" to include the actual approver's full name and the approval date (e.g., "*Sign-off:* Jane Doe / 2026-04-27"); ensure the updated literal replaces "[Name/Date]" under the "Sign-off" section so the document no longer contains an empty placeholder.
236-273:⚠️ Potential issue | 🔴 CriticalCRITICAL: Risks section is non-compliant (missing category + unresolved ownership)
The Risks section omits Dependencies, and Line 251 still has
TBD. Also, several “no risk” entries use full N/A placeholders/sign-offs instead of concise mitigation justification.As per coding guidelines: “STP Risks (II.5) must address ALL 6 standard categories…” and “When risk exists… full entry required… When no risk, only short justification in Mitigation field needed without Sign-off.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-virt/aaq-vmi-cpu-allocation-ratio-stp.md` around lines 236 - 273, The Risks section is missing the required "Dependencies" category, still contains a "TBD" sign-off on line 251, and uses full "N/A" placeholders instead of concise mitigation justifications; update the "Risks" block to include all six standard categories (including Dependencies), replace the "TBD" at the area currently marked on line 251 with a named owner or concrete sign-off for that risk entry, and for entries with no actual risk remove full N/A/sign-off fields and instead populate only the Mitigation field with a short justification (e.g., "No impact expected — mitigation not required"); use the section heading "Risks" and the existing subsections to locate and edit the relevant entries.
283-290:⚠️ Potential issue | 🟡 MinorMEDIUM: Scenario tiering for isolated CPU/memory checks should be Tier 1
Lines 283 and 289 classify isolated validations as Tier 2. These are single-feature validations and should be Tier 1.
As per coding guidelines: “STP Tier 1 scenarios apply to single feature, isolated tests… Tier 2 scenarios apply to end-to-end workflows, multi-feature integration, upgrade paths.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stps/sig-virt/aaq-vmi-cpu-allocation-ratio-stp.md` around lines 283 - 290, Change the scenario tiering for the two isolated checks from Tier 2 to Tier 1: update the bullet lines for "Test Scenario 1 — CPU quota reflects vmiCPUAllocationRatio" and "Test Scenario 2 — Memory quota reflects guest memory only" so their tags read "[Tier 1]" instead of "[Tier 2]"; leave the rest of the text and priorities (P0) unchanged.
🤖 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/aaq-vmi-cpu-allocation-ratio-stp.md`:
- Around line 216-217: There are multiple consecutive blank lines around the
"#### **3.1. Testing Tools & Frameworks**" header (and also at the other
indicated range), which violates the markdownlint rule; edit the markdown to
collapse any runs of empty lines to a single blank line so only single blank
lines separate sections and paragraphs (search for the header "#### **3.1.
Testing Tools & Frameworks**" and the other blank-run location and remove the
extra empty lines).
- Around line 5-13: The Metadata section is missing the structured Feature
Maturity declaration; update the "### **Metadata & Tracking**" block in
aaq-vmi-cpu-allocation-ratio-stp.md to add a `Feature Maturity` entry using the
required format (e.g., `Feature Maturity: DP: [version or N/A], TP: [version or
N/A], GA: [version]`) so that the metadata includes `DP/TP/GA` values alongside
existing fields like "Enhancement(s)" and "Feature Tracking".
- Around line 100-103: The API Extensions section currently exposes internal HCO
spec paths (spec.deployment.applicationAwareConfig.vmiCalcConfigName and
spec.deployment.virtualization.vmiCPUAllocationRatio); remove these internal
field names and rewrite the bullet to describe only the user-facing API impact
(for example: state whether there are any user-visible configuration knobs to
change VMI CPU allocation ratio, whether the change requires altering HCO
configuration via the user-facing console/API, and confirm "no CRD/schema
changes" if true). Ensure the new text does not include internal CRD paths or
component references and instead uses generic user-facing phrasing like "HCO
configuration option for VMI CPU allocation ratio" or "no change to public API
surface."
---
Duplicate comments:
In `@stps/sig-virt/aaq-vmi-cpu-allocation-ratio-stp.md`:
- Around line 140-152: Replace the unresolved "[ ] Name/Date" placeholders under
the out-of-scope items so each "*PM/Lead Agreement:*" entry contains the actual
approver name and date; specifically update the entries under the "**Multiple
ratio values (e.g., 1:1, 1:4)**", "**Upgrade testing**", and "**Windows guest
OS**" headings (and the earlier PM/Lead Agreement line) to a concrete "Name /
YYYY-MM-DD" value to meet the STP Out of Scope format.
- Around line 156-157: The "No test limitations" placeholder at the end of the
document needs an actual sign-off; update the footer section that currently
reads "None — reviewed and confirmed that no test limitations apply for this
release." by appending a real signer name and date in the required format
`*Sign-off:* [Name/Date]` (do not remove the statement if no limitations apply),
e.g., replace the existing placeholder line with the same sentence followed by
`*Sign-off:* [Your Name/YYYY-MM-DD]` so the STP Test Limitations section has a
recorded approver and date.
- Around line 306-320: Update the "IV. Sign-off and Approval" section to follow
STP guidelines by replacing the current unlabelled approvers list with explicit
role-based approvers (at minimum QE Lead, PM, Dev Lead) and keep all entries in
the "Name/GitHubHandle" format; keep the existing Reviewers list but ensure each
reviewer entry remains "Full Name/GitHubHandle", and add three approver entries
labeled "QE Lead:", "PM:", and "Dev Lead:" with corresponding
"Name/GitHubHandle" values (removing any placeholder text).
- Around line 62-70: The NFR section titled "Non-Functional Requirements (NFRs)"
currently uses blanket "N/A" entries and lacks PM/UX rationale for UI; update
that block to explicitly justify coverage or non-coverage for each required
domain (Monitoring/Observability, UI, Documentation, Performance, Security,
Scalability) rather than leaving "N/A", include concrete monitoring
signals/metrics and alerting needs if applicable, add PM/UX reasoning for the UI
NFR explaining whether and why testing the CPU allocation ratio adds customer
value, and expand the Documentation entry to reference the specific docs changes
(how CPU allocation ratio affects quota calculations and recommended configs) so
the reviewer can verify completeness in the "Non-Functional Requirements (NFRs)"
section.
- Line 79: Replace the placeholder sign-off text in the STP by updating the line
that reads "*Sign-off:* [Name/Date]" to include the actual approver's full name
and the approval date (e.g., "*Sign-off:* Jane Doe / 2026-04-27"); ensure the
updated literal replaces "[Name/Date]" under the "Sign-off" section so the
document no longer contains an empty placeholder.
- Around line 236-273: The Risks section is missing the required "Dependencies"
category, still contains a "TBD" sign-off on line 251, and uses full "N/A"
placeholders instead of concise mitigation justifications; update the "Risks"
block to include all six standard categories (including Dependencies), replace
the "TBD" at the area currently marked on line 251 with a named owner or
concrete sign-off for that risk entry, and for entries with no actual risk
remove full N/A/sign-off fields and instead populate only the Mitigation field
with a short justification (e.g., "No impact expected — mitigation not
required"); use the section heading "Risks" and the existing subsections to
locate and edit the relevant entries.
- Around line 283-290: Change the scenario tiering for the two isolated checks
from Tier 2 to Tier 1: update the bullet lines for "Test Scenario 1 — CPU quota
reflects vmiCPUAllocationRatio" and "Test Scenario 2 — Memory quota reflects
guest memory only" so their tags read "[Tier 1]" instead of "[Tier 2]"; leave
the rest of the text and priorities (P0) unchanged.
🪄 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: f074f9a2-dfdd-4ff9-80aa-7a07f8ab4ed9
📒 Files selected for processing (1)
stps/sig-virt/aaq-vmi-cpu-allocation-ratio-stp.md
| ### **Metadata & Tracking** | ||
|
|
||
| - **Enhancement(s):** No VEP or HLD exists for this change; the feature is tracked via Jira epic [CNV-73947](https://redhat.atlassian.net/browse/CNV-73947) | ||
| - **Feature Tracking:** https://redhat.atlassian.net/browse/CNV-61868 | ||
| - **Epic Tracking:** https://redhat.atlassian.net/browse/CNV-73947 | ||
| - **QE Owner(s):** Akriti Gupta (akritigupta@redhat.com) | ||
| - **Owning SIG:** sig-virt | ||
| - **Participating SIGs:** sig-virt | ||
|
|
There was a problem hiding this comment.
HIGH: Metadata is missing structured Feature Maturity field
Metadata currently lacks the required DP/TP/GA maturity declaration.
As per coding guidelines: “Feature Maturity in STP metadata must use structured format: DP: [version or N/A], TP: [version or N/A], GA: [version]…”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stps/sig-virt/aaq-vmi-cpu-allocation-ratio-stp.md` around lines 5 - 13, The
Metadata section is missing the structured Feature Maturity declaration; update
the "### **Metadata & Tracking**" block in aaq-vmi-cpu-allocation-ratio-stp.md
to add a `Feature Maturity` entry using the required format (e.g., `Feature
Maturity: DP: [version or N/A], TP: [version or N/A], GA: [version]`) so that
the metadata includes `DP/TP/GA` values alongside existing fields like
"Enhancement(s)" and "Feature Tracking".
| - [x] **API Extensions** | ||
| - *List new or modified APIs:* HCO `spec.deployment.applicationAwareConfig.vmiCalcConfigName` (existing field); | ||
| HCO `spec.deployment.virtualization.vmiCPUAllocationRatio` (existing field); no new CRD fields | ||
| - *Testing impact:* No new test infrastructure required; existing HCO patching patterns apply |
There was a problem hiding this comment.
HIGH: API Extensions section exposes internal implementation fields
Lines 101–102 enumerate internal HCO spec paths. This section must be rewritten to user-facing API impact only.
As per coding guidelines: “STPs must describe features from user perspective only — no API field names, CRD names, internal component references…” and “API Extensions… must mention only user-facing APIs.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stps/sig-virt/aaq-vmi-cpu-allocation-ratio-stp.md` around lines 100 - 103,
The API Extensions section currently exposes internal HCO spec paths
(spec.deployment.applicationAwareConfig.vmiCalcConfigName and
spec.deployment.virtualization.vmiCPUAllocationRatio); remove these internal
field names and rewrite the bullet to describe only the user-facing API impact
(for example: state whether there are any user-visible configuration knobs to
change VMI CPU allocation ratio, whether the change requires altering HCO
configuration via the user-facing console/API, and confirm "no CRD/schema
changes" if true). Ensure the new text does not include internal CRD paths or
component references and instead uses generic user-facing phrasing like "HCO
configuration option for VMI CPU allocation ratio" or "no change to public API
surface."
|
|
||
| #### **3.1. Testing Tools & Frameworks** |
There was a problem hiding this comment.
LOW: Remove consecutive blank lines to satisfy markdown linting
There are multiple consecutive blank lines in these ranges; collapse to single blank lines.
As per coding guidelines: “Markdown files must follow .markdownlint.yaml configuration… no multiple consecutive blank lines.”
Also applies to: 274-275
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stps/sig-virt/aaq-vmi-cpu-allocation-ratio-stp.md` around lines 216 - 217,
There are multiple consecutive blank lines around the "#### **3.1. Testing Tools
& Frameworks**" header (and also at the other indicated range), which violates
the markdownlint rule; edit the markdown to collapse any runs of empty lines to
a single blank line so only single blank lines separate sections and paragraphs
(search for the header "#### **3.1. Testing Tools & Frameworks**" and the other
blank-run location and remove the extra empty lines).
7 participants
STP Metadata
VEP issue:
What this PR does
STP for Adding Support for vmiCPUAllocationRatio in AAQ
Special notes for your reviewer
Summary by CodeRabbit