2 Commits
Author SHA1 Message Date
sneak 622ec6d18d Remove the clone container even when the deploy is cancelled (closes #215)
Check / check (pull_request) Skipped
The deferred removal used the deploy context, so a cancelled deploy
(a newer push, or the build timeout) never sent the remove request and
left the clone container and its volume behind. It now uses
context.WithoutCancel, and the test gains a case that cancels the
context while the clone is running.

Model: opus-5-5
2026-09-23 09:28:11 +00:00
sneak 86df8f558a Remove the git clone container's anonymous volume with it (closes #215)
Check / check (pull_request) Skipped
The pinned alpine/git image declares a volume at /git, so Docker gives
every clone container an anonymous volume. The container was removed
without its volumes, leaving one volume behind per deploy. The removal
now also removes the container's anonymous volumes, on success and on
failure. A test runs a clone against a fake Docker API and checks the
removal asks for volumes to be removed.

Model: opus-5-5
2026-09-23 09:06:48 +00:00
3 changed files with 98 additions and 2 deletions
+3
View File
@@ -20,6 +20,9 @@ regress.
# Completed Steps # 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` - 2026-09-23: Fixed the flaky `t.TempDir` cleanup race in `internal/handlers`
(the one fixed in `internal/service/webhook` by #198): (the one fixed in `internal/service/webhook` by #198):
`TestHandleWebhookProcessesValidWebhook` now waits with the webhook service's `TestHandleWebhookProcessesValidWebhook` now waits with the webhook service's
+5 -2
View File
@@ -656,11 +656,14 @@ func (c *Client) performClone(
return nil, err 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() { defer func() {
_ = c.docker.ContainerRemove( _ = c.docker.ContainerRemove(
ctx, context.WithoutCancel(ctx),
gitContainerID.String(), gitContainerID.String(),
container.RemoveOptions{Force: true}, container.RemoveOptions{Force: true, RemoveVolumes: true},
) )
}() }()
+90
View File
@@ -1,9 +1,18 @@
package docker //nolint:testpackage // tests unexported regexps and Client struct package docker //nolint:testpackage // tests unexported regexps and Client struct
import ( import (
"context"
"errors" "errors"
"fmt"
"log/slog" "log/slog"
"net/http"
"net/http/httptest"
"net/url"
"path/filepath"
"strings"
"testing" "testing"
"github.com/docker/docker/client"
) )
// mainBranch is the branch name used across validation tests. // 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")
}
})
}
}