envoyproxy / envoyproxy/gateway

TestEGUpgrade teardown flake: Delete gatewayclasses/upgrade: EOF

Open
#9,467 1 comment 0 reactions 0 assignees View on GitHub
stale triage
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.