-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Bug 1683057: Fix router metrics test flakes #24075
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
Bug 1683057: Fix router metrics test flakes #24075
Conversation
|
@ironcladlou: This pull request references Bugzilla bug 1683057, which is valid. The bug has been moved to the POST state. 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 kubernetes/test-infra repository. |
frobware
left a comment
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.
LGTM, though I think doing _ := foo() was clearer without the assignment. Either way, it is ignored.
Oops... my editor's linter auto-refactored these! Wonder if the std linter likes one or the other better. And yeah, I guess I should actually do something with the error anyway. Too bad the editor couldn't do that for me too... |
0c9db13 to
b9c5de4
Compare
The router metrics tests assume they're talking to a single router pod. During the v4 refactor, this assumption broke, and the test connection to the router was load-balanced to router pods behind the router service. This made the test flaky. Refactor the test to discover and operate against a single ingress endpoint.
b9c5de4 to
ea01775
Compare
|
@frobware rolled back the unnecessary refactor, PTAL |
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: frobware, ironcladlou 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 |
|
@ironcladlou: All pull requests linked via external trackers have merged. Bugzilla bug 1683057 has been moved to the MODIFIED 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 kubernetes/test-infra repository. |
|
I think this substantially increased flakes in AWS Going to revert, then we can retest in e2e-aws premerge |
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.
|
#24085 should fix the AWS flake |
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.
The router metrics tests assume they're talking to a single router pod. During
the v4 refactor, this assumption broke, and the test connection to the router
was load-balanced to router pods behind the router service. This made the test
flaky.
Refactor the test to discover and operate against a single ingress endpoint.