kube: relax flaky GOAWAY concurrent test assertion#66672
Merged
Conversation
jakealti
added
kubernetes-access
no-changelog
rosstimothy
reviewed
May 12, 2026
| // Some scheduler timings produce only network-level failures | ||
| // (broken pipe, conn reset) with no rewind path traversed. | ||
| // That's fine for what this test asserts, so log instead of requiring. | ||
| t.Logf("requests caught by GOAWAY 429 translation: %d/%d", retried.Load(), concurrency) |
Contributor
There was a problem hiding this comment.
Does logging this provide any value? If we don't care to assert a particular value, then should we just remove the log and assertion?
Contributor
Author
There was a problem hiding this comment.
Not really. I've removed it.
rosstimothy
approved these changes
May 12, 2026
espadolini
approved these changes
May 13, 2026
public-teleport-github-review-bot
Bot
removed request for
marcoandredinis and
tigrato
tigrato
approved these changes
May 13, 2026
This was referenced May 13, 2026
ivan-bax
pushed a commit
to ivan-bax/teleport
that referenced
this pull request
May 22, 2026
…6710) * kube: handle net/http rewind body GOAWAY error (gravitational#66605) * kube: handle net/http rewind body GOAWAY error * kube: fix data race in GOAWAY concurrent test * kube: match wrapped rewind body errors too * kube: set GetBody on buffered create requests * kube: relax flaky GOAWAY concurrent test assertion (gravitational#66672) * kube: relax flaky goaway concurrent assertion * kube: drop unused retried counter and log
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
4 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.
Follow-up to #66605. The soft
require.NotZero(retried.Load(), ...)check inTestGOAWAYHandling_Concurrentwas flaky: some scheduler timings produce only network-level failures without exercising the rewind path, so the 429 count legitimately reaches zero.Demoted to
t.Logf. The strict assertion that catches the actual bug (require.Zero(rewindLeaks, "rewind-body error must never reach the client")) is unchanged.Flake: https://github.com/gravitational/teleport.e/actions/runs/25729502557/job/75550786265
Local testing
go test -count=200 -race -run '^TestGOAWAYHandling_Concurrent$' ./lib/kube/proxy/— 200/200 pass in 61s.stress -p 8 ./goaway.test -test.run='^TestGOAWAYHandling_Concurrent$' -test.timeout=30s— 958 runs, 0 failures over ~3 minutes.formatStatusResponseErrorand re-ran-count=20 -race: the test fails reliably (20/20) withrewind-body error must never reach the client. The strict assertion still catches the original bug.