diff --git a/TODO.md b/TODO.md index 72043db..27bd714 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: Deployment log files are now stored under `logs//` instead of `logs///`, so downloads keep working after the upaas container is recreated; logs written under an old hostname directory are diff --git a/internal/docker/client.go b/internal/docker/client.go index 168f3fa..9e6abc5 100644 --- a/internal/docker/client.go +++ b/internal/docker/client.go @@ -656,11 +656,14 @@ 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. The removal must + // still run when the deploy is cancelled. defer func() { _ = c.docker.ContainerRemove( - ctx, + context.WithoutCancel(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..3ea3378 100644 --- a/internal/docker/validation_test.go +++ b/internal/docker/validation_test.go @@ -1,9 +1,18 @@ package docker //nolint:testpackage // tests unexported regexps and Client struct import ( + "context" "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 +158,84 @@ 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, fails, or is cancelled. +func TestPerformCloneRemovesContainerVolumes(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + exitCode int + cancel bool + }{ + {name: "succeeds", exitCode: 0}, + {name: "fails", exitCode: 1}, + {name: "cancelled", cancel: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithCancel(t.Context()) + t.Cleanup(cancel) + + 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") && tt.cancel: + // Cancel the deploy while the clone is running. + cancel() + <-r.Context().Done() + case strings.HasSuffix(r.URL.Path, "/wait"): + _, _ = fmt.Fprintf(w, `{"StatusCode":%d}`, tt.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(ctx, 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") + } + }) + } +}