From 8338add6c986545f00d90ab39f6e2df6f9e2b0cd Mon Sep 17 00:00:00 2001 From: bilby91 Date: Sat, 16 May 2026 12:18:44 -0300 Subject: [PATCH 1/2] applecontainer: properly wire named volume mounts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bridge's toFilesystem mapped MountType=volume to a virtiofs bind against the spec's source string. Apple's apiserver doesn't recognize that shape for named volumes and normalizes the source via URL(fileURLWithPath:), which resolves it relative to the launching process's CWD — yielding paths like /private/tmp/runner/_ that don't exist, and surface as `errno 2` / "vminitd.log doesn't exist" when the VM tries to bootstrap. Fix: resolve the volume name through ClientVolume.inspect to fetch the backing image path and filesystem format, then construct a real Filesystem.volume(...) — matching the shape apple/container's own CLI emits for `container run -v :/dest`. toFilesystem becomes async since ClientVolume.inspect is async; the call site loops instead of map. Surfaced while bringing up a compose-source devcontainer (crunchloop/dap) on the applecontainer backend through compose.Orchestrator — the app service's claude-config named volume hit this path. Add TestRunContainer_NamedVolume as a regression guard: creates a volume via the CLI, runs a container with MountVolume against it, asserts the inspect path round-trips a MountVolume mount with a non-empty source. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Sources/ACBridge/lifecycle.swift | 30 +++++++--- .../lifecycle_darwin_arm64_test.go | 60 +++++++++++++++++++ 2 files changed, 82 insertions(+), 8 deletions(-) diff --git a/applecontainer-bridge/Sources/ACBridge/lifecycle.swift b/applecontainer-bridge/Sources/ACBridge/lifecycle.swift index ec58f10..b4a7f62 100644 --- a/applecontainer-bridge/Sources/ACBridge/lifecycle.swift +++ b/applecontainer-bridge/Sources/ACBridge/lifecycle.swift @@ -103,7 +103,11 @@ private func runContainer(spec: RunSpecJSON) async throws { var cfg = ContainerConfiguration(id: spec.id, image: img.description, process: process) cfg.platform = platform cfg.labels = spec.labels ?? [:] - cfg.mounts = try (spec.mounts ?? []).map(toFilesystem) + var resolvedMounts: [Filesystem] = [] + for m in spec.mounts ?? [] { + resolvedMounts.append(try await toFilesystem(m)) + } + cfg.mounts = resolvedMounts cfg.capAdd = spec.capAdd ?? [] cfg.useInit = spec.initProcess ?? false // Enable Rosetta when running an amd64 container on an arm64 @@ -232,7 +236,7 @@ private func imageConfigUser(_ cfg: ImageConfig?) -> ProcessConfiguration.User { return .id(uid: 0, gid: 0) } -private func toFilesystem(_ m: MountJSON) throws -> Filesystem { +private func toFilesystem(_ m: MountJSON) async throws -> Filesystem { var options: MountOptions = [] if m.readOnly ?? false { options.append("ro") @@ -246,13 +250,23 @@ private func toFilesystem(_ m: MountJSON) throws -> Filesystem { case "tmpfs": return .tmpfs(destination: m.target, options: options) case "volume": - // PR-C scope decision: treat named volumes as virtiofs binds - // against the supplied source. Real named-volume support (with - // the apiserver pre-creating the volume) is a later PR. - guard let src = m.source, !src.isEmpty else { - throw BridgeError.invalidArgument("volume mount on this backend currently requires source (named-volume support is deferred)") + // Named volume: source carries the volume name. Resolve it + // through ClientVolume.inspect to fetch the backing image + // path + filesystem format the apiserver expects, then build + // a proper Filesystem.volume. Treating the spec as a virtiofs + // bind (PR-C's interim shape) makes the apiserver resolve the + // name against CWD and fail with errno 2 at bootstrap. + guard let name = m.source, !name.isEmpty else { + throw BridgeError.invalidArgument("named volume mount requires source (volume name)") } - return .virtiofs(source: src, destination: m.target, options: options) + let vol = try await ClientVolume.inspect(name) + return .volume( + name: vol.name, + format: vol.format, + source: vol.source, + destination: m.target, + options: options + ) default: throw BridgeError.invalidArgument("unknown mount type \"\(m.type)\"") } diff --git a/runtime/applecontainer/lifecycle_darwin_arm64_test.go b/runtime/applecontainer/lifecycle_darwin_arm64_test.go index 3f5d633..a9f6946 100644 --- a/runtime/applecontainer/lifecycle_darwin_arm64_test.go +++ b/runtime/applecontainer/lifecycle_darwin_arm64_test.go @@ -176,6 +176,66 @@ func TestRunContainer_BindMount(t *testing.T) { } } +// TestRunContainer_NamedVolume creates a container with a named volume +// mount and verifies the inspect path reports a MountVolume entry +// pointing at the volume's backing image (not a virtiofs bind against +// the launcher CWD). Regression guard for the bug where named volumes +// fell through to virtiofs and bootstrapped with errno 2. +func TestRunContainer_NamedVolume(t *testing.T) { + rt := runtimeOrSkip(t) + ctx := context.Background() + const ( + id = "ac-namedvol-test" + volume = "ac-namedvol-test-vol" + ) + _ = rt.RemoveContainer(ctx, id, runtime.RemoveOptions{Force: true}) + cliRun(t, "volume", "rm", volume) + t.Cleanup(func() { + _ = rt.RemoveContainer(ctx, id, runtime.RemoveOptions{Force: true}) + cliRun(t, "volume", "rm", volume) + }) + + cliRunStrict(t, "volume", "create", volume) + cliRunStrict(t, + "run", "--rm", "--name", "ac-alpine-warmup-vol", + "docker.io/library/alpine:latest", "/bin/true", + ) + + _, err := rt.RunContainer(ctx, runtime.RunSpec{ + Image: "docker.io/library/alpine:latest", + Name: id, + Cmd: []string{"sleep", "60"}, + Mounts: []runtime.MountSpec{ + {Type: runtime.MountVolume, Source: volume, Target: "/mnt/data"}, + }, + }) + if err != nil { + t.Fatalf("RunContainer with named volume: %v", err) + } + + details, err := rt.InspectContainer(ctx, id) + if err != nil { + t.Fatalf("InspectContainer: %v", err) + } + var found bool + for _, m := range details.Mounts { + if m.Target == "/mnt/data" && m.Type == string(runtime.MountVolume) { + found = true + // The apiserver substitutes the volume's on-disk image + // path as the canonical source. We don't pin the exact + // path (changes across apple/container releases) but it + // must not be empty and must not be the launcher CWD. + if m.Source == "" { + t.Errorf("named volume mount has empty source; expected backing path") + } + } + } + if !found { + t.Errorf("named volume mount not round-tripped; details.Mounts=%+v want target=/mnt/data type=volume", + details.Mounts) + } +} + // waitForState polls InspectContainer until the desired state is // observed or the timeout fires. Apple's runtime status transitions // asynchronously through the apiserver event loop. From 15d2c2906edeee914dc9cacecca538817c72b02c Mon Sep 17 00:00:00 2001 From: bilby91 Date: Sat, 16 May 2026 13:50:38 -0300 Subject: [PATCH 2/2] test: assert named volume mount source isn't rooted at launcher CWD MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The original bug produced a non-empty source — the failing path was "/private/tmp//" — so the existing "Source != ''" check would have passed it. Add an explicit "source isn't CWD-prefixed" guard so this test catches the exact shape of the regression. Per CodeRabbit review on PR #67. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../applecontainer/lifecycle_darwin_arm64_test.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/runtime/applecontainer/lifecycle_darwin_arm64_test.go b/runtime/applecontainer/lifecycle_darwin_arm64_test.go index a9f6946..990f357 100644 --- a/runtime/applecontainer/lifecycle_darwin_arm64_test.go +++ b/runtime/applecontainer/lifecycle_darwin_arm64_test.go @@ -5,6 +5,8 @@ package applecontainer import ( "context" "errors" + "os" + "path/filepath" "strings" "testing" "time" @@ -217,6 +219,11 @@ func TestRunContainer_NamedVolume(t *testing.T) { if err != nil { t.Fatalf("InspectContainer: %v", err) } + cwd, err := os.Getwd() + if err != nil { + t.Fatalf("Getwd: %v", err) + } + cwd = filepath.Clean(cwd) var found bool for _, m := range details.Mounts { if m.Target == "/mnt/data" && m.Type == string(runtime.MountVolume) { @@ -224,10 +231,16 @@ func TestRunContainer_NamedVolume(t *testing.T) { // The apiserver substitutes the volume's on-disk image // path as the canonical source. We don't pin the exact // path (changes across apple/container releases) but it - // must not be empty and must not be the launcher CWD. + // must not be empty and must not be the launcher CWD — + // the original bug had a non-empty source rooted at the + // launcher's working directory (apple's virtiofs path + // resolution), so empty-source alone wouldn't catch it. if m.Source == "" { t.Errorf("named volume mount has empty source; expected backing path") } + if strings.HasPrefix(filepath.Clean(m.Source), cwd+string(filepath.Separator)) { + t.Errorf("named volume mount source rooted at launcher CWD: source=%q cwd=%q", m.Source, cwd) + } } } if !found {