From cb07494a9206429c5c5ca1e0257fd42bb816d15e Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 1 Oct 2026 23:07:34 +0000 Subject: [PATCH] Revert "Say that a deploy keeps only an app's volumes when the app has none" This reverts commit cc786a1e84f951021bc805154662ff4ba1ba76f2, at the owner's request. It removes the app page sentence for an app with no volume mounts, the matching deploy log line and its constant, their tests, the README sentence and the TODO.md entry. The environment variable hint keeps its alert-warning class, which is in the committed stylesheet. 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 deletions(-) delete mode 100644 internal/handlers/app_no_volumes_test.go delete mode 100644 internal/service/deploy/deploy_volumes_test.go diff --git a/README.md b/README.md index f6e67b2..a563186 100644 --- a/README.md +++ b/README.md @@ -278,9 +278,6 @@ 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 7a0c2fe..bf60f7f 100644 --- a/TODO.md +++ b/TODO.md @@ -38,11 +38,6 @@ regress. style, instead of asking for a container restart, which keeps the old values (#255). -- 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: Builds attach a BuildKit session, as the docker command line does, so a base image that is not on the host is pulled instead of the build failing with "no active sessions" on Docker Engine 27. Container logs, and so the diff --git a/internal/handlers/app.go b/internal/handlers/app.go index 7258327..e03faf8 100644 --- a/internal/handlers/app.go +++ b/internal/handlers/app.go @@ -21,7 +21,6 @@ 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" ) @@ -195,7 +194,6 @@ 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 deleted file mode 100644 index 65b96bd..0000000 --- a/internal/handlers/app_no_volumes_test.go +++ /dev/null @@ -1,45 +0,0 @@ -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 d6e7635..ed43395 100644 --- a/internal/service/deploy/deploy.go +++ b/internal/service/deploy/deploy.go @@ -56,12 +56,6 @@ 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 @@ -375,11 +369,6 @@ 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 deleted file mode 100644 index 35aed12..0000000 --- a/internal/service/deploy/deploy_volumes_test.go +++ /dev/null @@ -1,79 +0,0 @@ -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 0889c3d..2137c71 100644 --- a/templates/app_detail.html +++ b/templates/app_detail.html @@ -318,8 +318,6 @@ - {{else}} -

{{.NoVolumesWarning}}

{{end}}
{{ .CSRFField }} -- 2.54.0