-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
e2e: fix router metrics test flake on AWS #24085
Conversation
/test e2e-aws |
/lgtm |
/retest |
The GCP errors are something else:
The AWS failure (https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/pr-logs/pull/24085/pull-ci-openshift-origin-master-e2e-aws/13324) shows a bug in my fix here — the weighted.go test shouldn't use PROXY because a custom router pod is used for the test. I've fixed that bug. /retest |
On AWS, the default router speaks PROXY protocol. The fix in openshift#24075 switched some router tests to (correctly) speak to the router directly. However, the fix did not update client code to speak PROXY to the router on AWS. The tests still sometimes pass on AWS by coincidence (as other non-test clients send traffic to the router through the LB, causing router stats to sometimes match test expectations.) Fix the tests so that test clients talking to routers use PROXY protocol on AWS.
e1012d5
to
1abf50a
Compare
/test e2e-aws |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ironcladlou, Miciah The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
func expectRouteStatusCodeRepeatedExec(ns, execPodName, url, host string, statusCode int, times int, proxy bool) error { | ||
var extraArgs []string | ||
if proxy { | ||
extraArgs = append(extraArgs, "--haproxy-protocol") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This argument isn't in the version of curl
bundled in UBI...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's so old though... curl/curl@6baeb6d
In my dev clusters, the image that gets used is gcr.io/kubernetes-e2e-test-images/agnhost:2.6
which has curl 7.61.1 (x86_64-alpine-linux-musl) libcurl/7.61.1 LibreSSL/2.5.5 zlib/1.2.11 libssh2/1.8.2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/retest How did AWS pass then? |
/test e2e-gcp-upgrade |
Force merging to unblock queues |
On AWS, the default router speaks PROXY protocol. The fix in
#24075 switched some router tests to
(correctly) speak to the router directly. However, the fix did not update client
code to speak PROXY to the router on AWS. The tests still sometimes pass on AWS
by coincidence (as other non-test clients send traffic to the router through the
LB, causing router stats to sometimes match test expectations.)
Fix the tests so that test clients talking to routers use PROXY protocol on AWS.