OCPBUGS-83399: e2e/ote/ccm-aws fix hairpin traffic on NLB and CLB in EUSC#449
Conversation
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
openshift-ci-robot
added
the
jira/valid-reference
|
Skipping CI for Draft Pull Request. |
openshift-ci-robot
added
the
jira/invalid-bug
|
@mtulio: This pull request references Jira Issue OCPBUGS-83399, 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. |
|
/test e2e-aws |
|
@mtulio: trigger 2 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/d43eb630-3907-11f1-9530-b25f861ba42c-0 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughUpdated the required version of Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 inconclusive)
✅ Passed checks (7 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/testwith openshift/installer/main/e2e-aws-eusc-techpreview openshift/origin#30994 openshift/installer#10453 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/cloud-controller-manager-aws-tests-ext/go.mod (1)
24-25: Prefer an organization-owned fork for long-term CI stability.Using a personal fork in the
replacedirective works currently, but it increases supply-chain risk for long-term builds and maintenance. If this is expected to persist beyond the short term, migrate to an org-owned fork or add an in-code comment documenting when this should be removed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/cloud-controller-manager-aws-tests-ext/go.mod` around lines 24 - 25, The replace directive pointing k8s.io/cloud-provider-aws/tests/e2e to the personal fork github.com/mtulio/... should be replaced with an organization-owned fork or documented for removal: update the replace directive (the line starting with "replace k8s.io/cloud-provider-aws/tests/e2e => github.com/mtulio/openshift-cloud-provider-aws/tests/e2e v0.0.0-...") to reference your org's fork (e.g., github.com/<org>/openshift-cloud-provider-aws/tests/e2e) and a stable tag/commit, or if this is temporary add an inline comment above the replace line noting why it exists and the exact date or PR after which it should be removed; ensure the module path and commit hash in the replace directive are updated accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cmd/cloud-controller-manager-aws-tests-ext/go.mod`:
- Around line 24-25: The replace directive pointing
k8s.io/cloud-provider-aws/tests/e2e to the personal fork github.com/mtulio/...
should be replaced with an organization-owned fork or documented for removal:
update the replace directive (the line starting with "replace
k8s.io/cloud-provider-aws/tests/e2e =>
github.com/mtulio/openshift-cloud-provider-aws/tests/e2e v0.0.0-...") to
reference your org's fork (e.g.,
github.com/<org>/openshift-cloud-provider-aws/tests/e2e) and a stable
tag/commit, or if this is temporary add an inline comment above the replace line
noting why it exists and the exact date or PR after which it should be removed;
ensure the module path and commit hash in the replace directive are updated
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5e1219d7-0a47-4c57-b18a-887411178a7d
⛔ Files ignored due to path filters (3)
cmd/cloud-controller-manager-aws-tests-ext/go.sumis excluded by!**/*.sumcmd/cloud-controller-manager-aws-tests-ext/vendor/k8s.io/cloud-provider-aws/tests/e2e/loadbalancer.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/modules.txtis excluded by!**/vendor/**
📒 Files selected for processing (1)
cmd/cloud-controller-manager-aws-tests-ext/go.mod
|
Applied fixes in the NLB hairpin upstream, re-testing: /test e2e-aws |
|
@mtulio: trigger 2 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/3505eac0-3916-11f1-900d-9fd05351db9a-0 |
|
/test e2e-aws-ovn |
|
/testwith openshift/installer/main/e2e-aws-eusc-techpreview openshift/origin#30994 openshift/installer#10453 |
|
Summary of CI tests filtered by CCM e2e tests (
Failures on later are all related to the waiting for DNS propagation, looks like affected scenarios have in common service endpoints. More to come with detailed analysis. |
|
/testwith openshift/installer/main/e2e-aws-eusc-techpreview openshift/origin#30994 openshift/installer#10453 |
|
/testwith openshift/installer/main/e2e-aws-eusc-techpreview openshift/installer#10453 openshift/origin#30994 |
|
Triggering hypershift conformance one more time (last one passed all 3 CCM loadBalancer tests enabled for that profile): /payload-job periodic-ci-openshift-hypershift-release-4.22-periodics-e2e-aws-ovn-conformance |
|
@mtulio: 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/61019170-3947-11f1-9b35-091d267f866e-0 |
|
One more execution is completing without issues on CCM-AWS e2e for hypershift periodics-e2e-aws-ovn-conformance: 1/3 passing for |
|
Tests are now passing with 15 minutes curl timeout. See the results of jobs passing ✅:
Since SOA DNS on EUSC is 900s, we are leaving 5 minutes extra to the total timeout to prevent flakes, my last commit will perform that, also proposed changes to upstream: kubernetes/cloud-provider-aws#1383 to get CI signals |
|
/payload-job periodic-ci-openshift-hypershift-release-4.22-periodics-e2e-aws-ovn-conformance |
|
@mtulio: 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/fa95b870-39a1-11f1-8943-cbf681a29bbe-0 |
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 `@cmd/cloud-controller-manager-aws-tests-ext/go.mod`:
- Around line 24-25: The go.mod replace currently points to a personal fork
(replace k8s.io/cloud-provider-aws/tests/e2e =>
github.com/mtulio/openshift-cloud-provider-aws/tests/e2e
v0.0.0-20260416182551-8dd96ca1b639); change this before merging by either moving
that fork under org control (create an org-owned repo mirroring
github.com/mtulio/openshift-cloud-provider-aws and update the replace to that
org path) or, if retaining the temporary replace, create and link a tracking
issue that references the upstream PR/commit (8dd96ca1b639) and sets a deadline
to remove the replace so maintainers can verify and remove the personal-fork
replace once the upstream k8s.io/cloud-provider-aws/tests/e2e incorporates the
fix.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 50989e33-81cc-4fa5-a120-6e486b4543b7
⛔ Files ignored due to path filters (3)
cmd/cloud-controller-manager-aws-tests-ext/go.sumis excluded by!**/*.sumcmd/cloud-controller-manager-aws-tests-ext/vendor/k8s.io/cloud-provider-aws/tests/e2e/loadbalancer.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/modules.txtis excluded by!**/vendor/**
📒 Files selected for processing (1)
cmd/cloud-controller-manager-aws-tests-ext/go.mod
| // Use development branch @mtulio e2e-lb-fix-hairpin | ||
| replace k8s.io/cloud-provider-aws/tests/e2e => github.com/mtulio/openshift-cloud-provider-aws/tests/e2e v0.0.0-20260416182551-8dd96ca1b639 |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🌐 Web query:
Has commit 8dd96ca1b639 (or equivalent hairpin CLB/NLB e2e fix) been merged into k8s.io/cloud-provider-aws/tests/e2e upstream? If yes, what is the first upstream pseudo-version containing it?
💡 Result:
No, commit 8dd96ca1b639 (or equivalent hairpin CLB/NLB e2e fix) has not been found merged into k8s.io/cloud-provider-aws/tests/e2e upstream. Extensive searches across GitHub, pkg.go.dev pseudo-versions, issues, PRs, and releases did not locate the specific commit SHA or any explicit mention of a hairpin CLB/NLB e2e fix in the tests/e2e directory. While hairpin NLB fixes exist in the main code (e.g., PR #1214 merged around Aug 2025, included in v1.35.0), and e2e tests for hairpin were added/refactored (PRs #1161, #1215), no evidence confirms the queried commit or equivalent e2e fix matching the description. The latest pseudo-version listed is v0.0.0-20260408230731-47188c2263a4 (2026-04-08), but without commit details, cannot confirm inclusion.
Citations:
- 1: https://github.com/kubernetes/cloud-provider-aws
- 2: Fix hairpinning traffic on internal NLB by introducing TG attribute reconciler kubernetes/cloud-provider-aws#1214
- 3: Hairpin connection failed on Service type-LoadBalancer NLB with internal scheme kubernetes/cloud-provider-aws#1160
- 4: https://pkg.go.dev/k8s.io/cloud-provider-aws/tests/e2e
- 5: https://github.com/kubernetes/cloud-provider-aws/tree/master/tests/e2e
- 6: https://github.com/kubernetes/cloud-provider-aws/releases/tag/v1.35.0
Move this replace to an org-owned fork or track upstream merge before this can be merged.
This replace points to a personal fork (github.com/mtulio/...), which creates a reproducibility and ownership risk. The verification confirms that the hairpin e2e fix referenced by commit 8dd96ca1b639 has not yet been merged into the upstream k8s.io/cloud-provider-aws/tests/e2e.
Until that fix lands upstream, you need to either:
- Transfer the fork to org control, or
- Add a clear tracking issue linking to the upstream PR/issue that will eventually make this replace obsolete, with a deadline to remove it.
Do not merge with a personal fork dependency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/cloud-controller-manager-aws-tests-ext/go.mod` around lines 24 - 25, The
go.mod replace currently points to a personal fork (replace
k8s.io/cloud-provider-aws/tests/e2e =>
github.com/mtulio/openshift-cloud-provider-aws/tests/e2e
v0.0.0-20260416182551-8dd96ca1b639); change this before merging by either moving
that fork under org control (create an org-owned repo mirroring
github.com/mtulio/openshift-cloud-provider-aws and update the replace to that
org path) or, if retaining the temporary replace, create and link a tracking
issue that references the upstream PR/commit (8dd96ca1b639) and sets a deadline
to remove the replace so maintainers can verify and remove the personal-fork
replace once the upstream k8s.io/cloud-provider-aws/tests/e2e incorporates the
fix.
|
/payload-job periodic-ci-openshift-hypershift-release-4.22-periodics-e2e-aws-ovn-conformance |
|
@mtulio: 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/ac890790-3cfd-11f1-8240-f58e276a842c-0 |
|
/verified by #449 (comment) |
openshift-ci-robot
added
the
verified
|
@mtulio: This PR has been marked as verified by 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. |
|
/verified by #449 (comment) and #449 (comment) |
|
@mtulio: This PR has been marked as verified by 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. |
|
@mtulio: This pull request references Jira Issue OCPBUGS-83399, 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
|
@tthvo: This pull request references Jira Issue OCPBUGS-83399, 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. |
|
@mtulio: 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. |
damdo
left a comment
damdo
left a comment
There was a problem hiding this comment.
/approve
/lgtm
/hold
for testing
openshift-ci
Bot
added
the
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: damdo The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
Failed by unrelated reasons, one more try - specially because we already observed good signals: /payload-job periodic-ci-openshift-hypershift-release-4.22-periodics-e2e-aws-ovn-conformance |
|
@mtulio: 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/ec1331b0-3e51-11f1-884f-53b990b36352-0 |
5/5 cloud-provider-aws tests are passing, job failed to unrelated reason. /hold cancel |
openshift-ci
Bot
removed
the
do-not-merge/hold
|
@mtulio: Jira Issue Verification Checks: Jira Issue OCPBUGS-83399 Jira Issue OCPBUGS-83399 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 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. |
5 participants
Update cloud-provider-aws e2e tests to increasing resilience while making API calls to AWS as well running client-connectivity on service load balancer previously failing on regions with detected longer than expected DNS propagation times, such as AWS EUSC.
Summary by CodeRabbit
Depends on kubernetes/cloud-provider-aws#1383