OCPBUGS-111704: router/metrics: enable proxy protocol for client on AWS clusters - #31526
Conversation
…WS clusters The cluster-ingress-operator now supports PROXY protocol for NLB by default (OCPBUGS-63219) on AWS. Thus, we need to partially revert NE-2422 to use proxy protocol for the client. This addresses the below failed e2e test in AWS dualstack periodic jobs: [sig-network][Feature:Router] The HAProxy router should expose prometheus metrics for a route [apigroup:route.openshift.io] [Suite:openshift/conformance/parallel]
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
openshift-ci-robot
added
jira/valid-reference
|
@tthvo: This pull request references Jira Issue OCPBUGS-111704, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
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. |
|
@tthvo: This pull request references Jira Issue OCPBUGS-111704, 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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan includes up to 12 reviews per rolling hour; 11 remain after this review. WalkthroughThe router metrics test now enables proxy protocol for all AWS platforms. It removes the dual-stack detection and exclusion logic. ChangesRouter metrics test
Estimated code review effort: 1 (Trivial) | ~3 minutes Merge Risk: ⚪ Minimal · up to The PR is a small localized test adjustment; no actionable merge-blocking risk remains, and it is merge-ready after routine repository checks and review. Suggested labels: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
openshift-ci
Bot
added
the
ready-for-human-review
|
/payload-job periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-installer-dualstack-ipv6-primary |
|
@tthvo: 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/64e83130-9a91-11f1-980d-fbf2d837ad3c-0 |
|
/pipeline required |
|
Scheduling required tests: |
|
/payload-job periodic-ci-openshift-release-main-nightly-5.1-e2e-aws-ovn-installer-dualstack-ipv6-primary |
|
@tthvo: 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/d75cb250-9a9f-11f1-8f88-e757515d10b0-0 |
|
/cherry-pick release-5.0 |
|
@tthvo: once the present PR merges, I will cherry-pick it on top of 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. |
|
Risk analysis has seen new tests most likely introduced by this PR. New Test Risks for sha: f8da131
New tests seen in this PR at sha: f8da131
|
|
/verified by 5.0/5.1 payload testing (the router/metrics test passed) and presubmits (no regression) Based on #31526 (comment) and #31526 (comment), both 5.0/5.1 AWS dual-stack IPv4-primary and IPv6-primary periodic jobs passed full green. This PR indeed fixes the test |
openshift-ci-robot
added
the
verified
|
@tthvo: 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. |
|
/approve |
openshift-ci
Bot
added
the
approved
|
/lgtm Thanks! |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alebedev87, sadasu, tthvo 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 |
|
Cross reference: proxy protocol was set as default for NLBs in openshift/cluster-ingress-operator#1426 |
| // Dual-stack installations on AWS are forced to use NLB type, which | ||
| // doesn't accept proxy protocol (yet). | ||
| proxyProtocol = (platformType == configv1.AWSPlatformType) && !dualStackIPFamily | ||
| proxyProtocol = platformType == configv1.AWSPlatformType |
There was a problem hiding this comment.
Does this test ever run on upgraded clusters? If a cluster is installed at version 4.22 or earlier with its "default" IngressController configured to use an NLB, then the operator will configure the NLB and router not to use PROXY protocol, and the operator will not change the NLB or router to use PROXY protocol on upgrade.
It would be safer to check the service or router deployment to determine whether they are configured to use PROXY protocol. That would make the test more robust across OpenShift releases and across upgrade scenarios, such as the one that I described.
There was a problem hiding this comment.
@Miciah good catch. it's not a problem today because I see no variants of aws.*dualstack.*upgrade.* jobs. So, this will work, though fragile.
But I agree - our default ingress controller being upgraded from 4.22 will not have proxy protocol. If a upgrade variant of dualstack is ever added for 5.0, this test will break. Resolving the Router deployment proxy env variable is a better way to handle this.
But, I wouldn't say it's a blocker for this PR. Can be done in a follow up.
There was a problem hiding this comment.
Another +1 from me. Checking the router setting is the better way about this 👍
it's not a problem today because I see no variants of aws.*dualstack.upgrade. jobs. So, this will work, though fragile.
Though, I wanted to say that AWS dual-stack networking is, as of now, a day-1-only feature (set at install time without options to switch day-2) and is TechPreview in 4.22; thus, there's no upgrade path to 5.0+ (or 4.23+). So, we should be safe. The upgrade will only be available in 5.0+/4.23+.
|
@tthvo: 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. |
|
@tthvo: Jira Issue Verification Checks: Jira Issue OCPBUGS-111704 Jira Issue OCPBUGS-111704 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. |
|
@tthvo: new pull request created: #31533 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. |
|
Fix included in release 5.1.0-0.nightly-2026-08-20-065836 |
9 participants
The
cluster-ingress-operatornow supportsPROXYprotocol for NLB by default (OCPBUGS-63219) on AWS. Thus, we need to partially revert NE-2422 (#30934), specifically a97b2ad, to use proxy protocol for the client.This addresses the below failed e2e test in AWS dualstack periodic jobs:
[sig-network][Feature:Router] The HAProxy router should expose prometheus metrics for a route [apigroup:route.openshift.io] [Suite:openshift/conformance/parallel]Summary by CodeRabbit