From bc1c453ff55630326eac29c9f4e32e32136e6051 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Fern=C3=A1ndez?= Date: Wed, 28 Oct 2026 17:06:31 +0000 Subject: [PATCH] fix(compose): reattach existing projects without fetching features Every compose-source Up ran createFreshCompose -> layerFeatures, which fetched each feature from its registry and rebuilt dc-go-final- even when the project's containers already existed. A restart of a compose workspace with features therefore needed registry egress and paid a final-image rebuild; without egress (a docker-in-docker workspace in a guest with no route to ghcr.io) it could not start at all. The image path's attachExisting never had this cost. - Stamp dev.containers.compose-inputs-hash on the primary container: a digest of the container-shaping part of the resolved config (features as requested, with options; local feature content), the loaded compose project, the primary base image ID and the engine options that reshape the project. On a later Up with a matching label and an unmoved image tag, reattachCompose merges the metadata from the existing image's label and brings the project up against the existing images (native under the caller's AdoptExisting with present sidecar images not rebuilt; shell-out with --no-recreate). Any miss takes the full path unchanged. - Escape $ in the devcontainer.metadata LABEL: the Dockerfile parser expanded ${devcontainerId}, so docker-in-docker's volume source was recorded as "dind-var-lib-docker-" and every read of the label lost the per-workspace volume name. - OCI feature cache is local-first for digest refs and exact X.Y.Z tags; floating tags stay online-first and fall back to the recorded resolution only when the registry is unreachable. Extraction is atomic. Adds DiskStoreOptions.OCITransport and ContainerDetails.ImageRef. Cut 0.5.2 in the changelog. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UMTBosSzdEvXqCZ9c7VyLk --- CHANGELOG.md | 57 ++- README.md | 4 +- design/compose-native.md | 62 +++ design/features.md | 31 +- feature/disk_store.go | 8 + feature/dockerfile.go | 16 +- feature/dockerfile_test.go | 49 +++ feature/oci.go | 211 ++++++++- feature/oci_cache_test.go | 288 +++++++++++++ runtime/docker/inspect.go | 1 + runtime/runtime.go | 8 + test/integration/compose_dind_test.go | 133 ++++++ test/integration/compose_reattach_test.go | 500 ++++++++++++++++++++++ up.go | 95 +++- up_compose_host_ports_test.go | 2 +- up_compose_mounts_test.go | 2 +- up_compose_reattach.go | 340 +++++++++++++++ up_compose_reattach_test.go | 308 +++++++++++++ up_compose_sidecar_test.go | 6 +- 19 files changed, 2082 insertions(+), 39 deletions(-) create mode 100644 feature/oci_cache_test.go create mode 100644 test/integration/compose_reattach_test.go create mode 100644 up_compose_reattach.go create mode 100644 up_compose_reattach_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d3c773..a7c8dfd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,60 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.5.2] - 2026-10-28 + +### Fixed + +- **compose** — `Up` on a compose-source devcontainer whose containers + already exist no longer fetches every feature from its registry and + rebuilds the final image. The primary container now carries a + `dev.containers.compose-inputs-hash` label (`LabelComposeInputsHash`): + a digest of the container-shaping part of the resolved + `devcontainer.json` (features as requested, with options; local feature + content), the loaded compose project, the primary service's base image + ID and the engine options that reshape the project. When it matches, + `Up` reattaches as the image path's `attachExisting` does: it reads the + merged metadata from the existing image's `devcontainer.metadata` label + (entrypoint wrapper, mounts including the v0.5.1 volume/tmpfs fix, env, + privileged, capAdd, lifecycle commands) and starts the existing services + from their existing images — native under the caller's `AdoptExisting`, + shell-out with `up --no-recreate`. A restart therefore needs no + registry: before, a docker-in-docker compose workspace whose host had no + route to ghcr.io failed every start after a stop. Any mismatch (changed + config or compose file, new base image, a moved image tag, `Recreate`, + `PullAlways`) takes the full path as before. Upgrade note: containers + created by an earlier version have no label, so they take the full path. + The label is part of the primary service's config, so the native backend + without `AdoptExisting` recreates such a container on its first `Up` + after upgrading (and labels it); with `AdoptExisting`, or on the + shell-out backend, the container is kept unlabelled and keeps taking the + full path until it is recreated. +- **features** — the `devcontainer.metadata` label written into feature + images kept `$` unescaped, so the Dockerfile parser expanded + `${devcontainerId}` to nothing: docker-in-docker's + `dind-var-lib-docker-${devcontainerId}` mount was recorded as + `dind-var-lib-docker-`. Any later read of the label (reattach, or the + image used as a base) lost the per-workspace volume name. The label is + now written literally, so a feature image whose metadata contains `$` gets + a new digest when it is next built. +- **features** — OCI feature lookups are local-first where the answer + cannot change: a digest reference, or an exact `X.Y.Z` version tag + resolved before, is served from the disk cache without contacting the + registry. Floating tags (`2`, `2.17`, `latest`) are still resolved + online; when the registry is unreachable (no HTTP response), the last + recorded resolution is used if cached. A registry that answers with an + error is authoritative. Extraction is now atomic (temporary directory, + then rename). Policy: `design/features.md` §8.1. + +### Added + +- `LabelComposeInputsHash` (`dev.containers.compose-inputs-hash`), stamped + on the primary container of a compose-source devcontainer. +- `runtime.ContainerDetails.ImageRef`: the image reference a container was + created from (docker's `Config.Image`). +- `feature.DiskStoreOptions.OCITransport`: the HTTP transport for OCI + registry calls. + ## [0.5.1] - 2026-10-27 ### Fixed @@ -613,7 +667,8 @@ shelling out to `@devcontainers/cli`. - `events` is doc-tagged **experimental** until v1.0.0 — type shapes may evolve without a SemVer-major bump. -[Unreleased]: https://github.com/crunchloop/devcontainer/compare/v0.5.1...HEAD +[Unreleased]: https://github.com/crunchloop/devcontainer/compare/v0.5.2...HEAD +[0.5.2]: https://github.com/crunchloop/devcontainer/compare/v0.5.1...v0.5.2 [0.5.1]: https://github.com/crunchloop/devcontainer/compare/v0.5.0...v0.5.1 [0.5.0]: https://github.com/crunchloop/devcontainer/compare/v0.4.3...v0.5.0 [0.4.3]: https://github.com/crunchloop/devcontainer/compare/v0.4.2...v0.4.3 diff --git a/README.md b/README.md index 54dbbf8..1d7846a 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,7 @@ field/behavior the library covers. Legend: ✅ acted on · ⚠️ parsed but not | --- | --- | --- | | `image` | ✅ | Pull, run, exec | | `build` (`dockerfile`, `context`, `args`, `target`, `cacheFrom`) | ✅ | User Dockerfile + features layered atop | -| `dockerComposeFile`, `service`, `runServices` | ✅ | compose-go parse + either shell-out to `docker compose` (default) or an in-process orchestrator (`EngineOptions.ComposeBackend = ComposeBackendNative`). The native orchestrator drives any `Runtime` implementing the compose primitives. | +| `dockerComposeFile`, `service`, `runServices` | ✅ | compose-go parse + either shell-out to `docker compose` (default) or an in-process orchestrator (`EngineOptions.ComposeBackend = ComposeBackendNative`). The native orchestrator drives any `Runtime` implementing the compose primitives. An `Up` whose containers exist from unchanged inputs restarts them from their existing images, with the feature metadata read from the image label: no feature fetch, no rebuild. | **Container config** @@ -69,7 +69,7 @@ field/behavior the library covers. Legend: ✅ acted on · ⚠️ parsed but not | Field | Status | Notes | | --- | --- | --- | -| `features` (OCI / HTTPS / local) | ✅ | DAG ordering, options validation, content-addressed cache | +| `features` (OCI / HTTPS / local) | ✅ | DAG ordering, options validation, content-addressed cache (local-first for digests and exact `X.Y.Z` versions; floating tags fall back to the last resolution when the registry is unreachable) | | `dependsOn` / `installsAfter` / `overrideFeatureInstallOrder` | ✅ | | | Pre-baked-image short-circuit | ✅ | `devcontainer.metadata` label read on Up | | `devcontainer-lock.json` | ❌ | [#26](https://github.com/crunchloop/devcontainer/issues/26) | diff --git a/design/compose-native.md b/design/compose-native.md index df103b0..c334673 100644 --- a/design/compose-native.md +++ b/design/compose-native.md @@ -399,6 +399,59 @@ Input: DownPlan { ProjectName string, RemoveImages bool, RemoveVolumes bool } service that didn't become healthy. The dependent services are not started. +### 5.4 Engine-level reattach (both backends) + +The orchestrator's config-hash reuse (§5.1) decides per container, but +it runs after the engine has already prepared the primary image: before +v0.5.2 every compose `Up` fetched every feature from its registry and +rebuilt `dc-go-final-`, even when the project's containers already +existed. A restart therefore needed the feature registry, and failed +without it. The image path never had this cost: `attachExisting` reads +the merged metadata from the existing container's image label. + +`Engine.Up` now does the same for compose sources (`reattachCompose`): + +1. The full path stamps `dev.containers.compose-inputs-hash` on the + primary container. The hash covers the inputs fixed before any + feature is fetched: the container-shaping part of the resolved + `devcontainer.json` (workspace and mounts, users, + `updateRemoteUserUID`, `containerEnv`, `runArgs`, init, privileged, + capAdd, securityOpt, overrideCommand, and the features as requested + with their options, plus a content digest of each local feature), + the loaded compose project (services, networks, volumes, secrets, + configs), the local image ID of the primary service's base image, + the engine options that reshape the project + (`ComposeBackend`, `DisableHostPortPublishing`, + `StrictFeatureVersionMatch`) and the workspace folder's owner. + Lifecycle commands, `remoteEnv`, ports attributes and + customizations are left out: `Up` re-reads them every call, and they + never reach the container or its image. +2. On a later `Up` with an existing container (and neither `Recreate`, + `PullAlways` nor an `Engine.Build` override), the engine finds the + primary container by project, service and devcontainer id, and + reattaches when its label equals the freshly computed hash and the + image tag it was created from still names the image it runs. +3. Reattach parses the image's `devcontainer.metadata` label, merges it + into the resolved config exactly as `attachExisting` does, rebuilds + the run override (entrypoint wrapper, every mount type, env, + privileged, capAdd, …) and brings the project up through the + selected backend against the existing images: native under the + caller's `AdoptExisting`, skipping sidecar builds whose image is + present; shell-out with `up -d --no-recreate`. Because the image is + the same, the orchestrator's per-service config hash matches too, so + a native `Up` without `AdoptExisting` reuses the containers instead + of recreating them. +4. Any other case — no container, no label (containers created before + v0.5.2), a different hash, a moved tag — takes the full path, + unchanged. `AdoptExisting` keeps its meaning: it governs whether the + orchestrator may reuse a container whose config drifted; reattach + only removes the fetch and rebuild in front of it. + +Like the reference CLI starting an existing container, reattach does +not re-resolve floating feature tags or look at the build context of a +`build:` primary (only its definition and the base image ID); a caller +that wants either picked up passes `Recreate`. + ## 6. Mapping to `runtime.Runtime` methods Same shape as `runtime-applecontainer.md` §8. @@ -1126,6 +1179,15 @@ Resolved during this design draft (2026-05-14): (upstream `#286`), the capability flag flips to true and the warning self-suppresses. +14. **Compose `Up` reattaches without fetching features + (2026-10-28).** §5.4. An existing project whose stored inputs hash + matches is restarted from its existing images with the metadata + read from the primary image's label, as the image path's + `attachExisting` does. Motivation: a restart of a compose workspace + with features needed the feature registry (and paid a final-image + rebuild) on every start; a workspace without registry egress could + not restart at all. + --- ## Appendix A: relationship to `compose.md` diff --git a/design/features.md b/design/features.md index 9d2d496..a249e9d 100644 --- a/design/features.md +++ b/design/features.md @@ -314,14 +314,35 @@ features/ sha256-/ blob.tgz extracted/... - index.json # ref → digest map with TTL hints + tags/ + .json # {"ref": "", "digest": "sha256:…"} ``` Cache key is **content-addressed**: OCI digest or HTTPS body sha256. A -mutable tag (`:1`) resolves through `index.json` to the digest, then -hits the disk cache. This is intentionally different from devpod's -"hash-the-ref-string" scheme — content-addressed dedupes across mirror -URLs and prevents stale-ref bugs. +tag resolves to a digest, then hits the disk cache. This is +intentionally different from devpod's "hash-the-ref-string" scheme — +content-addressed dedupes across mirror URLs and prevents stale-ref +bugs. Extraction goes to a temporary directory in the entry and is +renamed into place, so a partial extraction is never taken for a hit. + +### 8.1 OCI resolution policy (v0.5.2) + +The cache is only useful offline if the registry is not asked first. +`DiskStore` decides per reference form: + +| Reference | Policy | +| --- | --- | +| `name@sha256:…` | Local first: a cached extraction of the digest is used with no registry call. | +| `name:X.Y.Z` (exact version) | Local first: the digest recorded under `tags/` the last time this tag was resolved is used with no registry call, if its extraction is cached. Published versions are immutable by the publishing convention (the reference CLI publishes `X.Y.Z` once and moves only `X`, `X.Y` and `latest`). | +| `name:X`, `name:X.Y`, `name:latest`, any other tag | Online first, so a moved tag is picked up. If the registry is **unreachable** (no HTTP response: DNS, connect, TLS, timeout), the recorded resolution is used if its extraction is cached. | + +A registry that answers with an error (404, 401, 403, …) is +authoritative: its error is returned even when a resolution is +recorded. With nothing recorded or the content evicted, an unreachable +registry fails the fetch as before. Every successful online fetch of a +tag rewrites its record (write to a temporary file, then rename). HTTPS +features are unchanged: their content hash is only known after the +download. Concurrency: a small lockfile (`extracted/.lock`) per cache entry so parallel `Resolve` calls for the same ref don't race. diff --git a/feature/disk_store.go b/feature/disk_store.go index fe4abfc..f0396d8 100644 --- a/feature/disk_store.go +++ b/feature/disk_store.go @@ -29,6 +29,12 @@ type DiskStoreOptions struct { // fetches. Useful for Authorization on private servers. HTTPSHeaders map[string]string + // OCITransport overrides the HTTP transport for OCI registry + // calls. Nil uses go-containerregistry's default (which honors the + // standard proxy environment variables). Tests use it to make the + // registry unreachable; callers may use it for a custom proxy. + OCITransport http.RoundTripper + // HTTPSClient overrides the default *http.Client used for HTTPS // fetches. If nil, the package's default client (5-minute timeout) // is used. Tests substitute this to drive httptest servers. @@ -41,6 +47,7 @@ type DiskStoreOptions struct { type DiskStore struct { cacheDir string ociKeychain authn.Keychain + ociTransport http.RoundTripper httpsHeaders map[string]string httpsClient *http.Client } @@ -66,6 +73,7 @@ func NewDiskStore(opts DiskStoreOptions) (*DiskStore, error) { return &DiskStore{ cacheDir: dir, ociKeychain: kc, + ociTransport: opts.OCITransport, httpsHeaders: opts.HTTPSHeaders, httpsClient: opts.HTTPSClient, }, nil diff --git a/feature/dockerfile.go b/feature/dockerfile.go index 8b26940..0806c67 100644 --- a/feature/dockerfile.go +++ b/feature/dockerfile.go @@ -160,8 +160,7 @@ func generateDockerfile(plan BuildPlan) (string, error) { if err != nil { return "", fmt.Errorf("build metadata label: %w", err) } - // Wrap the JSON in strconv-quoted form so embedded "/'/$ are unambiguous. - fmt.Fprintf(&b, "LABEL %s=%s\n\n", MetadataLabel, strconv.Quote(string(label))) + fmt.Fprintf(&b, "LABEL %s=%s\n\n", MetadataLabel, dockerfileLiteral(string(label))) user := "root" if plan.RemoteUser != "" { @@ -239,3 +238,16 @@ func copyFileContents(src, dst string, mode os.FileMode) error { } return out.Close() } + +// dockerfileLiteral quotes s as a double-quoted Dockerfile word that +// evaluates to s exactly. strconv.Quote handles quotes and +// backslashes, but the Dockerfile parser still expands $VAR and +// ${VAR} inside double quotes, so every $ is escaped as well. Without +// it a metadata label carrying a feature's +// "dind-var-lib-docker-${devcontainerId}" mount was stored as +// "dind-var-lib-docker-" (the build has no such variable), and any +// later read of the label — a reattach, or this image as the base of +// another build — lost the per-workspace volume name. +func dockerfileLiteral(s string) string { + return strings.ReplaceAll(strconv.Quote(s), "$", `\$`) +} diff --git a/feature/dockerfile_test.go b/feature/dockerfile_test.go index 28ca8d6..6f12f13 100644 --- a/feature/dockerfile_test.go +++ b/feature/dockerfile_test.go @@ -301,3 +301,52 @@ func TestParseMetadataLabel_Empty(t *testing.T) { t.Errorf("got %v, want nil", got) } } + +// The metadata LABEL must carry ${...} literally: the Dockerfile parser +// expands variables inside double quotes, so an unescaped +// "dind-var-lib-docker-${devcontainerId}" mount source was stored as +// "dind-var-lib-docker-" and every later read of the label lost the +// per-workspace volume name. +func TestGenerateBuildContext_LabelKeepsDollarLiteral(t *testing.T) { + f := fixtureFeature(t, "dind", "2.0") + f.Metadata.Mounts = []config.Mount{{ + Type: config.MountVolume, + Source: "dind-var-lib-docker-${devcontainerId}", + Target: "/var/lib/docker", + }} + dst := t.TempDir() + if err := GenerateBuildContext(BuildPlan{BaseImage: "alpine:3.20", Features: []config.ResolvedFeature{f}}, dst); err != nil { + t.Fatalf("GenerateBuildContext: %v", err) + } + df, err := os.ReadFile(filepath.Join(dst, "Dockerfile")) + if err != nil { + t.Fatal(err) + } + var label string + for _, line := range strings.Split(string(df), "\n") { + if strings.HasPrefix(line, "LABEL "+MetadataLabel+"=") { + label = line + } + } + if label == "" { + t.Fatalf("no metadata LABEL line in:\n%s", df) + } + if strings.Count(label, "$") != strings.Count(label, `\$`) { + t.Errorf("LABEL has an unescaped $: %s", label) + } + if !strings.Contains(label, `dind-var-lib-docker-\${devcontainerId}`) { + t.Errorf("LABEL lost the mount source variable: %s", label) + } +} + +func TestDockerfileLiteral(t *testing.T) { + for in, want := range map[string]string{ + `plain`: `"plain"`, + `a${b}c`: `"a\${b}c"`, + `$HOME and "q" \x`: `"\$HOME and \"q\" \\x"`, + } { + if got := dockerfileLiteral(in); got != want { + t.Errorf("dockerfileLiteral(%q) = %s, want %s", in, got, want) + } + } +} diff --git a/feature/oci.go b/feature/oci.go index 05ffaee..f669a03 100644 --- a/feature/oci.go +++ b/feature/oci.go @@ -2,30 +2,76 @@ package feature import ( "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" "fmt" "os" "path/filepath" + "regexp" "strings" "github.com/google/go-containerregistry/pkg/name" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/google/go-containerregistry/pkg/v1/remote/transport" ) // devContainerFeatureMediaType is the OCI artifact config media type // used by the devcontainers spec for feature publication. const devContainerFeatureMediaType = "application/vnd.devcontainers" +// OCI cache policy. A feature artifact is content-addressed, so a +// cached copy is exactly as good as a download of the same digest; the +// only question is when the registry must be asked which digest a +// reference means. DiskStore answers it like this: +// +// - Digest reference (name@sha256:…): local first. A cached copy of +// that digest is used without contacting the registry. +// - Exact version tag (name:X.Y.Z): local first. Published feature +// versions are immutable by the spec's publishing convention (the +// reference CLI publishes X.Y.Z once and moves only the floating +// X, X.Y and latest tags), so the digest recorded the last time +// this tag was resolved is used without contacting the registry. +// - Floating tag (name:X, name:X.Y, name:latest, any other tag): +// online first, as before, so a moved tag is picked up. If the +// registry cannot be reached at all — no HTTP response: DNS, +// connect, TLS or timeout failures — the digest recorded the last +// time this tag was resolved is used. A registry that answers with +// an error (not found, unauthorized, …) is authoritative and its +// error is returned, as is any failure when nothing is recorded. +// +// Tag resolutions are recorded under /oci/tags/ after every +// successful online fetch. The recorded digest is only used when its +// extracted content is still in the cache. func (s *DiskStore) fetchOCI(ctx context.Context, ref string) (Fetched, error) { parsed, err := name.ParseReference(ref) if err != nil { return Fetched{}, fmt.Errorf("parse OCI ref %q: %w", ref, err) } + repo := parsed.Context().Name() - img, err := remote.Image(parsed, - remote.WithContext(ctx), - remote.WithAuthFromKeychain(s.ociKeychain), - ) + switch r := parsed.(type) { + case name.Digest: + if f, ok := s.cachedOCI(repo, r.DigestStr()); ok { + return f, nil + } + case name.Tag: + if isExactVersionTag(r.TagStr()) { + if f, ok := s.cachedTag(r); ok { + return f, nil + } + } + } + + img, err := remote.Image(parsed, s.remoteOptions(ctx)...) if err != nil { + if tag, isTag := parsed.(name.Tag); isTag && ctx.Err() == nil && registryUnreachable(err) { + if f, ok := s.cachedTag(tag); ok { + return f, nil + } + } return Fetched{}, fmt.Errorf("fetch OCI image %s: %w", ref, err) } @@ -41,20 +87,137 @@ func (s *DiskStore) fetchOCI(ctx context.Context, ref string) (Fetched, error) { if err != nil { return Fetched{}, fmt.Errorf("OCI digest: %w", err) } - pinned := parsed.Context().Name() + "@" + digest.String() + f, err := s.extractOCI(ref, repo, digest, img) + if err != nil { + return Fetched{}, err + } + if tag, isTag := parsed.(name.Tag); isTag { + // Best effort: a failed record only costs a later offline or + // local-first lookup, never this fetch. + _ = s.recordTag(tag, digest.String()) + } + return f, nil +} + +// remoteOptions are the go-containerregistry options for every OCI +// registry call. +func (s *DiskStore) remoteOptions(ctx context.Context) []remote.Option { + opts := []remote.Option{ + remote.WithContext(ctx), + remote.WithAuthFromKeychain(s.ociKeychain), + } + if s.ociTransport != nil { + opts = append(opts, remote.WithTransport(s.ociTransport)) + } + return opts +} + +// exactVersionTag matches a full semantic version with no pre-release +// or build suffix: the only tag form DiskStore treats as immutable. +var exactVersionTag = regexp.MustCompile(`^[0-9]+\.[0-9]+\.[0-9]+$`) + +func isExactVersionTag(tag string) bool { return exactVersionTag.MatchString(tag) } + +// registryUnreachable reports whether err means no HTTP response came +// back from the registry. A *transport.Error carries a response status +// (the registry answered), so it is not an offline condition. +func registryUnreachable(err error) bool { + var te *transport.Error + return !errors.As(err, &te) +} + +// ociEntryDir is the cache entry for one feature digest: +// /oci/sha256-. +func (s *DiskStore) ociEntryDir(hex string) string { + return filepath.Join(s.cacheDir, "oci", "sha256-"+hex) +} + +// cachedOCI returns the cached, extracted feature for repo@digest, if +// the cache holds a complete extraction of it. +func (s *DiskStore) cachedOCI(repo, digest string) (Fetched, bool) { + h, err := v1.NewHash(digest) + if err != nil || h.Algorithm != "sha256" { + return Fetched{}, false + } + extractedDir := filepath.Join(s.ociEntryDir(h.Hex), "extracted") + if _, err := os.Stat(filepath.Join(extractedDir, installScript)); err != nil { + return Fetched{}, false + } + meta, err := parseMetadata(extractedDir) + if err != nil { + return Fetched{}, false + } + return Fetched{Dir: extractedDir, ResolvedRef: repo + "@" + h.String(), Metadata: meta}, true +} + +// tagRecord is the on-disk record of one tag resolution. +type tagRecord struct { + Ref string `json:"ref"` + Digest string `json:"digest"` +} + +func (s *DiskStore) tagRecordPath(tag name.Tag) string { + sum := sha256.Sum256([]byte(tag.Name())) + return filepath.Join(s.cacheDir, "oci", "tags", hex.EncodeToString(sum[:])+".json") +} + +// cachedTag returns the cached feature for the digest tag last +// resolved to, if both the record and its content are present. +func (s *DiskStore) cachedTag(tag name.Tag) (Fetched, bool) { + b, err := os.ReadFile(s.tagRecordPath(tag)) + if err != nil { + return Fetched{}, false + } + var rec tagRecord + if err := json.Unmarshal(b, &rec); err != nil || rec.Ref != tag.Name() { + return Fetched{}, false + } + return s.cachedOCI(tag.Context().Name(), rec.Digest) +} + +// recordTag records that tag resolved to digest, atomically (write a +// temporary file, then rename over the record). +func (s *DiskStore) recordTag(tag name.Tag, digest string) error { + path := s.tagRecordPath(tag) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + return err + } + b, err := json.Marshal(tagRecord{Ref: tag.Name(), Digest: digest}) + if err != nil { + return err + } + tmp, err := os.CreateTemp(filepath.Dir(path), ".tag-*") + if err != nil { + return err + } + if _, err := tmp.Write(b); err != nil { + tmp.Close() + os.Remove(tmp.Name()) + return err + } + if err := tmp.Close(); err != nil { + os.Remove(tmp.Name()) + return err + } + if err := os.Rename(tmp.Name(), path); err != nil { + os.Remove(tmp.Name()) + return err + } + return nil +} + +// extractOCI returns the cached extraction of digest, or downloads and +// extracts img's single layer into the cache. +func (s *DiskStore) extractOCI(ref, repo string, digest v1.Hash, img v1.Image) (Fetched, error) { + if f, ok := s.cachedOCI(repo, digest.String()); ok { + return f, nil + } + pinned := repo + "@" + digest.String() // Cache layout: /oci/sha256-/extracted/ entryDir := filepath.Join(s.cacheDir, "oci", "sha256-"+digest.Hex) extractedDir := filepath.Join(entryDir, "extracted") - // Cache hit: extracted dir already has the metadata file. - if _, err := os.Stat(filepath.Join(extractedDir, metadataFile)); err == nil { - meta, err := parseMetadata(extractedDir) - if err == nil { - return Fetched{Dir: extractedDir, ResolvedRef: pinned, Metadata: meta}, nil - } - } - if err := os.MkdirAll(entryDir, 0o755); err != nil { return Fetched{}, fmt.Errorf("mkdir %s: %w", entryDir, err) } @@ -77,15 +240,29 @@ func (s *DiskStore) fetchOCI(ctx context.Context, ref string) (Fetched, error) { } defer rc.Close() - if err := os.RemoveAll(extractedDir); err != nil { - return Fetched{}, err + // Extract beside the entry and rename into place, so the cache + // never holds a partial extraction that the local-first lookups + // above would take for a complete one. + tmpDir, err := os.MkdirTemp(entryDir, ".extract-*") + if err != nil { + return Fetched{}, fmt.Errorf("mkdir extract tmpdir: %w", err) } - if err := extractTar(rc, extractedDir); err != nil { + defer os.RemoveAll(tmpDir) + if err := extractTar(rc, tmpDir); err != nil { return Fetched{}, fmt.Errorf("extract OCI feature %s: %w", ref, err) } - if _, err := os.Stat(filepath.Join(extractedDir, installScript)); err != nil { + if _, err := os.Stat(filepath.Join(tmpDir, installScript)); err != nil { return Fetched{}, fmt.Errorf("OCI feature %s: missing %s after extract: %w", ref, installScript, err) } + if _, err := parseMetadata(tmpDir); err != nil { + return Fetched{}, err + } + if err := os.RemoveAll(extractedDir); err != nil { + return Fetched{}, err + } + if err := os.Rename(tmpDir, extractedDir); err != nil { + return Fetched{}, fmt.Errorf("install extracted feature %s: %w", ref, err) + } meta, err := parseMetadata(extractedDir) if err != nil { return Fetched{}, err diff --git a/feature/oci_cache_test.go b/feature/oci_cache_test.go new file mode 100644 index 0000000..96b7d10 --- /dev/null +++ b/feature/oci_cache_test.go @@ -0,0 +1,288 @@ +package feature + +import ( + "archive/tar" + "bytes" + "context" + "errors" + "io" + "log" + "net" + "net/http" + "net/http/httptest" + "os" + "strings" + "sync/atomic" + "testing" + + "github.com/google/go-containerregistry/pkg/authn" + "github.com/google/go-containerregistry/pkg/name" + "github.com/google/go-containerregistry/pkg/registry" + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/empty" + "github.com/google/go-containerregistry/pkg/v1/mutate" + "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/google/go-containerregistry/pkg/v1/static" + "github.com/google/go-containerregistry/pkg/v1/types" + + "github.com/crunchloop/devcontainer/config" +) + +// Tests for the OCI cache policy documented on fetchOCI: digest refs +// and exact version tags are local-first, floating tags are online +// first with an offline fallback to the recorded resolution, and a +// registry that answers with an error is authoritative. + +// registryMode selects what gatedTransport does with a request. +type registryMode int32 + +const ( + modeOnline registryMode = iota // pass through + modeUnreachable // fail like a refused connection + modeDenied // answer 403 for everything +) + +// gatedTransport counts registry calls and can make the registry +// unreachable or have it refuse every request. +type gatedTransport struct { + mode atomic.Int32 + calls atomic.Int64 +} + +func (g *gatedTransport) RoundTrip(req *http.Request) (*http.Response, error) { + g.calls.Add(1) + switch registryMode(g.mode.Load()) { + case modeUnreachable: + return nil, &net.OpError{Op: "dial", Net: "tcp", Err: errors.New("connect: connection refused (test)")} + case modeDenied: + return &http.Response{ + StatusCode: http.StatusForbidden, + Status: "403 Forbidden", + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: io.NopCloser(strings.NewReader(`{"errors":[{"code":"DENIED","message":"denied (test)"}]}`)), + Request: req, + }, nil + } + return http.DefaultTransport.RoundTrip(req) +} + +type cacheFixture struct { + host string + gate *gatedTransport + store *DiskStore +} + +func newCacheFixture(t *testing.T) *cacheFixture { + t.Helper() + srv := httptest.NewServer(registry.New(registry.Logger(log.New(io.Discard, "", 0)))) + t.Cleanup(srv.Close) + gate := &gatedTransport{} + store, err := NewDiskStore(DiskStoreOptions{ + CacheDir: t.TempDir(), + OCIKeychain: authn.NewMultiKeychain(), + OCITransport: gate, + }) + if err != nil { + t.Fatalf("NewDiskStore: %v", err) + } + return &cacheFixture{host: strings.TrimPrefix(srv.URL, "http://"), gate: gate, store: store} +} + +func (c *cacheFixture) ref(tag string) string { return c.host + "/features/demo:" + tag } + +// push publishes a feature artifact whose install.sh echoes marker +// under tag, and returns its manifest digest. +func (c *cacheFixture) push(t *testing.T, tag, marker string) v1.Hash { + t.Helper() + var buf bytes.Buffer + tw := tar.NewWriter(&buf) + for _, f := range []struct{ name, body string }{ + {"devcontainer-feature.json", `{"id":"demo","version":"1.2.3"}`}, + {"install.sh", "#!/bin/sh\necho " + marker + "\n"}, + } { + if err := tw.WriteHeader(&tar.Header{Name: f.name, Mode: 0o755, Size: int64(len(f.body)), Typeflag: tar.TypeReg}); err != nil { + t.Fatal(err) + } + if _, err := tw.Write([]byte(f.body)); err != nil { + t.Fatal(err) + } + } + if err := tw.Close(); err != nil { + t.Fatal(err) + } + img, err := mutate.AppendLayers( + mutate.ConfigMediaType(empty.Image, types.MediaType(devContainerFeatureMediaType)), + static.NewLayer(buf.Bytes(), "application/vnd.devcontainers.layer.v1+tar")) + if err != nil { + t.Fatal(err) + } + ref, err := name.ParseReference(c.ref(tag)) + if err != nil { + t.Fatal(err) + } + if err := remote.Write(ref, img, remote.WithTransport(http.DefaultTransport)); err != nil { + t.Fatalf("push %s: %v", ref, err) + } + d, err := img.Digest() + if err != nil { + t.Fatal(err) + } + return d +} + +func (c *cacheFixture) fetch(t *testing.T, ref string) (Fetched, error) { + t.Helper() + return c.store.Fetch(context.Background(), ref, config.FeatureSourceOCI) +} + +func (c *cacheFixture) mustFetch(t *testing.T, ref string) Fetched { + t.Helper() + f, err := c.fetch(t, ref) + if err != nil { + t.Fatalf("Fetch %s: %v", ref, err) + } + return f +} + +func (c *cacheFixture) set(m registryMode) { + c.gate.mode.Store(int32(m)) + c.gate.calls.Store(0) +} + +func wantDigest(t *testing.T, f Fetched, d v1.Hash) { + t.Helper() + if !strings.HasSuffix(f.ResolvedRef, "@"+d.String()) { + t.Fatalf("ResolvedRef = %s, want digest %s", f.ResolvedRef, d) + } +} + +func TestOCICache_DigestRefIsLocalFirst(t *testing.T) { + c := newCacheFixture(t) + d := c.push(t, "1.2.3", "v1") + c.mustFetch(t, c.ref("1.2.3")) + + c.set(modeUnreachable) + f := c.mustFetch(t, c.host+"/features/demo@"+d.String()) + wantDigest(t, f, d) + if n := c.gate.calls.Load(); n != 0 { + t.Fatalf("digest ref with cached content made %d registry calls, want 0", n) + } +} + +func TestOCICache_DigestRefNotCachedGoesOnline(t *testing.T) { + c := newCacheFixture(t) + d := c.push(t, "1.2.3", "v1") + f := c.mustFetch(t, c.host+"/features/demo@"+d.String()) + wantDigest(t, f, d) + if c.gate.calls.Load() == 0 { + t.Fatal("uncached digest ref made no registry call") + } +} + +func TestOCICache_ExactVersionTagIsLocalFirst(t *testing.T) { + c := newCacheFixture(t) + d := c.push(t, "1.2.3", "v1") + wantDigest(t, c.mustFetch(t, c.ref("1.2.3")), d) + + // Online, but the recorded resolution of an exact version is used + // without asking the registry. + c.set(modeOnline) + wantDigest(t, c.mustFetch(t, c.ref("1.2.3")), d) + if n := c.gate.calls.Load(); n != 0 { + t.Fatalf("cached exact version made %d registry calls, want 0", n) + } + + c.set(modeUnreachable) + wantDigest(t, c.mustFetch(t, c.ref("1.2.3")), d) +} + +func TestOCICache_FloatingTagIsOnlineFirst(t *testing.T) { + c := newCacheFixture(t) + d1 := c.push(t, "1", "v1") + wantDigest(t, c.mustFetch(t, c.ref("1")), d1) + + d2 := c.push(t, "1", "v2") + if d1 == d2 { + t.Fatal("test artifacts share a digest") + } + c.set(modeOnline) + wantDigest(t, c.mustFetch(t, c.ref("1")), d2) + if c.gate.calls.Load() == 0 { + t.Fatal("floating tag resolved without asking the registry") + } +} + +func TestOCICache_FloatingTagOfflineUsesRecordedResolution(t *testing.T) { + c := newCacheFixture(t) + d := c.push(t, "latest", "v1") + wantDigest(t, c.mustFetch(t, c.ref("latest")), d) + + c.set(modeUnreachable) + f := c.mustFetch(t, c.ref("latest")) + wantDigest(t, f, d) + if f.Metadata.ID != "demo" { + t.Fatalf("Metadata.ID = %q, want demo", f.Metadata.ID) + } +} + +func TestOCICache_OfflineWithoutRecordFails(t *testing.T) { + c := newCacheFixture(t) + c.push(t, "1", "v1") + c.push(t, "1.2.3", "v1") + + c.set(modeUnreachable) + for _, tag := range []string{"1", "1.2.3"} { + if _, err := c.fetch(t, c.ref(tag)); err == nil { + t.Fatalf("offline fetch of never-resolved %s succeeded", tag) + } + } +} + +func TestOCICache_RegistryErrorIsAuthoritative(t *testing.T) { + c := newCacheFixture(t) + c.push(t, "1", "v1") + c.mustFetch(t, c.ref("1")) + + // The registry answers, with a refusal: that is not an offline + // condition, so the recorded resolution must not paper over it. + c.set(modeDenied) + if _, err := c.fetch(t, c.ref("1")); err == nil { + t.Fatal("fetch succeeded although the registry refused it") + } +} + +func TestOCICache_RecordNeedsContent(t *testing.T) { + c := newCacheFixture(t) + d := c.push(t, "1.2.3", "v1") + f := c.mustFetch(t, c.ref("1.2.3")) + wantDigest(t, f, d) + + // Evict the content but keep the tag record: the local-first path + // must fall through to the registry instead of returning a + // dangling directory. + if err := os.RemoveAll(c.store.ociEntryDir(d.Hex)); err != nil { + t.Fatal(err) + } + c.set(modeOnline) + wantDigest(t, c.mustFetch(t, c.ref("1.2.3")), d) + if c.gate.calls.Load() == 0 { + t.Fatal("evicted content was not re-fetched") + } +} + +func TestIsExactVersionTag(t *testing.T) { + for tag, want := range map[string]bool{ + "2.17.0": true, + "10.0.12": true, + "2": false, + "2.17": false, + "latest": false, + "2.17.0-rc.1": false, + "v2.17.0": false, + "dev": false, + } { + if got := isExactVersionTag(tag); got != want { + t.Errorf("isExactVersionTag(%q) = %v, want %v", tag, got, want) + } + } +} diff --git a/runtime/docker/inspect.go b/runtime/docker/inspect.go index d05ce7c..6afcc76 100644 --- a/runtime/docker/inspect.go +++ b/runtime/docker/inspect.go @@ -62,6 +62,7 @@ func (r *Runtime) InspectContainer(ctx context.Context, id string) (*runtime.Con out.User = c.Config.User out.Env = append([]string(nil), c.Config.Env...) out.Labels = copyLabels(c.Config.Labels) + out.ImageRef = c.Config.Image } if c.HostConfig != nil { out.Privileged = c.HostConfig.Privileged diff --git a/runtime/runtime.go b/runtime/runtime.go index 4f07e9e..cea9c02 100644 --- a/runtime/runtime.go +++ b/runtime/runtime.go @@ -230,6 +230,14 @@ type ContainerDetails struct { Mounts []MountInspect Labels map[string]string + // ImageRef is the image reference the container was created from + // (docker's Config.Image), e.g. "dc-go-final-:latest", as + // opposed to Container.Image, which inspect reports as the resolved + // image ID. Empty when the backend does not surface it. The compose + // reattach path uses it to rebuild the service's image: field + // exactly as it was when the container was created. + ImageRef string + // Security options as actually applied to the container, read back // from the backend's inspect (docker HostConfig). Lets callers // verify that RunSpec.Privileged/CapAdd/SecurityOpt — including the diff --git a/test/integration/compose_dind_test.go b/test/integration/compose_dind_test.go index 71d764f..e185d68 100644 --- a/test/integration/compose_dind_test.go +++ b/test/integration/compose_dind_test.go @@ -11,6 +11,7 @@ import ( "time" devcontainer "github.com/crunchloop/devcontainer" + "github.com/crunchloop/devcontainer/runtime" ) // dindFeatureRef is the upstream docker-in-docker feature. Its metadata @@ -139,3 +140,135 @@ services: }) } } + +// waitInnerDockerd waits for the docker-in-docker feature's dockerd. +func waitInnerDockerd(t *testing.T, ctx context.Context, eng *devcontainer.Engine, ws *devcontainer.Workspace) { + t.Helper() + deadline := time.Now().Add(2 * time.Minute) + for { + res, err := eng.Exec(ctx, ws, devcontainer.ExecOptions{ + Cmd: []string{"docker", "info", "--format", "{{.Driver}}"}, + }) + if err == nil && res.ExitCode == 0 { + return + } + if time.Now().After(deadline) { + t.Fatalf("inner dockerd never became ready: err=%v res=%+v", err, res) + } + time.Sleep(2 * time.Second) + } +} + +// TestComposeSource_DockerInDockerRestartsWithoutRegistry is the +// restart half of the docker-in-docker check: up, pull an image into +// the inner dockerd, stop every service, cut the feature registry +// off, up again. The second Up must reattach without fetching the +// feature, the /var/lib/docker volume must still be mounted, and +// `docker run hello-world` must work from the image the first boot +// pulled onto that volume. Before compose reattach, the second Up +// re-fetched docker-in-docker from ghcr.io and failed offline. +func TestComposeSource_DockerInDockerRestartsWithoutRegistry(t *testing.T) { + if testing.Short() { + t.Skip() + } + + for _, bk := range composeBackends { + t.Run(bk.name, func(t *testing.T) { + h := newReattachHarness(t, bk.backend) + dir := t.TempDir() + mustWrite(t, filepath.Join(dir, "docker-compose.yml"), ` +services: + app: + image: `+dindBaseImage+` + command: ["sh", "-c", "while sleep 1000; do :; done"] +`) + mustWrite(t, filepath.Join(dir, ".devcontainer", "devcontainer.json"), `{ + "dockerComposeFile": "../docker-compose.yml", + "service": "app", + "workspaceFolder": "/workspaces/proj", + "features": { "`+dindFeatureRef+`": {} } + }`) + + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Minute) + defer cancel() + + start := time.Now() + ws1, err := h.eng.Up(ctx, devcontainer.UpOptions{ + LocalWorkspaceFolder: dir, + Recreate: true, + SkipLifecycle: true, + }) + if err != nil { + t.Fatalf("first Up: %v", err) + } + t.Logf("first Up: %s, %d feature fetch(es), %d registry request(s)", + time.Since(start).Round(time.Millisecond), h.store.fetches.Load(), h.gate.calls.Load()) + cleanupComposeWorkspace(t, h.eng, ws1) + waitInnerDockerd(t, ctx, h.eng, ws1) + // Pull while the network is there; the second boot runs it + // from the volume. + if got := execOut(t, ctx, h.eng, ws1, "docker", "pull", "-q", "hello-world"); got == "" { + t.Fatal("docker pull hello-world printed nothing") + } + + project := ws1.Container.Labels["com.docker.compose.project"] + cs, err := h.rt.ListContainers(ctx, runtime.LabelFilter{Match: map[string]string{"com.docker.compose.project": project}}) + if err != nil { + t.Fatal(err) + } + for _, c := range cs { + if err := h.rt.StopContainer(ctx, c.ID, runtime.StopOptions{Timeout: 10 * time.Second}); err != nil { + t.Fatalf("stop %s: %v", c.Name, err) + } + } + h.block() + + start = time.Now() + ws2, err := h.eng.Up(ctx, devcontainer.UpOptions{ + LocalWorkspaceFolder: dir, + AdoptExisting: true, + SkipLifecycle: true, + }) + if err != nil { + t.Fatalf("second Up with the registry blocked: %v", err) + } + t.Logf("second Up (registry blocked): %s, %d feature fetch(es), %d registry request(s)", + time.Since(start).Round(time.Millisecond), h.store.fetches.Load(), h.gate.calls.Load()) + if n := h.store.fetches.Load(); n != 0 { + t.Errorf("second Up fetched %d features, want 0", n) + } + if n := h.gate.calls.Load(); n != 0 { + t.Errorf("second Up made %d registry requests, want 0", n) + } + if ws2.Container.ID != ws1.Container.ID { + t.Errorf("second Up recreated the primary: %s -> %s", ws1.Container.ID, ws2.Container.ID) + } + var dockerMount bool + for _, m := range ws2.Container.Mounts { + if m.Target == "/var/lib/docker" { + dockerMount = m.Type == "volume" + } + } + if !dockerMount { + t.Fatalf("/var/lib/docker is not a volume after reattach: %+v", ws2.Container.Mounts) + } + if !ws2.Container.Privileged { + t.Errorf("primary not privileged after reattach") + } + + waitInnerDockerd(t, ctx, h.eng, ws2) + res, err := h.eng.Exec(ctx, ws2, devcontainer.ExecOptions{ + Cmd: []string{"docker", "run", "--rm", "--pull", "never", "hello-world"}, + }) + if err != nil { + t.Fatalf("Exec docker run: %v", err) + } + if res.ExitCode != 0 || !strings.Contains(res.Stdout, "Hello from Docker!") { + t.Fatalf("docker run hello-world after offline restart failed: exit=%d stdout=%q stderr=%q", + res.ExitCode, res.Stdout, res.Stderr) + } + t.Logf("docker run --pull never hello-world after the offline restart: %q", + strings.SplitN(strings.TrimSpace(res.Stdout), "\n", 2)[0]) + }) + } +} diff --git a/test/integration/compose_reattach_test.go b/test/integration/compose_reattach_test.go new file mode 100644 index 0000000..a3cae08 --- /dev/null +++ b/test/integration/compose_reattach_test.go @@ -0,0 +1,500 @@ +//go:build integration + +package integration + +import ( + "archive/tar" + "bytes" + "context" + "errors" + "io" + "log" + "net" + "net/http" + "net/http/httptest" + "os/exec" + "path/filepath" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/google/go-containerregistry/pkg/authn" + "github.com/google/go-containerregistry/pkg/name" + "github.com/google/go-containerregistry/pkg/registry" + "github.com/google/go-containerregistry/pkg/v1/empty" + "github.com/google/go-containerregistry/pkg/v1/mutate" + "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/google/go-containerregistry/pkg/v1/static" + "github.com/google/go-containerregistry/pkg/v1/types" + + devcontainer "github.com/crunchloop/devcontainer" + "github.com/crunchloop/devcontainer/config" + "github.com/crunchloop/devcontainer/events" + "github.com/crunchloop/devcontainer/feature" + "github.com/crunchloop/devcontainer/runtime" + "github.com/crunchloop/devcontainer/runtime/docker" +) + +// Compose reattach: an Up whose project containers already exist from +// identical inputs must restart them from the existing images, reading +// the feature metadata from the primary image's label — no feature +// fetch, no image build, so no registry. These tests serve a small +// feature from an in-process registry and cut the registry off before +// the second Up. + +// blockableTransport makes the feature registry unreachable on demand, +// failing the way a guest without egress does (a refused connection). +type blockableTransport struct { + blocked atomic.Bool + calls atomic.Int64 +} + +func (b *blockableTransport) RoundTrip(req *http.Request) (*http.Response, error) { + b.calls.Add(1) + if b.blocked.Load() { + return nil, &net.OpError{Op: "dial", Net: "tcp", Err: errors.New("connect: connection refused (registry blocked by test)")} + } + return http.DefaultTransport.RoundTrip(req) +} + +// countingStore counts Fetch calls on the wrapped store. +type countingStore struct { + inner feature.Store + fetches atomic.Int64 +} + +func (c *countingStore) Fetch(ctx context.Context, ref string, kind config.FeatureSourceKind) (feature.Fetched, error) { + c.fetches.Add(1) + return c.inner.Fetch(ctx, ref, kind) +} + +// reattachHarness is one engine with a blockable, counted feature +// store whose cache lives in the test's temp dir. +type reattachHarness struct { + eng *devcontainer.Engine + rt *docker.Runtime + gate *blockableTransport + store *countingStore +} + +func newReattachHarness(t *testing.T, backend devcontainer.ComposeBackend) *reattachHarness { + t.Helper() + gate := &blockableTransport{} + ds, err := feature.NewDiskStore(feature.DiskStoreOptions{ + CacheDir: t.TempDir(), + OCIKeychain: authn.NewMultiKeychain(authn.DefaultKeychain), + OCITransport: gate, + }) + if err != nil { + t.Fatalf("NewDiskStore: %v", err) + } + store := &countingStore{inner: ds} + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + rt, err := docker.New(ctx, docker.Options{}) + if err != nil { + t.Skipf("Docker daemon unavailable: %v", err) + } + t.Cleanup(func() { _ = rt.Close() }) + eng, err := devcontainer.New(devcontainer.EngineOptions{ + Runtime: rt, + FeatureStore: store, + ComposeBackend: backend, + }) + if err != nil { + t.Fatalf("New: %v", err) + } + return &reattachHarness{eng: eng, rt: rt, gate: gate, store: store} +} + +// block cuts the registry off and resets the counters. +func (h *reattachHarness) block() { + h.gate.blocked.Store(true) + h.reset() +} + +func (h *reattachHarness) reset() { + h.gate.calls.Store(0) + h.store.fetches.Store(0) +} + +// markerFeatureMetadata declares one of everything the compose +// run override carries from feature metadata: env, a capability, a +// project volume, an entrypoint and a lifecycle command. +const markerFeatureMetadata = `{ + "id": "marker", + "version": "1.0.0", + "options": {"flavor": {"type": "string", "default": "plain"}}, + "containerEnv": {"MARKER_FEATURE": "on"}, + "capAdd": ["SYS_PTRACE"], + "mounts": [{"source": "marker-data-${devcontainerId}", "target": "/marker-data", "type": "volume"}], + "entrypoint": "/usr/local/share/marker-entry.sh", + "postStartCommand": "echo poststart >> /marker-data/poststart.log" +}` + +const markerFeatureInstall = `#!/bin/sh +set -e +mkdir -p /usr/local/share +echo "$FLAVOR" > /usr/local/share/marker-flavor +cat > /usr/local/share/marker-entry.sh <<'EOF' +#!/bin/sh +mkdir -p /marker-data +echo entry >> /marker-data/entry.log +EOF +chmod +x /usr/local/share/marker-entry.sh +` + +// serveMarkerFeature starts an in-process registry holding the marker +// feature and returns its reference. +func serveMarkerFeature(t *testing.T) string { + t.Helper() + srv := httptest.NewServer(registry.New(registry.Logger(log.New(io.Discard, "", 0)))) + t.Cleanup(srv.Close) + + var buf bytes.Buffer + tw := tar.NewWriter(&buf) + for _, f := range []struct{ name, body string }{ + {"devcontainer-feature.json", markerFeatureMetadata}, + {"install.sh", markerFeatureInstall}, + } { + if err := tw.WriteHeader(&tar.Header{Name: f.name, Mode: 0o755, Size: int64(len(f.body)), Typeflag: tar.TypeReg}); err != nil { + t.Fatal(err) + } + if _, err := tw.Write([]byte(f.body)); err != nil { + t.Fatal(err) + } + } + if err := tw.Close(); err != nil { + t.Fatal(err) + } + img, err := mutate.AppendLayers( + mutate.ConfigMediaType(empty.Image, types.MediaType("application/vnd.devcontainers")), + static.NewLayer(buf.Bytes(), "application/vnd.devcontainers.layer.v1+tar")) + if err != nil { + t.Fatal(err) + } + ref := strings.TrimPrefix(srv.URL, "http://") + "/features/marker:1.0.0" + parsed, err := name.ParseReference(ref) + if err != nil { + t.Fatal(err) + } + if err := remote.Write(parsed, img); err != nil { + t.Fatalf("push marker feature: %v", err) + } + return ref +} + +// writeMarkerWorkspace writes a compose devcontainer with one app +// service and a sidecar, using the marker feature with flavor. +func writeMarkerWorkspace(t *testing.T, dir, featureRef, flavor string) { + t.Helper() + mustWrite(t, filepath.Join(dir, "docker-compose.yml"), ` +services: + app: + image: debian:bookworm-slim + command: ["sh", "-c", "while sleep 1000; do :; done"] + sidecar: + image: busybox:1.36 + command: ["sh", "-c", "while sleep 1000; do :; done"] +`) + mustWrite(t, filepath.Join(dir, ".devcontainer", "devcontainer.json"), `{ + "dockerComposeFile": "../docker-compose.yml", + "service": "app", + "workspaceFolder": "/workspaces/proj", + "features": { "`+featureRef+`": { "flavor": "`+flavor+`" } } + }`) +} + +// eventLog collects every event of one Up. +type eventLog struct { + ch chan events.Event + done chan struct{} + mu sync.Mutex + evs []events.Event +} + +func newEventLog() *eventLog { + l := &eventLog{ch: make(chan events.Event, 4096), done: make(chan struct{})} + go func() { + defer close(l.done) + for ev := range l.ch { + l.mu.Lock() + l.evs = append(l.evs, ev) + l.mu.Unlock() + } + }() + return l +} + +func (l *eventLog) close() []events.Event { + close(l.ch) + <-l.done + return l.evs +} + +func featureBuilds(evs []events.Event) int { + n := 0 + for _, ev := range evs { + if b, ok := ev.(events.BuildStartEvent); ok && b.Source == events.BuildSourceFeatures { + n++ + } + } + return n +} + +func reattachSkips(evs []events.Event) int { + n := 0 + for _, ev := range evs { + if s, ok := ev.(events.FeatureSkippedEvent); ok && s.Reason == "compose_reattach" { + n++ + } + } + return n +} + +// up runs one Up and returns the workspace and its events. +func (h *reattachHarness) up(t *testing.T, ctx context.Context, dir string, opts devcontainer.UpOptions) (*devcontainer.Workspace, []events.Event) { + t.Helper() + l := newEventLog() + opts.LocalWorkspaceFolder = dir + opts.Events = l.ch + ws, err := h.eng.Up(ctx, opts) + evs := l.close() + if err != nil { + t.Fatalf("Up: %v", err) + } + return ws, evs +} + +// stopProject stops every container of the workspace's project, as a +// host or VM stop leaves them. +func (h *reattachHarness) stopProject(t *testing.T, ctx context.Context, ws *devcontainer.Workspace) { + t.Helper() + project := ws.Container.Labels["com.docker.compose.project"] + cs, err := h.rt.ListContainers(ctx, runtime.LabelFilter{Match: map[string]string{"com.docker.compose.project": project}}) + if err != nil { + t.Fatalf("ListContainers: %v", err) + } + if len(cs) < 2 { + t.Fatalf("project %s has %d containers, want app and sidecar", project, len(cs)) + } + for _, c := range cs { + if err := h.rt.StopContainer(ctx, c.ID, runtime.StopOptions{Timeout: 5 * time.Second}); err != nil { + t.Fatalf("stop %s: %v", c.Name, err) + } + } +} + +// cleanupComposeWorkspace removes the project, its volumes and the +// images the engine tagged for it. +func cleanupComposeWorkspace(t *testing.T, eng *devcontainer.Engine, ws *devcontainer.Workspace) { + t.Helper() + t.Cleanup(func() { + ctx := context.Background() + project := ws.Container.Labels["com.docker.compose.project"] + _ = eng.Down(ctx, ws, devcontainer.DownOptions{Remove: true, RemoveVolumes: true}) + if out, err := exec.Command("docker", "ps", "-aq", "--filter", "label=com.docker.compose.project="+project).Output(); err == nil { + for _, id := range strings.Fields(string(out)) { + _ = exec.Command("docker", "rm", "-f", id).Run() + } + } + if out, err := exec.Command("docker", "volume", "ls", "-q", "--filter", "label=com.docker.compose.project="+project).Output(); err == nil { + for _, v := range strings.Fields(string(out)) { + _ = exec.Command("docker", "volume", "rm", "-f", v).Run() + } + } + _ = exec.Command("docker", "network", "rm", project+"_default").Run() + for _, prefix := range []string{"dc-go-final-", "dc-go-uid-", "dc-go-compose-base-"} { + _ = exec.Command("docker", "image", "rm", "-f", prefix+string(ws.ID)+":latest").Run() + } + }) +} + +func execOut(t *testing.T, ctx context.Context, eng *devcontainer.Engine, ws *devcontainer.Workspace, cmd ...string) string { + t.Helper() + res, err := eng.Exec(ctx, ws, devcontainer.ExecOptions{Cmd: cmd}) + if err != nil { + t.Fatalf("Exec %v: %v", cmd, err) + } + if res.ExitCode != 0 { + t.Fatalf("Exec %v: exit %d stdout=%q stderr=%q", cmd, res.ExitCode, res.Stdout, res.Stderr) + } + return strings.TrimSpace(res.Stdout) +} + +func lineCount(s string) int { + if s == "" { + return 0 + } + return len(strings.Split(s, "\n")) +} + +var composeBackends = []struct { + name string + backend devcontainer.ComposeBackend +}{ + {"native", devcontainer.ComposeBackendNative}, + {"shellout", devcontainer.ComposeBackendShellout}, +} + +// TestComposeReattach_RestartsWithoutRegistry: up, stop every +// service, cut the registry off, up again. The second Up must reattach +// — same containers, restarted, no fetch, no build — and still apply +// the feature's metadata from the image label: env, capability, +// volume, entrypoint and postStartCommand. +func TestComposeReattach_RestartsWithoutRegistry(t *testing.T) { + if testing.Short() { + t.Skip() + } + featureRef := serveMarkerFeature(t) + + for _, bk := range composeBackends { + for _, adopt := range []bool{false, true} { + sub := bk.name + if adopt { + sub += "/adopt" + } + t.Run(sub, func(t *testing.T) { + h := newReattachHarness(t, bk.backend) + ctx, cancel := context.WithTimeout(context.Background(), 8*time.Minute) + defer cancel() + dir := t.TempDir() + writeMarkerWorkspace(t, dir, featureRef, "plain") + + // First creation: unchanged — fetch, build, label. + ws1, evs1 := h.up(t, ctx, dir, devcontainer.UpOptions{Recreate: true}) + cleanupComposeWorkspace(t, h.eng, ws1) + if n := h.store.fetches.Load(); n != 1 { + t.Fatalf("first Up: %d feature fetches, want 1", n) + } + if featureBuilds(evs1) != 1 { + t.Fatalf("first Up: %d feature image builds, want 1", featureBuilds(evs1)) + } + if ws1.Container.Labels[devcontainer.LabelComposeInputsHash] == "" { + t.Fatalf("first Up: primary has no %s label: %v", devcontainer.LabelComposeInputsHash, ws1.Container.Labels) + } + if got := execOut(t, ctx, h.eng, ws1, "cat", "/marker-data/entry.log"); lineCount(got) != 1 { + t.Fatalf("first Up: entry.log = %q, want one entry", got) + } + + h.stopProject(t, ctx, ws1) + h.block() + + ws2, evs2 := h.up(t, ctx, dir, devcontainer.UpOptions{AdoptExisting: adopt}) + if n := h.store.fetches.Load(); n != 0 { + t.Errorf("reattach: %d feature fetches, want 0", n) + } + if n := h.gate.calls.Load(); n != 0 { + t.Errorf("reattach: %d registry requests, want 0", n) + } + if n := featureBuilds(evs2); n != 0 { + t.Errorf("reattach: %d feature image builds, want 0", n) + } + if n := reattachSkips(evs2); n != 1 { + t.Errorf("reattach: %d compose_reattach skip events, want 1", n) + } + if ws2.Container.ID != ws1.Container.ID { + t.Errorf("reattach recreated the primary: %s -> %s", ws1.Container.ID, ws2.Container.ID) + } + if ws2.Container.State != runtime.StateRunning { + t.Errorf("primary state after reattach = %s, want running", ws2.Container.State) + } + cs, err := h.rt.ListContainers(ctx, runtime.LabelFilter{Match: map[string]string{ + "com.docker.compose.project": ws2.Container.Labels["com.docker.compose.project"], + "com.docker.compose.service": "sidecar", + }}) + if err != nil || len(cs) != 1 || cs[0].State != runtime.StateRunning { + t.Errorf("sidecar after reattach = %+v (err %v), want one running", cs, err) + } + + // Metadata from the image label reached cfg and the container. + if ws2.Config.ContainerEnv["MARKER_FEATURE"] != "on" { + t.Errorf("cfg.ContainerEnv = %v, want MARKER_FEATURE=on", ws2.Config.ContainerEnv) + } + if !containsStr(ws2.Config.CapAdd, "SYS_PTRACE") || !hasCapability(ws2.Container.CapAdd, "SYS_PTRACE") { + t.Errorf("capAdd cfg=%v container=%v, want SYS_PTRACE", ws2.Config.CapAdd, ws2.Container.CapAdd) + } + var vol bool + for _, m := range ws2.Container.Mounts { + if m.Target == "/marker-data" && m.Type == "volume" { + vol = true + } + } + if !vol { + t.Errorf("no volume at /marker-data after reattach: %+v", ws2.Container.Mounts) + } + if got := execOut(t, ctx, h.eng, ws2, "printenv", "MARKER_FEATURE"); got != "on" { + t.Errorf("MARKER_FEATURE in container = %q", got) + } + // The entrypoint ran again on restart, and the lifecycle + // command from the label ran on both Ups, into the same volume. + if got := execOut(t, ctx, h.eng, ws2, "cat", "/marker-data/entry.log"); lineCount(got) != 2 { + t.Errorf("entry.log after reattach = %q, want two entries", got) + } + if got := execOut(t, ctx, h.eng, ws2, "cat", "/marker-data/poststart.log"); lineCount(got) != 2 { + t.Errorf("poststart.log after reattach = %q, want two entries", got) + } + if got := execOut(t, ctx, h.eng, ws2, "cat", "/usr/local/share/marker-flavor"); got != "plain" { + t.Errorf("marker-flavor = %q, want plain", got) + } + }) + } + } +} + +// TestComposeReattach_ConfigChangeRebuilds: a changed feature option +// changes the inputs hash, so the next Up takes the full path — it +// fetches and builds again (needing the registry) rather than +// reattaching to an image built from other inputs. +func TestComposeReattach_ConfigChangeRebuilds(t *testing.T) { + if testing.Short() { + t.Skip() + } + featureRef := serveMarkerFeature(t) + + for _, bk := range composeBackends { + t.Run(bk.name, func(t *testing.T) { + h := newReattachHarness(t, bk.backend) + ctx, cancel := context.WithTimeout(context.Background(), 8*time.Minute) + defer cancel() + dir := t.TempDir() + writeMarkerWorkspace(t, dir, featureRef, "plain") + + ws1, _ := h.up(t, ctx, dir, devcontainer.UpOptions{Recreate: true, SkipLifecycle: true}) + cleanupComposeWorkspace(t, h.eng, ws1) + h.stopProject(t, ctx, ws1) + + writeMarkerWorkspace(t, dir, featureRef, "spicy") + + // Offline, the changed inputs must not be satisfied by the + // old image: the full path runs and needs the registry. The + // exact-version tag is served from the store's cache, so the + // fetch succeeds; what matters is that it happens. + h.block() + ws2, evs := h.up(t, ctx, dir, devcontainer.UpOptions{SkipLifecycle: true}) + if n := h.store.fetches.Load(); n != 1 { + t.Errorf("Up after config change: %d feature fetches, want 1", n) + } + if n := featureBuilds(evs); n != 1 { + t.Errorf("Up after config change: %d feature image builds, want 1", n) + } + if n := reattachSkips(evs); n != 0 { + t.Errorf("Up after config change reattached (%d skip events)", n) + } + if bk.backend == devcontainer.ComposeBackendNative { + // Native without AdoptExisting recreates on drift. + if ws2.Container.ID == ws1.Container.ID { + t.Errorf("native: primary not recreated after config change") + } + if ws2.Container.Labels[devcontainer.LabelComposeInputsHash] == ws1.Container.Labels[devcontainer.LabelComposeInputsHash] { + t.Errorf("native: inputs hash label unchanged after config change") + } + if got := execOut(t, ctx, h.eng, ws2, "cat", "/usr/local/share/marker-flavor"); got != "spicy" { + t.Errorf("native: marker-flavor = %q, want spicy", got) + } + } + }) + } +} diff --git a/up.go b/up.go index bf51e9d..89649b7 100644 --- a/up.go +++ b/up.go @@ -2,6 +2,7 @@ package devcontainer import ( "context" + "errors" "fmt" "os" "path/filepath" @@ -159,6 +160,12 @@ const ( // - existing + UpOptions.Recreate=true → stop + remove, fresh create // - no existing container → fresh create // +// For compose sources, "attach" means the project's containers are +// started from their existing images when the primary container's +// LabelComposeInputsHash matches the current inputs; features are not +// fetched and no image is built. Otherwise the full path runs (fetch, +// build, compose up). See reattachCompose. +// // Image source only in this milestone. Build / compose return // runtime.ErrNotImplemented. func (e *Engine) Up(ctx context.Context, opts UpOptions) (*Workspace, error) { @@ -227,10 +234,13 @@ func (e *Engine) Up(ctx context.Context, opts UpOptions) (*Workspace, error) { var ws *Workspace switch { case isCompose: - // `docker compose up -d` is idempotent: already-running services - // stay, stopped ones restart, missing ones get created. So we - // always go through createFreshCompose, regardless of whether - // `existing` was found — compose handles the dispatch internally. + // createFreshCompose handles both cases: with an existing + // container it first tries reattachCompose (restart the project + // from its existing images, metadata from the image label, no + // feature fetch or build) and otherwise runs the full path, where + // `docker compose up -d` / the native orchestrator keep + // already-running services, restart stopped ones and create + // missing ones. // // Reattach caveat for host-side hooks: when compose finds an // existing container, its env is already baked. secretsCommand @@ -547,11 +557,32 @@ func (e *Engine) createFreshCompose(ctx context.Context, cfg *config.ResolvedCon return nil, err } + // Reattach when the project's containers already exist from inputs + // identical to these: the image path's attachExisting, for compose. + // No feature fetch, no image build, so a restart needs neither the + // feature registry nor the time to rebuild the final image. + if existingContainer { + ws, err := e.reattachCompose(ctx, cfg, opts, project, src, primary, projectName, workingDir) + if !errors.Is(err, errComposeReattachMiss) { + return ws, err + } + } + baseImage, err := e.prepareComposeServiceImage(ctx, cfg, primary, workingDir, opts) if err != nil { return nil, err } + // Hash the inputs before layerFeatures and applyMetadataMerge + // mutate cfg. A hash that cannot be computed only means the + // container gets no label and a later Up cannot reattach to it. + inputsHash := "" + if base, ierr := e.runtime.InspectImage(ctx, baseImage); ierr == nil && base != nil { + if h, herr := e.composeInputsHash(cfg, opts, project, base.ID); herr == nil { + inputsHash = h + } + } + finalImage, baseLayers, err := e.layerFeatures(ctx, cfg, baseImage, opts) if err != nil { return nil, err @@ -568,6 +599,26 @@ func (e *Engine) createFreshCompose(ctx context.Context, cfg *config.ResolvedCon return nil, err } + runOverride := e.composeRunOverride(ctx, cfg, opts, project, src, finalImage, extraEnv, inputsHash) + return e.upComposeBackend(ctx, cfg, opts, project, src, projectName, workingDir, + finalImage, runOverride, existingContainer, false) +} + +// composeRunOverride builds the run-time override layered onto the +// primary service from the merged cfg. Shared by the full path and +// reattachCompose so both produce the same service config (and so the +// same orchestrator config hash) for the same merged metadata. +// inputsHash, when non-empty, is stamped as LabelComposeInputsHash. +func (e *Engine) composeRunOverride( + ctx context.Context, + cfg *config.ResolvedConfig, + opts UpOptions, + project *composetypes.Project, + src *config.ComposeSource, + finalImage string, + extraEnv map[string]string, + inputsHash string, +) compose.Override { bindMounts := composeBindMounts(cfg, opts) overrideLabels := map[string]string{ @@ -575,6 +626,9 @@ func (e *Engine) createFreshCompose(ctx context.Context, cfg *config.ResolvedCon LabelLocalWorkspaceFolder: cfg.LocalWorkspaceFolder, LabelEngine: engineIdent, } + if inputsHash != "" { + overrideLabels[LabelComposeInputsHash] = inputsHash + } overrideEnv := mergeEnv(cfg.ContainerEnv, extraEnv) // When features/image metadata declare entrypoint scripts (e.g. @@ -607,7 +661,7 @@ func (e *Engine) createFreshCompose(ctx context.Context, cfg *config.ResolvedCon // feature-entrypoint chain — without these, features like // docker-in-docker that declare privileged/init/entrypoint silently // fail on compose-source devcontainers. - runOverride := compose.Override{ + return compose.Override{ Service: src.Service, ExtraBindMounts: bindMounts, ExtraMounts: composeMounts(cfg, opts), @@ -620,11 +674,25 @@ func (e *Engine) createFreshCompose(ctx context.Context, cfg *config.ResolvedCon Entrypoints: cfg.Entrypoints, OriginalEntrypoint: origEntrypoint, } +} +// upComposeBackend dispatches to the selected compose backend. +// reattach marks a reattachCompose call: the native backend then skips +// sidecar builds whose image is already present. +func (e *Engine) upComposeBackend( + ctx context.Context, + cfg *config.ResolvedConfig, + opts UpOptions, + project *composetypes.Project, + src *config.ComposeSource, + projectName, workingDir, finalImage string, + runOverride compose.Override, + existingContainer, reattach bool, +) (*Workspace, error) { switch e.opts.ComposeBackend { case ComposeBackendNative: return e.upComposeNative(ctx, cfg, opts, project, src, projectName, - workingDir, finalImage, runOverride) + workingDir, finalImage, runOverride, reattach) case ComposeBackendShellout: return e.upComposeShellout(ctx, cfg, opts, project, src, projectName, workingDir, finalImage, runOverride, existingContainer) @@ -755,11 +823,12 @@ func (e *Engine) upComposeNative( src *config.ComposeSource, projectName, workingDir, finalImage string, runOverride compose.Override, + reattach bool, ) (*Workspace, error) { if err := compose.ApplyBuildOverride(project, src.Service, finalImage); err != nil { return nil, err } - if err := e.buildComposeSidecarImages(ctx, project, src, projectName, workingDir, opts); err != nil { + if err := e.buildComposeSidecarImages(ctx, project, src, projectName, workingDir, opts, reattach); err != nil { return nil, err } if err := compose.ApplyRunOverride(project, src.Service, runOverride); err != nil { @@ -911,6 +980,7 @@ func (e *Engine) buildComposeSidecarImages( src *config.ComposeSource, projectName, workingDir string, opts UpOptions, + skipPresent bool, ) error { // Selection mirrors `docker compose up `: // the named services plus their transitive dependencies (the same @@ -950,6 +1020,17 @@ func (e *Engine) buildComposeSidecarImages( if !filepath.IsAbs(ctxPath) { ctxPath = filepath.Join(workingDir, ctxPath) } + if skipPresent { + // Reattach: the existing containers were created from this + // tag; rebuilding would only re-run the build (and maybe + // pull its base) to land on the same image. + if _, err := e.runtime.InspectImage(ctx, tag); err == nil { + if err := compose.ApplyBuildOverride(project, name, tag); err != nil { + return err + } + continue + } + } opts.bus.Emit(events.BuildStartEvent{Source: events.BuildSourceDockerfile, Ref: tag}) if _, err := e.runtime.BuildImage(ctx, runtime.BuildSpec{ ContextPath: ctxPath, diff --git a/up_compose_host_ports_test.go b/up_compose_host_ports_test.go index 5c43b57..8b7c30f 100644 --- a/up_compose_host_ports_test.go +++ b/up_compose_host_ports_test.go @@ -55,7 +55,7 @@ func hostPortUp(t *testing.T, eng *Engine, out chan events.Event) { }, } if _, err := eng.upComposeNative(context.Background(), cfg, opts, project, src, - "dc-test", t.TempDir(), "app:dev", runOverride); err != nil { + "dc-test", t.TempDir(), "app:dev", runOverride, false); err != nil { t.Fatalf("upComposeNative: %v", err) } } diff --git a/up_compose_mounts_test.go b/up_compose_mounts_test.go index bc85120..3070d62 100644 --- a/up_compose_mounts_test.go +++ b/up_compose_mounts_test.go @@ -125,7 +125,7 @@ func TestUpComposeNative_CarriesFeatureVolumeMount(t *testing.T) { project.Name = "dc-test" if _, err := eng.upComposeNative(context.Background(), cfg, opts, project, &config.ComposeSource{Service: "app"}, "dc-test", t.TempDir(), "app:dev", - mountsRunOverride(cfg, opts)); err != nil { + mountsRunOverride(cfg, opts), false); err != nil { t.Fatalf("upComposeNative: %v", err) } diff --git a/up_compose_reattach.go b/up_compose_reattach.go new file mode 100644 index 0000000..56e8f82 --- /dev/null +++ b/up_compose_reattach.go @@ -0,0 +1,340 @@ +package devcontainer + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "io" + "io/fs" + "os" + "path/filepath" + "sort" + + composetypes "github.com/compose-spec/compose-go/v2/types" + + "github.com/crunchloop/devcontainer/compose" + "github.com/crunchloop/devcontainer/config" + "github.com/crunchloop/devcontainer/events" + "github.com/crunchloop/devcontainer/feature" + "github.com/crunchloop/devcontainer/runtime" +) + +// LabelComposeInputsHash is stamped on the primary service container +// of a compose-source devcontainer. Its value is composeInputsHash of +// everything that went into that container's image and run override +// before any feature was fetched: the container-shaping part of the +// resolved devcontainer.json (features as requested, with their +// options), the loaded compose project, the primary service's base +// image ID, and the engine options that reshape the project. +// +// A later Up whose freshly computed hash equals the stored one +// reattaches: it reads the merged metadata from the existing +// container's image label and starts the project's existing services, +// with no feature fetch and no image build — the compose counterpart +// of the image path's attachExisting. See reattachCompose. +const LabelComposeInputsHash = "dev.containers.compose-inputs-hash" + +// composeInputsHashVersion is folded into the hash so a change to what +// the hash covers invalidates every stored value (the next Up takes the +// full path once and stamps the new form) instead of comparing +// incompatible digests. +const composeInputsHashVersion = 1 + +// maxLocalFeatureHashBytes bounds how much local feature content +// composeInputsHash reads. A local feature is a handful of scripts; a +// tree past this is not one, and the hash fails rather than reading it, +// which only costs the reattach (the full path still runs). +const maxLocalFeatureHashBytes = 64 << 20 + +// composeHashedConfig is the container-shaping subset of a freshly +// resolved (pre-feature-merge) ResolvedConfig. Fields that Up re-reads +// from devcontainer.json on every call and that never reach the +// container or its image — lifecycle commands, remoteEnv, ports +// attributes, customizations, the display name — are left out, so +// editing them does not cost a feature fetch and an image rebuild. +type composeHashedConfig struct { + DevcontainerID string + Source *config.ComposeSource + LocalWorkspaceFolder string + ContainerWorkspaceFolder string + WorkspaceMount *config.Mount + ContainerUser string + RemoteUser string + UpdateRemoteUserUID *bool + ContainerEnv map[string]string + Mounts []config.Mount + RunArgs []string + Init *bool + Privileged *bool + CapAdd []string + SecurityOpt []string + OverrideCommand *bool + Features []composeHashedFeature +} + +type composeHashedFeature struct { + Ref string + SourceKind config.FeatureSourceKind + Options map[string]any + // LocalContent is the content digest of a local feature's + // directory. OCI and HTTPS features are identified by their + // reference as written, matching the reference CLI, which does not + // re-resolve feature tags when it starts an existing container. + LocalContent string `json:",omitempty"` +} + +type composeHashInput struct { + Version int + Config composeHashedConfig + // Project is the loaded compose model before any engine override. + // The project's interpolation environment and working directory are + // not hashed: they are inputs to the model, already reflected in it. + Services composetypes.Services + Networks composetypes.Networks + Volumes composetypes.Volumes + Secrets composetypes.Secrets + Configs composetypes.Configs + // BaseImageID is the local image ID of the primary service's base + // image (its image:, or the engine's build of its build:). + BaseImageID string + // Engine options that reshape the project or the feature layering. + ComposeBackend ComposeBackend + DisableHostPortPublishing bool + StrictFeatureVersionMatch bool + // HostOwner is the workspace folder's owner, which selects the + // updateRemoteUserUID reconciliation applied on Linux. + HostOwner string +} + +// composeInputsHash returns the hex digest stored as +// LabelComposeInputsHash. cfg must be the freshly resolved config, +// before layerFeatures and applyMetadataMerge mutate it; project must +// be the loaded project before any Apply*Override. +func (e *Engine) composeInputsHash(cfg *config.ResolvedConfig, opts UpOptions, project *composetypes.Project, baseImageID string) (string, error) { + src, ok := cfg.Source.(*config.ComposeSource) + if !ok { + return "", fmt.Errorf("compose inputs hash: not a compose source") + } + if baseImageID == "" { + return "", fmt.Errorf("compose inputs hash: base image ID unknown") + } + configDir := composeWorkingDir(cfg, opts) + features := make([]composeHashedFeature, 0, len(cfg.Features)) + for _, f := range cfg.Features { + hf := composeHashedFeature{Ref: f.Ref, SourceKind: f.SourceKind, Options: f.Options} + if f.SourceKind == config.FeatureSourceLocal { + digest, err := hashDirContent(feature.ResolveLocalRef(configDir, f.Ref), maxLocalFeatureHashBytes) + if err != nil { + return "", fmt.Errorf("compose inputs hash: local feature %s: %w", f.Ref, err) + } + hf.LocalContent = digest + } + features = append(features, hf) + } + in := composeHashInput{ + Version: composeInputsHashVersion, + Config: composeHashedConfig{ + DevcontainerID: cfg.DevcontainerID, + Source: src, + LocalWorkspaceFolder: cfg.LocalWorkspaceFolder, + ContainerWorkspaceFolder: cfg.ContainerWorkspaceFolder, + WorkspaceMount: cfg.WorkspaceMount, + ContainerUser: cfg.ContainerUser, + RemoteUser: cfg.RemoteUser, + UpdateRemoteUserUID: cfg.UpdateRemoteUserUID, + ContainerEnv: cfg.ContainerEnv, + Mounts: cfg.Mounts, + RunArgs: cfg.RunArgs, + Init: cfg.Init, + Privileged: cfg.Privileged, + CapAdd: cfg.CapAdd, + SecurityOpt: cfg.SecurityOpt, + OverrideCommand: cfg.OverrideCommand, + Features: features, + }, + Services: project.Services, + Networks: project.Networks, + Volumes: project.Volumes, + Secrets: project.Secrets, + Configs: project.Configs, + BaseImageID: baseImageID, + ComposeBackend: e.opts.ComposeBackend, + DisableHostPortPublishing: e.opts.DisableHostPortPublishing, + StrictFeatureVersionMatch: e.opts.StrictFeatureVersionMatch, + } + if uid, gid, ok := statOwner(cfg.LocalWorkspaceFolder); ok { + in.HostOwner = fmt.Sprintf("%d:%d", uid, gid) + } + b, err := json.Marshal(in) + if err != nil { + return "", fmt.Errorf("compose inputs hash: %w", err) + } + sum := sha256.Sum256(b) + return hex.EncodeToString(sum[:]), nil +} + +// hashDirContent digests every regular file under root (relative path, +// mode and bytes, in lexical order), reading at most limit bytes. +func hashDirContent(root string, limit int64) (string, error) { + var paths []string + err := filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.Type().IsRegular() { + paths = append(paths, p) + } + return nil + }) + if err != nil { + return "", err + } + sort.Strings(paths) + h := sha256.New() + var total int64 + for _, p := range paths { + rel, err := filepath.Rel(root, p) + if err != nil { + return "", err + } + fi, err := os.Stat(p) + if err != nil { + return "", err + } + total += fi.Size() + if total > limit { + return "", fmt.Errorf("content exceeds %d bytes", limit) + } + _, _ = fmt.Fprintf(h, "%s\x00%o\x00%d\x00", filepath.ToSlash(rel), fi.Mode().Perm(), fi.Size()) // hash.Hash writes never fail + f, err := os.Open(p) + if err != nil { + return "", err + } + _, err = io.Copy(h, io.LimitReader(f, fi.Size())) + f.Close() + if err != nil { + return "", err + } + } + return hex.EncodeToString(h.Sum(nil)), nil +} + +// composePrimaryBaseRef is the image reference whose local ID stands +// for the primary service's base image in composeInputsHash: its +// image:, or the tag prepareComposeServiceImage builds its build: to. +func composePrimaryBaseRef(cfg *config.ResolvedConfig, svc *composetypes.ServiceConfig) string { + if svc.Build != nil { + return "dc-go-compose-base-" + cfg.DevcontainerID + ":latest" + } + return svc.Image +} + +// errComposeReattachMiss marks a reattach precondition that did not +// hold. Up then takes the full path; it is never returned to callers. +var errComposeReattachMiss = errors.New("compose reattach not possible") + +func reattachMiss(format string, args ...any) error { + return fmt.Errorf("%w: "+format, append([]any{errComposeReattachMiss}, args...)...) +} + +// reattachCompose is the compose counterpart of attachExisting. When +// the project's primary container exists and carries an +// LabelComposeInputsHash equal to the hash of the current inputs, it +// +// - reads the merged feature and image metadata from that +// container's image label, as attachExisting does, instead of +// fetching every feature and rebuilding the final image; +// - rebuilds the run override from it (entrypoint wrapper, mounts, +// env, privileged, capAdd, ...) exactly as the full path does; and +// - brings the project up through the selected backend against the +// existing images: the native orchestrator under the caller's +// AdoptExisting with sidecar builds skipped when their images are +// present, or `docker compose up --no-recreate` (which builds only +// missing images) on the shell-out backend. +// +// It returns (nil, errComposeReattachMiss-wrapped) when a precondition +// does not hold — no primary container, no or different hash label, +// the image tag the container was created from now names another +// image — and the caller falls back to the full path. cfg is not +// mutated on a miss. +func (e *Engine) reattachCompose( + ctx context.Context, + cfg *config.ResolvedConfig, + opts UpOptions, + project *composetypes.Project, + src *config.ComposeSource, + primary *composetypes.ServiceConfig, + projectName, workingDir string, +) (*Workspace, error) { + if opts.override != nil { + return nil, reattachMiss("image build requested") + } + if opts.PullPolicy == PullAlways { + return nil, reattachMiss("pull policy is always") + } + containers, err := e.runtime.ListContainers(ctx, runtime.LabelFilter{Match: map[string]string{ + compose.LabelComposeProject: projectName, + compose.LabelComposeService: src.Service, + LabelDevcontainerID: cfg.DevcontainerID, + }}) + if err != nil { + return nil, reattachMiss("list primary containers: %v", err) + } + if len(containers) != 1 { + return nil, reattachMiss("%d primary containers", len(containers)) + } + details, err := e.runtime.InspectContainer(ctx, containers[0].ID) + if err != nil || details == nil { + return nil, reattachMiss("inspect primary container: %v", err) + } + stored := details.Labels[LabelComposeInputsHash] + if stored == "" { + return nil, reattachMiss("primary container has no %s label", LabelComposeInputsHash) + } + base, err := e.runtime.InspectImage(ctx, composePrimaryBaseRef(cfg, primary)) + if err != nil || base == nil || base.ID == "" { + return nil, reattachMiss("base image not present locally: %v", err) + } + current, err := e.composeInputsHash(cfg, opts, project, base.ID) + if err != nil { + return nil, reattachMiss("%v", err) + } + if current != stored { + return nil, reattachMiss("inputs changed") + } + if details.ImageRef == "" { + return nil, reattachMiss("primary container image reference unknown") + } + img, err := e.runtime.InspectImage(ctx, details.ImageRef) + if err != nil || img == nil { + return nil, reattachMiss("image %s not present locally: %v", details.ImageRef, err) + } + if img.ID != details.Image { + // Someone re-tagged it (Engine.Build, a manual build): the tag + // no longer names the image the container runs, and a later + // recreate from it would silently switch images. + return nil, reattachMiss("image %s now names %s, container runs %s", details.ImageRef, img.ID, details.Image) + } + + // Committed: from here on cfg is merged from the label. + var layers []config.FeatureMetadata + if label := img.Labels[feature.MetadataLabel]; label != "" { + parsed, err := feature.ParseMetadataLabel(label) + if err != nil { + return nil, fmt.Errorf("compose reattach: parse %s label of %s: %w", feature.MetadataLabel, details.ImageRef, err) + } + layers = parsed + } + for _, f := range cfg.Features { + opts.bus.Emit(events.FeatureSkippedEvent{Ref: f.Ref, Reason: "compose_reattach"}) + } + applyMetadataMerge(cfg, layers, opts.LocalEnv) + + // Reattach never re-runs secretsCommand: the container's env is + // already baked (see UpOptions.RunSecretsCommand). + runOverride := e.composeRunOverride(ctx, cfg, opts, project, src, details.ImageRef, nil, stored) + return e.upComposeBackend(ctx, cfg, opts, project, src, projectName, workingDir, details.ImageRef, runOverride, true, true) +} diff --git a/up_compose_reattach_test.go b/up_compose_reattach_test.go new file mode 100644 index 0000000..67d240c --- /dev/null +++ b/up_compose_reattach_test.go @@ -0,0 +1,308 @@ +package devcontainer + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" + + composetypes "github.com/compose-spec/compose-go/v2/types" + + "github.com/crunchloop/devcontainer/compose" + "github.com/crunchloop/devcontainer/config" + "github.com/crunchloop/devcontainer/events" + "github.com/crunchloop/devcontainer/runtime" +) + +// hashFixture is a resolved compose config plus its loaded project, as +// Up has them before any feature is fetched. +type hashFixture struct { + eng *Engine + cfg *config.ResolvedConfig + opts UpOptions + project *composetypes.Project +} + +// newHashFixture writes the workspace into dir (shared between the +// fixtures a test compares, so paths and the devcontainer ID match) +// and resolves it. +func newHashFixture(t *testing.T, dir, devcontainerJSON, composeYAML string) *hashFixture { + t.Helper() + if err := os.MkdirAll(filepath.Join(dir, ".devcontainer"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "docker-compose.yml"), []byte(composeYAML), 0o644); err != nil { + t.Fatal(err) + } + cfgPath := filepath.Join(dir, ".devcontainer", "devcontainer.json") + if err := os.WriteFile(cfgPath, []byte(devcontainerJSON), 0o644); err != nil { + t.Fatal(err) + } + opts := UpOptions{LocalWorkspaceFolder: dir, ConfigPath: cfgPath, LocalEnv: map[string]string{"HOME": "/home/x"}} + cfg, err := Resolve(context.Background(), ResolveOptions{LocalWorkspaceFolder: dir, ConfigPath: cfgPath, LocalEnv: opts.LocalEnv}) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + src := cfg.Source.(*config.ComposeSource) + project, err := compose.Load(context.Background(), compose.LoadOptions{ + Files: src.Files, + WorkingDir: composeWorkingDir(cfg, opts), + ProjectName: composeProjectName(cfg), + Env: mapToEnvList(opts.LocalEnv), + }) + if err != nil { + t.Fatalf("compose.Load: %v", err) + } + eng, err := New(EngineOptions{Runtime: newFakeRuntime(), ComposeBackend: ComposeBackendNative}) + if err != nil { + t.Fatal(err) + } + return &hashFixture{eng: eng, cfg: cfg, opts: opts, project: project} +} + +func (f *hashFixture) hash(t *testing.T, baseID string) string { + t.Helper() + h, err := f.eng.composeInputsHash(f.cfg, f.opts, f.project, baseID) + if err != nil { + t.Fatalf("composeInputsHash: %v", err) + } + return h +} + +const hashBaseJSON = `{ + "dockerComposeFile": "../docker-compose.yml", + "service": "app", + "workspaceFolder": "/workspaces/proj", + "features": { "ghcr.io/devcontainers/features/docker-in-docker:2": { "moby": false } }, + "postStartCommand": "echo hi", + "customizations": { "vscode": { "extensions": ["a.b"] } } +}` + +const hashBaseCompose = ` +services: + app: + image: debian:bookworm-slim + command: ["sleep", "infinity"] + db: + image: postgres:17 +` + +func TestComposeInputsHash_Deterministic(t *testing.T) { + a := newHashFixture(t, t.TempDir(), hashBaseJSON, hashBaseCompose) + h1 := a.hash(t, "sha256:base") + if h2 := a.hash(t, "sha256:base"); h1 != h2 { + t.Fatalf("same inputs hashed differently: %s vs %s", h1, h2) + } + if len(h1) != 64 { + t.Fatalf("hash %q is not a sha256 hex digest", h1) + } +} + +// Everything that shapes the primary image or the project changes the +// hash, so a changed input takes the full path (fetch and build). +func TestComposeInputsHash_ChangesWithInputs(t *testing.T) { + cases := []struct { + name string + json, yaml string + baseID string + disablePort bool + }{ + {name: "base image ID", baseID: "sha256:other"}, + {name: "feature option", json: replaceOnce(t, hashBaseJSON, `"moby": false`, `"moby": true`)}, + {name: "feature version", json: replaceOnce(t, hashBaseJSON, "docker-in-docker:2", "docker-in-docker:2.17.0")}, + {name: "containerEnv", json: replaceOnce(t, hashBaseJSON, `"service": "app",`, `"service": "app", "containerEnv": {"A": "1"},`)}, + {name: "mounts", json: replaceOnce(t, hashBaseJSON, `"service": "app",`, `"service": "app", "mounts": ["type=volume,source=v,target=/v"],`)}, + {name: "primary service", yaml: replaceOnce(t, hashBaseCompose, `["sleep", "infinity"]`, `["sleep", "1000"]`)}, + {name: "sidecar service", yaml: replaceOnce(t, hashBaseCompose, "postgres:17", "postgres:18")}, + {name: "engine option", disablePort: true}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + dir := t.TempDir() + want := newHashFixture(t, dir, hashBaseJSON, hashBaseCompose).hash(t, "sha256:base") + json, yaml, baseID := hashBaseJSON, hashBaseCompose, "sha256:base" + if c.json != "" { + json = c.json + } + if c.yaml != "" { + yaml = c.yaml + } + if c.baseID != "" { + baseID = c.baseID + } + f := newHashFixture(t, dir, json, yaml) + f.eng.opts.DisableHostPortPublishing = c.disablePort + if got := f.hash(t, baseID); got == want { + t.Fatalf("hash unchanged after changing the %s", c.name) + } + }) + } +} + +// Fields Up re-reads from devcontainer.json on every call and that never +// reach the container or its image do not change the hash: editing a +// lifecycle command must not cost a feature fetch and a rebuild. +func TestComposeInputsHash_IgnoresNonContainerFields(t *testing.T) { + dir := t.TempDir() + want := newHashFixture(t, dir, hashBaseJSON, hashBaseCompose).hash(t, "sha256:base") + + edited := replaceOnce(t, hashBaseJSON, `"postStartCommand": "echo hi"`, + `"name": "renamed", "postStartCommand": "echo bye", "remoteEnv": {"X": "y"}, "forwardPorts": [3000]`) + edited = replaceOnce(t, edited, `["a.b"]`, `["c.d"]`) + if got := newHashFixture(t, dir, edited, hashBaseCompose).hash(t, "sha256:base"); got != want { + t.Fatalf("hash changed after editing name/lifecycle/remoteEnv/ports/customizations") + } +} + +func TestComposeInputsHash_LocalFeatureContent(t *testing.T) { + dir := t.TempDir() + json := replaceOnce(t, hashBaseJSON, `"ghcr.io/devcontainers/features/docker-in-docker:2": { "moby": false }`, `"./local-feature": {}`) + featDir := filepath.Join(dir, ".devcontainer", "local-feature") + if err := os.MkdirAll(featDir, 0o755); err != nil { + t.Fatal(err) + } + install := filepath.Join(featDir, "install.sh") + if err := os.WriteFile(install, []byte("#!/bin/sh\necho one\n"), 0o755); err != nil { + t.Fatal(err) + } + f := newHashFixture(t, dir, json, hashBaseCompose) + h1 := f.hash(t, "sha256:base") + if err := os.WriteFile(install, []byte("#!/bin/sh\necho two\n"), 0o755); err != nil { + t.Fatal(err) + } + if h2 := f.hash(t, "sha256:base"); h1 == h2 { + t.Fatal("hash unchanged after editing a local feature's install.sh") + } +} + +func TestComposeInputsHash_RequiresBaseImageID(t *testing.T) { + f := newHashFixture(t, t.TempDir(), hashBaseJSON, hashBaseCompose) + if _, err := f.eng.composeInputsHash(f.cfg, f.opts, f.project, ""); err == nil { + t.Fatal("hash computed without a base image ID") + } +} + +func replaceOnce(t *testing.T, s, old, new string) string { + t.Helper() + if strings.Count(s, old) != 1 { + t.Fatalf("%q does not occur exactly once", old) + } + return strings.Replace(s, old, new, 1) +} + +// listingFake answers ListContainers from the fakeRuntime's containers. +type listingFake struct{ *fakeRuntime } + +func (l listingFake) ListContainers(ctx context.Context, filter runtime.LabelFilter) ([]runtime.Container, error) { + l.mu.Lock() + defer l.mu.Unlock() + var out []runtime.Container + for _, d := range l.containersByID { + ok := true + for k, v := range filter.Match { + if d.Labels[k] != v { + ok = false + } + } + if ok { + out = append(out, d.Container) + } + } + return out, nil +} + +// reattachMissFixture is a compose workspace whose primary container +// exists with the hash of the current inputs; each test breaks one +// precondition and expects a miss (so Up falls back to the full path) +// without cfg being touched. +func reattachMissFixture(t *testing.T) (*hashFixture, *fakeRuntime, *runtime.ContainerDetails) { + t.Helper() + f := newHashFixture(t, t.TempDir(), hashBaseJSON, hashBaseCompose) + fr := newFakeRuntime() + eng, err := New(EngineOptions{Runtime: listingFake{fr}, ComposeBackend: ComposeBackendNative}) + if err != nil { + t.Fatal(err) + } + f.eng = eng + f.opts.bus = newEventBus(events.NewEmitter(nil), nil) + fr.imagesByRef["debian:bookworm-slim"] = &runtime.ImageDetails{ID: "sha256:base"} + fr.imagesByRef["dc-go-final-x:latest"] = &runtime.ImageDetails{ID: "sha256:final"} + c := &runtime.ContainerDetails{ + Container: runtime.Container{ID: "c1", Image: "sha256:final", State: runtime.StateExited}, + ImageRef: "dc-go-final-x:latest", + Labels: map[string]string{ + compose.LabelComposeProject: composeProjectName(f.cfg), + compose.LabelComposeService: "app", + LabelDevcontainerID: f.cfg.DevcontainerID, + LabelComposeInputsHash: f.hash(t, "sha256:base"), + }, + } + fr.containersByID["c1"] = c + return f, fr, c +} + +func (f *hashFixture) reattach(t *testing.T) error { + t.Helper() + src := f.cfg.Source.(*config.ComposeSource) + primary, err := compose.PrimaryService(f.project, src.Service) + if err != nil { + t.Fatal(err) + } + _, err = f.eng.reattachCompose(context.Background(), f.cfg, f.opts, f.project, src, primary, + composeProjectName(f.cfg), composeWorkingDir(f.cfg, f.opts)) + return err +} + +func TestReattachCompose_Misses(t *testing.T) { + // Control: with every precondition holding, reattach commits (here + // it then fails in the fake's orchestrator primitives, which is not + // a miss), so each case below misses for the reason it names. + t.Run("control", func(t *testing.T) { + f, _, _ := reattachMissFixture(t) + if err := f.reattach(t); errors.Is(err, errComposeReattachMiss) { + t.Fatalf("intact fixture missed: %v", err) + } + if len(f.cfg.Entrypoints) == 0 && f.cfg.OverrideCommand == nil { + t.Fatalf("committed reattach did not merge metadata into cfg") + } + }) + cases := map[string]func(f *hashFixture, fr *fakeRuntime, c *runtime.ContainerDetails){ + "no hash label (created before reattach existed)": func(_ *hashFixture, _ *fakeRuntime, c *runtime.ContainerDetails) { + delete(c.Labels, LabelComposeInputsHash) + }, + "inputs changed": func(_ *hashFixture, _ *fakeRuntime, c *runtime.ContainerDetails) { + c.Labels[LabelComposeInputsHash] = "0000" + }, + "base image moved": func(_ *hashFixture, fr *fakeRuntime, _ *runtime.ContainerDetails) { + fr.imagesByRef["debian:bookworm-slim"] = &runtime.ImageDetails{ID: "sha256:newbase"} + }, + "base image missing": func(_ *hashFixture, fr *fakeRuntime, _ *runtime.ContainerDetails) { + delete(fr.imagesByRef, "debian:bookworm-slim") + }, + "final image tag moved": func(_ *hashFixture, fr *fakeRuntime, _ *runtime.ContainerDetails) { + fr.imagesByRef["dc-go-final-x:latest"] = &runtime.ImageDetails{ID: "sha256:rebuilt"} + }, + "no primary container": func(_ *hashFixture, fr *fakeRuntime, _ *runtime.ContainerDetails) { + delete(fr.containersByID, "c1") + }, + "pull policy always": func(f *hashFixture, _ *fakeRuntime, _ *runtime.ContainerDetails) { + f.opts.PullPolicy = PullAlways + }, + } + for name, breakIt := range cases { + t.Run(name, func(t *testing.T) { + f, fr, c := reattachMissFixture(t) + breakIt(f, fr, c) + before := *f.cfg + err := f.reattach(t) + if !errors.Is(err, errComposeReattachMiss) { + t.Fatalf("reattach = %v, want a miss", err) + } + if len(f.cfg.Mounts) != len(before.Mounts) || len(f.cfg.Entrypoints) != len(before.Entrypoints) || f.cfg.OverrideCommand != before.OverrideCommand { + t.Fatalf("cfg mutated on a miss") + } + }) + } +} diff --git a/up_compose_sidecar_test.go b/up_compose_sidecar_test.go index cd2e228..9378b01 100644 --- a/up_compose_sidecar_test.go +++ b/up_compose_sidecar_test.go @@ -73,7 +73,7 @@ func TestBuildComposeSidecarImages_BuildsNonPrimaryServices(t *testing.T) { }) src := &config.ComposeSource{Service: "app"} - if err := eng.buildComposeSidecarImages(context.Background(), project, src, "dc-x", workingDir, sidecarUpOptions()); err != nil { + if err := eng.buildComposeSidecarImages(context.Background(), project, src, "dc-x", workingDir, sidecarUpOptions(), false); err != nil { t.Fatalf("buildComposeSidecarImages: %v", err) } @@ -120,7 +120,7 @@ func TestBuildComposeSidecarImages_HonorsRunServices(t *testing.T) { }) src := &config.ComposeSource{Service: "app", RunServices: []string{"db"}} - if err := eng.buildComposeSidecarImages(context.Background(), project, src, "dc-x", t.TempDir(), sidecarUpOptions()); err != nil { + if err := eng.buildComposeSidecarImages(context.Background(), project, src, "dc-x", t.TempDir(), sidecarUpOptions(), false); err != nil { t.Fatalf("buildComposeSidecarImages: %v", err) } @@ -147,7 +147,7 @@ func TestBuildComposeSidecarImages_BuildsDependenciesOfSelection(t *testing.T) { }) src := &config.ComposeSource{Service: "app", RunServices: []string{"app"}} - if err := eng.buildComposeSidecarImages(context.Background(), project, src, "dc-x", t.TempDir(), sidecarUpOptions()); err != nil { + if err := eng.buildComposeSidecarImages(context.Background(), project, src, "dc-x", t.TempDir(), sidecarUpOptions(), false); err != nil { t.Fatalf("buildComposeSidecarImages: %v", err) } if len(rt.builds) != 1 || rt.builds[0].Tag != "dc-x-db" {