Enable network tests - #12
Open
dmcgowan wants to merge 3 commits into
Open
Conversation
Port the network-only portions of the sandbox-tests branch's 'Add listen network sandbox tests' commit onto main, without any sandbox-suite changes: - NetworkSuite gains InboundTCPListen (host connects to a container's listener) and LoopbackWithinContainer (a container reaches itself over 127.0.0.1), alongside the existing OutboundTCP/UDP and DNSResolve tests. - Wire NewNetworkSuite into TestShim under the "net" feature flag; it was previously defined but never invoked. - Add the looptest testbin command (self-contained in-container loopback echo, used by LoopbackWithinContainer) and nc(1) listen mode (-l), with -v to report the bound socket. As in standard nc, that notice goes to stderr, in nc's own "Listening on <addr> <port>" format, and stdout is left carrying connection payload only -- so InboundTCPListen can learn an ephemeral port and still assert on payload without control output interleaved in the same stream. The "net" feature is not yet enabled on any runc profile; a bare runc container has no network setup at all in either profile (no CNI, no veth, no port mapping), so every test in this suite would fail. Follow-up commits add in-test network setup so these tests can run against runc too. Signed-off-by: Derek McGowan <derek@mcg.dev>
dmcgowan
force-pushed
the
enable-network-tests
branch
from
August 14, 2026 23:49
8ff72de to
80482de
Compare
dmcgowan
marked this pull request as ready for review
August 15, 2026 17:29
There was a problem hiding this comment.
Pull request overview
This PR enables and extends the shimtest networking conformance coverage by wiring NetworkSuite into the default runner, adding inbound/loopback test coverage, and (optionally) letting the test harness provide container networking via slirp4netns for shims/runtimes that don’t configure networking themselves.
Changes:
- Run
NetworkSuitefromTestShimwhen thenetfeature isn’t skipped, and expand the suite with inbound TCP listen and in-container loopback tests. - Extend the embedded
testbinwithnc -l(listen mode) and add a newlooptesthelper command used by the loopback test. - Add
Config.ProvideNetworksupport (JSON + docs + CI deps) and implement Linux-onlyslirp4netnsattachment/port forwarding helpers.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
testbin/testbin.go |
Add nc listen mode and new looptest helper to support network conformance tests. |
runner_test.go |
Invoke NetworkSuite when net isn’t skipped. |
rootfs.go |
Add looptest to the embedded rootfs command set. |
README.md |
Document provide_network and add new network tests to the test table. |
network_suite.go |
Add inbound TCP listen + loopback tests; add host-port selection and optional provided networking integration. |
main_test.go |
Add provide_network to JSON config schema and marshal/unmarshal flow. |
helpers.go |
Add withNewNetworkNamespace() spec opt for tests that need a dedicated netns. |
helpers_netns_other.go |
Provide non-Linux stubs/constants so non-Linux builds compile and skip provided-network paths. |
helpers_netns_linux.go |
Implement slirp4netns-based netns attachment + hostfwd API integration. |
config.go |
Add Config.ProvideNetwork to control harness-provided networking. |
.github/workflows/profiles/runc.json |
Enable harness-provided networking for runc profile. |
.github/workflows/profiles/runc-rootless.json |
Enable harness-provided networking for rootless runc profile. |
.github/workflows/ci.yml |
Install slirp4netns in CI to support provide_network runs. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+763
to
+765
| // looptest <token>: starts echosrv on an ephemeral port, waits for it to | ||
| // print the port, then connects to 127.0.0.1:<port>, sends the token, and | ||
| // prints the echoed response to stdout. |
Comment on lines
+69
to
+77
| // Leave this false for any shim that already gives containers real | ||
| // networking on its own (e.g. a VM-based shim bridging guest | ||
| // traffic to the host) — NetworkSuite then tests that default path | ||
| // directly, exactly as it always has. Set it true only for shims | ||
| // with no network setup of their own (e.g. bare runc, which leaves | ||
| // a rootless container in an empty, unconfigured network | ||
| // namespace): the suite's tests would otherwise be unsatisfiable | ||
| // through no fault of the shim, since providing container | ||
| // networking isn't part of the shim v2 API contract at all. |
| | `uid` | int | UID to run as; defaults to the current user's UID. If set to a value different from the current UID and the effective UID is 0, the harness re-execs itself as that user via `sudo` | | ||
| | `gid` | int | GID to run as | | ||
| | `format_mounts` | bool | Provide the rootfs as formatted erofs/ext4 images with a `format/mkdir/overlay` descriptor for the shim to mount. Default (`false`) extracts the rootfs and provides a pre-mounted overlay (or plain directory when rootless) | | ||
| | `provide_network` | bool | Have `NetworkSuite` set up a container's network connectivity itself (a dedicated network namespace plus a `slirp4netns` process attached to it) instead of assuming the shim already gives containers a working default network path. Leave `false` for shims with real networking of their own (e.g. VM-based shims); set `true` only for shims with no network setup at all (e.g. bare runc). Requires `slirp4netns` on `PATH`; skipped otherwise | |
Comment on lines
+108
to
+111
| if err := cmd.Start(); err != nil { | ||
| tb.Fatalf("attachContainerNetwork: start slirp4netns: %v", err) | ||
| } | ||
| readyW.Close() |
Give NetworkSuite the ability to set up a container's network connectivity itself, rather than assuming the shim already provides one. Bare containerd-shim-runc-v2 leaves containers with no network setup at all in either privilege mode: a rootless container gets a fresh, empty network namespace (only a down loopback), and a root container shares the host's own namespace but has no isolated, attachable namespace of its own. Neither can satisfy this suite's tests as shipped. Add attachContainerNetwork (helpers_netns_linux.go), which attaches a dedicated slirp4netns process to a container's network namespace between Task.Create (once the namespace exists) and Task.Start (before the container's entrypoint runs), using slirp4netns's --ready-fd as a race-free readiness barrier -- no retry logic is needed in the container's own networking code. The same mechanism works identically for root and rootless shims (slirp4netns needs only a target namespace, plus --userns-path when it isn't owned by the caller's own user namespace), so root and rootless are unified onto one code path via a new withNewNetworkNamespace CreateOCISpec opt that gives a root container an isolated namespace too. Skips (not fails) when slirp4netns isn't on PATH. This is opt-in via a new Config.ProvideNetwork field (JSON: provide_network), left false by default so shims with real networking of their own (e.g. VM-based shims bridging guest traffic to the host) continue to have their own default network path tested directly and unmodified. Enabled only in the runc and runc-rootless CI profiles. Inbound port forwarding uses a fixed, well-known port (networkSuiteInboundPort) registered via slirp4netns's API socket (add_hostfwd) before Task.Start, rather than a runtime-discovered ephemeral one: the forward is in place before the container's listener even exists, so there's no window for an early connection attempt to race its registration. DNS resolution needs the container's /etc/resolv.conf pointed at slirp4netns's DNS proxy; the bind mount providing it deliberately omits ro, since a read-only bind mount's required follow-up remount fails EPERM in a rootless user namespace, and this file is test-owned scratch data, not a security boundary. Testing this end-to-end surfaced two pre-existing bugs in testInboundTCPListen, latent because the test could never previously run far enough to hit them against bare runc: it asserted a socket echo that nc never produces, since nc copies its connection to stdout and its stdin to the connection but has no path echoing received data back over the same connection, and it closed the container's stdin FIFO without the Task.CloseIO call the shim needs to actually observe EOF on it (already documented and handled correctly in this same file's other tests). Fixed both: the test now confirms inbound delivery by waiting for the token in the container's own captured stdout, and issues CloseIO before Task.Wait. Also adds a defensive read deadline to nc -u's UDP receive (testbin.go) so a missing reply is a fast, legible failure rather than a hang -- not a retry, since attachContainerNetwork already guarantees the network is ready before the container's entrypoint ever runs. Verified locally against runc (root and rootless, repeated runs) with a real slirp4netns: all five NetworkSuite tests pass in both privilege modes, and the full non-network suite is unaffected. Signed-off-by: Derek McGowan <derek@mcg.dev>
testInboundTCPListen and looptest both asked the container to bind port 0 and then discovered the assigned port from the container's own output. Some shims proxy each socket call to the host independently rather than truly sharing one network stack; under such a shim, the guest and the host can each resolve "an ephemeral port" on their own, with no guarantee the two agree. The application only ever reports the guest's number, which is not necessarily where the host is actually listening — every dial to the reported port then hits a real, but wrong, host port with nothing on it. Request a concrete port instead, chosen at random from a fixed range outside Linux's default ephemeral range, with a short retry loop on conflict: - pickHostPort (network_suite.go) picks a free host port for testInboundTCPListen by binding and immediately releasing a listener, used unconditionally now rather than only when Config.ProvideNetwork forces a fixed port for a different reason (registering the inbound forward before Task.Start). This also lets the port-validation check become a direct equality assertion, so a guest/host port mismatch now fails with a clear diagnostic instead of surfacing as a bare connection-refused two steps later. - cmdLooptest (testbin) applies the same fix to its self-contained in-container listener/connector pair, which is just as exposed to a guest/host port mismatch despite never leaving the container. Neither retry loop recreates a container on a bind conflict — that would need a materially more complex retry around the whole task lifecycle for a residual, now much less likely race. That is a reasonable next step if this ever proves flaky in practice, not something to build preemptively. Signed-off-by: Derek McGowan <derek@mcg.dev>
dmcgowan
force-pushed
the
enable-network-tests
branch
from
August 17, 2026 23:26
c0bc51e to
5aa74b4
Compare
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.
Enable network tests for runc and nerdbox, using slirp4netns for rootless tests