OCPBUGS-63219: Remove NLB hairpin workaround from dual-stack test by gcs278 · Pull Request #31453 · openshift/origin

gcs278 · 2026-07-29T23:49:15Z

Summary

Remove the manual service.beta.kubernetes.io/aws-load-balancer-target-group-attributes=preserve_client_ip.enabled=false annotation workaround from the dual-stack NLB e2e test.

New NLB IngressControllers now default to PROXY protocol (openshift/cluster-ingress-operator#1426), which disables native client IP preservation and fixes hairpin connection failures without manual annotation.

Affected test: [sig-network-edge][OCPFeatureGate:AWSDualStackInstall][Feature:Router] should be reachable via IPv4 and IPv6 through a dual-stack ingress controller

Dependencies

OCPBUGS-63219: Support NLB protocol to configure proxy protocol and client IP preservation cluster-ingress-operator#1426 must merge first

The manual target-group-attributes annotation to disable client IP preservation is no longer needed. New NLB IngressControllers default to PROXY protocol (openshift/cluster-ingress-operator#1426), which fixes hairpin connection failures without manual intervention. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

openshift-merge-bot · 2026-07-29T23:49:17Z

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: automatic mode

openshift-ci · 2026-07-29T23:49:21Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

openshift-ci-robot · 2026-07-29T23:49:22Z

@gcs278: This pull request references Jira Issue OCPBUGS-63219, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (5.0.0) matches configured target version for branch (5.0.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @anuragthehatter

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Summary

Remove the manual service.beta.kubernetes.io/aws-load-balancer-target-group-attributes=preserve_client_ip.enabled=false annotation workaround from the dual-stack NLB e2e test.

New NLB IngressControllers now default to PROXY protocol (openshift/cluster-ingress-operator#1426), which disables native client IP preservation and fixes hairpin connection failures without manual annotation.

Dependencies

OCPBUGS-63219: Support NLB protocol to configure proxy protocol and client IP preservation cluster-ingress-operator#1426 must merge first

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 · 2026-07-29T23:50:18Z

Walkthrough

The dual-stack NLB ingress-controller test removes the annotation that disabled target group client IP preservation. The remaining labeling, route, backend, DNS, and curl verification steps are unchanged.

Changes

Dual-stack NLB test

Layer / File(s)	Summary
Remove client IP override `test/extended/router/dualstack.go`	The test proceeds directly from shard rollout to namespace labeling without disabling NLB target group client IP preservation.

Estimated code review effort: 1 (Trivial) | ~2 minutes

🚥 Pre-merge checks | ✅ 15

✅ Passed checks (15 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	dualstack.go only removes a service annotation; the two Ginkgo It titles are static and contain no dynamic or overly specific values.
Test Structure And Quality	✅ Passed	Two It blocks each target one reachability behavior; setup/cleanup and timeouts are present, and bare assertions match nearby router test patterns.
Microshift Test Compatibility	✅ Passed	No new Ginkgo tests were added; the existing dual-stack suite already has [apigroup:route.openshift.io][apigroup:operator.openshift.io][apigroup:config.openshift.io], so MicroShift skips it.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	PASS: PR only removes an annotation from existing dual-stack router tests; no new Ginkgo tests or multi-node/HA assumptions were added.
Topology-Aware Scheduling Compatibility	✅ Passed	Only a test annotation was removed; no manifests, controllers, affinities, selectors, replicas, or topology logic were added.
Ote Binary Stdout Contract	✅ Passed	The only code change removes an annotation; the file has no process-level stdout writes (no fmt.Print/Println/Printf, klog, or stdout logging in setup).
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	Only an annotation was removed from an existing dual-stack test; no new IPv4 assumptions or external connectivity were introduced.
No-Weak-Crypto	✅ Passed	Diff only removes a router-service annotation; no weak crypto, custom crypto, or secret comparison code is introduced.
Container-Privileges	✅ Passed	Only a test file changed, and it contains no privileged, hostPID/network/IPC, SYS_ADMIN, or allowPrivilegeEscalation settings.
No-Sensitive-Data-In-Logs	✅ Passed	The PR only removes an AWS target-group annotation from a test; no new logging or sensitive-data exposure appears in the changed file.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely identifies removal of the NLB hairpin workaround from the dual-stack test.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands.}

gcs278 · 2026-07-29T23:52:39Z

/test ?

gcs278 · 2026-07-29T23:55:56Z

/payload-job-with-prs periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ipi-ovn-dualstack openshift/cluster-ingress-operator#1426

openshift-ci · 2026-07-29T23:56:09Z

@gcs278: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ipi-ovn-dualstack

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/0b3cbda0-8ba9-11f1-920c-8c9cd0cf844f-0

gcs278 · 2026-07-31T02:29:56Z

/pipeline required

openshift-merge-bot · 2026-07-31T02:29:59Z

Scheduling required tests:
/test e2e-aws-csi
/test e2e-aws-ovn-fips
/test e2e-aws-ovn-microshift
/test e2e-aws-ovn-microshift-serial
/test e2e-aws-ovn-serial-1of2
/test e2e-aws-ovn-serial-2of2
/test e2e-gcp-csi
/test e2e-gcp-ovn
/test e2e-gcp-ovn-upgrade
/test e2e-metal-ipi-ovn-ipv6
/test e2e-vsphere-ovn
/test e2e-vsphere-ovn-upi

gcs278 · 2026-07-31T03:02:52Z

/retest

gcs278 · 2026-07-31T12:00:12Z

unrelated
/retest

gcs278 · 2026-07-31T12:00:22Z

unrelated
/retest

jcmoraisjr · 2026-07-31T13:46:10Z

Good job on the PROXY proto for NLB!

/lgtm

openshift-ci · 2026-07-31T13:48:24Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gcs278, jcmoraisjr

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~test/extended/router/OWNERS~~ [gcs278,jcmoraisjr]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci · 2026-07-31T15:36:05Z

@gcs278: all tests passed!

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

gcs278 · 2026-07-31T15:50:15Z

/test ?

gcs278 · 2026-07-31T15:50:33Z

@CodeRabbit what tests should we run to verify this dualstack e2e jobs still works that's being changed in this PR?

coderabbitai · 2026-07-31T15:51:35Z

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== Test file outline =='
ast-grep outline test/extended/router/dualstack.go --items all || true

echo '== NLB and target-group references in the test file =='
rg -n -C 8 'NLB|Network Load Balancer|target-group|preserve_client_ip|PROXY|dual.?stack' test/extended/router/dualstack.go || true

echo '== Repository references to the affected test and requested CI job =='
rg -n -i -C 3 'dualstack.*nlb|nlb.*dualstack|e2e-metal-ipi-ovn-dualstack|aws-load-balancer-target-group-attributes|preserve_client_ip' . \
  -g '!vendor/**' -g '!**/.git/**' || true

echo '== Candidate CI configuration files =='
fd -HI -t f '(config|jobs|release|periodic).*\.ya?ml$|.*\.ya?ml$' . \
  | xargs -r rg -l -i 'e2e-metal-ipi-ovn-dualstack|dualstack' \
  | head -50

Length of output: 7832

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== Complete affected NLB spec =='
sed -n '52,114p' test/extended/router/dualstack.go

echo '== Extended-test invocation and Ginkgo filtering documentation =='
rg -n -i -C 4 \
  'ginkgo\.(focus|skip)|--ginkgo\.(focus|skip)|--run-?regex|run.*extended|extended.*test' \
  README.md docs test hack openshift-hack 2>/dev/null || true

echo '== Test commands and suite definitions that include router tests =='
rg -n -i -C 4 \
  'test/extended|extended.*router|router.*extended|run-test' \
  Makefile* hack test openshift-hack 2>/dev/null || true

Length of output: 201113

@gcs278 The job you already requested is the required test:

/payload-job-with-prs periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ipi-ovn-dualstack openshift/cluster-ingress-operator#1426

Run it with openshift/cluster-ingress-operator#1426 included. This is required because the removed workaround depends on that PR enabling PROXY protocol by default for new NLB IngressController objects.

Verify that this spec passes in the job output:

should be reachable via IPv4 and IPv6 through a dual-stack ingress controller

The spec creates an AWS NLB router shard. It then verifies route DNS resolution and route access with both IPv4 and IPv6. This covers the former hairpin failure path.

No additional unit test is needed for this change. A unit test cannot validate AWS NLB provisioning or the end-to-end IPv4/IPv6 connection path.

_{You are interacting with an AI system.}

openshift-ci · 2026-07-31T15:52:20Z

@coderabbitai[bot]: user coderabbitai[bot] is not trusted for pull request #31453

gcs278 · 2026-07-31T15:57:12Z

We should definitely run the dualstack tests to verify before merging:
/test e2e-metal-ipi-ovn-dualstack
/test e2e-metal-ipi-ovn-dualstack-bgp
/test e2e-metal-ipi-ovn-dualstack-bgp-local-gw
/test e2e-metal-ipi-ovn-dualstack-local-gateway
/test e2e-openstack-dualstack-v6primary

gcs278 · 2026-07-31T15:57:30Z

/test e2e-metal-ipi-ovn-bgp-virt-dualstack

gcs278 · 2026-07-31T20:20:45Z

/payload-job periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-installer-dualstack-ipv6-primary-techpreview

openshift-ci · 2026-07-31T20:20:48Z

@gcs278: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-installer-dualstack-ipv6-primary-techpreview

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/4fe7b7f0-8d1d-11f1-98c7-43e19aae3884-0

gcs278 · 2026-07-31T20:21:07Z

/payload-job periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-installer-dualstack-ipv4-primary-techpreview

openshift-ci · 2026-07-31T20:21:11Z

@gcs278: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-installer-dualstack-ipv4-primary-techpreview

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/5d346980-8d1d-11f1-90bb-3970c871a3a9-0

gcs278 · 2026-07-31T20:21:46Z

I forgot to realize this test is an AWS-only test. Kicked off the two payload jobs that matter. I'll hold until i get a good CI result.
/hold

openshift-ci-robot added jira/severity-important Referenced Jira bug's severity is important for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. labels Jul 29, 2026

openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 29, 2026

openshift-ci-robot added the jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. label Jul 29, 2026

openshift-ci Bot requested a review from anuragthehatter July 29, 2026 23:49

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 29, 2026

coderabbitai Bot approved these changes Jul 29, 2026

View reviewed changes

openshift-ci Bot added the ready-for-human-review Indicates a PR has been reviewed by automated tools and is ready for human review label Jul 29, 2026

openshift-ci Bot mentioned this pull request Jul 29, 2026

OCPBUGS-63219: Support NLB protocol to configure proxy protocol and client IP preservation openshift/cluster-ingress-operator#1426

Merged

gcs278 marked this pull request as ready for review July 31, 2026 02:29

openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 31, 2026

openshift-ci Bot requested review from grzpiotrowski and rikatz July 31, 2026 02:30

openshift-ci Bot assigned jcmoraisjr Jul 31, 2026

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jul 31, 2026

openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 31, 2026

Uh oh!

Conversation

gcs278 commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Dependencies

Uh oh!

openshift-merge-bot Bot commented Jul 29, 2026

Uh oh!

openshift-ci Bot commented Jul 29, 2026

Uh oh!

openshift-ci-robot commented Jul 29, 2026

Summary

Dependencies

Uh oh!

coderabbitai Bot commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

gcs278 commented Jul 29, 2026

Uh oh!

gcs278 commented Jul 29, 2026

Uh oh!

openshift-ci Bot commented Jul 29, 2026

Uh oh!

gcs278 commented Jul 31, 2026

Uh oh!

openshift-merge-bot Bot commented Jul 31, 2026

Uh oh!

gcs278 commented Jul 31, 2026

Uh oh!

gcs278 commented Jul 31, 2026

Uh oh!

gcs278 commented Jul 31, 2026

Uh oh!

jcmoraisjr commented Jul 31, 2026

Uh oh!

openshift-ci Bot commented Jul 31, 2026

Uh oh!

openshift-ci Bot commented Jul 31, 2026

Uh oh!

gcs278 commented Jul 31, 2026

Uh oh!

gcs278 commented Jul 31, 2026

Uh oh!

coderabbitai Bot commented Jul 31, 2026

Uh oh!

openshift-ci Bot commented Jul 31, 2026

Uh oh!

gcs278 commented Jul 31, 2026

Uh oh!

gcs278 commented Jul 31, 2026

Uh oh!

gcs278 commented Jul 31, 2026

Uh oh!

openshift-ci Bot commented Jul 31, 2026

Uh oh!

gcs278 commented Jul 31, 2026

Uh oh!

openshift-ci Bot commented Jul 31, 2026

Uh oh!

gcs278 commented Jul 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gcs278 commented Jul 29, 2026 •

edited

Loading

coderabbitai Bot commented Jul 29, 2026 •

edited

Loading