OCPBUGS-66145: fix(e2e): Ensure release rollout in NodePool tests#7284
OCPBUGS-66145: fix(e2e): Ensure release rollout in NodePool tests#7284devguyio wants to merge 1 commit intoopenshift:mainfrom
Conversation
|
@devguyio: This pull request explicitly references no jira issue. 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. |
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Review skipped — only excluded labels are configured. (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAfter running all NodePool test cases for a HostedCluster, the test re-fetches the NodePool and, if replicas indicate workers exist, waits up to 30 minutes for HostedCluster conditions to stabilize by calling Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
|
Skipping CI for Draft Pull Request. |
|
/test e2e-aws |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: devguyio 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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
test/e2e/nodepool_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
test/e2e/nodepool_test.go
🧬 Code graph analysis (1)
test/e2e/nodepool_test.go (1)
test/e2e/util/util.go (1)
WaitForImageRollout(569-610)
⏰ 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). (2)
- GitHub Check: Red Hat Konflux / hypershift-cli-mce-211-on-pull-request
- GitHub Check: Red Hat Konflux / hypershift-release-mce-211-on-pull-request
40979d8 to
e7209d7
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/e2e/nodepool_test.go (1)
372-375: Consider derivinghasWorkerNodesfrom post‑test state instead of hard‑codingtrue
ValidateHostedClusterConditionstreatshasWorkerNodesas a semantic switch: when false, it expects a different ClusterVersion condition set for clusters without workers (see the helper intest/e2e/util/util.go). Hard‑codingtruehere assumes every NodePool test always leaves the HostedCluster with workers; if any current or future test intentionally ends with zero workers, this wait can sit for the full 30‑minute timeout.If that assumption isn’t strictly guaranteed, consider:
- Computing
hasWorkerNodesfrom the current cluster state afterRun(e.g., listing worker nodes again), or- Threading an explicit “expects workers at end” flag from each
NodePoolTest, or- At minimum, documenting in a comment that all NodePool tests must restore worker nodes before returning so this call remains valid.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
test/e2e/nodepool_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
test/e2e/nodepool_test.go
🧬 Code graph analysis (1)
test/e2e/nodepool_test.go (1)
test/e2e/util/util.go (1)
ValidateHostedClusterConditions(2874-2903)
e7209d7 to
fba0b87
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
test/e2e/nodepool_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
test/e2e/nodepool_test.go
🧬 Code graph analysis (1)
test/e2e/nodepool_test.go (1)
test/e2e/util/util.go (1)
ValidateHostedClusterConditions(2874-2903)
⏰ 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). (3)
- GitHub Check: Red Hat Konflux / hypershift-cli-mce-211-on-pull-request
- GitHub Check: Red Hat Konflux / hypershift-operator-main-on-pull-request
- GitHub Check: Red Hat Konflux / hypershift-release-mce-211-on-pull-request
fba0b87 to
bb7159d
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
/test verify |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
@coderabbitai summary |
✅ Actions performedSummary regeneration triggered. |
|
/test e2e-aws |
bb7159d to
a820809
Compare
Some node pool e2e tests modify a NodePool's configuration in Run() , which triggers rollouts that might affect CVO conditions. This might cause failures in the after() validations of the e2e framework which expect that a HostedCluster is not progressing. Wait for the HostedCluster to stabilize after running each NodePool test Assisted-by: Claude (via Claude Code) Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>
a820809 to
2541cc8
Compare
|
/test e2e-aws |
|
PR needs rebase. 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. |
|
@devguyio: This pull request references Jira Issue OCPBUGS-66145, 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. |
|
/jira refresh |
|
@csrwng: This pull request references Jira Issue OCPBUGS-66145, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
No GitHub users were found matching the public email listed for the QA contact in Jira (yli2@redhat.com), skipping review request. 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. |
|
@devguyio: The following tests failed, say
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. |
What this PR does / why we need it:
Some node pool e2e tests modify a NodePool's configuration in Run() , which triggers rollouts that might affect CVO conditions.
This might cause failures in the after() validations of the e2e framework which expect that a HostedCluster is not progressing such as this failure
This PR introduces a change to wait for the HostedCluster to stabilize after running each NodePool test
Special notes for your reviewer:
Checklist: