Repository navigation
e2e: fix the ingressauth flake: retry a refused handshake that ends without the router's alert - #2961
Merged
Taahir Ahmed (ahmedtd) merged 1 commit intoOct 20, 2026
Conversation
… alert TestIngressAuthentication proves a refusal by the router's TLS alert. The router closes the connection right after sending it, and the port-forward or Go's HTTP/2 client sometimes reports that close first, as "http2: client conn could not be established", so the test failed with the refusal unproven. A failure without an alert is now retried on a fresh port-forward, three times at most; an alert is still required to pass.
Davanum Srinivas (dims)
changed the title
Davanum Srinivas (dims)
marked this pull request as ready for review
October 20, 2026 13:28
Lior Lieberman (LiorLieberman)
requested a review
from Taahir Ahmed (ahmedtd)
October 20, 2026 14:45
Taahir Ahmed (ahmedtd)
approved these changes
Oct 20, 2026
2 tasks done
Collaborator
|
#3051 similar fix for TestActorArbitraryPortAccess |
Benjamin Elder (BenTheElder)
added
area/tests
Sneha-at (Sneha-at)
pushed a commit
to Sneha-at/substrate
that referenced
this pull request
Oct 22, 2026
TestActorArbitraryPortAccess has been flaking in e2e since the router moved to static-mtls in agent-substrate#2390, e.g. https://github.com/agent-substrate/substrate/actions/runs/39549120410/job/119054007125 client-go closes the whole port-forward when any one connection on it errors: https://github.com/kubernetes/client-go/blob/v0.37.0/tools/portforward/portforward.go#L549-L550 RouterClient shared one CONNECT port-forward for the life of the client. So one reset from the router failed the tunnel in flight and every Connect after it, which then got "connection refused" from the dead local port. agent-substrate#2961 fixed the same thing in ingressauth by forwarding per attempt. <!-- The PR title becomes the release note. Write it for users. --> Why is this needed? (Or link the issue. Open one first for features, API, install or CI changes.) Fixes a trivial test flake bug, test only change. Spotted while reviewing PRs like agent-substrate#2502 <!-- e.g. "AI-assisted, I reviewed and tested it." No AI co-author trailers. --> AI use: AI-assisted, I reviewed it. <!-- Breaking change? Describe what users must do on upgrade and ask for the `breaking-change` label. --> - [x] Tests pass - [x] Docs updated
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
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.
TestIngressAuthenticationfails about one e2e job in three since static-mtls became the e2e default (5 of the 15 pr-workflow runs since #2390 merged): the refused client's request ends withhttp2: client conn could not be establishedinstead of the router's TLS alert, and the test calls the refusal unproven. The router closes the connection right after the alert, and the port-forward (portforward.go:549 ... write: broken pipe) or Go's HTTP/2 client reports that close first.Recent instances, all on the
httpslistener:Change. A refusal that fails without a TLS alert is retried on a fresh port-forward, three times at most. An alert is still required, and a client the router lets through still fails the test.
Tests.
go vetand the suite compile; the retry only shows under the flake, which CI's e2e jobs exercise.