test: bind git HTTP test server to ephemeral port#1085
Merged
Conversation
The integration-test git HTTP server hardcoded binding 0.0.0.0:8080. Gateway containers share the BuildKit worker's network namespace, so when multiple servers ran concurrently the second net.Listen failed with EADDRINUSE. This made TestSourceGitChecksumPreservesTagMetadata (two t.Parallel() subtests) and TestGomodGitAuth/HTTP flaky. The server now accepts port "0" to request an ephemeral port from the kernel and reports the actually-bound port in its ready event. StartHTTPGitServer records that real port back into Attr.HTTPPort so the spec and client gitconfig generation that run afterward use it. Tests now request port "0" instead of a fixed 8080. Trade-off: callers must start the HTTP server before generating specs or client gitconfig, since the concrete port is only known once the server is online. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Brian Goff <cpuguy83@gmail.com>
cpuguy83
force-pushed
the
cpuguy83/musical-waffle
branch
from
1e21532 to
37ff73a
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes flaky integration test failures (TestSourceGitChecksumPreservesTagMetadata and TestGomodGitAuth/HTTP) caused by port collisions when multiple parallel test git HTTP servers all attempted to bind to the hardcoded port 8080. The fix uses ephemeral port allocation (port "0") and propagates the actual bound port back through the existing ReadyEvent protocol.
Changes:
- The server binary (
cmd/server) now resolves the actual bound port from the listener when"0"is requested, and emits it in the ready event. Error paths correctly close the listener. StartHTTPGitServerrecords the real port back intoAttr.HTTPPortso downstream spec and gitconfig generation use the correct port.- Test call sites in
source_test.goandgomod_git_auth_test.gorequest port"0"instead of"8080".
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/git_services/cmd/server/main.go | Resolves the actual bound port from the TCP listener after binding, uses it in the ready event, and adds listener.Close() on error paths. |
| test/git_services/teststate.go | Writes the actual port from the ready event back into ts.Attr.HTTPPort so downstream consumers observe the real port. |
| test/git_services/attributes.go | Updates the HTTPPort field documentation to describe ephemeral port behavior. |
| test/source_test.go | Changes HTTPPort from "8080" to "0". |
| test/gomod_git_auth_test.go | Changes HTTPPort from "8080" to "0" and updates the comment to explain ephemeral port semantics. |
The port-related fields (Attributes.HTTPPort/SSHPort, ReadyEvent.Port, ServerResult.Port, GenerateSpec's port local) were strings. A port is fundamentally a number, and keeping it a string forced a fallible string->int conversion path. Modeling it as int means only the infallible int->string conversion happens, and only at the edges that genuinely need a string (the listen-address exec arg, the gomod auth host:port key, and the source URL). The server now emits the bound port as a JSON number in its ready event, and the SSH server script emits its port unquoted to match. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Collaborator
Author
|
GHA appears to be wedged. |
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.
Problem
TestSourceGitChecksumPreservesTagMetadata(and occasionallyTestGomodGitAuth/HTTP) failed intermittently with:The integration-test git HTTP server hardcoded binding
0.0.0.0:8080. The gateway containers that run these servers share the BuildKit worker's network namespace, so when two servers were alive at the same time the secondnet.ListenhitEADDRINUSE.TestSourceGitChecksumPreservesTagMetadataruns twot.Parallel()subtests, each starting an HTTP git server on 8080, andTestGomodGitAuth/HTTPuses 8080 too. The failure only surfaced when these overlapped under the test scheduler, which is why it was flaky.Fix
Let the kernel assign a free port instead of hardcoding 8080:
cmd/server(runServe): accepts"0"as the requested port, then reads the actual bound port fromlistener.Addr()and reports it in the ready event.StartHTTPGitServer: records the real port back intoAttr.HTTPPortso the spec and client gitconfig generation that run afterward use it.source_test.go/gomod_git_auth_test.go: request port"0"instead of8080.Trade-off
Callers must start the HTTP server before generating specs or client gitconfig, since the concrete port is only known once the server is online. Current call sites already follow this order.
SSH (port 2222) is left unchanged — only one SSH server is started per test process, so it can't collide.