refactor: convert filesystem migration markers to plugin properties

The docker-options DOCKER_OPTIONS_<PHASE>.migrated sentinels and the storage data/storage-registry/migrations/<app> flag files are now stored as per-app properties (`migrated-<phase>` and `legacy-mounts-migrated` respectively), so they survive a property-store backup/restore and align with the existing `migrated-from-files`, `env-migrated`, and `nginx-conf-sigil-migrated` patterns. The new markers are written only when a non-empty legacy source was actually migrated, distinguishing apps that never had legacy state from apps that did and were drained. A one-cycle upgrade-cycle conversion drains any leftover filesystem sentinels into the new properties and removes the files; for docker-options the conversion deliberately runs before the global `migrated-from-files` short-circuit so installs upgrading from the previous release still get their `.migrated` files converted.
2026-05-23 23:47:58 -04:00
parent 5df399273a
commit b2320121fe
9 changed files with 381 additions and 59 deletions
--- 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_<PHASE>` 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_<PHASE>` 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 |
--- 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 |
--- 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_<PHASE> file was drained into the property
+// store. Surfaces in the property store as `docker-options/<app>/migrated-<phase>`.
+func migratedPropertyKey(phase string) string {
+	return "migrated-" + strings.ToLower(phase)
+}
+
+// convertLegacyMigratedMarker drains a leftover DOCKER_OPTIONS_<PHASE>.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-<phase>` 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")
--- 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
--- 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/<appName>`.
+// 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/<app>`.
+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 <app>`.
+// 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
+// <app>`. 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/<app>) 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 {
--- 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
--- 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())
--- 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
 }
+
--- 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-<hash> 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/<app>. 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
+}