diff --git a/TODO.md b/TODO.md index 4b9ed0d..923d1d5 100644 --- a/TODO.md +++ b/TODO.md @@ -20,6 +20,15 @@ regress. # Completed Steps +- 2026-09-29: Built images are tagged `upaas-:`, git's short + form of the commit built, instead of the deployment number. A redeploy of a + commit gives its tag to the new image; the old one is kept while the app runs + it or Rollback would start it, then removed by its ID, found among the images + the app's deployments recorded. The removal of old images now keeps every + image any app runs or would roll back to. The clone now reads the commit from + git's output, which Docker's log headers had hidden, so manual deploys record + their commit too (#239). + - 2026-09-29: A failed build now fails the deploy with the build's own error instead of a later "failed to inspect image", and the deployment log shows the end of the build output before that error. upaas refuses to build on a Docker diff --git a/internal/docker/client.go b/internal/docker/client.go index 2af6f52..a8e1e32 100644 --- a/internal/docker/client.go +++ b/internal/docker/client.go @@ -25,6 +25,7 @@ import ( "github.com/docker/docker/client" "github.com/docker/docker/pkg/archive" "github.com/docker/docker/pkg/jsonmessage" + "github.com/docker/docker/pkg/stdcopy" "github.com/docker/go-connections/nat" controlapi "github.com/moby/buildkit/api/services/control" buildkitclient "github.com/moby/buildkit/client" @@ -491,6 +492,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 @@ -553,7 +555,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, @@ -583,19 +585,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 @@ -850,11 +877,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{ @@ -921,7 +950,7 @@ func (c *Client) runGitClone( return nil, fmt.Errorf("error waiting for git container: %w", err) case status := <-statusCh: // Always capture logs for the result - logs, _ := c.ContainerLogs(ctx, containerID, "100") + logs := c.gitContainerOutput(ctx, containerID) if status.StatusCode != 0 { return nil, fmt.Errorf( @@ -932,23 +961,62 @@ func (c *Client) runGitClone( ) } - // Parse commit SHA from output (looks for "COMMIT:" 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:" +// gitContainerOutput returns the last 100 lines the git container wrote. +// Docker puts a header before each line a container without a terminal +// writes; stdcopy removes them so that the lines can be parsed. +func (c *Client) gitContainerOutput( + ctx context.Context, + containerID ContainerID, +) string { + reader, err := c.docker.ContainerLogs(ctx, containerID.String(), container.LogsOptions{ + ShowStdout: true, + ShowStderr: true, + Tail: "100", + }) + if err != nil { + return "" + } -// 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 { + defer func() { _ = reader.Close() }() + + var output strings.Builder + + _, _ = stdcopy.StdCopy(&output, &output, reader) + + return output.String() +} + +// 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 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) } diff --git a/internal/docker/validation_test.go b/internal/docker/validation_test.go index f496655..c56cb56 100644 --- a/internal/docker/validation_test.go +++ b/internal/docker/validation_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "log/slog" "net/http" "net/http/httptest" @@ -16,6 +17,7 @@ import ( "time" "github.com/docker/docker/client" + "github.com/docker/docker/pkg/stdcopy" controlapi "github.com/moby/buildkit/api/services/control" ) @@ -203,6 +205,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(`{}`)) } @@ -244,6 +248,59 @@ func TestPerformCloneRemovesContainerVolumes(t *testing.T) { } } +// 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) { + _, _ = stdcopy.NewStdWriter(w, stdcopy.Stderr).Write([]byte("Cloning into '/repo'...\n")) + _, _ = stdcopy.NewStdWriter(w, stdcopy.Stdout).Write([]byte("COMMIT:" + cloneCommit + "\n")) + _, _ = stdcopy.NewStdWriter(w, stdcopy.Stdout).Write([]byte("SHORT_SHA:1a2b3c4\n")) +} + +// TestCloneRepoReadsCommit runs a clone against a fake Docker API and +// checks that the commit checked out is read from the clone's output, in +// full and in git's short form. +func TestCloneRepoReadsCommit(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.HasSuffix(r.URL.Path, "/containers/create"): + _, _ = w.Write([]byte(`{"Id":"gitcontainer"}`)) + 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()} + + result, err := c.CloneRepo( + t.Context(), "git@example.com:repo.git", mainBranch, "", "fake-key", + t.TempDir(), t.TempDir(), + ) + if err != nil { + t.Fatal(err) + } + + if result.CommitSHA != cloneCommit || result.ShortSHA != "1a2b3c4" { + t.Errorf("got commit %q, short %q", result.CommitSHA, result.ShortSHA) + } +} + // 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. diff --git a/internal/models/deployment.go b/internal/models/deployment.go index 874b8d4..a024c75 100644 --- a/internal/models/deployment.go +++ b/internal/models/deployment.go @@ -203,11 +203,12 @@ func (d *Deployment) insert(ctx context.Context) error { func (d *Deployment) update(ctx context.Context) error { query := ` UPDATE deployments SET - image_id = ?, container_id = ?, status = ?, logs = ?, finished_at = ? + commit_sha = ?, image_id = ?, container_id = ?, status = ?, logs = ?, + finished_at = ? WHERE id = ?` _, err := d.db.Exec(ctx, query, - d.ImageID, d.ContainerID, d.Status, d.Logs, d.FinishedAt, d.ID, + d.CommitSHA, d.ImageID, d.ContainerID, d.Status, d.Logs, d.FinishedAt, d.ID, ) return err @@ -295,6 +296,45 @@ func FindDeploymentsByAppID( return deployments, nil } +// FindDeploymentImageIDs returns the IDs of the images an app's +// deployments built or rolled back to. +func FindDeploymentImageIDs( + ctx context.Context, + deployDB *database.Database, + appID string, +) ([]string, error) { + rows, err := deployDB.Query(ctx, ` + SELECT DISTINCT image_id FROM deployments + WHERE app_id = ? AND image_id IS NOT NULL`, + appID, + ) + if err != nil { + return nil, fmt.Errorf("querying deployment image IDs: %w", err) + } + + defer func() { _ = rows.Close() }() + + var imageIDs []string + + for rows.Next() { + var imageID string + + scanErr := rows.Scan(&imageID) + if scanErr != nil { + return nil, fmt.Errorf("scanning deployment image ID: %w", scanErr) + } + + imageIDs = append(imageIDs, imageID) + } + + rowsErr := rows.Err() + if rowsErr != nil { + return nil, fmt.Errorf("iterating deployment image IDs: %w", rowsErr) + } + + return imageIDs, nil +} + // LatestDeploymentForApp finds the most recent deployment for an app. // //nolint:nilnil // returning nil,nil is idiomatic for "not found" in Active Record diff --git a/internal/service/deploy/deploy.go b/internal/service/deploy/deploy.go index 4245be2..ed43395 100644 --- a/internal/service/deploy/deploy.go +++ b/internal/service/deploy/deploy.go @@ -735,44 +735,103 @@ func (svc *Service) recordDeployedImage( return nil } -// removeUnusedImages removes the app's tags (upaas-:, set by -// buildImage) except those of the image the running container uses and the -// one Rollback would start. Docker deletes an image only once no other tag, -// such as another app's, and no container still uses it. +// removeUnusedImages removes the app's images except those an app's running +// container uses or its Rollback would start. Docker deletes a tagged image +// only once no other tag, such as another app's, and no container still +// uses it. func (svc *Service) removeUnusedImages( ctx context.Context, app *models.App, deployment *models.Deployment, ) { - tags, err := svc.docker.ListImageTags(ctx, "upaas-"+app.Name) + images, err := svc.findAppImages(ctx, app) if err != nil { svc.log.Error("failed to list app images", "error", err, "app", app.Name) return } - for _, tag := range slices.Sorted(maps.Keys(tags)) { - imageID := tags[tag].String() - if imageID == app.ImageID.String || imageID == app.PreviousImageID.String { + keep, err := svc.imagesToKeep(ctx) + if err != nil { + svc.log.Error("failed to list the images apps use", "error", err, "app", app.Name) + + return + } + + for _, name := range slices.Sorted(maps.Keys(images)) { + if keep[images[name].String()] { continue } - removeErr := svc.docker.RemoveImageTag(ctx, tag) + removeErr := svc.docker.RemoveImageTag(ctx, name) if removeErr != nil { svc.log.Error("failed to remove old image", - "error", removeErr, "app", app.Name, "tag", tag) + "error", removeErr, "app", app.Name, "image", name) _ = deployment.AppendLog( ctx, - "WARNING: failed to remove old image "+tag+": "+removeErr.Error(), + "WARNING: failed to remove old image "+name+": "+removeErr.Error(), ) continue } - _ = deployment.AppendLog(ctx, "Removed old image: "+tag) + _ = deployment.AppendLog(ctx, "Removed old image: "+name) } } +// findAppImages returns the app's images, each under the name it is removed +// by: its tag, upaas-: as set by buildImage, or its ID if +// it has none. A redeploy of a commit gives the commit's tag to the new +// image, so the old one is found among the images the app's deployments +// recorded. +func (svc *Service) findAppImages( + ctx context.Context, + app *models.App, +) (map[string]docker.ImageID, error) { + images, err := svc.docker.ListImageTags(ctx, "upaas-"+app.Name) + if err != nil { + return nil, fmt.Errorf("failed to list image tags: %w", err) + } + + untagged, err := svc.docker.ListUntaggedImages(ctx) + if err != nil { + return nil, fmt.Errorf("failed to list untagged images: %w", err) + } + + recorded, err := models.FindDeploymentImageIDs(ctx, svc.db, app.ID) + if err != nil { + return nil, fmt.Errorf("failed to find deployment images: %w", err) + } + + for _, imageID := range untagged { + if slices.Contains(recorded, imageID.String()) { + images[imageID.String()] = imageID + } + } + + return images, nil +} + +// imagesToKeep returns the IDs of the image each app's running container +// uses and the one its Rollback would start. It covers every app because +// apps that build the same commit can share an image, and a redeploy can +// take the tag that kept the image for one of them. +func (svc *Service) imagesToKeep(ctx context.Context) (map[string]bool, error) { + apps, err := models.AllApps(ctx, svc.db) + if err != nil { + return nil, fmt.Errorf("failed to list apps: %w", err) + } + + keep := make(map[string]bool) + + for _, app := range apps { + keep[app.ImageID.String] = true + keep[app.PreviousImageID.String] = true + } + + return keep, nil +} + // cleanupCancelledDeploy removes orphan resources left by a cancelled deployment. func (svc *Service) cleanupCancelledDeploy( ctx context.Context, @@ -912,14 +971,14 @@ func (svc *Service) buildImage( app *models.App, deployment *models.Deployment, ) (docker.ImageID, error) { - workDir, cleanup, err := svc.cloneRepository(ctx, app, deployment) + workDir, shortSHA, cleanup, err := svc.cloneRepository(ctx, app, deployment) if err != nil { return "", err } defer cleanup() - imageTag := fmt.Sprintf("upaas-%s:%d", app.Name, deployment.ID) + imageTag := "upaas-" + app.Name + ":" + shortSHA // Create log writer that flushes build output to deployment logs every second logWriter := newDeploymentLogWriter(ctx, deployment) @@ -954,11 +1013,14 @@ func (svc *Service) buildImage( return imageID, nil } +// cloneRepository clones the app's repository for a build. It returns the +// directory of the clone, git's short form of the commit checked out, and a +// function that removes the clone. func (svc *Service) cloneRepository( ctx context.Context, app *models.App, deployment *models.Deployment, -) (string, func(), error) { +) (string, string, func(), error) { // Use a subdirectory of DataDir for builds since it's mounted from the host // and accessible to Docker for bind mounts (unlike /tmp inside the container). // Structure: builds//-/ @@ -975,7 +1037,7 @@ func (svc *Service) cloneRepository( fmt.Errorf("failed to create builds dir: %w", err), ) - return "", nil, fmt.Errorf("failed to create builds dir: %w", err) + return "", "", nil, fmt.Errorf("failed to create builds dir: %w", err) } buildDir, err := os.MkdirTemp(appBuildsDir, fmt.Sprintf("%d-*", deployment.ID)) @@ -987,7 +1049,7 @@ func (svc *Service) cloneRepository( fmt.Errorf("failed to create temp dir: %w", err), ) - return "", nil, fmt.Errorf("failed to create temp dir: %w", err) + return "", "", nil, fmt.Errorf("failed to create temp dir: %w", err) } cleanup := func() { _ = os.RemoveAll(buildDir) } @@ -1023,7 +1085,7 @@ func (svc *Service) cloneRepository( fmt.Errorf("failed to clone repo: %w", cloneErr), ) - return "", nil, fmt.Errorf("failed to clone repo: %w", cloneErr) + return "", "", nil, fmt.Errorf("failed to clone repo: %w", cloneErr) } svc.processCloneResult(ctx, app, deployment, cloneResult, commitSHA) @@ -1031,7 +1093,7 @@ func (svc *Service) cloneRepository( // Return the 'work' subdirectory where the repo was cloned workDir := filepath.Join(buildDir, "work") - return workDir, cleanup, nil + return workDir, cloneResult.ShortSHA, cleanup, nil } // processCloneResult handles the result of a git clone operation. diff --git a/internal/service/deploy/deploy_build_test.go b/internal/service/deploy/deploy_build_test.go index a2cf9f4..655db3d 100644 --- a/internal/service/deploy/deploy_build_test.go +++ b/internal/service/deploy/deploy_build_test.go @@ -34,6 +34,8 @@ func TestBuildImageLogsBuildErrorBeforeDeployError(t *testing.T) { switch { case strings.HasSuffix(r.URL.Path, "/containers/create"): _, _ = w.Write([]byte(`{"Id":"gitcontainer"}`)) + case strings.HasSuffix(r.URL.Path, "/logs"): + writeCloneOutput(w, "abc1234") case strings.HasSuffix(r.URL.Path, "/version"): _, _ = w.Write([]byte(`{"Version":"27.3.1","ApiVersion":"1.47"}`)) case strings.HasSuffix(r.URL.Path, "/build"): @@ -77,7 +79,7 @@ func TestBuildImageLogsBuildErrorBeforeDeployError(t *testing.T) { // The service has no notify service: the app has no ntfy topic and no // Slack webhook, so the build failure notification sends nothing. - svc := deploy.NewTestServiceWithConfig(log, cfg, dockerClient) + svc := deploy.NewTestServiceWithConfig(log, cfg, db, dockerClient) _, err = svc.BuildImage(ctx, app, deployment) require.EqualError(t, err, "failed to build image: exit code: 1") diff --git a/internal/service/deploy/deploy_cleanup_test.go b/internal/service/deploy/deploy_cleanup_test.go index 42f7954..4742731 100644 --- a/internal/service/deploy/deploy_cleanup_test.go +++ b/internal/service/deploy/deploy_cleanup_test.go @@ -20,7 +20,7 @@ func TestCleanupCancelledDeploy_RemovesBuildDir(t *testing.T) { tmpDir := t.TempDir() cfg := &config.Config{DataDir: tmpDir} - svc := deploy.NewTestServiceWithConfig(slog.Default(), cfg, nil) + svc := deploy.NewTestServiceWithConfig(slog.Default(), cfg, nil, nil) // Create a fake build directory matching the deployment pattern appName := "test-app" @@ -59,7 +59,7 @@ func TestCleanupCancelledDeploy_NoBuildDir(t *testing.T) { tmpDir := t.TempDir() cfg := &config.Config{DataDir: tmpDir} - svc := deploy.NewTestServiceWithConfig(slog.Default(), cfg, nil) + svc := deploy.NewTestServiceWithConfig(slog.Default(), cfg, nil, nil) // Should not panic when build dir doesn't exist svc.CleanupCancelledDeploy(context.Background(), "nonexistent-app", 1, "") diff --git a/internal/service/deploy/deploy_images_test.go b/internal/service/deploy/deploy_images_test.go index 086ccb3..006077c 100644 --- a/internal/service/deploy/deploy_images_test.go +++ b/internal/service/deploy/deploy_images_test.go @@ -3,14 +3,20 @@ package deploy_test import ( "context" "database/sql" + "encoding/json" + "fmt" + "io" "log/slog" + "maps" "net/http" "net/http/httptest" "os" + "slices" "strings" "sync" "testing" + "github.com/docker/docker/pkg/stdcopy" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/fx/fxtest" @@ -23,45 +29,135 @@ import ( "sneak.berlin/go/upaas/internal/service/deploy" ) -// TestRecordDeployedImageRemovesOldImages runs the step after a deploy -// against a fake Docker API. Image one is also tagged for another app, -// image two was the previous image, three the current one, four is new. -func TestRecordDeployedImageRemovesOldImages(t *testing.T) { - t.Parallel() +// fakeImageAPI is a fake Docker API that keeps images and their tags as +// Docker does: a build gives its tag to the image it builds, and removing +// an image's last tag, or an untagged image by its ID, deletes the image. +// It also answers the steps of a git clone that reports shortSHA. +type fakeImageAPI struct { + mu sync.Mutex + images map[string][]string // image ID -> tags + shortSHA string // the commit's short hash the clone reports + nextID string // ID of the image the next build creates + built []string // the tag of each build + removed []string // each tag or ID removed + forced bool // whether a removal was forced +} - var ( - mu sync.Mutex - removed []string - forced bool - ) +func (api *fakeImageAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) { + api.mu.Lock() + defer api.mu.Unlock() - srv := httptest.NewServer(http.HandlerFunc( - func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") + w.Header().Set("Content-Type", "application/json") - switch { - case r.Method == http.MethodDelete: - _, name, _ := strings.Cut(r.URL.Path, "/images/") + _, name, isImage := strings.Cut(r.URL.Path, "/images/") - mu.Lock() + switch { + case strings.HasSuffix(r.URL.Path, "/images/json"): + dangling := strings.Contains(r.URL.Query().Get("filters"), "dangling") + api.listImages(w, dangling) + case isImage && r.Method == http.MethodDelete: + api.forced = api.forced || r.URL.Query().Get("force") != "" + api.removeImage(w, name) + case isImage && strings.HasSuffix(name, "/json"): + api.inspectImage(w, strings.TrimSuffix(name, "/json")) + case strings.HasSuffix(r.URL.Path, "/build"): + tag := r.URL.Query().Get("t") + api.built = append(api.built, tag) + api.untag(tag) + api.images[api.nextID] = append(api.images[api.nextID], tag) + case strings.HasSuffix(r.URL.Path, "/version"): + _, _ = w.Write([]byte(`{"Version":"27.3.1","ApiVersion":"1.47"}`)) + case strings.HasSuffix(r.URL.Path, "/containers/create"): + _, _ = w.Write([]byte(`{"Id":"gitcontainer"}`)) + case strings.HasSuffix(r.URL.Path, "/logs"): + writeCloneOutput(w, api.shortSHA) + default: + // The other steps of the git clone, which succeeds. + _, _ = w.Write([]byte(`{}`)) + } +} - removed = append(removed, name) - forced = forced || r.URL.Query().Get("force") != "" - mu.Unlock() +// listImages lists the untagged images, or else the tagged ones. +func (api *fakeImageAPI) listImages(w http.ResponseWriter, dangling bool) { + list := []map[string]any{} - _, _ = w.Write([]byte(`[]`)) - case strings.HasSuffix(r.URL.Path, "/images/json"): - _, _ = w.Write([]byte(`[ - {"Id":"sha256:one","RepoTags":["upaas-myapp:1","upaas-otherapp:7"]}, - {"Id":"sha256:two","RepoTags":["upaas-myapp:2"]}, - {"Id":"sha256:three","RepoTags":["upaas-myapp:3"]}, - {"Id":"sha256:four","RepoTags":["upaas-myapp:4"]} - ]`)) - default: - _, _ = w.Write([]byte(`{}`)) - } - }, - )) + for _, id := range slices.Sorted(maps.Keys(api.images)) { + if (len(api.images[id]) == 0) == dangling { + list = append(list, map[string]any{"Id": id, "RepoTags": api.images[id]}) + } + } + + _ = json.NewEncoder(w).Encode(list) +} + +func (api *fakeImageAPI) inspectImage(w http.ResponseWriter, name string) { + for id, tags := range api.images { + if id == name || slices.Contains(tags, name) { + _ = json.NewEncoder(w).Encode(map[string]any{"Id": id, "RepoTags": tags}) + + return + } + } + + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message":"No such image"}`)) +} + +func (api *fakeImageAPI) removeImage(w http.ResponseWriter, name string) { + api.removed = append(api.removed, name) + + id := name + if _, isID := api.images[name]; !isID { + id = api.untag(name) + } + + if len(api.images[id]) == 0 { + delete(api.images, id) + } + + _, _ = w.Write([]byte(`[]`)) +} + +// untag removes tag from the image that has it, leaving the image, and +// returns the image's ID. +func (api *fakeImageAPI) untag(tag string) string { + for id, tags := range api.images { + if slices.Contains(tags, tag) { + api.images[id] = slices.DeleteFunc(tags, func(t string) bool { return t == tag }) + + return id + } + } + + return "" +} + +// state returns each image's tags, the tag of each build and each tag or +// ID removed. +func (api *fakeImageAPI) state() (map[string][]string, []string, []string) { + api.mu.Lock() + defer api.mu.Unlock() + + return maps.Clone(api.images), slices.Clone(api.built), slices.Clone(api.removed) +} + +// writeCloneOutput writes the line of a git clone's output that gives the +// commit's short hash, as Docker sends a container's log: after a header. +func writeCloneOutput(w io.Writer, shortSHA string) { + out := stdcopy.NewStdWriter(w, stdcopy.Stdout) + + _, _ = fmt.Fprintf(out, "SHORT_SHA:%s\n", shortSHA) +} + +// newImageTestService returns a deploy service that uses api as its +// Docker API, and its database. +func newImageTestService( + t *testing.T, + api *fakeImageAPI, +) (*deploy.Service, *database.Database) { + t.Helper() + + srv := httptest.NewServer(api) t.Cleanup(srv.Close) log := slog.New(slog.NewTextHandler(os.Stderr, nil)) @@ -77,30 +173,152 @@ func TestRecordDeployedImageRemovesOldImages(t *testing.T) { t.Cleanup(lifecycle.RequireStop) db := database.NewTestDatabase(t) - ctx := context.Background() + dataDir := t.TempDir() + cfg := &config.Config{DataDir: dataDir, HostDataDir: dataDir} + + return deploy.NewTestServiceWithConfig(log, cfg, db, dockerClient), db +} + +// saveApp saves an app with the given current and previous image. +func saveApp( + t *testing.T, + db *database.Database, + name, imageID, previousImageID string, +) *models.App { + t.Helper() app := models.NewApp(db) - app.ID = "myapp-id" - app.Name = "myapp" - app.ImageID = sql.NullString{String: "sha256:three", Valid: true} - app.PreviousImageID = sql.NullString{String: "sha256:two", Valid: true} - require.NoError(t, app.Save(ctx)) + app.ID = name + "-id" + app.Name = name + app.ImageID = sql.NullString{String: imageID, Valid: true} + app.PreviousImageID = sql.NullString{String: previousImageID, Valid: true} + require.NoError(t, app.Save(context.Background())) + + return app +} + +// TestRecordDeployedImageRemovesOldImages runs the step after a deploy +// against a fake Docker API. Image one has a tag from before images were +// tagged with their commit, and is also tagged for another app. Image two +// was the previous image, three the current one, four is new. Image five is +// the other app's previous image, which a redeploy of its commit left +// without the other app's tag. +func TestRecordDeployedImageRemovesOldImages(t *testing.T) { + t.Parallel() + + api := &fakeImageAPI{images: map[string][]string{ + "sha256:one": {"upaas-myapp:140", "upaas-otherapp:1a2b3c4"}, + "sha256:two": {"upaas-myapp:2b3c4d5"}, + "sha256:three": {"upaas-myapp:3c4d5e6"}, + "sha256:four": {"upaas-myapp:4d5e6f7"}, + "sha256:five": {"upaas-myapp:5e6f7a8"}, + }} + svc, db := newImageTestService(t, api) + ctx := context.Background() + + app := saveApp(t, db, "myapp", "sha256:three", "sha256:two") + saveApp(t, db, "otherapp", "sha256:other", "sha256:five") deployment := models.NewDeployment(db) deployment.AppID = app.ID require.NoError(t, deployment.Save(ctx)) - svc := deploy.NewTestServiceWithConfig(log, &config.Config{}, dockerClient) - - err = svc.RecordDeployedImage(ctx, app, deployment, "sha256:four") + err := svc.RecordDeployedImage(ctx, app, deployment, "sha256:four") require.NoError(t, err) assert.Equal(t, "sha256:four", app.ImageID.String) assert.Equal(t, "sha256:three", app.PreviousImageID.String) - mu.Lock() - defer mu.Unlock() + images, _, removed := api.state() - assert.Equal(t, []string{"upaas-myapp:1", "upaas-myapp:2"}, removed) - assert.False(t, forced, "old image tags must be removed without force") + assert.Equal(t, []string{"upaas-myapp:140", "upaas-myapp:2b3c4d5"}, removed) + assert.Equal(t, map[string][]string{ + "sha256:one": {"upaas-otherapp:1a2b3c4"}, + "sha256:three": {"upaas-myapp:3c4d5e6"}, + "sha256:four": {"upaas-myapp:4d5e6f7"}, + "sha256:five": {"upaas-myapp:5e6f7a8"}, + }, images) + assert.False(t, api.forced, "old images must be removed without force") +} + +// TestRedeployRemovesImagesLeftWithoutTag deploys commits against a fake +// Docker API, some of them again. A build takes the commit's tag from the +// image an earlier build of it made. That image is kept, without a tag, +// while the app runs it or Rollback would start it, and is removed by its +// ID once neither does. +func TestRedeployRemovesImagesLeftWithoutTag(t *testing.T) { + t.Parallel() + + // The app runs commit abc1234 and would roll back to def5678. The last + // deploy, of commit 0123abc, failed after its build. + api := &fakeImageAPI{images: map[string][]string{ + "sha256:built-abc1234": {"upaas-myapp:abc1234"}, + "sha256:built-def5678": {"upaas-myapp:def5678"}, + "sha256:failed-0123abc": {"upaas-myapp:0123abc"}, + }} + svc, db := newImageTestService(t, api) + ctx := context.Background() + + app := saveApp(t, db, "myapp", "sha256:built-abc1234", "sha256:built-def5678") + + for imageID := range api.images { + deployment := models.NewDeployment(db) + deployment.AppID = app.ID + deployment.ImageID = sql.NullString{String: imageID, Valid: true} + require.NoError(t, deployment.Save(ctx)) + } + + deployCommit := func(shortSHA, imageID string) { + t.Helper() + + api.mu.Lock() + api.shortSHA = shortSHA + api.nextID = imageID + api.mu.Unlock() + + deployment := models.NewDeployment(db) + deployment.AppID = app.ID + require.NoError(t, deployment.Save(ctx)) + + built, err := svc.BuildImage(ctx, app, deployment) + require.NoError(t, err) + require.NoError(t, svc.RecordDeployedImage(ctx, app, deployment, built)) + } + + // The failed deploy's image loses its tag, and nothing uses it. + deployCommit("0123abc", "sha256:retried-0123abc") + + images, _, _ := api.state() + assert.Equal(t, map[string][]string{ + "sha256:built-abc1234": {"upaas-myapp:abc1234"}, + "sha256:retried-0123abc": {"upaas-myapp:0123abc"}, + }, images) + + // The running image loses its tag and becomes the one Rollback starts. + deployCommit("0123abc", "sha256:rebuilt-0123abc") + + images, _, _ = api.state() + assert.Equal(t, map[string][]string{ + "sha256:rebuilt-0123abc": {"upaas-myapp:0123abc"}, + "sha256:retried-0123abc": {}, + }, images) + assert.Equal(t, "sha256:retried-0123abc", app.PreviousImageID.String) + + // Once Rollback no longer needs it, the untagged image is removed. + deployCommit("4567def", "sha256:built-4567def") + + images, built, removed := api.state() + assert.Equal(t, map[string][]string{ + "sha256:built-4567def": {"upaas-myapp:4567def"}, + "sha256:rebuilt-0123abc": {"upaas-myapp:0123abc"}, + }, images) + assert.Equal(t, []string{ + "upaas-myapp:0123abc", "upaas-myapp:0123abc", "upaas-myapp:4567def", + }, built) + assert.Equal(t, []string{ + "sha256:failed-0123abc", "upaas-myapp:def5678", // first deploy + "upaas-myapp:abc1234", // second deploy + "sha256:retried-0123abc", // third deploy + }, removed) + assert.False(t, api.forced, "old images must be removed without force") } diff --git a/internal/service/deploy/export_test.go b/internal/service/deploy/export_test.go index 95ce82e..8abfc34 100644 --- a/internal/service/deploy/export_test.go +++ b/internal/service/deploy/export_test.go @@ -9,6 +9,7 @@ import ( "strings" "sneak.berlin/go/upaas/internal/config" + "sneak.berlin/go/upaas/internal/database" "sneak.berlin/go/upaas/internal/docker" "sneak.berlin/go/upaas/internal/models" ) @@ -44,15 +45,18 @@ func (svc *Service) UnlockApp(appID string) { svc.unlockApp(appID) } -// NewTestServiceWithConfig creates a Service with config and docker client for testing. +// NewTestServiceWithConfig creates a Service with config, database and +// docker client for testing. func NewTestServiceWithConfig( log *slog.Logger, cfg *config.Config, + db *database.Database, dockerClient *docker.Client, ) *Service { return &Service{ log: log, config: cfg, + db: db, docker: dockerClient, } }