fix(windows): refuse container: sibling containers explicitly instead of failing opaquely - #168
Merged
Merged
Conversation
…ad of failing opaquely
A Windows job using `container:` fails, and until now nothing said why. The
observed symptom is a bare `##[error]docker: command not found` in Set up job
(luthermonson/shipmates runs 31973561588, 31973696389, 31987299956). It is not
the runner refusing the directive — the runner never emits its Linux-only
refusal, it prepares the job and goes looking for docker.
Two independent things are wrong, and fixing only the visible one would have
made diagnosis worse rather than better.
1. No docker CLI. runtime.go prepends C:\actions-runner to PATH with a comment
claiming it is there for "the docker.exe we copy into the runner dir". No
code in this repo has ever copied a docker.exe anywhere; the only installs
are into the runner IMAGE (images/runner-ci-windows -> C:\go\bin). So any
Windows job on a stock image — the servercore default in image_windows.go,
or whatever a workflow names in `container:` — has no docker on PATH.
2. Sibling containers are Linux-only, structurally. dind's create path pins the
OCI spec to linux/<arch>, the snapshotter to overlayfs, the runtime to
io.containerd.runc.v2, bind-mounts /etc/hosts, /etc/hostname and
/etc/resolv.conf as Type:"bind", and resolves every -v through the runner
snapshot's overlayfs upper/lowerdirs. Windows containerd has none of that.
Supplying a docker.exe would only move the failure to a missing-snapshotter
error three layers down.
So the honest state is: ephemerd honours `container:` on Windows by making it
the runner image, and the job's steps do run inside it, but the Actions runner
also drives its own docker create/exec for the same directive and that half is
not implemented. This does not implement it. It makes the boundary explicit at
the two places an operator can see it, which is what
docs/arch/dind-bind-translation.md already listed as the alternative to
building a Windows translation layer ("a clean 'not supported' rejection at
request time").
- pkg/dind: checkWindowsSiblingGate rejects POST /containers/create on a
Windows host with 501 and a message naming the cause and the way out. Pure
function beside checkPrivilegedGate, and ordered after request-shape
validation and the privileged gate so their advice still wins.
- pkg/scheduler: resolveImage warns when a job declares `container:` AND
targets Windows — the one combination that cannot work. Silent for Linux and
macOS `container:` jobs and for Windows images that came from
[runner.images.<repo>].windows, so the warning stays worth reading.
- pkg/runtime: correct the docker.exe comment, which sent one investigation
hunting for a copy that does not exist. Also drop two stale
"Docker-in-Docker is not supported / use Kaniko or Buildah" comments that
predate pkg/dind.
- containment.yml: probe `docker create` on the Windows node. GATES only on
"must not succeed" — true on every build, so it cannot go red because a node
runs an older ephemerd — and REPORTS which error shape came back. Nothing in
.github/workflows uses `container:` on any platform, so this is the only
standing record of the fact.
No `container:` smoke test is added: the path does not work end to end, so
there is nothing to assert green.
Verified: GOOS=windows and GOOS=linux `go build ./...`, `go vet ./...` under
both, and `go test ./pkg/... ./cmd/...` on a Windows host — dind 274 pass /
9 skip, runtime 146/3, scheduler 237/2, all skips pre-existing Linux-only
tests. All 15 new test cases executed.
Review defect on PR #168. The probe added in 9a6c07b turned the whole Windows containment job red on every run, including — especially — when its assertion held. actions/runner appends if ((Test-Path -LiteralPath variable:\LASTEXITCODE)) { exit $LASTEXITCODE } to every powershell/pwsh script (ScriptHandlerHelpers.FixUpScriptContents). The probe's PASS path is `docker create` exiting NON-zero — that is the assertion — after which the script printed "all checks passed" and fell off the end with no explicit exit. The appended line then found $LASTEXITCODE still holding docker's code and exited with it, failing the step; Summary counts that outcome and exits 1. Every other step in the file survives only by accident of ending on a cmdlet. This one is the first whose success path ends on a deliberately failing native command. Reproduced locally against the real step text extracted from the YAML, with the runner's tail appended, driven by a stub docker on PATH. Before: PASS path exit 1. After, all four paths: create refused (assertion holds) -> step exit 0 create succeeded (real regression) -> step exit 1 no docker CLI on PATH -> SKIP, exit 0 no DOCKER_HOST -> SKIP, exit 0 `exit 0` on the pass branch rather than resetting $global:LASTEXITCODE; both were verified to work, the explicit exit does not depend on the appended line existing. Commented as load-bearing so it is not "tidied" to match its neighbours. This is the only step 9a6c07b added and the only native command it introduced. Also corrected while here: - The commit body of 9a6c07b implied the string "Container operations are only supported on Linux runners" was a real refusal. It is not; that literal string appears nowhere in this repo. The actual claim is docs/_archived/arch/overview.md:313-315 — "GitHub's runner binary blocks container operations (services:, container:) on Windows. This is hardcoded in the runner, not something ephemerd can fix." The shipmates logs DISPROVE both halves: the runner blocks nothing, and the blocker is ephemerd's, so it IS something ephemerd can fix. That archived paragraph is why this was never investigated and should be treated as retracted. - Scope, stated where each half is read: the 501 does NOT fix the reported bug. A stock Windows image has no docker CLI, so the job dies at PATH resolution and never opens a connection. The gate only helps images that ship a CLI; the scheduler warning is the only thing that changes for the reported case. - `services:` has the identical problem and gets no scheduler warning, because parseContainerImage reads the `container` key and nothing else. Noted at the warning. - runtime.go: replacing the false docker.exe rationale with a plausible guess was no better. The PATH prepend's real purpose is not recorded anywhere; say that instead of inventing one. - setPlatformGOOS mutates a package global — safe only while nothing in pkg/dind calls t.Parallel(). Documented with what to do if that changes. - New: TestWindowsHost_OnlyCreateIsGated pins the negative across 11 endpoints (version, info, ping, images, pull, push, build, container list, networks, system df) plus TestWindowsHost_ContainerCreateGateIsMethodScoped. Nothing previously asserted that the gate's blast radius stays one route, which is the property the Windows legs of build-images.yml and containment.yml depend on. Proven non-vacuous by mutation: hoisting the gate into the router fails 9 of the 11 subtests plus the method test (version and ping route ahead of the injection point). - resolveImage's quiet-path test now covers "darwin" as well as "macos"; both spellings reach it from different call sites. Verified: GOOS=windows and GOOS=linux `go build ./...`, `go vet ./...` under both, `go test ./pkg/... ./cmd/...` all exit 0 on a Windows host. dind 287 pass / 9 skip, runtime 146/3, scheduler 238/2, zero failures; every skip is a pre-existing Linux-only test. 28 test nodes across the new tests, each confirmed executed by name.
luthermonson
added a commit
that referenced
this pull request
Aug 18, 2026
…sharpen it Two changes to the Known Limitations entry. Merge-order hazard. The entry asserted "ephemerd has no code path that rejects them on Windows". True on main today, false the moment PR #168 lands its 501 gate for Windows sibling-container creation. Rewritten to describe the outcome (the job fails; where it fails depends on image and version) rather than the absence of a code path, so it reads correctly whichever of the two merges second. The deep-failure bullet now says the symptom is "either an explicit not-implemented from the daemon or a raw snapshotter/runtime error out of containerd", which covers both worlds. Sharpened verdict. The previous closing said "no verdict either way", which was right when written but undersold the evidence now confirmed against main: handleContainerCreate pins linux/<arch>, the overlayfs snapshotter and io.containerd.runc.v2 unconditionally (pkg/dind/containers.go), and Windows containerd only offers the windows/windows-lcow snapshotters (pkg/dind/cleanup.go). A Docker CLI genuinely only moves the failure deeper, so the entry now says "not supported" and explains why. That same pinning also condemns the `docker run` workaround this section had recommended since forever — container creation through the fake daemon is Linux-only, so the workaround fails on a Windows-native job for the same reason the thing it worked around does. Removed it rather than keep pointing people at it. `docker build`/`docker push` are unaffected (BuildKit path, exercised on Windows by build-images.yml), and Linux jobs are unaffected everywhere, so both are called out. Also scoped the adjacent dind entry, which claimed `docker run` works with no OS qualification and described the socket transport as if /var/run/docker.sock were universal, and scoped the "One Image, Every Host" line at :91 to Linux images (same defect already fixed at :585).
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
1 participant
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.
A Windows job using
container:fails with a 15-line log that dies insideSet up job:Not a refusal. The runner never reaches
Checking docker versionorInitialize containers— it fails at PATH resolution. The Linux leg of the same matrix reaches all of it. Runs are inluthermonson/shipmatesPR #48.The old claim was wrong, and its origin is known
Container operations are only supported on Linux runnersdoes not exist in this repo. It came fromdocs/_archived/arch/overview.md.resolveImage(pkg/scheduler/scheduler.go:112-125) is OS-agnostic andpkg/github/client.go:274-313parsescontainer.imagewith no OS gate.But
container:still cannot work on Windows — for a deeper reasonephemerd honours
container:by making it the runner image, so steps do run in the right image. The Actions runner then independently drives its own docker create/exec for the same directive, and that half is structurally Linux-only:pkg/dind/containers.go:383pinslinux/<arch>:586pins theoverlayfssnapshotter — Windows containerd has onlywindows/windows-lcow(pkg/dind/cleanup.go:285-289):589pinsio.containerd.runc.v2— Windows needsrunhcs.v1:511,532,544bind/etc/hosts,/etc/hostname,/etc/resolv.confasType:"bind"runtime.go:1263,containers.go:1483)So adding a docker CLI would only move the failure deeper.
docs/arch/dind-bind-translation.md:230-236,258-260already recorded this as deferred work needing "its own translation layer or a clean not-supported rejection at request time."Everything else in the shim works on Windows and is exercised today:
version,info,pull,push,images,build.What this does
Takes the second option the arch doc prescribes — makes the boundary visible rather than implementing the feature.
pkg/dind/containers.go—checkWindowsSiblingGatereturns 501 naming cause and workaround, instead of a missing-snapshotter error three layers down. Ordered so request-shape and privileged advice still win.pkg/scheduler/scheduler.go:117-132— warns only forcontainer:and Windows. Silent for Linux/macOScontainer:and for[runner.images.<repo>].windows.pkg/runtime/runtime.go:923— corrected a false comment claiming the PATH prepend existed for "the docker.exe we copy into the runner dir". Nothing has ever copied a docker.exe anywhere; the only installs go into the runner image atC:\go\bin. Found independently by three agents.runtime.go:47,:885— fixed two stale "Docker-in-Docker is not supported / use Kaniko or Buildah" comments predatingpkg/dind..github/workflows/containment.yml— probesdocker createon Windows, gating only on "must not succeed" so it cannot false-red an un-upgraded node, and reporting which error shape came back.No smoke test added — the path does not work, so there is nothing to assert green.
Verification
GOOS=windows/GOOS=linuxbuilds and both vets exit 0.go test -count=1 ./pkg/... ./cmd/...exit 0 on a real Windows host: dind 274 pass / 9 skip, runtime 146/3, scheduler 237/2 — every skip pre-existing and Linux-only. All 15 new cases confirmed executed individually, not inferred from the packageok.Genuinely unknown
Whether
ACTIONS_RUNNER_CONTAINER_HOOKSwould work. It is arguably correct for every platform — ephemerd already runs the runner inside the image, so a null hook collapses the directive and removes the double pull on Linux too — but it changes the working Linux path and could not be validated without the fleet.The practical guidance for users is
[runner.images.<repo>].windowsinstead ofcontainer:.