From 76858126e2cf57a423a1c45a9f3b526a1cdbb235 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Fri, 2 Oct 2026 01:43:28 +0200 Subject: [PATCH] Revert "Say that a deploy keeps only an app's volumes when the app has none" The owner asked for this commit to be reverted in full (https://git.eeqj.de/sneak/upaas/pulls/254#issuecomment-109763). It removes the no-volume-mounts sentence from the app page and the deploy log, the constant and tests behind it, the README sentence, and its TODO.md entry. Later TODO.md entries stay. https://git.eeqj.de/sneak/upaas/issues/248 stays closed as rejected. 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}}