WIP: [net] STP for 3rd party IP request (CNV-67524) #7
Conversation
yossisegev
force-pushed
the
net-ip-request-stp
branch
from
71dbb7d to
f74211e
Compare
|
@servolkov @Anatw @azhivovk @EdDev @rnetser @orelmisan |
|
/retest tox |
1 similar comment
|
/retest tox |
stps/sig-network/ip-request-stp.md
Outdated
| * verify the address is pingable | ||
| * verify this interface can maintain TCP connectivity | ||
| * the feature should be applied on both IPv4/6 families | ||
| * verify the assigned address and connectivity are maintained after VM reboot |
There was a problem hiding this comment.
What if you create a VM without the annotation, than stop the VM and add the annotation - after you start the VM it should have the new requested address? I think it worth a test case.
There was a problem hiding this comment.
This is an invalid scenario; the feature is about requesting new VMs with the IPAM address.
There was a problem hiding this comment.
@yossisegev this should be mentioned in the stp
yossisegev
force-pushed
the
net-ip-request-stp
branch
from
f74211e to
e926454
Compare
WalkthroughAdds a new OpenShift Virtualization Software Test Plan that defines requirements, test strategy, environment, scenarios, traceability, UDN manifests, use of the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
openshift-virtualization-qe-bot-3
added
branch-main
size/L
and removed
commented-rnetser
commented-Anatw
commented-azhivovk
labels
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
stps/sig-network/ip-request-stp.md (2)
7-15: Minor: Fix markdown table formatting and consider conciseness improvements.Several markdown formatting issues should be addressed:
- Lines 7–15 and lines 124–145: Table pipe style is inconsistent—leading pipes present but trailing pipes missing. Ensure tables have both leading and trailing pipes for consistent formatting.
- Line 21: "prior to formal test planning" could be simplified to "before formal test planning" for conciseness.
These are style improvements and do not affect content readability.
Also applies to: 59-80, 124-124, 134-145
159-170: Clarify entry criteria and integration boundary.The Entry Criteria section (lines 159–170) correctly identifies that testing begins with pre-created UDN and VM specs with the
addressesannotation. However, ensure that Section I (Motivation and Requirements Review), Section II.3 (Non-Goals), and Section II.8 (Known Limitations) all explicitly and consistently state that no integration testing with third-party IPAM providers will be performed; testing only validates the feature mechanism itself with manually-provided inputs. This was flagged in past review comments and should be reinforced to prevent scope creep or stakeholder misunderstandings.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
stps/sig-network/ip-request-stp.md
🧰 Additional context used
🪛 LanguageTool
stps/sig-network/ip-request-stp.md
[grammar] ~3-~3: Use a hyphen to join words.
Context: ...cific IP for a VM in UDN through a third party integration - Quality Engineering ...
(QB_NEW_EN_HYPHEN)
[style] ~21-~21: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...re's value, technology, and testability prior to formal test planning. #### **1. Requir...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
[grammar] ~77-~77: Ensure spelling is correct
Context: ...Templating VM spec with the addresses annottaion. | It's the responsibility of the 3rd p...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~140-~140: In American English, abbreviations like “etc.” require a period.
Context: ... | [e.g., Specific NICs for SR-IOV, GPU etc] ...
(ETC_PERIOD)
[grammar] ~193-~193: Use a hyphen to join words.
Context: ...here will be no integration with the 3rd party providers; 3rd party is responsibl...
(QB_NEW_EN_HYPHEN)
[grammar] ~201-~201: Ensure spelling is correct
Context: ... requirements are tested. * The testing scenarioa and goals are specified in "4. Test Str...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
stps/sig-network/ip-request-stp.md
10-10: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
57-57: Spaces inside code span elements
(MD038, no-space-in-code)
124-124: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
124-124: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: can-be-merged
- GitHub Check: tox
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
stps/sig-network/ip-request-stp.md (1)
21-21: Optional: Consider more concise phrasing.Line 21 uses "prior to" which could be shortened to "before" for slightly better readability without changing meaning: "...and testability before formal test planning."
Also applies to: 21-21
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
stps/sig-network/ip-request-stp.md
🧰 Additional context used
🪛 LanguageTool
stps/sig-network/ip-request-stp.md
[grammar] ~3-~3: Use a hyphen to join words.
Context: ...cific IP for a VM in UDN through a third party integration - Quality Engineering ...
(QB_NEW_EN_HYPHEN)
[style] ~21-~21: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...re's value, technology, and testability prior to formal test planning. #### **1. Requir...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
[grammar] ~77-~77: Ensure spelling is correct
Context: ...Templating VM spec with the addresses annottaion. | It's the responsibility of the 3rd ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~139-~139: In American English, abbreviations like “etc.” require a period.
Context: ... | [e.g., Specific NICs for SR-IOV, GPU etc] | | **Stor...
(ETC_PERIOD)
[grammar] ~192-~192: Use a hyphen to join words.
Context: ...here will be no integration with the 3rd party providers; 3rd party is responsibl...
(QB_NEW_EN_HYPHEN)
[grammar] ~200-~200: Ensure spelling is correct
Context: ... requirements are tested. * The testing scenarioa and goals are specified in "4. Test Str...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: can-be-merged
- GitHub Check: tox
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
Not that I know of, but I am rechecking with the devs, and I will add them in the STP if there any tier-1 tests. |
yossisegev
force-pushed
the
net-ip-request-stp
branch
from
807b4e4 to
cf94011
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @stps/sig-network/ip-request-stp.md:
- Line 62: Fix the typo in the sentence fragment "UDN manifest with desirred IP
subnet, and a VM manifest with a dedicated annotation to specify" by changing
"desirred" to "desired" so the line reads "...UDN manifest with desired IP
subnet..."; update this occurrence in the stps/sig-network/ip-request-stp.md
content (search for the phrase "UDN manifest with desirred" to locate it).
- Line 77: Update the typo in the documentation: change the word "presistence"
to "persistence" in the bullet "[P0] Verify IP presistence (over reboot,
migration, ...)" so it reads "[P0] Verify IP persistence (over reboot,
migration, ...)".
- Line 84: Replace the phrase "User Defined Network" with the hyphenated
compound adjective "User-Defined Network" where UDN is defined (the line
containing "UDN: User Defined Network (OVN-Kubernetes overlay network)") so the
entry reads "UDN: User-Defined Network (OVN-Kubernetes overlay network)". Ensure
the hyphenation is applied only to the compound adjective form in the UDN
definition.
- Around line 98-104: Fill in PM/Lead sign-off (Name/Date) for each non-goal row
listed so they are not left blank: "Templating VM spec with the `addresses`
annotation", "Creating C/UDN with IP range", "Secondary UDN interfaces", and
"Annotated address out of UDN range"; update the checkbox cells in the table to
include the approving PM/Lead name and date next to each item (e.g., replace "[
] Name/Date" with "[x] Alice Smith 2026-01-11" or similar) so the STP has
explicit stakeholder agreement before approval.
- Line 213: Rewrite the sentence at line 213 to a complete, grammatical
explanation that tier-1 testing is not possible due to the feature relying on
OVN-k8s (e.g., "All verification will be performed in Tier‑2 because this
feature depends on OVN‑k8s components that kubevirt developers cannot access in
Tier‑1 environments."), update Section III's test table to explicitly label each
scenario as Tier‑1 or Tier‑2 (mark E2E/cluster-level scenarios as Tier‑2 and
unit/isolated functional checks as Tier‑1 where applicable), and add a short
constraint note in the tests section explaining why Tier‑1 tests cannot be
executed (mention OVN‑k8s dependency, required cluster configuration, or
environment access limitations) so reviewers can see the exact reason.
🧹 Nitpick comments (1)
stps/sig-network/ip-request-stp.md (1)
177-182: Clarify entry criteria regarding test automation setup.Line 177 mentions "creating the C/UDN resource" and "VM manifest with the new
addressesannotation," but notes these will be patched manually since third-party integration isn't available. Clarify whether automated test cases or test fixtures need to be prepared for these setup steps, and document the expected test data format/structure.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
stps/sig-network/ip-request-stp.md
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-08T17:18:15.313Z
Learnt from: yossisegev
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 7
File: stps/sig-network/ip-request-stp.md:0-0
Timestamp: 2026-01-08T17:18:15.313Z
Learning: In the repository RedHatQE/openshift-virtualization-tests-design-docs, the preferred style is "3rd party" without hyphenation, not "third-party".
Applied to files:
stps/sig-network/ip-request-stp.md
🪛 LanguageTool
stps/sig-network/ip-request-stp.md
[grammar] ~3-~3: Use a hyphen to join words.
Context: ...cific IP for a VM in UDN through a third party integration - Quality Engineering ...
(QB_NEW_EN_HYPHEN)
[style] ~22-~22: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...re's value, technology, and testability prior to formal test planning. #### **1. Requir...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
[grammar] ~77-~77: Ensure spelling is correct
Context: ... claimed IP address. - [P0] Verify IP presistence (over reboot, migration, ...) - [P1] ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[uncategorized] ~84-~84: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...t Conventions (if applicable):** * UDN: User Defined Network (OVN-Kubernetes overlay network...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~205-~205: Use a hyphen to join words.
Context: ...here will be no integration with the 3rd party providers; 3rd party is responsibl...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (4)
stps/sig-network/ip-request-stp.md (4)
3-3: Align terminology with repository style: use "3rd party" instead of "third party".Per repository learnings, the preferred style is "3rd party" without hyphenation.
🔧 Proposed fix
-## **Request a specific IP for a VM in UDN through a third party integration - Quality Engineering Plan** +## **Request a specific IP for a VM in UDN through a 3rd party integration - Quality Engineering Plan**
125-125: Align terminology with repository style: use "3rd party" instead of "3rd-party".Per repository learnings, the preferred style is "3rd party" without hyphenation, not "3rd-party" with hyphen.
🔧 Proposed fix
-| Usability Testing | N | The annotation is done by the 3rd-party integrator, therefore no new UI capability to test. +| Usability Testing | N | The annotation is done by the 3rd party integrator, therefore no new UI capability to test.
205-205: Align all terminology instances with repository style: use "3rd party" consistently.Per repository learnings, the preferred style is "3rd party" (no hyphen). Update both instances on line 205.
🔧 Proposed fix
-- There will be no integration with the 3rd party providers; 3rd party is responsible for providing valid UDN and annotated VM specs, which will be the starting point of the testing in CNV. +- There will be no integration with the 3rd party providers; the 3rd party is responsible for providing valid UDN and annotated VM specs, which will be the starting point of the testing in CNV.
49-49: Fix typo: "exechanged" → "exchanged".🐛 Proposed fix
-| Several meetings held + design docs exechanged. +| Several meetings held + design docs exchanged.Likely an incorrect or invalid review comment.
| | Non-Goal | Rationale | PM/ Lead Agreement | | ||
| |:----------------------------------------------------|:---------------------------------------------------------------------------------------------------------------------------------------------------|:------------------- | | ||
| | Templating VM spec with the `addresses` annotation. | It's the responsibility of the 3rd party integrator, and not accessible by us.<br/>We cannot and will not test the integration with the 3rd party. | [ ] Name/Date | | ||
| | | Our side start with an annotated VM manifest. | | | ||
| | Creating C/UDN with IP range. | Same | [ ] Name/Date | | ||
| | Secondary UDN interfaces. | Not supported | [ ] Name/Date | | ||
| | Annotated address out of UDN range. | Unsupported by OVN-k8s and behavior is unexpected. | [ ] Name/Date | |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd stps/sig-network && cat -n ip-request-stp.md | sed -n '95,115p'Repository: RedHatQE/openshift-virtualization-tests-design-docs
Length of output: 2322
Critical: All non-goal checkboxes lack PM/Lead sign-off and must be completed before STP approval.
Per the document's own guidance (line 108), non-goals without stakeholder agreement are considered risks and must be escalated. All four non-goals currently have empty sign-off checkboxes: Templating VM spec with the addresses annotation, Creating C/UDN with IP range, Secondary UDN interfaces, and Annotated address out of UDN range. Each requires PM/Lead approval with name and date before the STP can proceed.
🤖 Prompt for AI Agents
In @stps/sig-network/ip-request-stp.md around lines 98 - 104, Fill in PM/Lead
sign-off (Name/Date) for each non-goal row listed so they are not left blank:
"Templating VM spec with the `addresses` annotation", "Creating C/UDN with IP
range", "Secondary UDN interfaces", and "Annotated address out of UDN range";
update the checkbox cells in the table to include the approving PM/Lead name and
date next to each item (e.g., replace "[ ] Name/Date" with "[x] Alice Smith
2026-01-11" or similar) so the STP has explicit stakeholder agreement before
approval.
cf94011 to
d7c2dfc
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @stps/sig-network/ip-request-stp.md:
- Around line 94-107: The Non-Goals table in the "Non-Goals (Testing Scope
Exclusions)" section has empty PM/Lead agreement cells for all entries, which
per the document policy must be signed off or escalated; update the Non-Goals
table rows (the four entries under "Templating VM spec...", "Creating C/UDN with
IP range", "Secondary UDN interfaces", and "Annotated address out of UDN range")
by adding PM/Lead names and dates in the "PM/ Lead Agreement" column, or if
sign-off cannot be obtained, add explicit risk entries referencing these
specific non-goals into Section II.7 (Escalations/Risks) with owner and
mitigation so the document meets the stated requirement.
- Line 79: Fix the typos in the markdown: change the phrase "Verify IP
presistence" to "Verify IP persistence" and change "User Defined Network" to
hyphenated "User-Defined Network" wherever those exact phrases appear in the
document (e.g., the line containing "Verify IP presistence (over reboot,
migration, ...)" and the line using "User Defined Network").
- Line 3: Change the phrase in the title/header from "third party" to "3rd
party" (no hyphen) so the line reads "Request a specific IP for a VM in UDN
through a 3rd party integration - Quality Engineering Plan", and in the metadata
table remove the duplicated "Enhancement" entry by consolidating the two
identical enhancement rows into a single row (keep one occurrence and delete the
redundant one) so the table has only one Enhancement entry.
- Around line 212-230: Fix the grammatical error in Section III's explanatory
sentence: replace "the feature as an OVN-k8s so kubevirt developers cannot test
it tier-1." with a clearer phrasing such as "the feature requires OVN‑k8s, so
kubevirt developers cannot test it in tier‑1." Update the sentence in the IP
request STP text under "III. Test Scenarios & Traceability" accordingly to
preserve the intended meaning and readability.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
stps/sig-network/ip-request-stp.md
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-08T17:18:15.313Z
Learnt from: yossisegev
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 7
File: stps/sig-network/ip-request-stp.md:0-0
Timestamp: 2026-01-08T17:18:15.313Z
Learning: In the repository RedHatQE/openshift-virtualization-tests-design-docs, the preferred style is "3rd party" without hyphenation, not "third-party".
Applied to files:
stps/sig-network/ip-request-stp.md
🪛 LanguageTool
stps/sig-network/ip-request-stp.md
[grammar] ~3-~3: Use a hyphen to join words.
Context: ...cific IP for a VM in UDN through a third party integration - Quality Engineering ...
(QB_NEW_EN_HYPHEN)
[style] ~22-~22: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...re's value, technology, and testability prior to formal test planning. #### **1. Requir...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
[grammar] ~79-~79: Ensure spelling is correct
Context: ... claimed IP address. - [P0] Verify IP presistence (over reboot, migration, ...) - [P1] ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[uncategorized] ~86-~86: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...t Conventions (if applicable):** * UDN: User Defined Network (OVN-Kubernetes overlay network...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~207-~207: Use a hyphen to join words.
Context: ...here will be no integration with the 3rd party providers; 3rd party is responsibl...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (7)
stps/sig-network/ip-request-stp.md (7)
20-55: Section I (Motivation & Requirements Review) is thorough and complete.All requirement and technology review checks are marked verified with sufficient detail. NFR coverage is well-documented.
65-89: Section II.1 (Scope) is well-defined with clear priorities.The in-scope items are appropriately scoped and aligned with MTV IPAM testing patterns. P0/P1 prioritization is sensible.
133-143: Section II.4.C (Potential Areas) is complete and well-justified.All dependencies, monitoring, cross-integrations, and UI considerations are properly addressed.
144-160: Section II.5 (Test Environment) is well-specified.Environment requirements are clearly defined, including IPv4/6 coverage. Test infrastructure needs are appropriate for the feature scope.
173-184: Section II.6 (Entry Criteria) clearly defines when testing can begin.Criteria are appropriate, and the transparent note about starting from annotated VM creation (rather than full 3rd-party integration) sets correct expectations.
201-208: Section II.8 (Known Limitations) is clear and appropriately transparent.Limitations are well-articulated and consistent with the feature's current scope. The note about starting from annotated VM specs (rather than full 3rd-party integration) aligns with testing strategy.
233-242: Section IV (Sign-off) identifies stakeholders but approval signatures are pending.Reviewer list is comprehensive. For final STP approval, add sign-off rows with date/approval confirmation from each stakeholder, especially given the PR review comments from rnetser asking about tier1 tests (now answered in Section III).
yossisegev
force-pushed
the
net-ip-request-stp
branch
from
d7c2dfc to
ff6bc87
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @stps/sig-network/ip-request-stp.md:
- Line 179: Summary: Fix a capitalization typo in the Entry Criteria section.
Locate the Entry Criteria text containing the word "VIrtualization" and replace
it with the correctly capitalized "Virtualization" (i.e., change the lowercase
"I" after "V" to a lowercase "i") so the phrase reads "Virtualization" in that
sentence.
- Line 193: In the Risk Category table row for "Timeline/Schedule" replace the
misspelled word "Targetted" with "Targeted" in the cell text ("Targetted for
4.21 but test automation won't be ready for code-freeze." → "Targeted for 4.21
but test automation won't be ready for code-freeze."), ensuring the rest of the
sentence and spacing remain unchanged.
🧹 Nitpick comments (1)
stps/sig-network/ip-request-stp.md (1)
22-22: Optional: Minor wordiness refinement.The phrase "prior to formal test planning" could be simplified to "before formal test planning" for greater clarity, though the current phrasing is acceptable in formal documentation.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
stps/sig-network/ip-request-stp.md
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-08T17:18:15.313Z
Learnt from: yossisegev
Repo: RedHatQE/openshift-virtualization-tests-design-docs PR: 7
File: stps/sig-network/ip-request-stp.md:0-0
Timestamp: 2026-01-08T17:18:15.313Z
Learning: In the repository RedHatQE/openshift-virtualization-tests-design-docs, the preferred style is "3rd party" without hyphenation, not "third-party".
Applied to files:
stps/sig-network/ip-request-stp.md
🪛 LanguageTool
stps/sig-network/ip-request-stp.md
[grammar] ~3-~3: Use a hyphen to join words.
Context: ...pecific IP for a VM in UDN through a 3rd party integration - Quality Engineering ...
(QB_NEW_EN_HYPHEN)
[style] ~22-~22: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...re's value, technology, and testability prior to formal test planning. #### **1. Requir...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
[grammar] ~193-~193: Ensure spelling is correct
Context: ...------------| | Timeline/Schedule | Targetted for 4.21 but test automation won't be r...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~207-~207: Use a hyphen to join words.
Context: ...here will be no integration with the 3rd party providers; 3rd party is responsibl...
(QB_NEW_EN_HYPHEN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: tox
🔇 Additional comments (6)
stps/sig-network/ip-request-stp.md (6)
3-3: Style choice aligns with repository conventions.The title uses "3rd party" without hyphenation, which is the preferred style for this repository per established conventions. The LanguageTool suggestion to hyphenate can be safely disregarded.
100-106: Obtain stakeholder sign-off for Non-Goals or escalate as risks.The Non-Goals table (lines 100-106) has three items with empty PM/Lead Agreement cells. Per the document's own guidance (line 110), non-goals without stakeholder agreement must be escalated as risks. Either obtain explicit sign-off from PM/Lead (filling in Name/Date), or formally escalate these to the Risks section with mitigation strategies.
Current unsignored non-goals:
- Line 102: Templating VM spec with the
addressesannotation- Line 104: Creating C/UDN with IP range
- Line 106: Annotated address out of UDN range
214-214: Tier-2 testing clarification addresses prior reviewer concern.The explicit statement that "Everything will be tested in tier-2, as the feature is an OVN-k8s so kubevirt developers cannot test it in tier-1" directly addresses the earlier concern about tier-1 test availability. This rationale is sound given that OVN-k8s behavior is a dependency.
212-228: Test scenarios and traceability are comprehensive.Section III provides clear mapping from requirements to test scenarios with appropriate priorities. Coverage includes all critical functionality (visibility, connectivity, persistence), error cases, and IPv4/6 variants. The tier-2 testing constraint is well-rationalized given OVN-k8s dependencies.
232-240: Sign-off section complete for review phase; ensure formal approvals added before merge.The reviewers and stakeholders are clearly identified. For final merge, ensure this section is updated with actual approval dates and signatures from each reviewer. Consider using a more formal tracking mechanism (e.g., checkbox with name/date) to match the non-goals approval pattern elsewhere in the document.
201-207: Known Limitations section clearly scopes testing boundaries.The Known Limitations section properly identifies three key constraints:
- No ARM64 support
- Primary interface only (no secondary)
- No 3rd-party provider integration (testing starts from UDN/VM manifest)
These align with earlier Non-Goals and Entry Criteria sections, providing consistent scope management throughout the document.
|
|
||
| - [ ] Requirements and design documents are **approved and merged** | ||
| - [ ] Test environment can be **set up and configured** (see Section II.5 - Test Environment) | ||
| - [ ] The starting point of this testing, from OpenShift VIrtualization networking perspective, is by performing the 2\ |
There was a problem hiding this comment.
Fix typo in Entry Criteria section.
"VIrtualization" has an incorrect capital letter; should be "Virtualization".
💚 Proposed fix
- - The starting point of this testing, from OpenShift VIrtualization networking perspective, is by performing the 2\
+ - The starting point of this testing, from OpenShift Virtualization networking perspective, is by performing the 2\🤖 Prompt for AI Agents
In @stps/sig-network/ip-request-stp.md at line 179, Summary: Fix a
capitalization typo in the Entry Criteria section. Locate the Entry Criteria
text containing the word "VIrtualization" and replace it with the correctly
capitalized "Virtualization" (i.e., change the lowercase "I" after "V" to a
lowercase "i") so the phrase reads "Virtualization" in that sentence.
|
|
||
| | Risk Category | Specific Risk for This Feature | Mitigation Strategy | Status | | ||
| |:--------------------- |:---------------------------------------------------------------------------------------------------------------|:--------------------|:------------| | ||
| | Timeline/Schedule | Targetted for 4.21 but test automation won't be ready for code-freeze. | Negotiated. | [discussed] | |
There was a problem hiding this comment.
Fix spelling error in Risk Category table.
"Targetted" should be "Targeted" (single 't').
💚 Proposed fix
- | Timeline/Schedule | Targetted for 4.21 but test automation won't be ready for code-freeze. | Negotiated. | [discussed] |
+ | Timeline/Schedule | Targeted for 4.21 but test automation won't be ready for code-freeze. | Negotiated. | [discussed] |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | Timeline/Schedule | Targetted for 4.21 but test automation won't be ready for code-freeze. | Negotiated. | [discussed] | | |
| | Timeline/Schedule | Targeted for 4.21 but test automation won't be ready for code-freeze. | Negotiated. | [discussed] | |
🧰 Tools
🪛 LanguageTool
[grammar] ~193-~193: Ensure spelling is correct
Context: ...------------| | Timeline/Schedule | Targetted for 4.21 but test automation won't be r...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
In @stps/sig-network/ip-request-stp.md at line 193, In the Risk Category table
row for "Timeline/Schedule" replace the misspelled word "Targetted" with
"Targeted" in the cell text ("Targetted for 4.21 but test automation won't be
ready for code-freeze." → "Targeted for 4.21 but test automation won't be ready
for code-freeze."), ensuring the rest of the sentence and spacing remain
unchanged.
| | Check | Done | Details/Notes | Comments | | ||
| |:--------------------------------------- |:-----|:----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:--------- | | ||
| | **Review Requirements** | [V] | Reviewed the relevant requirements. | | | ||
| | **Understand Value** | [V] | Allow seamless migration to OpenShift, with minimal network changes to customer's workloads. | | |
There was a problem hiding this comment.
this is not the value of what is being tested here; the value is allowing customers to integrate with 3rd party IPAMs
please remove any ref to MTV; while this feature is using the underlying openshift functionality, the stp is only about the tested feature.
you can mention in a comment that the same mechanism is applied but iiuc this feature is going to be proposed to customers.
with that in mind (and unlike what i thought when this originally came up in a discussion with the team), this feature needs to be tested separate of mtv use cases
|
/wip |
openshift-virtualization-qe-bot
changed the title
9 participants
STP Metadata
VEP issue: https://issues.redhat.com/browse/CNV-67524
What this PR does
STP for new feature of Request a specific IP for a VM in UDN through a third party integration
Special notes for your reviewer
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.