diff --git a/TODO.md b/TODO.md index 5f47fa4..d0b661c 100644 --- a/TODO.md +++ b/TODO.md @@ -20,6 +20,9 @@ regress. # Completed Steps +- 2026-09-23: The git clone container is now removed together with its anonymous + volume (the `alpine/git` image declares one at `/git`), so a deploy no longer + leaves a Docker volume behind (#215). - 2026-09-23: Fixed the flaky `t.TempDir` cleanup race in `internal/handlers` (the one fixed in `internal/service/webhook` by #198): `TestHandleWebhookProcessesValidWebhook` now waits with the webhook service's diff --git a/internal/docker/client.go b/internal/docker/client.go index 168f3fa..eb56a73 100644 --- a/internal/docker/client.go +++ b/internal/docker/client.go @@ -656,11 +656,13 @@ func (c *Client) performClone( return nil, err } + // The git image declares a volume, so Docker gives each clone container + // an anonymous volume; remove it with the container. defer func() { _ = c.docker.ContainerRemove( ctx, gitContainerID.String(), - container.RemoveOptions{Force: true}, + container.RemoveOptions{Force: true, RemoveVolumes: true}, ) }() diff --git a/internal/docker/validation_test.go b/internal/docker/validation_test.go index 2d033c8..9a57b82 100644 --- a/internal/docker/validation_test.go +++ b/internal/docker/validation_test.go @@ -2,8 +2,16 @@ package docker //nolint:testpackage // tests unexported regexps and Client struc import ( "errors" + "fmt" "log/slog" + "net/http" + "net/http/httptest" + "net/url" + "path/filepath" + "strings" "testing" + + "github.com/docker/docker/client" ) // mainBranch is the branch name used across validation tests. @@ -149,3 +157,67 @@ func TestCloneRepoRejectsInjection(t *testing.T) { }) } } + +// TestPerformCloneRemovesContainerVolumes runs a clone against a fake Docker +// API and checks that the clone container is removed together with its +// anonymous volumes, whether the clone succeeds or fails. +func TestPerformCloneRemovesContainerVolumes(t *testing.T) { + t.Parallel() + + for _, exitCode := range []int{0, 1} { + t.Run(fmt.Sprintf("exit code %d", exitCode), func(t *testing.T) { + t.Parallel() + + removeQuery := make(chan url.Values, 1) + + srv := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + + switch { + case r.Method == http.MethodDelete: + removeQuery <- r.URL.Query() + case strings.HasSuffix(r.URL.Path, "/containers/create"): + _, _ = w.Write([]byte(`{"Id":"gitcontainer"}`)) + case strings.HasSuffix(r.URL.Path, "/wait"): + _, _ = fmt.Fprintf(w, `{"StatusCode":%d}`, exitCode) + default: + _, _ = w.Write([]byte(`{}`)) + } + }, + )) + t.Cleanup(srv.Close) + + dockerAPI, err := client.NewClientWithOpts( + client.WithHost("tcp://" + srv.Listener.Addr().String()), + ) + if err != nil { + t.Fatal(err) + } + + c := &Client{docker: dockerAPI, log: slog.Default()} + + dir := t.TempDir() + cfg := &cloneConfig{ + repoURL: "git@example.com:repo.git", + branch: mainBranch, + sshPrivateKey: "fake-key", + containerDir: filepath.Join(dir, "repo"), + hostDir: filepath.Join(dir, "repo"), + keyFile: filepath.Join(dir, "deploy_key"), + hostKeyFile: filepath.Join(dir, "deploy_key"), + } + + _, _ = c.performClone(t.Context(), cfg) + + select { + case query := <-removeQuery: + if query.Get("v") != "1" { + t.Errorf("clone container removed without its volumes: %v", query) + } + default: + t.Error("clone container was not removed") + } + }) + } +}