add httproute weight based routing mesh conformance tests #3827
Conversation
k8s-ci-robot
added
release-note
k8s-ci-robot
added
the
size/L
guicassolato
left a comment
guicassolato
left a comment
There was a problem hiding this comment.
/lgtm
Left a small suggestion that doesn't have to block the PR though.
|
|
||
| // Assert request succeeds before doing our distribution check | ||
| client.MakeRequestAndExpectEventuallyConsistentResponse(t, expected, s.TimeoutConfig) | ||
| for i := 0; i < 10; i++ { |
There was a problem hiding this comment.
Worth adding some entropy to each iteration to mitigate the risk of coordinated execution perhaps? For example: introducing jitter, randomising the payload, or anything else that varies the sample across iterations (without affecting expected weight distribution).
There was a problem hiding this comment.
I think this is a great idea, but I'm OK with doing that as a second step.
There was a problem hiding this comment.
agreed, thanks @guicassolato ! will open a follow up issue
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: guicassolato, LiorLieberman 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 |
|
This looks good to me, thanks @LiorLieberman! /lgtm |
|
Minor lint error @LiorLieberman |
LiorLieberman
force-pushed
the
httproute-weights
branch
from
d7ea33e to
e464433
Compare
k8s-ci-robot
removed
the
lgtm
LiorLieberman
force-pushed
the
httproute-weights
branch
from
e464433 to
5553968
Compare
|
/lgtm Thanks, @LiorLieberman! |
k8s-ci-robot
added
the
lgtm
…-sigs#3827) * add httproute weights mesh conformance tests * gofmt
5 participants
What type of PR is this?
/area conformance-test
Add mesh tests for request header modifier
This is a follow up to #3729
What this PR does / why we need it:
Add mesh tests coverage
Which issue(s) this PR fixes:
Related #3581
Does this PR introduce a user-facing change?: