Remove the git clone container's anonymous volume with it (closes #215)
The pinned `alpine/git` image declares a volume at `/git`, so every clone container got an anonymous volume, and the container was removed without its volumes, leaving one volume behind per deploy. The clone container is now removed together with its volumes, whether the clone succeeds, fails or is cancelled; the removal uses a context that outlives cancellation. Tests cover success, failure and cancellation against a fake Docker API, and a manual check against real Docker is recorded on the PR. The old app container's own anonymous volumes are still kept on redeploy, since deleting them could discard app data. Model: opus-5-5
This commit was merged in pull request #217.
This commit is contained in:
@@ -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/<appname>/`
|
||||
instead of `logs/<hostname>/<appname>/`, so downloads keep working after the
|
||||
upaas container is recreated; logs written under an old hostname directory are
|
||||
|
||||
@@ -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},
|
||||
)
|
||||
}()
|
||||
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user