Add e2e test for round robin load balancing #3390
Conversation
k8s-ci-robot
added
size/M
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
@zrdaley thanks! |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aledbf, zrdaley 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 |
k8s-ci-robot
added
the
approved
| _, body, errs := gorequest.New(). | ||
| Get(f.IngressController.HTTPURL). | ||
| Set("Host", host). | ||
| Retry(10, 1*time.Second, http.StatusNotFound, http.StatusServiceUnavailable). |
There was a problem hiding this comment.
Why do you need this? Does the endpoint expected to be flaky?
| Set("Host", host). | ||
| Retry(10, 1*time.Second, http.StatusNotFound, http.StatusServiceUnavailable). | ||
| End() | ||
| Expect(len(errs)).Should(BeNumerically("==", 0)) |
There was a problem hiding this comment.
When you do this, the error message is useless. Please use Expect(errs).Should(BeEmpty()) instead (or if you know any better one - the idea is to reveal more about the error not just say n != m).
@zrdaley have you tried with a single Nginx worker? From #3303:
|
@zrdaley I'd suggest something like |
|
@ElvinEfendi I addressed all of your comments in a new PR: #3407 Initially I thought that the request distribution was off because of bad request responses (hence the |
What this PR does / why we need it:
This adds e2e-testing for round robin load balancing.
Which issue this PR fixes: #3303
Special notes for your reviewer:
Sometimes when testing, the request distribution wasn't an equal split three ways. I'm assuming due to a retry occurring.
The acceptable range was added to combat this type of test flakiness: https://github.com/kubernetes/ingress-nginx/pull/3390/files#diff-ea1cd91e199561de47a82f58b325412cR78