From 2f4bd0ba8dee89e77448b783f535a842e9225e30 Mon Sep 17 00:00:00 2001 From: Mike Sul Date: Thu, 8 Oct 2026 17:15:42 +0200 Subject: [PATCH 1/4] fix(start): remove containers left by a failed start When docker compose up fails, containers that were created but not started stay behind. On the next attempt, compose starts them again, and Docker 29.7.2 lets such a container run without its published ports after a host port conflict. A retried update then reports success while a service is unreachable. Removing these containers right after the failure makes every attempt create them afresh, so a retry fails the same way the first attempt did. Signed-off-by: Mike Sul Assisted-by: Claude Code:claude-opus-5-5 --- pkg/compose/start.go | 71 ++++++++++++++++++++++++++++++++------------ 1 file changed, 52 insertions(+), 19 deletions(-) diff --git a/pkg/compose/start.go b/pkg/compose/start.go index 88694d0..5989556 100644 --- a/pkg/compose/start.go +++ b/pkg/compose/start.go @@ -7,6 +7,7 @@ import ( "github.com/containerd/containerd/platforms" "os" "os/exec" + "strings" ) type ( @@ -65,28 +66,11 @@ func StartApps(ctx context.Context, cfg *Config, appURIs []string, options ...St if opts.ProgressHandler != nil { opts.ProgressHandler(app, AppStartStatusStarting, nil) } - cmd := exec.Command("docker", "compose", "up", "-d", "--remove-orphans") - cmd.Dir = cfg.GetAppComposeDir(app.Name()) - if opts.Verbose { - // Directly connect to stdout/stderr, so we can see the output in real time - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - } else { - // Capture stdout/stderr for error reporting - var stdout, stderr bytes.Buffer - cmd.Stdout = &stdout - cmd.Stderr = &stderr - } - if err := cmd.Run(); err != nil { + if err := composeUp(cfg.GetAppComposeDir(app.Name()), opts.Verbose); err != nil { if opts.ProgressHandler != nil { opts.ProgressHandler(app, AppStartStatusFailed, err) } - if opts.Verbose { - return fmt.Errorf("failed to start %s: %w", app, err) - } else { - return fmt.Errorf("failed to start %s: %w\n\tstdout: %s\n\tstderr: %s", app, err, cmd.Stdout, cmd.Stderr) - } - + return fmt.Errorf("failed to start %s: %w", app, err) } if opts.ProgressHandler != nil { opts.ProgressHandler(app, AppStartStatusStarted, nil) @@ -94,3 +78,52 @@ func StartApps(ctx context.Context, cfg *Config, appURIs []string, options ...St } return nil } + +// composeUp runs `docker compose up` in the given app compose directory. +// If it fails, the app containers that were created but not started are removed. +func composeUp(composeDir string, verbose bool) error { + cmd := exec.Command("docker", "compose", "up", "-d", "--remove-orphans") + cmd.Dir = composeDir + var stdout, stderr bytes.Buffer + if verbose { + // Directly connect to stdout/stderr, so we can see the output in real time + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + } else { + // Capture stdout/stderr for error reporting + cmd.Stdout = &stdout + cmd.Stderr = &stderr + } + err := cmd.Run() + if err == nil { + return nil + } + if !verbose { + err = fmt.Errorf("%w\n\tstdout: %s\n\tstderr: %s", err, stdout.String(), stderr.String()) + } + if errRm := removeCreatedContainers(composeDir); errRm != nil { + err = fmt.Errorf("%w\n\tfailed to clean up after the failed start: %s", err, errRm.Error()) + } + return err +} + +// removeCreatedContainers removes the app containers that `docker compose up` created but failed to start. +// Starting such a container again may succeed without its published ports (Docker 29.7.2 does so after +// a host port conflict), so the next start attempt could report success for an app that is broken. +func removeCreatedContainers(composeDir string) error { + cmd := exec.Command("docker", "compose", "ps", "--all", "--quiet", "--status", "created") + cmd.Dir = composeDir + out, err := cmd.Output() + if err != nil { + return fmt.Errorf("failed to list containers in the created state: %w", err) + } + ids := strings.Fields(string(out)) + if len(ids) == 0 { + return nil + } + cmd = exec.Command("docker", append([]string{"rm"}, ids...)...) + if out, err := cmd.CombinedOutput(); err != nil { + return fmt.Errorf("failed to remove containers in the created state: %w: %s", err, out) + } + return nil +} From 30a1e5cc05afd1fc85bcd4dbe5dcb215e317d9ed Mon Sep 17 00:00:00 2001 From: Mike Sul Date: Thu, 8 Oct 2026 17:42:45 +0200 Subject: [PATCH 2/4] fix(install): check progress reporter on containerd load On a dockerd that uses the containerd image store, loading images without a progress reporter panics with a nil pointer dereference at the first "Loaded image:" message: the containerd branch calls the reporter unconditionally while the graphdriver branch checks it. Install calls the loader without a reporter when no progress is requested, so such an install crashes composectl. Signed-off-by: Mike Sul Assisted-by: Claude Code:claude-opus-5-5 --- pkg/compose/image_loader.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/compose/image_loader.go b/pkg/compose/image_loader.go index 3c810e0..c27738e 100644 --- a/pkg/compose/image_loader.go +++ b/pkg/compose/image_loader.go @@ -396,7 +396,7 @@ func reportProgressIfContainerd(body io.ReadCloser, imageURIs []imageURI2RefCoun if strings.HasPrefix(jm.Stream, "Error unpacking image") { return fmt.Errorf("error unpacking image: %s", jm.Stream) } - if strings.HasPrefix(jm.Stream, "Loaded image: ") { + if reporter != nil && strings.HasPrefix(jm.Stream, "Loaded image: ") { imageRef := strings.TrimPrefix(jm.Stream, "Loaded image: ") // Remove end line character if present imageRef = strings.TrimSuffix(imageRef, "\n") From 4875f3753ff1fb0e184bed1ef3ab6e60af265fa5 Mon Sep 17 00:00:00 2001 From: Mike Sul Date: Thu, 8 Oct 2026 17:42:46 +0200 Subject: [PATCH 3/4] test(compose): run e2e on dockerd 29 with both stores Devices run dockerd both with the overlay2 graphdriver and with the containerd image store, and the test environment covered only the former on dockerd 25. The test dockerd is now stock 29.7.2 with containerd 2.4.1, built on first start, and CONTAINERD_SNAPSHOTTER selects the image store; CI runs the suite once per store. Publishing test apps reads layer sizes from an overlay2 docker store, which the containerd image store does not have. A separate publisher dockerd, always on overlay2, now plays the FoundriesFactory CI so the device dockerd can use either store. The patched image loading in TestAppImageLoading needs the Foundries dockerd patches, which the stock daemon lacks; it now runs only when DOCKERD_FIO_PATCH=1. Signed-off-by: Mike Sul Assisted-by: Claude Code:claude-opus-5-5 --- .github/workflows/ci.yml | 6 ++++++ dev-shell.sh | 3 ++- test/compose/.env.test | 5 +++++ test/compose/docker-compose.yml | 29 ++++++++++++++++++++++++-- test/compose/dockerd/Dockerfile | 10 +++++++++ test/fixtures/composectl_cmds.go | 12 +++++++++-- test/fixtures/layers_meta.go | 2 +- test/integration/image_loading_test.go | 3 +++ 8 files changed, 64 insertions(+), 6 deletions(-) create mode 100644 test/compose/dockerd/Dockerfile diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f78d26f..e0802c1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,7 +26,13 @@ jobs: test: name: Build and run all tests + strategy: + fail-fast: false + matrix: + containerd-snapshotter: [false, true] runs-on: ubuntu-latest + env: + CONTAINERD_SNAPSHOTTER: ${{ matrix.containerd-snapshotter }} steps: - uses: actions/checkout@v3 - name: build diff --git a/dev-shell.sh b/dev-shell.sh index 0f31ab1..dc2a40f 100755 --- a/dev-shell.sh +++ b/dev-shell.sh @@ -1,7 +1,8 @@ down() { docker compose --env-file=test/compose/.env.test -f test/compose/docker-compose.yml down --remove-orphans # remove the docker runtime and compose app store volumes - docker volume rm compose_docker-data compose_docker-runtime compose_reset-apps + docker volume rm compose_docker-data compose_docker-runtime compose_reset-apps \ + compose_publisher-data compose_publisher-runtime } trap down EXIT diff --git a/test/compose/.env.test b/test/compose/.env.test index 06b1c22..9056fc1 100644 --- a/test/compose/.env.test +++ b/test/compose/.env.test @@ -7,3 +7,8 @@ REG_DIR=$CPS_DIR/registry REG_CERT_DIR=$REG_DIR/certs # This must be relative to SRC_DIR path REG_CERT=test/compose/registry/certs/registry.crt + +# Image store of the test dockerd: false - the overlay2 graphdriver, true - the containerd image store +CONTAINERD_SNAPSHOTTER=false +# Set to 1 when the test dockerd includes the Foundries patches +DOCKERD_FIO_PATCH= diff --git a/test/compose/docker-compose.yml b/test/compose/docker-compose.yml index 52cadf0..fb8bb2e 100644 --- a/test/compose/docker-compose.yml +++ b/test/compose/docker-compose.yml @@ -12,8 +12,11 @@ services: - ${REG_DIR}/config.yml:/etc/distribution/config.yml dockerd: - image: ghcr.io/foundriesio/moby:25.0.3_fio - command: ["dockerd", "-D", "-H", "unix:///var/run/docker/docker.sock"] + image: composeapp-test-dockerd:29.7.2 + build: + context: ${CPS_DIR}/dockerd + command: ["dockerd", "-D", "-H", "unix:///var/run/docker/docker.sock", + "--feature", "containerd-snapshotter=${CONTAINERD_SNAPSHOTTER}"] privileged: true volumes: - docker-runtime:/var/run/docker @@ -21,6 +24,20 @@ services: - ${REG_DIR}/daemon.json:/etc/docker/daemon.json:ro - reset-apps:/var/sota/reset-apps + # Plays the role of the FoundriesFactory CI when tests publish apps: test images are pulled into it + # and the layer sizes are read from its overlay2 store, whatever image store the dockerd above uses. + dockerd-publisher: + image: composeapp-test-dockerd:29.7.2 + build: + context: ${CPS_DIR}/dockerd + command: ["dockerd", "-D", "-H", "unix:///var/run/docker-publisher/docker.sock", + "--feature", "containerd-snapshotter=false"] + privileged: true + volumes: + - publisher-runtime:/var/run/docker-publisher + - publisher-data:/var/lib/docker + - ${REG_DIR}/daemon.json:/etc/docker/daemon.json:ro + composectl: build: context: ${SRC_DIR} @@ -34,11 +51,14 @@ services: - docker-runtime:/var/run/docker - docker-data:/var/lib/docker - reset-apps:/var/sota/reset-apps + - publisher-runtime:/var/run/docker-publisher + - publisher-data:/var/lib/docker-publisher:ro working_dir: ${SRC_DIR} depends_on: - registry - registry-org - dockerd + - dockerd-publisher environment: - GOCACHE=${SRC_DIR}/.cache - DOCKER_HOST=unix:///var/run/docker/docker.sock @@ -46,8 +66,13 @@ services: - SRC_DIR=${SRC_DIR} - STOREROOT=/var/sota/reset-apps - COMPOSEROOT=/var/sota/compose-apps + - DOCKERD_FIO_PATCH=${DOCKERD_FIO_PATCH} + - PUBLISHER_DOCKER_HOST=unix:///var/run/docker-publisher/docker.sock + - PUBLISHER_DOCKER_DATA_ROOT=/var/lib/docker-publisher volumes: docker-data: docker-runtime: reset-apps: + publisher-data: + publisher-runtime: diff --git a/test/compose/dockerd/Dockerfile b/test/compose/dockerd/Dockerfile new file mode 100644 index 0000000..3f766ee --- /dev/null +++ b/test/compose/dockerd/Dockerfile @@ -0,0 +1,10 @@ +# Stock dockerd with a newer containerd, for the test environment. +# Built by `docker compose` on first start; rebuild with `docker compose build dockerd`. +ARG DOCKER_VERSION=29.7.2 +FROM docker:${DOCKER_VERSION}-dind + +ARG CONTAINERD_VERSION=2.4.1 +ARG TARGETARCH +# Replace the containerd shipped with Docker; dockerd starts the one found on its PATH +RUN wget -qO- https://github.com/containerd/containerd/releases/download/v${CONTAINERD_VERSION}/containerd-static-${CONTAINERD_VERSION}-linux-${TARGETARCH}.tar.gz \ + | tar -xz -C /usr/local diff --git a/test/fixtures/composectl_cmds.go b/test/fixtures/composectl_cmds.go index 3a66b4b..5385323 100644 --- a/test/fixtures/composectl_cmds.go +++ b/test/fixtures/composectl_cmds.go @@ -123,10 +123,18 @@ func (a *App) getAppImages(t *testing.T, composeFilePath string) []string { return images } +// publisherDockerCommand returns a docker CLI command run against the publisher dockerd, +// the daemon that the layers meta script reads image layer sizes from. +func publisherDockerCommand(args ...string) *exec.Cmd { + c := exec.Command("docker", args...) + c.Env = append(os.Environ(), "DOCKER_HOST="+os.Getenv("PUBLISHER_DOCKER_HOST")) + return c +} + func (a *App) pullImages(t *testing.T) { images := a.GetAppImages(t) for _, image := range images { - c := exec.Command("docker", "pull", image) + c := publisherDockerCommand("pull", image) output, cmdErr := c.CombinedOutput() Checkf(t, cmdErr, "failed to pull app images: %s\n", output) } @@ -139,7 +147,7 @@ func (a *App) removeImages(t *testing.T) { if _, ok := removedImages[image]; ok { continue } - c := exec.Command("docker", "image", "rm", image) + c := publisherDockerCommand("image", "rm", image) output, cmdErr := c.CombinedOutput() Checkf(t, cmdErr, "failed to pull app images: %s\n", output) removedImages[image] = true diff --git a/test/fixtures/layers_meta.go b/test/fixtures/layers_meta.go index c42ab87..2bf4949 100644 --- a/test/fixtures/layers_meta.go +++ b/test/fixtures/layers_meta.go @@ -12,7 +12,7 @@ func GenerateLayersMetaFile(t *testing.T, appsRootDir string) string { tmpDir := t.TempDir() layerSizesFile := filepath.Join(tmpDir, "layers_meta.json") c := exec.Command("python", os.Getenv("LAYERS_SIZE_SCRIPT"), "--apps-root", - appsRootDir, "-o", layerSizesFile) + appsRootDir, "-o", layerSizesFile, "--docker-data-root", os.Getenv("PUBLISHER_DOCKER_DATA_ROOT")) output, err := c.CombinedOutput() Checkf(t, err, "failed to run command to gather app layer sizes: %s", string(output)) b, err := os.ReadFile(layerSizesFile) diff --git a/test/integration/image_loading_test.go b/test/integration/image_loading_test.go index f41eda1..601c19b 100644 --- a/test/integration/image_loading_test.go +++ b/test/integration/image_loading_test.go @@ -45,6 +45,9 @@ services: err = loadImages(t, context.Background(), cli, composeApp, layersRoot) f.Check(t, err) + if os.Getenv("DOCKERD_FIO_PATCH") != "1" { + t.Skip("skipping image loading that requires the Foundries patches to dockerd; set DOCKERD_FIO_PATCH=1 to run it") + } err = loadImages(t, context.Background(), cli, composeApp, layersRoot, compose.WithProgressReporting(progressHandler), compose.WithBlobReadingFromStore(), compose.WithRefWithDigest()) f.Check(t, err) From 1c69844f6946c1e274e84adf36ec85c6d9d6c08a Mon Sep 17 00:00:00 2001 From: Mike Sul Date: Thu, 8 Oct 2026 18:47:45 +0200 Subject: [PATCH 4/4] test(compose): run e2e on the fio-patched dockerd too Devices run dockerd with the Foundries patches, while composectl must also work with a stock dockerd on any host. DOCKERD_FIO_PATCH=1 now builds the test dockerd from ghcr.io/foundriesio/moby:29.7.2_fio instead of the stock dind image, besides enabling the patched image loading test. CI runs the suite for each daemon and image store. Signed-off-by: Mike Sul Assisted-by: Claude Code:claude-opus-5-5 --- .github/workflows/ci.yml | 2 ++ test/compose/.env.test | 2 +- test/compose/docker-compose.yml | 6 +++++- test/compose/dockerd/Dockerfile | 11 ++++++++--- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e0802c1..4b60f72 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,10 +29,12 @@ jobs: strategy: fail-fast: false matrix: + dockerd: [stock, fio] containerd-snapshotter: [false, true] runs-on: ubuntu-latest env: CONTAINERD_SNAPSHOTTER: ${{ matrix.containerd-snapshotter }} + DOCKERD_FIO_PATCH: ${{ matrix.dockerd == 'fio' && '1' || '' }} steps: - uses: actions/checkout@v3 - name: build diff --git a/test/compose/.env.test b/test/compose/.env.test index 9056fc1..ac81c99 100644 --- a/test/compose/.env.test +++ b/test/compose/.env.test @@ -10,5 +10,5 @@ REG_CERT=test/compose/registry/certs/registry.crt # Image store of the test dockerd: false - the overlay2 graphdriver, true - the containerd image store CONTAINERD_SNAPSHOTTER=false -# Set to 1 when the test dockerd includes the Foundries patches +# Empty: run tests against the stock dockerd; 1: against the dockerd with the Foundries patches DOCKERD_FIO_PATCH= diff --git a/test/compose/docker-compose.yml b/test/compose/docker-compose.yml index fb8bb2e..b2a70e5 100644 --- a/test/compose/docker-compose.yml +++ b/test/compose/docker-compose.yml @@ -12,9 +12,11 @@ services: - ${REG_DIR}/config.yml:/etc/distribution/config.yml dockerd: - image: composeapp-test-dockerd:29.7.2 + image: composeapp-test-dockerd:29.7.2${DOCKERD_FIO_PATCH:+-fio} build: context: ${CPS_DIR}/dockerd + args: + DOCKERD_FIO_PATCH: ${DOCKERD_FIO_PATCH} command: ["dockerd", "-D", "-H", "unix:///var/run/docker/docker.sock", "--feature", "containerd-snapshotter=${CONTAINERD_SNAPSHOTTER}"] privileged: true @@ -30,6 +32,8 @@ services: image: composeapp-test-dockerd:29.7.2 build: context: ${CPS_DIR}/dockerd + args: + DOCKERD_FIO_PATCH: "" command: ["dockerd", "-D", "-H", "unix:///var/run/docker-publisher/docker.sock", "--feature", "containerd-snapshotter=false"] privileged: true diff --git a/test/compose/dockerd/Dockerfile b/test/compose/dockerd/Dockerfile index 3f766ee..545f6f1 100644 --- a/test/compose/dockerd/Dockerfile +++ b/test/compose/dockerd/Dockerfile @@ -1,8 +1,13 @@ -# Stock dockerd with a newer containerd, for the test environment. -# Built by `docker compose` on first start; rebuild with `docker compose build dockerd`. +# dockerd with a newer containerd, for the test environment. +# Built by `docker compose` on first start; rebuild with `docker compose build`. ARG DOCKER_VERSION=29.7.2 -FROM docker:${DOCKER_VERSION}-dind +# Empty: the stock dockerd; 1: the dockerd with the Foundries patches +ARG DOCKERD_FIO_PATCH= +FROM docker:${DOCKER_VERSION}-dind AS dockerd +FROM ghcr.io/foundriesio/moby:${DOCKER_VERSION}_fio AS dockerd-fio + +FROM dockerd${DOCKERD_FIO_PATCH:+-fio} ARG CONTAINERD_VERSION=2.4.1 ARG TARGETARCH # Replace the containerd shipped with Docker; dockerd starts the one found on its PATH