Tag built images with the commit's short hash (closes #239)

Builds are tagged upaas-<app>:<short hash>, git's short form of the commit checked out, instead of the deployment number. The clone prints the short hash and fails without one. A redeploy of a commit gives the tag to the new image. The cleanup after a deploy now also finds the app's untagged images among those its deployments recorded, and removes them by ID once the app neither runs them nor would roll back to them. It keeps every image any app uses, since apps that build the same commit can share one. commit_sha is now saved on update so manual deploys keep the commit read from the clone. Model: opus-5-5
2026-10-01 22:33:20 +02:00
parent 047945cabf
commit 9488e2faef
10 changed files with 701 additions and 123 deletions
@@ -505,6 +505,7 @@ type cloneConfig struct {
 type CloneResult struct {
 	Output    string // Combined stdout/stderr from git clone
 	CommitSHA string // The HEAD commit SHA after clone/checkout
+	ShortSHA  string // git's short form of CommitSHA (git rev-parse --short)
 }

 // CloneRepo clones a git repository using SSH and optionally checks out a
@@ -567,7 +568,7 @@ func (c *Client) RemoveImage(ctx context.Context, imageID ImageID) error {
 }

 // ListImageTags returns the tags in the given repository, such as
-// "upaas-myapp:12" in "upaas-myapp", each with the ID of its image.
+// "upaas-myapp:1a2b3c4" in "upaas-myapp", each with the ID of its image.
 // Tags the same image has in other repositories are left out.
 func (c *Client) ListImageTags(
 	ctx context.Context,
@@ -597,19 +598,44 @@ func (c *Client) ListImageTags(
 	return tags, nil
 }

-// RemoveImageTag removes a tag such as "upaas-myapp:12", without force.
-// Docker then deletes the image, and the untagged images it was built on,
-// only if no other tag and no container still uses it.
-func (c *Client) RemoveImageTag(ctx context.Context, tag string) error {
+// ListUntaggedImages returns the IDs of the images that have no tag, such
+// as one whose tag a later build gave to the image it built.
+func (c *Client) ListUntaggedImages(ctx context.Context) ([]ImageID, error) {
+	if c.docker == nil {
+		return nil, ErrNotConnected
+	}
+
+	images, err := c.docker.ImageList(ctx, image.ListOptions{
+		Filters: filters.NewArgs(filters.Arg("dangling", "true")),
+	})
+	if err != nil {
+		return nil, fmt.Errorf("failed to list untagged images: %w", err)
+	}
+
+	imageIDs := make([]ImageID, 0, len(images))
+
+	for _, img := range images {
+		imageIDs = append(imageIDs, ImageID(img.ID))
+	}
+
+	return imageIDs, nil
+}
+
+// RemoveImageTag removes the tag name, such as "upaas-myapp:1a2b3c4",
+// without force. Docker then deletes the image, and the untagged images it
+// was built on, only if no other tag and no container still uses it. If name
+// is instead the ID of an untagged image, it removes that image unless a
+// container uses it.
+func (c *Client) RemoveImageTag(ctx context.Context, name string) error {
 	if c.docker == nil {
 		return ErrNotConnected
 	}

-	_, err := c.docker.ImageRemove(ctx, tag, image.RemoveOptions{
+	_, err := c.docker.ImageRemove(ctx, name, image.RemoveOptions{
 		PruneChildren: true,
 	})
 	if err != nil && !client.IsErrNotFound(err) {
-		return fmt.Errorf("failed to remove image tag %s: %w", tag, err)
+		return fmt.Errorf("failed to remove image %s: %w", name, err)
 	}

 	return nil
@@ -905,11 +931,13 @@ func (c *Client) createGitContainer(
 		// Clone without depth limit so we can checkout any commit, then checkout specific SHA
 		script = `git clone --branch "$CLONE_BRANCH" "$CLONE_URL" /repo` +
 			` && cd /repo && git checkout "$CLONE_SHA"` +
-			` && echo COMMIT:$(git rev-parse HEAD)`
+			` && echo COMMIT:$(git rev-parse HEAD)` +
+			` && echo SHORT_SHA:$(git rev-parse --short HEAD)`
 	} else {
 		// Shallow clone of branch HEAD, then output commit SHA
 		script = `git clone --depth 1 --branch "$CLONE_BRANCH" "$CLONE_URL" /repo` +
-			` && cd /repo && echo COMMIT:$(git rev-parse HEAD)`
+			` && cd /repo && echo COMMIT:$(git rev-parse HEAD)` +
+			` && echo SHORT_SHA:$(git rev-parse --short HEAD)`
 	}

 	env := []string{
@@ -987,23 +1015,37 @@ func (c *Client) runGitClone(
 			)
 		}

-		// Parse commit SHA from output (looks for "COMMIT:<sha>" line)
-		commitSHA := parseCommitSHA(logs)
+		// Parse the commit from the "COMMIT:" and "SHORT_SHA:" lines.
+		result := &CloneResult{
+			Output:    logs,
+			CommitSHA: parseCommitSHA(logs, commitMarker),
+			ShortSHA:  parseCommitSHA(logs, shortSHAMarker),
+		}

-		return &CloneResult{Output: logs, CommitSHA: commitSHA}, nil
+		// The short hash names the image the deploy builds.
+		if result.ShortSHA == "" {
+			return nil, fmt.Errorf("%w: no short commit hash in its output: %s",
+				ErrGitCloneFailed, logs)
+		}
+
+		return result, nil
 	}
 }

-// commitMarker is the prefix used to identify commit SHA in clone output.
-const commitMarker = "COMMIT:"
+// Prefixes of the lines in the clone output that carry the commit checked
+// out, in full and in git's short form.
+const (
+	commitMarker   = "COMMIT:"
+	shortSHAMarker = "SHORT_SHA:"
+)

-// parseCommitSHA extracts the commit SHA from git clone output.
-// It looks for a line starting with "COMMIT:" and returns the SHA after it.
-func parseCommitSHA(output string) string {
+// parseCommitSHA extracts a commit SHA from git clone output.
+// It looks for a line starting with marker and returns the SHA after it.
+func parseCommitSHA(output, marker string) string {
 	for line := range strings.SplitSeq(output, "\n") {
 		line = strings.TrimSpace(line)

-		sha, found := strings.CutPrefix(line, commitMarker)
+		sha, found := strings.CutPrefix(line, marker)
 		if found {
 			return strings.TrimSpace(sha)
 		}
@@ -16,6 +16,7 @@ import (
 	"testing"
 	"time"

+	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/client"
 	"github.com/docker/docker/pkg/stdcopy"
 	controlapi "github.com/moby/buildkit/api/services/control"
@@ -205,6 +206,8 @@ func TestPerformCloneRemovesContainerVolumes(t *testing.T) {
 						<-r.Context().Done()
 					case strings.HasSuffix(r.URL.Path, "/wait"):
 						_, _ = fmt.Fprintf(w, `{"StatusCode":%d}`, tt.exitCode)
+					case strings.HasSuffix(r.URL.Path, "/logs"):
+						writeCloneOutput(w)
 					default:
 						_, _ = w.Write([]byte(`{}`))
 					}
@@ -221,18 +224,7 @@ func TestPerformCloneRemovesContainerVolumes(t *testing.T) {

 			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)
+			_, _ = c.performClone(ctx, testCloneConfig(t))

 			select {
 			case query := <-removeQuery:
@@ -246,6 +238,38 @@ func TestPerformCloneRemovesContainerVolumes(t *testing.T) {
 	}
 }

+// testCloneConfig returns the settings of a clone in these tests, with its
+// files in a temporary directory.
+func testCloneConfig(t *testing.T) *cloneConfig {
+	t.Helper()
+
+	dir := t.TempDir()
+
+	return &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"),
+	}
+}
+
+// cloneCommit is the commit the fake clones in these tests check out.
+const cloneCommit = "1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b"
+
+// writeCloneOutput writes the output of a git clone of cloneCommit as
+// Docker sends a container's log: each line after a header.
+func writeCloneOutput(w io.Writer) {
+	stdout := stdcopy.NewStdWriter(w, stdcopy.Stdout)
+	stderr := stdcopy.NewStdWriter(w, stdcopy.Stderr)
+
+	_, _ = stderr.Write([]byte("Cloning into '/repo'...\n"))
+	_, _ = stdout.Write([]byte("COMMIT:" + cloneCommit + "\n"))
+	_, _ = stdout.Write([]byte("SHORT_SHA:1a2b3c4\n"))
+}
+
 // TestPerformBuildUsesBuildKit runs a build against a fake Docker API and
 // checks that it asks for BuildKit and that BuildKit's progress reaches the
 // build log as plain text.
@@ -501,12 +525,11 @@ func serveSession(
 // TestPerformCloneReadsFramedLogs runs a clone against a fake Docker API that
 // sends the clone container's output in frames, as Docker does for a
 // container without a terminal, and checks that the output comes back as
-// plain text and that the commit is read from it.
+// plain text and that the commit, in full and in git's short form, is read
+// from it.
 func TestPerformCloneReadsFramedLogs(t *testing.T) {
 	t.Parallel()

-	const commit = "1647b43aa6b211686719313bc6372c3693c54ca9"
-
 	srv := httptest.NewServer(http.HandlerFunc(
 		func(w http.ResponseWriter, r *http.Request) {
 			w.Header().Set("Content-Type", "application/json")
@@ -517,10 +540,7 @@ func TestPerformCloneReadsFramedLogs(t *testing.T) {
 			case strings.HasSuffix(r.URL.Path, "/wait"):
 				_, _ = w.Write([]byte(`{"StatusCode":0}`))
 			case strings.HasSuffix(r.URL.Path, "/logs"):
-				_, _ = stdcopy.NewStdWriter(w, stdcopy.Stderr).
-					Write([]byte("Cloning into '/repo'...\n"))
-				_, _ = stdcopy.NewStdWriter(w, stdcopy.Stdout).
-					Write([]byte("COMMIT:" + commit + "\n"))
+				writeCloneOutput(w)
 			default:
 				_, _ = w.Write([]byte(`{}`))
 			}
@@ -537,28 +557,126 @@ func TestPerformCloneReadsFramedLogs(t *testing.T) {

 	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"),
-	}
-
-	result, err := c.performClone(t.Context(), cfg)
+	result, err := c.performClone(t.Context(), testCloneConfig(t))
 	if err != nil {
 		t.Fatal(err)
 	}

-	want := "Cloning into '/repo'...\nCOMMIT:" + commit + "\n"
+	want := "Cloning into '/repo'...\nCOMMIT:" + cloneCommit + "\nSHORT_SHA:1a2b3c4\n"
 	if result.Output != want {
 		t.Errorf("got clone output %q, want %q", result.Output, want)
 	}

-	if result.CommitSHA != commit {
-		t.Errorf("got commit %q, want %q", result.CommitSHA, commit)
+	if result.CommitSHA != cloneCommit || result.ShortSHA != "1a2b3c4" {
+		t.Errorf("got commit %q, short %q", result.CommitSHA, result.ShortSHA)
+	}
+}
+
+// TestPerformCloneFailsWithoutShortSHA runs a clone against a fake Docker API
+// whose clone succeeds but prints no "SHORT_SHA:" line, and checks that the
+// clone fails, since the short hash names the image the deploy builds.
+func TestPerformCloneFailsWithoutShortSHA(t *testing.T) {
+	t.Parallel()
+
+	srv := httptest.NewServer(http.HandlerFunc(
+		func(w http.ResponseWriter, r *http.Request) {
+			w.Header().Set("Content-Type", "application/json")
+
+			switch {
+			case strings.HasSuffix(r.URL.Path, "/containers/create"):
+				_, _ = w.Write([]byte(`{"Id":"gitcontainer"}`))
+			case strings.HasSuffix(r.URL.Path, "/wait"):
+				_, _ = w.Write([]byte(`{"StatusCode":0}`))
+			case strings.HasSuffix(r.URL.Path, "/logs"):
+				_, _ = stdcopy.NewStdWriter(w, stdcopy.Stdout).
+					Write([]byte("COMMIT:" + cloneCommit + "\n"))
+			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()}
+
+	_, err = c.performClone(t.Context(), testCloneConfig(t))
+	if !errors.Is(err, ErrGitCloneFailed) {
+		t.Errorf("got error %v, want %v", err, ErrGitCloneFailed)
+	}
+}
+
+// TestPerformCloneAsksGitForShortSHA runs a clone of a branch's last commit
+// and a clone of a given commit against a fake Docker API, and checks that
+// the command each clone container is created with prints git's own short
+// form of the commit checked out.
+func TestPerformCloneAsksGitForShortSHA(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name      string
+		commitSHA string
+	}{
+		{name: "branch", commitSHA: ""},
+		{name: "commit", commitSHA: cloneCommit},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			created := make(chan container.Config, 1)
+
+			srv := httptest.NewServer(http.HandlerFunc(
+				func(w http.ResponseWriter, r *http.Request) {
+					w.Header().Set("Content-Type", "application/json")
+
+					switch {
+					case strings.HasSuffix(r.URL.Path, "/containers/create"):
+						var cfg container.Config
+
+						_ = json.NewDecoder(r.Body).Decode(&cfg)
+						created <- cfg
+
+						_, _ = w.Write([]byte(`{"Id":"gitcontainer"}`))
+					case strings.HasSuffix(r.URL.Path, "/wait"):
+						_, _ = w.Write([]byte(`{"StatusCode":0}`))
+					case strings.HasSuffix(r.URL.Path, "/logs"):
+						writeCloneOutput(w)
+					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()}
+
+			cfg := testCloneConfig(t)
+			cfg.commitSHA = tt.commitSHA
+
+			_, err = c.performClone(t.Context(), cfg)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			cmd := strings.Join((<-created).Cmd, " ")
+			if !strings.Contains(cmd, "echo SHORT_SHA:$(git rev-parse --short HEAD)") {
+				t.Errorf("clone command %q does not print git's short hash", cmd)
+			}
+		})
 	}
 }