conformance: wait for mesh weighted routes to be programmed - #5214
conformance: wait for mesh weighted routes to be programmed#5214Automaat wants to merge 2 commits into
Conversation
Signed-off-by: Marcin Skalski <skalskimarcin33@gmail.com>
|
Welcome @Automaat! |
|
Hi @Automaat. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
|
Thanks @Automaat! This change looks correct to me, but I'll defer to one of the mesh leads to confirm. /ok-to-test |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Automaat, robscott 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 |
Signed-off-by: Marcin Skalski <skalskimarcin33@gmail.com>
|
I think this change might be somewhat conflating three distinct concerns - config propagation time, statistical variance (which is the stated reason for Providing an improved allowance for variable config propagation times and respecting
If the test can pass before the route config is applied though, then allowing a longer waiting period to ensure the config is applied doesn't seem to address an actual logical problem in this test case? |
|
@mikemorris Fair point; my description wasn't clear about this. There's no signal meaning "config is programmed" for a mesh, so the distribution check covers propagation and variance together, and I don't see a way to split them. It's safe to lean on because it can't pass early, and it requires the exact number of active backends. A pass implies the route is programmed. On your last question, I think that's two different checks. I looked at #5040 and #5122 first. Those gate on a signal that means ready. The closest mesh analogue is route status, so I measured it on Kuma (local k3d, apply to weights live in the sidecar):
Accepted lands ~1.2s before the dataplane has the config, more than the whole current budget, so a status gate alone wouldn't fix this. That's the gap you meant about confirming applied config. Happy to add one if you want the shape to match, but it wouldn't change flakiness. Across 13 runs: HTTPRoute 0.31-1.81s, GRPCRoute 0.18-2.03s. Both straddle the current budget. |
|
I'm increasingly feeling like the best approach here would be to split all of the weighted tests into two phases:
That splits the convergence time from the functionality that's actually being tested - does the configured weight actually take effect? |
|
@youngnick this wouldn't solve the actual issue I have since echo backends are available. The issue is the propagation time from applying policy on cp to it actually taking effect on dataplanes. There are multiple steps that need to be synced and computed in the meantime, before acutal config is propagated to dataplane |
|
Ah, I see now. I agree that this will help with the problem you describe then. |
What type of PR is this?
/kind flake
/area conformance-test
What this PR does / why we need it:
MeshGRPCRouteWeightandMeshHTTPRouteWeightstart sampling the traffic distribution before the route is programmed, so they fail against implementations that take more than a second to converge.Neither test gates on the route.
MakeRequestAndExpectEventuallyConsistentResponseonly needs three consecutive 200s, and in the mesh fixtures the parentechoService already fronts the echo-v1 and echo-v2 pods, so requests succeed with no route at all. In one failing run that gate passed 164ms after the route was created. The check then retriesMaxTestRetries(10) times with no delay, so the real budget is however long 10 batches take: under two seconds. Every attempt sees the pre-route 50/50 split.ExpectWeightedDistributionBatchreplaces the fixed retry count. It polls atDefaultPollIntervaluntilMaxTimeToConsistency, so the budget becomes the configured timeout rather than the sampling speed, and the fast path still returns on the first success. #5040 and #5122 fixed the same problem for the TCP and UDP tests by adding an explicit readiness gate; the mesh tests never got one. The other fourMaxTestRetriescallers are unchanged.Measured on Kuma, from apply to weights visible in the proxy config: HTTPRoute 0.31-1.81s, GRPCRoute 0.18-2.03s. Both straddle the current budget. That matches the roughly 50% failure rate in CI. kumahq/kuma#18322 skips the gRPC test downstream until this lands.
Does this PR introduce a user-facing change?:
Was AI used in preparing this PR?
AIL:3 - the change and this description were AI-drafted from my investigation of the failure, and reviewed by me before submission.
https://claude.ai/code/session_014tscnFJQ4AwC4CDdsz6P1m