Revert "Say that a deploy keeps only an app's volumes when the app has none"
Check / check (pull_request) Successful in 4m29s
Check / check (pull_request) Successful in 4m29s
The owner asked for this commit to be reverted in full (#254 (comment)). 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. #248 stays closed as rejected. Model: opus-5-5
This commit was merged in pull request #263.
This commit is contained in:
@@ -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
|
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.
|
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
|
## License
|
||||||
|
|
||||||
WTFPL
|
WTFPL
|
||||||
|
|||||||
@@ -38,11 +38,6 @@ regress.
|
|||||||
style, instead of asking for a container restart, which keeps the old values
|
style, instead of asking for a container restart, which keeps the old values
|
||||||
(#255).
|
(#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,
|
- 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
|
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
|
with "no active sessions" on Docker Engine 27. Container logs, and so the
|
||||||
|
|||||||
@@ -21,7 +21,6 @@ import (
|
|||||||
"sneak.berlin/go/upaas/internal/database"
|
"sneak.berlin/go/upaas/internal/database"
|
||||||
"sneak.berlin/go/upaas/internal/models"
|
"sneak.berlin/go/upaas/internal/models"
|
||||||
"sneak.berlin/go/upaas/internal/service/app"
|
"sneak.berlin/go/upaas/internal/service/app"
|
||||||
"sneak.berlin/go/upaas/internal/service/deploy"
|
|
||||||
"sneak.berlin/go/upaas/templates"
|
"sneak.berlin/go/upaas/templates"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -195,7 +194,6 @@ func (h *Handlers) HandleAppDetail() http.HandlerFunc {
|
|||||||
"EnvVars": envVars,
|
"EnvVars": envVars,
|
||||||
"Labels": labels,
|
"Labels": labels,
|
||||||
"Volumes": volumes,
|
"Volumes": volumes,
|
||||||
"NoVolumesWarning": deploy.NoVolumesWarning,
|
|
||||||
"Ports": ports,
|
"Ports": ports,
|
||||||
"Deployments": deployments,
|
"Deployments": deployments,
|
||||||
"LatestDeployment": latestDeployment,
|
"LatestDeployment": latestDeployment,
|
||||||
|
|||||||
@@ -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)
|
|
||||||
}
|
|
||||||
@@ -56,12 +56,6 @@ var (
|
|||||||
ErrNoPreviousImage = errors.New("no previous image available for rollback")
|
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.
|
// logFlushInterval is how often to flush buffered logs to the database.
|
||||||
const logFlushInterval = time.Second
|
const logFlushInterval = time.Second
|
||||||
|
|
||||||
@@ -375,11 +369,6 @@ func (svc *Service) Deploy(
|
|||||||
|
|
||||||
svc.logWebhookPayload(bgCtx, deployment, webhookEvent)
|
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)
|
err = svc.updateAppStatusBuilding(bgCtx, app)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
|
|||||||
@@ -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)
|
|
||||||
}
|
|
||||||
@@ -318,8 +318,6 @@
|
|||||||
</tbody>
|
</tbody>
|
||||||
</table>
|
</table>
|
||||||
</div>
|
</div>
|
||||||
{{else}}
|
|
||||||
<p class="alert-warning">{{.NoVolumesWarning}}</p>
|
|
||||||
{{end}}
|
{{end}}
|
||||||
<form method="POST" action="/apps/{{.App.ID}}/volumes" class="flex flex-col sm:flex-row gap-2 items-end">
|
<form method="POST" action="/apps/{{.App.ID}}/volumes" class="flex flex-col sm:flex-row gap-2 items-end">
|
||||||
{{ .CSRFField }}
|
{{ .CSRFField }}
|
||||||
|
|||||||
Reference in New Issue
Block a user