2 Commits
Author SHA1 Message Date
sneak 4dbb654b79 Remove images from earlier deploys after a successful deploy (closes #216)
Check / check (pull_request) Skipped
After a successful deploy, upaas now removes the app's images other than
the one the running container uses and the previous one, which rollback
starts. Removing an image also removes the untagged images it was built on
unless another image still needs them. Images left by other stages of a
multi-stage build are not tied to the app and stay.

Model: opus-5-5
2026-09-23 09:49:43 +00:00
clawbot ddcd179841 Keep deployment logs findable after the container is recreated (closes #214)
Deployment logs were stored under a directory named after the container's hostname, and the path was worked out again from the current hostname on download. Docker gives a recreated container a new hostname, so every older log download returned 404. New logs now go to `logs/<appname>/` with no hostname in the path. Logs written by older versions under an old hostname directory are still found by looking one directory deeper, inside the same confined log root. Tests cover both the hostname-free path and downloading an old-layout log.

Model: opus-5-5
2026-09-23 11:44:30 +02:00
7 changed files with 215 additions and 8 deletions
+7
View File
@@ -20,6 +20,13 @@ regress.
# Completed Steps # Completed Steps
- 2026-09-23: After a successful deploy, upaas removes the app's images other
than the running one and the one rollback would use, together with the
untagged images they were built on (#216).
- 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` - 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
+25
View File
@@ -537,6 +537,31 @@ func (c *Client) RemoveImage(ctx context.Context, imageID ImageID) error {
return nil return nil
} }
// ListImageIDs returns the IDs of all images tagged in the given repository,
// such as "upaas-myapp".
func (c *Client) ListImageIDs(
ctx context.Context,
repository string,
) ([]ImageID, error) {
if c.docker == nil {
return nil, ErrNotConnected
}
images, err := c.docker.ImageList(ctx, image.ListOptions{
Filters: filters.NewArgs(filters.Arg("reference", repository)),
})
if err != nil {
return nil, fmt.Errorf("failed to list images: %w", err)
}
ids := make([]ImageID, 0, len(images))
for _, img := range images {
ids = append(ids, ImageID(img.ID))
}
return ids, nil
}
func (c *Client) performBuild( func (c *Client) performBuild(
ctx context.Context, ctx context.Context,
opts BuildImageOptions, opts BuildImageOptions,
+21 -1
View File
@@ -6,9 +6,11 @@ import (
"encoding/json" "encoding/json"
"errors" "errors"
"fmt" "fmt"
"io/fs"
"net/http" "net/http"
"net/url" "net/url"
"os" "os"
"path"
"path/filepath" "path/filepath"
"strconv" "strconv"
"strings" "strings"
@@ -640,7 +642,7 @@ func (h *Handlers) HandleDeploymentLogDownload() http.HandlerFunc {
} }
defer func() { _ = root.Close() }() defer func() { _ = root.Close() }()
file, openErr := root.Open(relPath) file, openErr := openDeploymentLog(root, relPath)
if openErr != nil { if openErr != nil {
http.NotFound(writer, request) 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. // containerLogsAPITail is the default number of log lines for the container logs API.
const containerLogsAPITail = "100" const containerLogsAPITail = "100"
+50
View File
@@ -69,6 +69,56 @@ func TestHandleDeploymentLogDownloadServesLegitimateFile(t *testing.T) {
assert.Contains(t, recorder.Body.String(), "deploy log contents") 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 // TestHandleDeploymentLogDownloadRejectsPathTraversal verifies the
// os.Root containment guard. A traversal-shaped app name drives the // os.Root containment guard. A traversal-shaped app name drives the
// resolved log path out of the deploy log directory onto a sentinel // resolved log path out of the deploy log directory onto a sentinel
+59 -7
View File
@@ -261,15 +261,14 @@ func (svc *Service) GetBuildDir(appName string) string {
// GetLogFilePath returns the path to the log file for a deployment. // GetLogFilePath returns the path to the log file for a deployment.
// Returns empty string if the path cannot be determined. // 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( func (svc *Service) GetLogFilePath(
app *models.App, app *models.App,
deployment *models.Deployment, deployment *models.Deployment,
) string { ) string {
hostname, err := os.Hostname()
if err != nil {
hostname = "unknown"
}
// Get commit SHA // Get commit SHA
sha := "" sha := ""
if deployment.CommitSHA.Valid && deployment.CommitSHA.String != "" { 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) 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 // GetLogDir returns the root directory under which all deployment log
@@ -535,6 +534,8 @@ func (svc *Service) runBuildAndDeploy(
return err return err
} }
svc.removeUnusedImages(bgCtx, app, deployment)
// Use context.WithoutCancel to ensure health check completes even if // Use context.WithoutCancel to ensure health check completes even if
// the parent context is cancelled (e.g., HTTP request ends). // the parent context is cancelled (e.g., HTTP request ends).
go svc.checkHealthAfterDelay(bgCtx, app, deployment) go svc.checkHealthAfterDelay(bgCtx, app, deployment)
@@ -715,6 +716,57 @@ func (svc *Service) checkCancelled(
return ErrDeployCancelled return ErrDeployCancelled
} }
// removeUnusedImages removes the app's images (tagged upaas-<app>:<deployment>
// by buildImage) except the one the running container uses and the one
// Rollback would start. Removing an image also removes the untagged images
// it was built on, unless another image still needs them.
func (svc *Service) removeUnusedImages(
ctx context.Context,
app *models.App,
deployment *models.Deployment,
) {
images, err := svc.docker.ListImageIDs(ctx, "upaas-"+app.Name)
if err != nil {
svc.log.Error("failed to list app images", "error", err, "app", app.Name)
return
}
for _, imageID := range unusedImages(images, app) {
removeErr := svc.docker.RemoveImage(ctx, imageID)
if removeErr != nil {
svc.log.Error("failed to remove old image",
"error", removeErr, "app", app.Name, "image", imageID)
_ = deployment.AppendLog(
ctx,
"WARNING: failed to remove old image "+
imageID.String()+": "+removeErr.Error(),
)
continue
}
_ = deployment.AppendLog(ctx, "Removed old image: "+imageID.String())
}
}
// unusedImages returns the images that are neither the app's current image
// nor its previous image, which Rollback uses.
func unusedImages(images []docker.ImageID, app *models.App) []docker.ImageID {
var unused []docker.ImageID
for _, imageID := range images {
if imageID.String() == app.ImageID.String ||
imageID.String() == app.PreviousImageID.String {
continue
}
unused = append(unused, imageID)
}
return unused
}
// cleanupCancelledDeploy removes orphan resources left by a cancelled deployment. // cleanupCancelledDeploy removes orphan resources left by a cancelled deployment.
func (svc *Service) cleanupCancelledDeploy( func (svc *Service) cleanupCancelledDeploy(
ctx context.Context, ctx context.Context,
@@ -1294,7 +1346,7 @@ func (svc *Service) failDeployment(
} }
// writeLogsToFile writes the deployment logs to a file on disk. // 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) { func (svc *Service) writeLogsToFile(app *models.App, deployment *models.Deployment) {
if !deployment.Logs.Valid || deployment.Logs.String == "" { if !deployment.Logs.Valid || deployment.Logs.String == "" {
return return
@@ -0,0 +1,48 @@
package deploy_test
import (
"database/sql"
"testing"
"github.com/stretchr/testify/assert"
"sneak.berlin/go/upaas/internal/docker"
"sneak.berlin/go/upaas/internal/models"
"sneak.berlin/go/upaas/internal/service/deploy"
)
const currentImage = "sha256:current"
func TestUnusedImages_KeepsCurrentAndRollbackImage(t *testing.T) {
t.Parallel()
app := &models.App{
ImageID: sql.NullString{String: currentImage, Valid: true},
PreviousImageID: sql.NullString{String: "sha256:previous", Valid: true},
}
images := []docker.ImageID{
"sha256:oldest",
"sha256:previous",
"sha256:older",
currentImage,
}
assert.Equal(t,
[]docker.ImageID{"sha256:oldest", "sha256:older"},
deploy.UnusedImages(images, app),
)
}
func TestUnusedImages_FirstDeployHasNoRollbackImage(t *testing.T) {
t.Parallel()
app := &models.App{
ImageID: sql.NullString{String: currentImage, Valid: true},
}
images := []docker.ImageID{currentImage, "sha256:failed"}
assert.Equal(t,
[]docker.ImageID{"sha256:failed"},
deploy.UnusedImages(images, app),
)
}
+5
View File
@@ -90,6 +90,11 @@ func (svc *Service) GetBuildDirExported(appName string) string {
return svc.GetBuildDir(appName) return svc.GetBuildDir(appName)
} }
// UnusedImages exposes unusedImages for testing.
func UnusedImages(images []docker.ImageID, app *models.App) []docker.ImageID {
return unusedImages(images, app)
}
// BuildContainerOptionsExported exposes buildContainerOptions for testing. // BuildContainerOptionsExported exposes buildContainerOptions for testing.
func (svc *Service) BuildContainerOptionsExported( func (svc *Service) BuildContainerOptionsExported(
ctx context.Context, ctx context.Context,