OCPBUGS-82046: Revert "remove some duplicate ovnk test cases" #30975
OCPBUGS-82046: Revert "remove some duplicate ovnk test cases" #30975arkadeepsen wants to merge 2 commits intoopenshift:mainfrom
Conversation
This reverts commit 2692e1d.
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
openshift-ci-robot
added
jira/valid-reference
|
@arkadeepsen: This pull request references Jira Issue OCPBUGS-82046, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughAdds new test cases validating NetworkAttachmentDefinition (NAD) object creation from UserDefinedNetwork (UDN) and ClusterUserDefinedNetwork (CUDN) objects, including helper functions for NAD manifest validation and updated imports. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes ✨ 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 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: arkadeepsen The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/jira refresh |
openshift-ci-robot
added
jira/valid-bug
|
@arkadeepsen: This pull request references Jira Issue OCPBUGS-82046, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/extended/networking/network_segmentation.go`:
- Around line 1533-1540: The OwnerReference Name field is hardcoded to
"test-net" inside the ExpectWithOffset assertion; change it to use the udnName
parameter so the OwnerReference is constructed with Name: udnName (consistent
with the earlier usage of udnName and with assertClusterNADManifest). Locate the
ExpectWithOffset(1, nad.OwnerReferences) block and replace the literal
"test-net" with udnName, ensuring other fields (UID: types.UID(udnUID),
BlockOwnerDeletion, Controller, APIVersion, Kind) remain 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5640bb55-188a-4008-8825-d8a30202beb0
📒 Files selected for processing (1)
test/extended/networking/network_segmentation.go
| ExpectWithOffset(1, nad.OwnerReferences).To(Equal([]metav1.OwnerReference{{ | ||
| APIVersion: "k8s.ovn.org/v1", | ||
| Kind: "UserDefinedNetwork", | ||
| Name: "test-net", | ||
| UID: types.UID(udnUID), | ||
| BlockOwnerDeletion: pointer.Bool(true), | ||
| Controller: pointer.Bool(true), | ||
| }})) |
There was a problem hiding this comment.
Hardcoded "test-net" should use udnName parameter.
Line 1536 hardcodes Name: "test-net" in the OwnerReference, but the function accepts udnName as a parameter. This is inconsistent with:
- Line 1531 which correctly uses
udnName assertClusterNADManifest(line 1615) which correctly usesName: udnName
While currently the function is only called with testUdnName="test-net", this will silently break if reused with a different name.
🔧 Proposed fix
ExpectWithOffset(1, nad.OwnerReferences).To(Equal([]metav1.OwnerReference{{
APIVersion: "k8s.ovn.org/v1",
Kind: "UserDefinedNetwork",
- Name: "test-net",
+ Name: udnName,
UID: types.UID(udnUID),
BlockOwnerDeletion: pointer.Bool(true),
Controller: pointer.Bool(true),
}}))📝 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.
| ExpectWithOffset(1, nad.OwnerReferences).To(Equal([]metav1.OwnerReference{{ | |
| APIVersion: "k8s.ovn.org/v1", | |
| Kind: "UserDefinedNetwork", | |
| Name: "test-net", | |
| UID: types.UID(udnUID), | |
| BlockOwnerDeletion: pointer.Bool(true), | |
| Controller: pointer.Bool(true), | |
| }})) | |
| ExpectWithOffset(1, nad.OwnerReferences).To(Equal([]metav1.OwnerReference{{ | |
| APIVersion: "k8s.ovn.org/v1", | |
| Kind: "UserDefinedNetwork", | |
| Name: udnName, | |
| UID: types.UID(udnUID), | |
| BlockOwnerDeletion: pointer.Bool(true), | |
| Controller: pointer.Bool(true), | |
| }})) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/extended/networking/network_segmentation.go` around lines 1533 - 1540,
The OwnerReference Name field is hardcoded to "test-net" inside the
ExpectWithOffset assertion; change it to use the udnName parameter so the
OwnerReference is constructed with Name: udnName (consistent with the earlier
usage of udnName and with assertClusterNADManifest). Locate the
ExpectWithOffset(1, nad.OwnerReferences) block and replace the literal
"test-net" with udnName, ensuring other fields (UID: types.UID(udnUID),
BlockOwnerDeletion, Controller, APIVersion, Kind) remain unchanged.
|
/payload-job periodic-ci-openshift-release-main-ci-4.22-e2e-aws-ovn |
|
@arkadeepsen: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/1fcd6cd0-328d-11f1-8977-164157712a22-0 |
|
@arkadeepsen: This pull request references Jira Issue OCPBUGS-82046, which is valid. 3 validation(s) were run on this bug
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Scheduling required tests: |
|
/retest |
1 similar comment
|
/retest |
|
@arkadeepsen: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Job Failure Risk Analysis for sha: 3fc02a8
|
2 participants
This PR reverts #30820 (2692e1d) which removed UDN tests from origin repo. The reason the tests were removed was because the same tests will be run via OTE. However, since the OTE PR is still not merged, we are currently not running these tests at all.
Along with the revert the CNI version is also bumped to 1.1.0 so that the tests don’t fail.