Remove the clone container even when the deploy is cancelled (closes #215)

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

TODO.md

@@ -20,6 +20,13 @@ 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
+  still found (#214).
 - 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

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},
 		)
 	}()
 

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")
+			}
+		})
+	}
+}

internal/handlers/app.go

@@ -6,9 +6,11 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"io/fs"
 	"net/http"
 	"net/url"
 	"os"
+	"path"
 	"path/filepath"
 	"strconv"
 	"strings"
@@ -640,7 +642,7 @@ func (h *Handlers) HandleDeploymentLogDownload() http.HandlerFunc {
 		}
 		defer func() { _ = root.Close() }()
 
-		file, openErr := root.Open(relPath)
+		file, openErr := openDeploymentLog(root, relPath)
 		if openErr != nil {
 			http.NotFound(writer, request)
 
@@ -665,6 +667,24 @@ func (h *Handlers) HandleDeploymentLogDownload() http.HandlerFunc {
 	}
 }
 
+// openDeploymentLog opens a deployment log file inside the log root.
+// Logs written by older versions sit one directory deeper, under the
+// hostname of the container that wrote them, so when the file is not at
+// relPath it is looked for under any directory directly below the root.
+func openDeploymentLog(root *os.Root, relPath string) (*os.File, error) {
+	file, err := root.Open(relPath)
+	if err == nil {
+		return file, nil
+	}
+
+	matches, globErr := fs.Glob(root.FS(), path.Join("*", filepath.ToSlash(relPath)))
+	if globErr != nil || len(matches) == 0 {
+		return nil, err
+	}
+
+	return root.Open(matches[0])
+}
+
 // containerLogsAPITail is the default number of log lines for the container logs API.
 const containerLogsAPITail = "100"
 

internal/handlers/log_download_test.go

@@ -69,6 +69,56 @@ func TestHandleDeploymentLogDownloadServesLegitimateFile(t *testing.T) {
 	assert.Contains(t, recorder.Body.String(), "deploy log contents")
 }
 
+// TestGetLogFilePathHasNoHostname verifies a deployment log is stored
+// directly under logs/<appname>/, with no hostname directory in between,
+// so it is still found after the container is recreated with a new
+// hostname.
+func TestGetLogFilePathHasNoHostname(t *testing.T) {
+	t.Parallel()
+
+	testCtx := setupTestHandlers(t)
+	createdApp := createTestApp(t, testCtx, "log-path-app")
+
+	deployment := models.NewDeployment(testCtx.database)
+	deployment.AppID = createdApp.ID
+
+	logPath := testCtx.deploySvc.GetLogFilePath(createdApp, deployment)
+
+	assert.Equal(t,
+		filepath.Join(testCtx.deploySvc.GetLogDir(), createdApp.Name),
+		filepath.Dir(logPath),
+	)
+}
+
+// TestHandleDeploymentLogDownloadServesLogFromOldHostnameDir verifies a
+// log written by an older version, under the hostname of a container that
+// has since been recreated, can still be downloaded.
+func TestHandleDeploymentLogDownloadServesLogFromOldHostnameDir(t *testing.T) {
+	t.Parallel()
+
+	testCtx := setupTestHandlers(t)
+	createdApp := createTestApp(t, testCtx, "log-old-hostname-app")
+
+	deployment := models.NewDeployment(testCtx.database)
+	deployment.AppID = createdApp.ID
+	deployment.Status = models.DeploymentStatusSuccess
+	require.NoError(t, deployment.Save(context.Background()))
+
+	logDir := testCtx.deploySvc.GetLogDir()
+	newPath := testCtx.deploySvc.GetLogFilePath(createdApp, deployment)
+	relPath, relErr := filepath.Rel(logDir, newPath)
+	require.NoError(t, relErr)
+
+	oldPath := filepath.Join(logDir, "old-container-hostname", relPath)
+	require.NoError(t, os.MkdirAll(filepath.Dir(oldPath), 0o750))
+	require.NoError(t, os.WriteFile(oldPath, []byte("old deploy log contents"), 0o600))
+
+	recorder := doLogDownload(t, testCtx, createdApp.ID, deployment.ID)
+
+	assert.Equal(t, http.StatusOK, recorder.Code)
+	assert.Contains(t, recorder.Body.String(), "old deploy log contents")
+}
+
 // TestHandleDeploymentLogDownloadRejectsPathTraversal verifies the
 // os.Root containment guard. A traversal-shaped app name drives the
 // resolved log path out of the deploy log directory onto a sentinel

internal/service/deploy/deploy.go

@@ -261,15 +261,14 @@ func (svc *Service) GetBuildDir(appName string) string {
 
 // GetLogFilePath returns the path to the log file for a deployment.
 // Returns empty string if the path cannot be determined.
+//
+// The path must not depend on the container's hostname: Docker assigns a
+// new one whenever the container is recreated, and older logs would then
+// no longer be found.
 func (svc *Service) GetLogFilePath(
 	app *models.App,
 	deployment *models.Deployment,
 ) string {
-	hostname, err := os.Hostname()
-	if err != nil {
-		hostname = "unknown"
-	}
-
 	// Get commit SHA
 	sha := ""
 	if deployment.CommitSHA.Valid && deployment.CommitSHA.String != "" {
@@ -291,7 +290,7 @@ func (svc *Service) GetLogFilePath(
 		filename = fmt.Sprintf("%s_%s.log.txt", app.Name, timestamp)
 	}
 
-	return filepath.Join(svc.config.DataDir, "logs", hostname, app.Name, filename)
+	return filepath.Join(svc.config.DataDir, "logs", app.Name, filename)
 }
 
 // GetLogDir returns the root directory under which all deployment log
@@ -1294,7 +1293,7 @@ func (svc *Service) failDeployment(
 }
 
 // writeLogsToFile writes the deployment logs to a file on disk.
-// Structure: DataDir/logs/<hostname>/<appname>/<appname>_<sha>_<timestamp>.log.txt
+// Structure: DataDir/logs/<appname>/<appname>_<sha>_<timestamp>.log.txt
 func (svc *Service) writeLogsToFile(app *models.App, deployment *models.Deployment) {
 	if !deployment.Logs.Valid || deployment.Logs.String == "" {
 		return