diff --git a/docs/advanced-usage/docker-options.md b/docs/advanced-usage/docker-options.md index 326f76862..ae4039d3a 100644 --- a/docs/advanced-usage/docker-options.md +++ b/docs/advanced-usage/docker-options.md @@ -235,8 +235,11 @@ dokku docker-options:list node-js-app --phase deploy ### Internal properties -The following property is recorded internally by the docker-options plugin and is not exposed via `docker-options:report`: +The following properties are recorded internally by the docker-options plugin and are not exposed via `docker-options:report`: -| Property | Description | Source | -|---|---|---| -| `migrated-from-files` | Global migration sentinel that records that the legacy `DOCKER_OPTIONS_` flat-file store has been drained into plugin properties | `plugins/docker-options/functions.go` writes `"true"` once the install-time migration runs | +| Property | Scope | Description | Source | +|---|---|---|---| +| `migrated-from-files` | global | Global migration sentinel that records that the legacy `DOCKER_OPTIONS_` flat-file store has been drained into plugin properties | `plugins/docker-options/functions.go` writes `"true"` once the install-time migration runs | +| `migrated-build` | per-app | Per-app marker recording that the app's legacy `DOCKER_OPTIONS_BUILD` file was drained into the `_default_.build` property list. Only set when the legacy file contained non-empty content | `plugins/docker-options/functions.go` writes `"true"` after the per-phase drain | +| `migrated-deploy` | per-app | Per-app marker recording that the app's legacy `DOCKER_OPTIONS_DEPLOY` file was drained into the `_default_.deploy` property list. Only set when the legacy file contained non-empty content | `plugins/docker-options/functions.go` writes `"true"` after the per-phase drain | +| `migrated-run` | per-app | Per-app marker recording that the app's legacy `DOCKER_OPTIONS_RUN` file was drained into the `_default_.run` property list. Only set when the legacy file contained non-empty content | `plugins/docker-options/functions.go` writes `"true"` after the per-phase drain | diff --git a/docs/advanced-usage/persistent-storage.md b/docs/advanced-usage/persistent-storage.md index c033e734e..8989224dc 100644 --- a/docs/advanced-usage/persistent-storage.md +++ b/docs/advanced-usage/persistent-storage.md @@ -227,3 +227,13 @@ By default, Dokku will execute your buildpack app processes as the `herokuishuse > this user must exist in your herokuish image. Additionally, the default `docker-local` scheduler that comes with Dokku will ensure your storage mounts are owned by either `herokuishuser` or the overridden value you have set in `DOKKU_APP_USER`. See the [docker-local scheduler documentation](/docs/deployment/schedulers/docker-local.md#disabling-chown-of-persistent-storage) docs for more information. + +## Properties + +### Internal properties + +The following property is recorded internally by the storage plugin and is not exposed via `storage:report`: + +| Property | Scope | Description | Source | +|---|---|---|---| +| `legacy-mounts-migrated` | per-app | Per-app marker recording that the app's legacy `-v` docker-options entries were drained into named storage entries plus attachments. Only set when at least one `-v` line was actually migrated; apps that have never had legacy mounts never receive this marker | `plugins/storage/migrate.go` writes `"true"` after a successful drain | diff --git a/plugins/docker-options/functions.go b/plugins/docker-options/functions.go index 27535f977..a04c985cd 100644 --- a/plugins/docker-options/functions.go +++ b/plugins/docker-options/functions.go @@ -245,22 +245,66 @@ func CommandReport(appName string, format string, infoFlag string) error { return ReportSingleApp(appName, format, infoFlag) } -// migrateLegacyDockerOptionsFiles converts pre-properties DOCKER_OPTIONS_* -// files into property lists. It is gated by a single global marker so it never -// re-runs - even if a user later restores a DOCKER_OPTIONS_* file by hand. -func migrateLegacyDockerOptionsFiles() error { - if common.PropertyExists("docker-options", "--global", "migrated-from-files") { +// migratedPropertyKey returns the per-app property name recording that +// an app's DOCKER_OPTIONS_ file was drained into the property +// store. Surfaces in the property store as `docker-options//migrated-`. +func migratedPropertyKey(phase string) string { + return "migrated-" + strings.ToLower(phase) +} + +// convertLegacyMigratedMarker drains a leftover DOCKER_OPTIONS_.migrated +// sentinel into the new per-phase property and removes the file. Runs +// as part of the upgrade-cycle pass; no-op when the file is absent. +// TODO(post-deprecation): remove this helper and its caller. +func convertLegacyMigratedMarker(appName, phase string) error { + migratedPath := filepath.Join(common.AppRoot(appName), "DOCKER_OPTIONS_"+strings.ToUpper(phase)+".migrated") + if !common.FileExists(migratedPath) { return nil } + if err := common.PropertyWrite("docker-options", appName, migratedPropertyKey(phase), "true"); err != nil { + return err + } + return os.Remove(migratedPath) +} +// migrateLegacyDockerOptionsFiles converts pre-properties +// DOCKER_OPTIONS_* files into property lists. Idempotency comes from +// two layers: a per-phase per-app `migrated-` property recording +// what was drained, and a global `migrated-from-files` short-circuit so +// the steady-state install path returns immediately. +// +// The upgrade-cycle conversion of leftover `.migrated` filesystem +// sentinels runs BEFORE the global gate so users upgrading from the +// previous release - who have `migrated-from-files` set AND `.migrated` +// files on disk - still get those files drained into properties. +func migrateLegacyDockerOptionsFiles() error { apps, err := common.DokkuApps() if err != nil { if errors.Is(err, common.NoAppsExist) { - return common.PropertyWrite("docker-options", "--global", "migrated-from-files", "true") + if !common.PropertyExists("docker-options", "--global", "migrated-from-files") { + return common.PropertyWrite("docker-options", "--global", "migrated-from-files", "true") + } + return nil } return err } + // Upgrade-cycle conversion: drain any `.migrated` sentinels left + // behind by the previous release into the new per-phase properties. + // Always runs; cheap when nothing to do. + // TODO(post-deprecation): remove this loop. + for _, appName := range apps { + for _, phase := range availablePhases { + if err := convertLegacyMigratedMarker(appName, phase); err != nil { + return err + } + } + } + + if common.PropertyExists("docker-options", "--global", "migrated-from-files") { + return nil + } + for _, appName := range apps { for _, phase := range availablePhases { legacyPath := filepath.Join(common.AppRoot(appName), "DOCKER_OPTIONS_"+strings.ToUpper(phase)) @@ -273,16 +317,19 @@ func migrateLegacyDockerOptionsFiles() error { return err } - if err := common.PropertyListWrite("docker-options", appName, propertyKey(DefaultProcessType, phase), lines); err != nil { - return err + if len(lines) > 0 { + if err := common.PropertyListWrite("docker-options", appName, propertyKey(DefaultProcessType, phase), lines); err != nil { + return err + } + if err := common.PropertyWrite("docker-options", appName, migratedPropertyKey(phase), "true"); err != nil { + return err + } + common.LogInfo1(fmt.Sprintf("Migrated %s to docker-options properties", legacyPath)) } - migratedPath := legacyPath + ".migrated" - if err := os.Rename(legacyPath, migratedPath); err != nil { - return fmt.Errorf("Unable to rename migrated file %s: %s", legacyPath, err.Error()) + if err := os.Remove(legacyPath); err != nil { + return fmt.Errorf("Unable to remove migrated file %s: %s", legacyPath, err.Error()) } - - common.LogInfo1(fmt.Sprintf("Migrated %s to docker-options properties", legacyPath)) } } @@ -312,6 +359,7 @@ func readLegacyOptionsFile(path string) ([]string, error) { // removeMigratedLegacyFiles deletes any leftover DOCKER_OPTIONS_*.migrated // files for an app. Called from post-delete so we don't leak files into a // directory that is about to be torn down anyway. +// TODO(post-deprecation): remove this helper and the call in TriggerPostDelete. func removeMigratedLegacyFiles(appName string) { for _, phase := range availablePhases { path := filepath.Join(common.AppRoot(appName), "DOCKER_OPTIONS_"+strings.ToUpper(phase)+".migrated") diff --git a/plugins/docker-options/migration_test.go b/plugins/docker-options/migration_test.go index d3a1595fe..80995008f 100644 --- a/plugins/docker-options/migration_test.go +++ b/plugins/docker-options/migration_test.go @@ -4,6 +4,7 @@ import ( "os" "os/user" "path/filepath" + "strings" "testing" "github.com/dokku/dokku/plugins/common" @@ -89,21 +90,28 @@ func TestMigrateLegacyDockerOptionsFiles_MigratesAndIsIdempotent(t *testing.T) { t.Errorf("migrated-from-files marker not set after first migration") } + migratedPhases := map[string]map[string]bool{ + "alpha": {"build": true, "deploy": true}, + "beta": {"deploy": true}, + } for _, app := range []string{"alpha", "beta"} { for _, phase := range []string{"BUILD", "DEPLOY", "RUN"} { legacy := filepath.Join(dokkuRoot, app, "DOCKER_OPTIONS_"+phase) migrated := legacy + ".migrated" - if app == "alpha" && (phase == "BUILD" || phase == "DEPLOY") || app == "beta" && phase == "DEPLOY" { - if _, err := os.Stat(migrated); err != nil { - t.Errorf("expected %s to exist: %v", migrated, err) - } - if _, err := os.Stat(legacy); !os.IsNotExist(err) { - t.Errorf("expected %s to be gone, got err=%v", legacy, err) - } - } else { - if _, err := os.Stat(legacy); !os.IsNotExist(err) { - t.Errorf("did not expect %s to exist, got err=%v", legacy, err) + if _, err := os.Stat(legacy); !os.IsNotExist(err) { + t.Errorf("expected %s to be gone, got err=%v", legacy, err) + } + if _, err := os.Stat(migrated); !os.IsNotExist(err) { + t.Errorf("expected %s to be gone (no more rename), got err=%v", migrated, err) + } + lowerPhase := strings.ToLower(phase) + exists := common.PropertyExists("docker-options", app, "migrated-"+lowerPhase) + if migratedPhases[app][lowerPhase] { + if !exists { + t.Errorf("expected migrated-%s property for %s to be set", lowerPhase, app) } + } else if exists { + t.Errorf("did not expect migrated-%s property for %s to be set", lowerPhase, app) } } } @@ -135,6 +143,89 @@ func TestMigrateLegacyDockerOptionsFiles_MigratesAndIsIdempotent(t *testing.T) { } } +// TestMigrateLegacyDockerOptionsFiles_ConvertsLegacyMigratedMarker +// covers the upgrade-cycle path where a previous-release `.migrated` +// sentinel exists on disk. The conversion writes the per-phase +// property and removes the file. +func TestMigrateLegacyDockerOptionsFiles_ConvertsLegacyMigratedMarker(t *testing.T) { + dokkuRoot := setupMigrationEnv(t) + + if err := os.MkdirAll(filepath.Join(dokkuRoot, "alpha"), 0755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + sentinel := filepath.Join(dokkuRoot, "alpha", "DOCKER_OPTIONS_DEPLOY.migrated") + if err := os.WriteFile(sentinel, []byte{}, 0644); err != nil { + t.Fatalf("WriteFile sentinel: %v", err) + } + + if err := migrateLegacyDockerOptionsFiles(); err != nil { + t.Fatalf("migrateLegacyDockerOptionsFiles: %v", err) + } + + if !common.PropertyExists("docker-options", "alpha", "migrated-deploy") { + t.Errorf("expected migrated-deploy property to be set") + } + if _, err := os.Stat(sentinel); !os.IsNotExist(err) { + t.Errorf("expected sentinel file gone, got err=%v", err) + } +} + +// TestMigrateLegacyDockerOptionsFiles_ConvertsMigratedMarkerEvenWhenGloballyMigrated +// is the regression test for the upgrade-cycle ordering fix: users on +// the previous release have `migrated-from-files` ALREADY set AND +// `.migrated` sentinels on disk. The conversion pass must run before +// the global short-circuit so these sentinels still get drained. +func TestMigrateLegacyDockerOptionsFiles_ConvertsMigratedMarkerEvenWhenGloballyMigrated(t *testing.T) { + dokkuRoot := setupMigrationEnv(t) + + if err := os.MkdirAll(filepath.Join(dokkuRoot, "alpha"), 0755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + sentinel := filepath.Join(dokkuRoot, "alpha", "DOCKER_OPTIONS_DEPLOY.migrated") + if err := os.WriteFile(sentinel, []byte{}, 0644); err != nil { + t.Fatalf("WriteFile sentinel: %v", err) + } + if err := common.PropertyWrite("docker-options", "--global", "migrated-from-files", "true"); err != nil { + t.Fatalf("seed global marker: %v", err) + } + + if err := migrateLegacyDockerOptionsFiles(); err != nil { + t.Fatalf("migrateLegacyDockerOptionsFiles: %v", err) + } + + if !common.PropertyExists("docker-options", "alpha", "migrated-deploy") { + t.Errorf("expected migrated-deploy property to be set despite global short-circuit") + } + if _, err := os.Stat(sentinel); !os.IsNotExist(err) { + t.Errorf("expected sentinel file gone, got err=%v", err) + } +} + +// TestMigrateLegacyDockerOptionsFiles_SkipsEmptyContentFiles verifies +// that a legacy file containing only comments/whitespace gets removed +// but does NOT receive a per-phase property write - the property is +// only set when actual content was drained. +func TestMigrateLegacyDockerOptionsFiles_SkipsEmptyContentFiles(t *testing.T) { + dokkuRoot := setupMigrationEnv(t) + + writeLegacyDockerOptionsFile(t, dokkuRoot, "alpha", "DEPLOY", "# only a comment\n\n \n") + + if err := migrateLegacyDockerOptionsFiles(); err != nil { + t.Fatalf("migrateLegacyDockerOptionsFiles: %v", err) + } + + if common.PropertyExists("docker-options", "alpha", "migrated-deploy") { + t.Errorf("did not expect migrated-deploy property to be set for empty-content file") + } + legacy := filepath.Join(dokkuRoot, "alpha", "DOCKER_OPTIONS_DEPLOY") + if _, err := os.Stat(legacy); !os.IsNotExist(err) { + t.Errorf("expected empty-content legacy file removed, got err=%v", err) + } + if common.PropertyExists("docker-options", "alpha", "_default_.deploy") { + t.Errorf("did not expect _default_.deploy property list for empty-content file") + } +} + func equalStrings(a, b []string) bool { if len(a) != len(b) { return false diff --git a/plugins/storage/migrate.go b/plugins/storage/migrate.go index 07cb087a4..46bb8eba2 100644 --- a/plugins/storage/migrate.go +++ b/plugins/storage/migrate.go @@ -12,11 +12,18 @@ import ( dockeroptions "github.com/dokku/dokku/plugins/docker-options" ) -// migrationFlagDir contains per-app flag files written after a successful -// per-app migration; presence means the app's legacy -v lines have been -// drained into the attachment store. Lives under the registry directory -// so it doesn't collide with the property-store path -// `config/storage/`. +// MigratedProperty is the per-app marker recording that legacy +// docker-options `-v` lines have been drained into named storage +// entries plus attachments. Written via the property store so it is +// visible to property-store backup/restore tooling, replacing the old +// filesystem flag file at `data/storage-registry/migrations/`. +const MigratedProperty = "legacy-mounts-migrated" + +// migrationFlagDir / migrationFlagFile remain for one release cycle so +// the upgrade-cycle helper convertLegacyMigrationFlag can drain any +// leftover flag files into the new property. +// TODO(post-deprecation): remove both functions and the +// migrationFlagDir entry in triggers.go's install directory list. func migrationFlagDir() string { return filepath.Join(RegistryDirectory(), "migrations") } @@ -25,15 +32,17 @@ func migrationFlagFile(appName string) string { return filepath.Join(migrationFlagDir(), appName) } -// MigrateApp re-runs the legacy migration for a single app, ignoring -// the per-app flag file so an operator can force-rescan an app whose -// docker-options state changed after the bulk install-time pass. -// Used by `dokku storage:migrate `. +// MigrateApp re-runs the legacy migration for a single app so an +// operator can force-rescan an app whose docker-options state changed +// after the bulk install-time pass. Used by `dokku storage:migrate +// `. The marker property is intentionally left in place; migrateApp +// only writes it when something was actually drained, so a re-run on +// an already-drained app with no new `-v` lines preserves the existing +// "had legacy state, drained" signal. func MigrateApp(appName string) error { - if err := os.MkdirAll(migrationFlagDir(), 0755); err != nil { - return fmt.Errorf("unable to create migration flag dir: %w", err) + if err := convertLegacyMigrationFlag(appName); err != nil { + return err } - _ = os.Remove(migrationFlagFile(appName)) if err := migrateApp(appName); err != nil { return fmt.Errorf("storage migration failed for app %q: %w", appName, err) } @@ -42,12 +51,9 @@ func MigrateApp(appName string) error { // MigrateLegacyMounts walks every app and converts its legacy `-v` // docker-options entries into named storage entries plus attachments. -// Idempotent: per-app flag files short-circuit re-runs. +// Idempotent: the per-app legacy-mounts-migrated property short-circuits +// re-runs. func MigrateLegacyMounts() error { - if err := os.MkdirAll(migrationFlagDir(), 0755); err != nil { - return fmt.Errorf("unable to create migration flag dir: %w", err) - } - apps, err := common.DokkuApps() if err != nil { if errors.Is(err, common.NoAppsExist) { @@ -57,7 +63,10 @@ func MigrateLegacyMounts() error { } for _, app := range apps { - if _, err := os.Stat(migrationFlagFile(app)); err == nil { + if err := convertLegacyMigrationFlag(app); err != nil { + return err + } + if common.PropertyExists(PluginName, app, MigratedProperty) { continue } if err := migrateApp(app); err != nil { @@ -67,6 +76,27 @@ func MigrateLegacyMounts() error { return nil } +// convertLegacyMigrationFlag drains the legacy filesystem flag file +// (data/storage-registry/migrations/) into the new +// legacy-mounts-migrated property and removes the file. Runs as part +// of the per-app loop so installs upgrading from the previous release +// surface the marker in the property store without re-running the +// drain logic. No-op when the flag file is absent. +// TODO(post-deprecation): remove this helper and its callers. +func convertLegacyMigrationFlag(appName string) error { + path := migrationFlagFile(appName) + if _, err := os.Stat(path); err != nil { + if os.IsNotExist(err) { + return nil + } + return err + } + if err := common.PropertyWrite(PluginName, appName, MigratedProperty, "true"); err != nil { + return err + } + return os.Remove(path) +} + // migrateApp performs the per-app migration. Order is intentional: write // the entry, write the attachment, drain the docker-options line. Any // step failing aborts before drain, so the original behavior is @@ -99,13 +129,20 @@ func migrateApp(appName string) error { } sort.Strings(mounts) + if len(mounts) == 0 { + // No legacy `-v` lines for this app. Leave the marker unset so + // `storage:report` (and future tooling) distinguishes apps that + // never had legacy state from apps that did and were drained. + return nil + } + for _, mount := range mounts { if err := migrateMount(appName, mount, phaseMap[mount]); err != nil { return err } } - return touchMigrationFlag(appName) + return common.PropertyWrite(PluginName, appName, MigratedProperty, "true") } func migrateMount(appName string, mount string, phases []string) error { @@ -167,10 +204,6 @@ func migrateMount(appName string, mount string, phases []string) error { return nil } -func touchMigrationFlag(appName string) error { - return common.TouchFile(migrationFlagFile(appName)) -} - func filterMountLines(lines []string) []string { out := []string{} for _, line := range lines { diff --git a/plugins/storage/migrate_test.go b/plugins/storage/migrate_test.go index 415b51666..484615f64 100644 --- a/plugins/storage/migrate_test.go +++ b/plugins/storage/migrate_test.go @@ -112,9 +112,9 @@ func TestMigrateAppSinglePhase(t *testing.T) { t.Errorf("deploy options after migration = %v, want [--restart=on-failure:5]", deployAfter) } - // Migration flag file is in place. - if _, err := os.Stat(migrationFlagFile("alpha")); err != nil { - t.Errorf("migration flag missing: %v", err) + // Property marker is in place. + if !common.PropertyExists(PluginName, "alpha", MigratedProperty) { + t.Errorf("legacy-mounts-migrated property missing") } } @@ -230,8 +230,8 @@ func TestMigrateLegacyMountsBulkAndFlagFastPath(t *testing.T) { } for _, app := range []string{"alpha", "beta"} { - if _, err := os.Stat(migrationFlagFile(app)); err != nil { - t.Errorf("flag file missing for %s: %v", app, err) + if !common.PropertyExists(PluginName, app, MigratedProperty) { + t.Errorf("legacy-mounts-migrated property missing for %s", app) } atts, err := LoadAttachments(app) if err != nil { @@ -242,9 +242,9 @@ func TestMigrateLegacyMountsBulkAndFlagFastPath(t *testing.T) { } } - // Re-stage a -v on alpha and rerun MigrateLegacyMounts. The flag - // file short-circuits per-app migration, so the new line stays in - // docker-options. + // Re-stage a -v on alpha and rerun MigrateLegacyMounts. The + // property marker short-circuits per-app migration, so the new + // line stays in docker-options. if err := common.PropertyListWrite("docker-options", "alpha", "_default_.deploy", []string{"-v /sneaky:/x"}); err != nil { t.Fatalf("re-stage alpha: %v", err) } @@ -324,6 +324,91 @@ func TestMigrateAppChownsLegacyEntryFiles(t *testing.T) { } } +// TestMigrateAppNoMountsDoesNotMarkMigrated confirms that an app with +// no legacy `-v` lines leaves the legacy-mounts-migrated property +// unset. Apps that have never had legacy state are distinguishable +// from apps that did and were drained. +func TestMigrateAppNoMountsDoesNotMarkMigrated(t *testing.T) { + _, dokkuRoot := setupMigrationEnv(t) + stageApp(t, dokkuRoot, "alpha", map[string][]string{ + "deploy": {"--restart=on-failure:5"}, + }) + + if err := migrateApp("alpha"); err != nil { + t.Fatalf("migrateApp: %v", err) + } + + if common.PropertyExists(PluginName, "alpha", MigratedProperty) { + t.Errorf("legacy-mounts-migrated property should not be set for an app with no -v lines") + } + + atts, err := LoadAttachments("alpha") + if err != nil { + t.Fatalf("LoadAttachments: %v", err) + } + if len(atts) != 0 { + t.Errorf("expected 0 attachments, got %d", len(atts)) + } +} + +// TestMigrateLegacyMountsConvertsLegacyFlagFile is the upgrade-cycle +// regression: on an install upgrading from the previous release, the +// per-app filesystem flag file under data/storage-registry/migrations/ +// must be drained into the property store and removed. +func TestMigrateLegacyMountsConvertsLegacyFlagFile(t *testing.T) { + _, dokkuRoot := setupMigrationEnv(t) + stageApp(t, dokkuRoot, "alpha", map[string][]string{ + "deploy": {"--restart=on-failure:5"}, + }) + + flagPath := migrationFlagFile("alpha") + if err := common.TouchFile(flagPath); err != nil { + t.Fatalf("touch flag file: %v", err) + } + + if err := MigrateLegacyMounts(); err != nil { + t.Fatalf("MigrateLegacyMounts: %v", err) + } + + got := common.PropertyGetDefault(PluginName, "alpha", MigratedProperty, "") + if got != "true" { + t.Errorf("legacy-mounts-migrated = %q, want %q", got, "true") + } + if _, err := os.Stat(flagPath); !os.IsNotExist(err) { + t.Errorf("expected flag file gone, got err=%v", err) + } +} + +// TestMigrateLegacyMountsRespectsExistingProperty confirms that the +// property is the gate: an app with a `-v` line is skipped when the +// property is already set, leaving the legacy line in docker-options. +func TestMigrateLegacyMountsRespectsExistingProperty(t *testing.T) { + _, dokkuRoot := setupMigrationEnv(t) + stageApp(t, dokkuRoot, "alpha", map[string][]string{ + "deploy": {"-v /var/log:/log"}, + }) + + if err := common.PropertyWrite(PluginName, "alpha", MigratedProperty, "true"); err != nil { + t.Fatalf("seed property: %v", err) + } + + if err := MigrateLegacyMounts(); err != nil { + t.Fatalf("MigrateLegacyMounts: %v", err) + } + + got := phaseOptions(t, "alpha", "deploy") + if !equalSorted(got, []string{"-v /var/log:/log"}) { + t.Errorf("expected -v line preserved, got %v", got) + } + atts, err := LoadAttachments("alpha") + if err != nil { + t.Fatalf("LoadAttachments: %v", err) + } + if len(atts) != 0 { + t.Errorf("expected 0 attachments (property gated migration), got %d", len(atts)) + } +} + func equalSorted(a, b []string) bool { if len(a) != len(b) { return false diff --git a/plugins/storage/triggers.go b/plugins/storage/triggers.go index 278b723cf..19081a389 100644 --- a/plugins/storage/triggers.go +++ b/plugins/storage/triggers.go @@ -37,7 +37,10 @@ func TriggerInstall() error { // The install trigger runs as root; the directories it just made are // root-owned, but the dokku user is the one that runs storage:create - // and writes entry / migration-flag files into them. + // and writes entry files into them. migrationFlagDir() stays in the + // list for one release cycle so the upgrade-cycle conversion in + // MigrateLegacyMounts can read pre-existing flag files. + // TODO(post-deprecation): drop migrationFlagDir() from this list. for _, dir := range []string{RegistryDirectory(), EntriesDirectory(), migrationFlagDir()} { if err := os.MkdirAll(dir, 0755); err != nil { return fmt.Errorf("Unable to create %s: %s", dir, err.Error()) diff --git a/tests/unit/docker-options-2.bats b/tests/unit/docker-options-2.bats index f7f7f289f..9fe3678f6 100644 --- a/tests/unit/docker-options-2.bats +++ b/tests/unit/docker-options-2.bats @@ -264,3 +264,4 @@ web: python3 -u web.py worker: python3 -u worker.py EOF } + diff --git a/tests/unit/storage.bats b/tests/unit/storage.bats index a93c40428..803c80047 100644 --- a/tests/unit/storage.bats +++ b/tests/unit/storage.bats @@ -420,6 +420,13 @@ teardown() { assert_success assert_output "1" + # Property marker is set after a successful migration with mounts. + run /bin/bash -c "sudo test -f /var/lib/dokku/config/storage/$TEST_APP/legacy-mounts-migrated" + assert_success + run /bin/bash -c "sudo cat /var/lib/dokku/config/storage/$TEST_APP/legacy-mounts-migrated" + assert_success + assert_output "true" + # Cleanup: unmount + destroy the synthesized legacy entry. We need # the entry name from list-entries since legacy- is content-derived. legacy_name=$(dokku storage:list-entries --format json | jq -r '.[] | select(.name | startswith("legacy-")) | .name' | head -1) @@ -438,3 +445,44 @@ teardown() { assert_success assert_output_contains "deprecated" } + +@test "(storage:migrate) leaves legacy-mounts-migrated unset for apps with no legacy -v lines" { + # Fresh app with no -v lines: storage:migrate must NOT write the + # legacy-mounts-migrated property. Distinguishes "never had legacy + # state" from "had legacy state, drained". + sudo rm -f "/var/lib/dokku/config/storage/$TEST_APP/legacy-mounts-migrated" + + run /bin/bash -c "dokku storage:migrate $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "sudo test -f /var/lib/dokku/config/storage/$TEST_APP/legacy-mounts-migrated" + assert_failure +} + +@test "(storage:migrate) converts legacy migration flag file into property" { + # Stage an upgrade-from-old-code scenario: a leftover flag file under + # data/storage-registry/migrations/. storage:migrate (which runs + # convertLegacyMigrationFlag before migrateApp) must drain it into + # the property and remove the file. + flag_dir="/var/lib/dokku/data/storage-registry/migrations" + sudo mkdir -p "$flag_dir" + sudo touch "$flag_dir/$TEST_APP" + sudo chown dokku:dokku "$flag_dir/$TEST_APP" + sudo rm -f "/var/lib/dokku/config/storage/$TEST_APP/legacy-mounts-migrated" + + run /bin/bash -c "dokku storage:migrate $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "sudo test -f /var/lib/dokku/config/storage/$TEST_APP/legacy-mounts-migrated" + assert_success + run /bin/bash -c "sudo cat /var/lib/dokku/config/storage/$TEST_APP/legacy-mounts-migrated" + assert_success + assert_output "true" + + run /bin/bash -c "sudo test -e $flag_dir/$TEST_APP" + assert_failure +}