From 80a15adfa3e5382ef8441628321c984424b93a90 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 1 Oct 2026 18:51:52 +0000 Subject: [PATCH] Say that a deploy keeps only an app's volumes when the app has none (closes #248) An app with no volume mounts now shows, in its Volume Mounts section, that the files it writes are lost whenever a deploy or rollback replaces its container and that a restart keeps them. Each deploy of such an app writes the same sentence into its log, after the webhook payload and before the clone. The README says in one sentence that a deploy or rollback starts a new container that keeps only the files in the app's volume mounts. Model: opus-5-5 --- README.md | 3 + TODO.md | 5 ++ internal/handlers/app.go | 2 + internal/handlers/app_no_volumes_test.go | 45 +++++++++++ internal/service/deploy/deploy.go | 11 +++ .../service/deploy/deploy_volumes_test.go | 79 +++++++++++++++++++ templates/app_detail.html | 2 + 7 files changed, 147 insertions(+) create mode 100644 internal/handlers/app_no_volumes_test.go create mode 100644 internal/service/deploy/deploy_volumes_test.go diff --git a/README.md b/README.md index a563186..f6e67b2 100644 --- a/README.md +++ b/README.md @@ -278,6 +278,9 @@ exist yet, upaas has Docker create it as an empty directory, owned by root, when the app's container starts; there is no need to create it first. An existing host path is left as it is. This needs Docker Engine 23.0 or later. +A deploy or rollback replaces the app's container with a new one, which keeps +only the files the app wrote to its volume mounts. + ## License WTFPL diff --git a/TODO.md b/TODO.md index 3b56966..f39a7d6 100644 --- a/TODO.md +++ b/TODO.md @@ -20,6 +20,11 @@ regress. # Completed Steps +- 2026-10-01: An app with no volume mounts says on its page, and in the log of + each deploy, that a deploy or rollback loses the files it writes and a restart + keeps them; the README's Volume mounts section says a deploy or rollback keeps + only the files the app wrote to its volume mounts (#248). + - 2026-10-01: An app's first deploy no longer fails when a volume's host path does not exist yet: upaas asks Docker to create a missing host path when the app's container starts and to leave an existing one alone, so nobody has to diff --git a/internal/handlers/app.go b/internal/handlers/app.go index e03faf8..7258327 100644 --- a/internal/handlers/app.go +++ b/internal/handlers/app.go @@ -21,6 +21,7 @@ import ( "sneak.berlin/go/upaas/internal/database" "sneak.berlin/go/upaas/internal/models" "sneak.berlin/go/upaas/internal/service/app" + "sneak.berlin/go/upaas/internal/service/deploy" "sneak.berlin/go/upaas/templates" ) @@ -194,6 +195,7 @@ func (h *Handlers) HandleAppDetail() http.HandlerFunc { "EnvVars": envVars, "Labels": labels, "Volumes": volumes, + "NoVolumesWarning": deploy.NoVolumesWarning, "Ports": ports, "Deployments": deployments, "LatestDeployment": latestDeployment, diff --git a/internal/handlers/app_no_volumes_test.go b/internal/handlers/app_no_volumes_test.go new file mode 100644 index 0000000..65b96bd --- /dev/null +++ b/internal/handlers/app_no_volumes_test.go @@ -0,0 +1,45 @@ +package handlers_test + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "sneak.berlin/go/upaas/internal/models" + "sneak.berlin/go/upaas/internal/service/deploy" +) + +// TestAppPageSaysFilesAreLostOnlyWhenAppHasNoVolumes checks that the app page +// shows deploy.NoVolumesWarning until the app gets a volume mount. +func TestAppPageSaysFilesAreLostOnlyWhenAppHasNoVolumes(t *testing.T) { + t.Parallel() + + testCtx := setupTestHandlers(t) + createdApp := createTestApp(t, testCtx, "no-volumes-app") + + renderAppPage := func() string { + request := httptest.NewRequestWithContext( + t.Context(), http.MethodGet, "/apps/"+createdApp.ID, nil, + ) + request = addChiURLParams(request, map[string]string{"id": createdApp.ID}) + recorder := httptest.NewRecorder() + + testCtx.handlers.HandleAppDetail().ServeHTTP(recorder, request) + require.Equal(t, http.StatusOK, recorder.Code) + + return recorder.Body.String() + } + + assert.Contains(t, renderAppPage(), deploy.NoVolumesWarning) + + volume := models.NewVolume(testCtx.database) + volume.AppID = createdApp.ID + volume.HostPath = "/srv/no-volumes-app" + volume.ContainerPath = "/data" + require.NoError(t, volume.Save(t.Context())) + + assert.NotContains(t, renderAppPage(), deploy.NoVolumesWarning) +} diff --git a/internal/service/deploy/deploy.go b/internal/service/deploy/deploy.go index 4245be2..9af52d0 100644 --- a/internal/service/deploy/deploy.go +++ b/internal/service/deploy/deploy.go @@ -56,6 +56,12 @@ var ( ErrNoPreviousImage = errors.New("no previous image available for rollback") ) +// NoVolumesWarning is shown on the page of an app with no volume mounts and +// written into each of its deploy logs. +const NoVolumesWarning = "This app has no volume mounts, so the files it writes " + + "are lost whenever a deploy or rollback replaces its container; " + + "a restart of the container keeps them." + // logFlushInterval is how often to flush buffered logs to the database. const logFlushInterval = time.Second @@ -369,6 +375,11 @@ func (svc *Service) Deploy( svc.logWebhookPayload(bgCtx, deployment, webhookEvent) + volumes, err := app.GetVolumes(bgCtx) + if err == nil && len(volumes) == 0 { + _ = deployment.AppendLog(bgCtx, NoVolumesWarning) + } + err = svc.updateAppStatusBuilding(bgCtx, app) if err != nil { return err diff --git a/internal/service/deploy/deploy_volumes_test.go b/internal/service/deploy/deploy_volumes_test.go new file mode 100644 index 0000000..35aed12 --- /dev/null +++ b/internal/service/deploy/deploy_volumes_test.go @@ -0,0 +1,79 @@ +package deploy_test + +import ( + "context" + "log/slog" + "os" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "sneak.berlin/go/upaas/internal/config" + "sneak.berlin/go/upaas/internal/database" + "sneak.berlin/go/upaas/internal/docker" + "sneak.berlin/go/upaas/internal/logger" + "sneak.berlin/go/upaas/internal/models" + "sneak.berlin/go/upaas/internal/service/deploy" + "sneak.berlin/go/upaas/internal/service/notify" +) + +// deployLog deploys a new app, with one volume mount when withVolume is set, +// and returns the deploy's log. Docker is not connected, so the deploy fails +// at the git clone, after the start of its log is written. +func deployLog(t *testing.T, withVolume bool) string { + t.Helper() + + log := logger.NewForTest(slog.New(slog.NewTextHandler(os.Stderr, nil))) + dataDir := t.TempDir() + cfg := &config.Config{DataDir: dataDir, HostDataDir: dataDir} + db := database.NewTestDatabase(t) + + dockerClient, err := docker.New(nil, docker.Params{Logger: log, Config: cfg}) + require.NoError(t, err) + + notifySvc, err := notify.New(nil, notify.ServiceParams{Logger: log}) + require.NoError(t, err) + + svc, err := deploy.New(nil, deploy.ServiceParams{ + Logger: log, Config: cfg, Database: db, + Docker: dockerClient, Notify: notifySvc, + }) + require.NoError(t, err) + + ctx := context.Background() + + app := models.NewApp(db) + app.ID = "volumesapp-id" + app.Name = "volumesapp" + app.Branch = "main" + require.NoError(t, app.Save(ctx)) + + if withVolume { + volume := models.NewVolume(db) + volume.AppID = app.ID + volume.HostPath = "/srv/volumesapp" + volume.ContainerPath = "/data" + require.NoError(t, volume.Save(ctx)) + } + + err = svc.Deploy(ctx, app, nil, false) + require.ErrorIs(t, err, docker.ErrNotConnected) + + deployments, err := app.GetDeployments(ctx, 1) + require.NoError(t, err) + require.Len(t, deployments, 1) + + return deployments[0].Logs.String +} + +func TestDeployLogSaysFilesAreLostOnlyWhenAppHasNoVolumes(t *testing.T) { + t.Parallel() + + logWithoutVolumes := deployLog(t, false) + assert.Equal(t, 1, strings.Count(logWithoutVolumes, deploy.NoVolumesWarning), + logWithoutVolumes) + + assert.NotContains(t, deployLog(t, true), deploy.NoVolumesWarning) +} diff --git a/templates/app_detail.html b/templates/app_detail.html index eb6f146..7166a69 100644 --- a/templates/app_detail.html +++ b/templates/app_detail.html @@ -318,6 +318,8 @@ + {{else}} +

{{.NoVolumesWarning}}

{{end}}
{{ .CSRFField }}