envoyproxy / envoyproxy/gateway
TestEGUpgrade teardown flake: Delete gatewayclasses/upgrade: EOF
- Dominant language
- Go
- Stars
- 3k
- Forks
- 864
- Avg merge
- 2d 2h
- Merged PRs (30d)
- 140
Description
### What happened?
`TestEGUpgrade` intermittently fails during teardown when the gateway-api
conformance suite's cleanup deletes the `upgrade` GatewayClass, surfacing a
transient connection-level error:
--- FAIL: TestEGUpgrade/EGUpgrade/Upgrade_from_an_older_eg_release_should_succeed
...
Delete "https://.../apis/gateway.networking.k8s.io/v1/gatewayclasses/upgrade": EOF
This is a test-infrastructure flake, not a regression in the code under test.
It has shown up on unrelated PRs (e.g. it appeared on the e2e-test jobs of
#9460, which only changes HTTPRoute redirect handling) — the failing DELETE is
part of the suite's registered `t.Cleanup`, run after the test body already
succeeded.
Failed CI run (PR #9460): https://github.com/envoyproxy/gateway/actions/runs/29031092521
Affected jobs: the `e2e-test (...)` matrix entries in that run.
### Root cause
The teardown DELETE goes through `suite.Client`, which the gateway-api
conformance suite rebuilds **per test** via `client.New(suite.RestConfig, ...)`
in `setClientsetForTest`
([suite.go](https://github.com/kubernetes-sigs/gateway-api/blob/v1.6.0/conformance/utils/suite/suite.go)).
The underlying DELETE is issued by `Applier.MustApplyWithCleanup`'s cleanup
closure ([apply.go](https://github.com/kubernetes-sigs/gateway-api/blob/v1.6.0/conformance/utils/kubernetes/apply.go)),
which already treats `IsNotFound` as success.
The gap is in client-go's own retry logic. `rest.Request.request()` only retries
**GET** on transient connection errors:
```go
isErrRetryableFunc := func(req *http.Request, err error) bool {
if req.Method != "GET" {
return false // DELETE / other writes are NOT retried
}
if net.IsConnectionReset(err) || net.IsProbableEOF(err) || net.IsHTTP2ConnectionLost(err) {
return true
}
return false
}
So when the DELETE reaches the apiserver but the response is lost (EOF /
connection reset / HTTP/2 GOAWAY / timeout), the error is returned to the test
immediately and teardown fails — even though the resource was already deleted
(or will report NotFound on a retry).
Proposed fix
Install a retrying http.RoundTripper via rest.Config.WrapTransport in
test/utils/kubernetes's NewClient. The transport wrapper must be on
RestConfig (not a client.Client wrapper) precisely because the conformance
suite rebuilds suite.Client from suite.RestConfig per test — a client-level
wrapper would be bypassed by setClientsetForTest, while WrapTransport is
inherited by every client.New(suite.RestConfig, ...).
Retry policy (conservative):
- Methods: GET, HEAD, DELETE only. Never POST/PATCH/PUT.
- Body: only retry when the body is replayable — req.Body == nil,
req.Body == http.NoBody, or req.GetBody != nil. (client-go builds DELETE
bodies from bytes.NewReader(bodyBytes), and the stdlib auto-populates
GetBody for *bytes.Reader, so DeleteOptions bodies are replayable.
http.NoBody must be checked explicitly because the stdlib substitutes it for
a zero-length body instead of nil.)
- Before each retry after the first: re-derive the body via req.GetBody()
and assign it to a req.Clone(...) (Clone copies GetBody but does not reset
Body).
- Errors retried: io.EOF, io.ErrUnexpectedEOF, connection reset,
HTTP/2 GOAWAY/connection-lost, net.Error timeouts.
- HTTP status codes are NOT retried (5xx/429/404 returned as-is), so a 404
from a retried DELETE flows through to MustApplyWithCleanup's existing
IsNotFound → success handling.
- Close any partial resp.Body before retrying; honor context cancellation in
the backoff select.
- Compose with any pre-existing cfg.WrapTransport via transport.Wrappers
rather than overwriting it.
A draft implementation + unit tests exist in worktree
worktree-e2e-eof-retry (modifies test/utils/kubernetes/client.go, adds
client_test.go). Happy to open a PR once this issue is tracked.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with test/utils/kubernetes/client.go and client_test.go in the worktree-e2e-eof-retry draft, then inspect how NewClient configures RestConfig and WrapTransport. Run the unit tests and verify the requested retry behavior, replayable-body handling, response cleanup, cancellation, and composition with existing wrappers.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, kubernetes
- Domain
- testing
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 45/100