OCPBUGS-65957: test(e2e): fix TestNodePool test failure when installing cilium as external cni - #7263
OCPBUGS-65957: test(e2e): fix TestNodePool test failure when installing cilium as external cni#7263wewang58 wants to merge 1 commit into
Conversation
openshift-ci-robot
added
the
jira/valid-reference
WalkthroughAdds a platform-specific guard in the node pool test to ensure at least one worker node exists when running on Azure (ARO HCP) with Cilium as the external CNI by setting NodePoolReplicas to 1 in that scenario. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Comment |
openshift-ci
Bot
added
do-not-merge/needs-area
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
added
the
area/testing
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wewang58 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 |
|
e2e test : TestNodePool/HostedCluster0/ValidateHostedCluster/InstallCilium/InstallCilium and TestNodePool/HostedCluster2/ValidateHostedCluster/InstallCilium/InstallCilium passed in the job, other TestNodePool/HostedCluster2/Main/TestAdditionalTrustBundlePropagation/AdditionalTrustBundlePropagationTest expand_more failure seems it's not related. |
celebdor
changed the title
openshift-ci-robot
added
jira/severity-low
|
@wewang58: This pull request references Jira Issue OCPBUGS-65957, 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 |
openshift-ci-robot
added
jira/valid-bug
|
@wewang58: This pull request references Jira Issue OCPBUGS-65957, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: 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. |
|
@openshift-ci-robot: GitHub didn't allow me to request PR reviews from the following users: wewang58. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. 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 kubernetes-sigs/prow repository. |
|
Since #7423 is merged, the pr can be reviewed. |
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
openshift-ci-robot
added
jira/invalid-bug
|
@wewang58: This pull request references Jira Issue OCPBUGS-65957, which is invalid:
Comment 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. |
|
/retest |
|
@wewang58: The following test 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. |
…ternal cni This commit fixes the TestNodePool test failure when installing cilium as an external CNI by adjusting the test assertions and configurations to account for external CNIs.
c3da0bc to
a7f35f1
Compare
|
@wewang58: This pull request references Jira Issue OCPBUGS-65957, which is invalid:
Comment 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/e2e/nodepool_test.go`:
- Around line 213-217: The code references clusterOpts.ExternalCNIProvider which
doesn't exist; add a new field ExternalCNIProvider string to the
PlatformAgnosticOptions struct (or to its embedded core.RawCreateOptions if you
prefer centralizing raw flags) and wire it into any flag/option parsing so
clusterOpts.ExternalCNIProvider is populated before the check in
nodepool_test.go; ensure the new field name matches the check and update
usages/tests accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a74e7e46-8812-4abe-a49f-a1c8738ecbb0
📒 Files selected for processing (1)
test/e2e/nodepool_test.go
| // On ARO HCP with Cilium, we need at least one worker node to install cilium-olm operator | ||
| // before individual tests run. The cilium-olm deployment requires worker nodes to schedule its pods. | ||
| if globalOpts.Platform == hyperv1.AzurePlatform && clusterOpts.ExternalCNIProvider == "cilium" { | ||
| clusterOpts.NodePoolReplicas = 1 | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify ExternalCNIProvider field definition and usage patterns
echo "=== ExternalCNIProvider field definition and usage ==="
rg -n 'ExternalCNIProvider' --type=go -B2 -A2
echo ""
echo "=== Cilium provider references with context ==="
rg -n '"cilium"' --type=go -B2 -A2Repository: openshift/hypershift
Length of output: 3461
🏁 Script executed:
#!/bin/bash
# Find ExternalCNIProvider definition and assignment patterns
echo "=== Search for ExternalCNIProvider struct/field definition ==="
rg -n 'ExternalCNIProvider' --type=go
echo ""
echo "=== Search for other CNI provider strings (openstack, none, etc.) ==="
rg -n '"openstack"|"none"|"aws"|"external"' --type=go | grep -i cni | head -20
echo ""
echo "=== Search for CNI provider enum or constant definitions ==="
rg -n 'const.*CNI|type.*CNI' --type=go | head -20
echo ""
echo "=== Check clusterOpts structure around line 213 ==="
sed -n '190,230p' test/e2e/nodepool_test.goRepository: openshift/hypershift
Length of output: 3103
🏁 Script executed:
#!/bin/bash
# Find DefaultClusterOptions function and struct definition
echo "=== Search for DefaultClusterOptions function ==="
rg -n 'func.*DefaultClusterOptions' --type=go -A10
echo ""
echo "=== Search for struct containing ExternalCNIProvider ==="
rg -n 'type.*struct' --type=go | head -30
echo ""
echo "=== Search for ExternalCNIProvider assignment/initialization ==="
rg -n 'ExternalCNIProvider\s*=' --type=go
echo ""
echo "=== Broader search for ExternalCNIProvider in all files ==="
find . -name "*.go" -type f ! -path "*/vendor/*" -exec grep -l "ExternalCNIProvider" {} \;Repository: openshift/hypershift
Length of output: 3940
🏁 Script executed:
#!/bin/bash
# Find PlatformAgnosticOptions struct definition
echo "=== Search for PlatformAgnosticOptions struct definition ==="
rg -n 'type PlatformAgnosticOptions' --type=go -A50
echo ""
echo "=== Check full DefaultClusterOptions function ==="
sed -n '181,250p' test/e2e/util/options.goRepository: openshift/hypershift
Length of output: 6735
🏁 Script executed:
#!/bin/bash
# Search for ExternalCNIProvider in core package
echo "=== Search for core.RawCreateOptions struct definition ==="
rg -n 'type RawCreateOptions' --type=go -A50 | head -100
echo ""
echo "=== Direct search for ExternalCNIProvider in all non-vendor Go files ==="
find . -name "*.go" -type f ! -path "*/vendor/*" -exec grep -l "ExternalCNIProvider" {} \;
echo ""
echo "=== Check if code compiles by looking for test files ==="
ls -la test/e2e/nodepool_test.goRepository: openshift/hypershift
Length of output: 7315
🏁 Script executed:
#!/bin/bash
# Find core package RawCreateOptions
echo "=== Search for core.RawCreateOptions in cmd/cluster/core ==="
find cmd/cluster/core -name "*.go" -exec grep -l "type RawCreateOptions" {} \;
echo ""
echo "=== Read the core RawCreateOptions struct ==="
find cmd/cluster/core -name "*.go" -exec grep -n "type RawCreateOptions" -A100 {} + | head -150Repository: openshift/hypershift
Length of output: 7182
Code will not compile — ExternalCNIProvider field does not exist on clusterOpts.
The referenced field clusterOpts.ExternalCNIProvider does not exist in either the PlatformAgnosticOptions struct or its embedded core.RawCreateOptions struct. The field would need to be added to one of these structs before this code can compile.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/e2e/nodepool_test.go` around lines 213 - 217, The code references
clusterOpts.ExternalCNIProvider which doesn't exist; add a new field
ExternalCNIProvider string to the PlatformAgnosticOptions struct (or to its
embedded core.RawCreateOptions if you prefer centralizing raw flags) and wire it
into any flag/option parsing so clusterOpts.ExternalCNIProvider is populated
before the check in nodepool_test.go; ensure the new field name matches the
check and update usages/tests accordingly.
|
/jira refresh |
|
@wewang58: This pull request references Jira Issue OCPBUGS-65957, which is invalid:
Comment 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 |
openshift-ci-robot
added
jira/valid-bug
|
@wewang58: This pull request references Jira Issue OCPBUGS-65957, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: 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. |
|
@openshift-ci-robot: GitHub didn't allow me to request PR reviews from the following users: wewang58. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. 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 kubernetes-sigs/prow repository. |
|
/retest |
|
/retest-required |
|
/retest |
|
/test e2e-aks-cilium |
|
Stale PRs are closed after 21d of inactivity. If this PR is still relevant, comment to refresh it or remove the stale label. If this PR is safe to close now please do so with /lifecycle stale |
openshift-ci
Bot
added
the
lifecycle/stale
Test Failure Analysis CompleteJob Information
Test Failure AnalysisErrorSummaryThese three failures are not caused by the PR's code changes (which only touch Root CauseThe root cause is a stale Konflux pipeline association for this long-lived PR:
Recommendations
Evidence
|
|
Stale PRs rot after 14d of inactivity. Mark the PR as fresh by commenting If this PR is safe to close now please do so with /lifecycle rotten |
openshift-ci
Bot
added
lifecycle/rotten
|
Rotten PRs close after 7d of inactivity. Reopen the PR by commenting /close |
|
@openshift-ci[bot]: Closed this PR. 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 kubernetes-sigs/prow repository. |
|
@wewang58: This pull request references Jira Issue OCPBUGS-65957. The bug has been updated to no longer refer to the pull request using the external bug tracker. All external bug links have been closed. The bug has been moved to the NEW state. 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. |
3 participants
What this PR does / why we need it:
The pr update NodePoolReplicas is 1 for NodePoolTests, because On ARO HCP with Cilium, we need at least one worker node to install cilium-olm operator before individual tests run
Which issue(s) this PR fixes:
Fixes:
Special notes for your reviewer:
The pr is depend on :#7077, because ExternalCNIProvider is defined in pr7077
Checklist:
Summary by CodeRabbit