enable websockets - #2
Merged
Merged
Conversation
Signed-off-by: Peter Jausovec <peter.jausovec@solo.io>
EItanya
approved these changes
Jun 1, 2026
Merged
EItanya
pushed a commit
that referenced
this pull request
Aug 18, 2026
…-substrate#1002) Fixes agent-substrate#1001 `TestK8sResolverEndpointSliceUpdates` intermittently fails at `resolver_test.go:215` — 8 of the 30 most recent failed `pr-workflow` runs (~27%): ``` updated state.Addresses = [{Addr: "10.0.0.1:443", ServerName: "", }], want [{Addr: "10.0.0.1:443", ServerName: "", } {Addr: "10.0.0.2:443", ServerName: "", }] ``` `Build` starts a goroutine that calls `updateState` once `WaitForCacheSync` returns. That report is deliberate: a service with no EndpointSlices never fires `AddFunc`, so without it the resolver would stay silent instead of telling gRPC the answer is an empty set. But `WaitForCacheSync` polls at `syncedPollPeriod = 100ms`, so it fires roughly 100ms after Build — and if the test has not yet created the second slice by then, that update still carries only `10.0.0.1` and sits in the channel ahead of the real one. The second `select` took whatever came next, so it asserted against the stale update. Locally Build-to-Create is 0.6ms, well ahead of the timer, which is why this only shows up on loaded runners. The resolver is not at fault — it promises eventual convergence, not that the first update after a change is final, and a duplicate update costs gRPC nothing. So both waits now go through one `waitForAddrs` helper that consumes updates until the set matches, with a timeout so a genuinely broken resolver still fails and reports the last set it saw. Note this changes the first wait as well: it asserted the *first* update equals `[10.0.0.1]`, and now waits for that set instead. The same argument applies there — nothing promises the first update is final. ## Verification `-count=N` proves nothing here: the unfixed test passes locally at any count because the window is never hit. A/B with the Build→Create delay as the only variable: | delay | old assertion | new assertion | |---|---|---| | 0ms | 5/5 pass | 5/5 pass | | 150ms | **0/5 pass** | **5/5 pass** | Measured timeline with the 150ms stall in place: ``` [ 0.5ms] update #1: [10.0.0.1] <- AddFunc for slice1 first select takes it ... 150ms stall ... [101.1ms] update #2: [10.0.0.1] <- the WaitForCacheSync goroutine test creates slice2 [151.4ms] update #3: [10.0.0.1, 10.0.0.2] ``` The probe tests used for this are not included. Rebased over agent-substrate#1013. That fixes a different bug — concurrent `updateState` calls letting an older address set win — and does not close this one: the two updates here are ~100ms apart, so the queue has nothing to coalesce. Re-measured on top of it, unchanged: at a 150ms delay the old assertion is 0/5 and the new one 5/5.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
enable websockets on the gateway.