github/dokku
0
0
Fork 0
Code Issues Pull Requests Actions 5 Packages Projects Releases Wiki Activity

fix: honor storage attachment process types at deploy time

Browse Source 
An attachment's process type was recorded, reported and used to scope `storage:mount --replace` and `storage:unmount --all`, but nothing read it when the volume was actually mounted, so `storage:mount <app> <entry> --container-dir <path> --process-type web` bound the volume into every process rather than only `web`. Both schedulers now scope a mount to the process type it was declared for, and containers that belong to no process type - a `dokku run` one-off, an `app.json` deploy task, a k3s cron job - receive default-scoped mounts only. A container path can now be claimed by a single mount per process type, and `storage:migrate` drains process-scoped `-v` lines out of `docker-options` rather than only the default-scoped ones.
This commit is contained in:
Jose Diaz-Gonzalez
2026-09-21 18:08:42 -04:00
parent e00e939f25
commit b612285392
20 changed files with 1075 additions and 114 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
docs/advanced-usage/persistent-storage.md
View File

@@ -247,7 +247,7 @@ dokku storage:mount node-js-app /var/lib/dokku/data/storage/node-js-app:/app/sto
dokku storage:mount node-js-app some-docker-volume:/app/storage
```
In the first example, Dokku will then mount the shared contents of `/var/lib/dokku/data/storage/node-js-app` to `/app/storage` inside the container. The mount point is *not* relative to your app's working directory, and is instead relative to the root (`/`) of the container. Mounts are only available for containers created via `run` and by the deploy process, and not during the build process. In addition, the host path is never auto-created by either Dokku or Docker, and should be an explicit path, not one relative to the current working directory.
In the first example, Dokku will then mount the shared contents of `/var/lib/dokku/data/storage/node-js-app` to `/app/storage` inside the container. The mount point is *not* relative to your app's working directory, and is instead relative to the root (`/`) of the container. Mounts are only available for containers created via `run` and by the deploy process, and not during the build process. A mount is bound into every process type unless `--process-type` narrows it - see [scoping a mount to a process type](#scoping-a-mount-to-a-process-type). In addition, the host path is never auto-created by either Dokku or Docker, and should be an explicit path, not one relative to the current working directory.
> If the `/storage` path within the container had pre-existing content, the container files will be over-written. This may be an issue for users that create assets at build time but then mount a directory at the same place during runtime. Files are not merged.
@@ -268,6 +268,40 @@ Once persistent storage is mounted, the app requires a restart. See the [process
dokku ps:restart app-name
```
#### Scoping a mount to a process type
> [!IMPORTANT]
> New as of 0.38.28
By default a mount is bound into every one of an app's containers. `--process-type` narrows it to a single Procfile process type, which is how a worker gets a scratch volume the web process has no business seeing:
```shell
dokku storage:create node-js-scratch
dokku storage:mount node-js-app node-js-scratch --container-dir /scratch --process-type worker
```
The magic `_default_` process type is the unscoped case, and it is what a mount gets when `--process-type` is omitted. Default-scoped mounts are bound into every process; a mount scoped to a named process type is bound only into that one.
Some containers belong to no process type at all - a `dokku run` one-off, the ephemeral containers that run `app.json` deploy tasks, and k3s cron jobs. Those receive default-scoped mounts only, since there is no process type for a named scope to match. On k3s this applies to the deploy phase; `--phase run` attachments are not mounted there at all.
A container path can be claimed by only one mount per process type, so `storage:mount` rejects a container path that an overlapping scope already holds. The default scope overlaps every named one:
```shell
dokku storage:create node-js-data
dokku storage:create node-js-cache
dokku storage:mount node-js-app node-js-data --container-dir /app/storage
# fails: the default-scoped mount above also applies to web
dokku storage:mount node-js-app node-js-cache --container-dir /app/storage --process-type web
```
Two *named* scopes never apply to the same container, so they may each bind a different entry at the same container path:
```shell
dokku storage:mount node-js-app node-js-data --container-dir /app/storage --process-type web
dokku storage:mount node-js-app node-js-cache --container-dir /app/storage --process-type worker
```
#### Replacing the entire mount set
> [!IMPORTANT]

2
docs/deployment/schedulers/k3s.md
View File

@@ -1158,6 +1158,8 @@ git push dokku master
For a hostPath-backed PV (no StorageClass), pass `<path>` as the second positional argument and omit `--storage-class-name`. The plugin renders both the PV and the PVC into the entry's helm release. The `--reclaim-policy` flag (`Retain` or `Delete`) controls whether the underlying PV survives `storage:destroy`. Annotations and labels set via `storage:annotations:set` and `storage:labels:set` propagate to both the PVC and the PV so backup tools (Velero, Longhorn snapshots) can find them.
`storage:mount --process-type <proc>` scopes a volume to a single process type, so only that deployment's pods mount the PVC; a mount left in the default `_default_` scope is mounted by every deployment. Cron jobs are keyed by a generated cron ID rather than a Procfile process type, so they mount default-scoped volumes only. Only deploy-phase attachments are mounted on k3s - `--phase run` has no effect here.
The legacy `storage:mount <app> <host>:<container>` colon form is rejected on k3s apps; create a named entry instead. See [Persistent Storage](/docs/advanced-usage/persistent-storage.md) for the full command reference.
### Chart upgrade callbacks

2
docs/development/plugin-triggers.md
View File

@@ -875,7 +875,7 @@ APP="$1"; IMAGE_SOURCE_TYPE="$2"
### `docker-args-process-deploy`
- Description: emits docker arguments scoped to a specific Procfile process type. The `docker-options` plugin implements this trigger to surface options registered via `docker-options:add --process <PROC>`. `$PROC_TYPE` may be empty (or set to the magic `_default_` value) to signify default-scope docker deploy options.
- Description: emits docker arguments scoped to a specific Procfile process type. The `docker-options` plugin implements this trigger to surface options registered via `docker-options:add --process <PROC>`, and the `storage` plugin implements it to emit the `-v` flags for mounts made with `storage:mount --process-type <PROC>`. `$PROC_TYPE` may be empty (or set to the magic `_default_` value) to signify default-scope docker deploy options. Note that the two plugins split the work differently: `docker-options` emits only its process-specific scope here and its default scope through `docker-args-deploy`, while `storage` emits both scopes here and does not implement `docker-args-deploy` at all.
- Invoked by: `dokku deploy`
- Arguments: `$APP $IMAGE_SOURCE_TYPE $IMAGE_TAG [$PROC_TYPE $CONTAINER_INDEX]`
- Example:

20
plugins/scheduler-k3s/chart.go
View File

@@ -16,6 +16,7 @@ import (
"github.com/dokku/dokku/plugins/config"
"github.com/dokku/dokku/plugins/cron"
"github.com/dokku/dokku/plugins/registry"
"github.com/dokku/dokku/plugins/storage"
"github.com/kballard/go-shellquote"
)
@@ -335,9 +336,12 @@ func BuildAppChart(ctx context.Context, appName, imageTag string, opts BuildOpti
}
}
processVolumes := []ProcessVolume{}
// Volumes every workload in the app receives, whichever process type it
// belongs to. Storage attachments are layered on top per process type
// inside the loops below.
baseVolumes := []ProcessVolume{}
if shmSize := getComputedShmSize(appName); shmSize != "" {
processVolumes = append(processVolumes, ProcessVolume{
baseVolumes = append(baseVolumes, ProcessVolume{
Name: "shmem",
MountPath: "/dev/shm",
EmptyDir: &ProcessVolumeEmptyDir{
@@ -351,11 +355,9 @@ func BuildAppChart(ctx context.Context, appName, imageTag string, opts BuildOpti
if err != nil {
return cleanup(fmt.Errorf("error loading storage app mounts: %w", err))
}
deployVolumes, err := ToProcessVolumes(deployMountPairs)
if err != nil {
if err := ValidateMountPairs(deployMountPairs); err != nil {
return cleanup(err)
}
processVolumes = append(processVolumes, deployVolumes...)
for processType, processCount := range processes {
healthchecks, ok := appJSON.Healthchecks[processType]
@@ -411,7 +413,7 @@ func BuildAppChart(ctx context.Context, appName, imageTag string, opts BuildOpti
ProcessType: ProcessType_Worker,
Replicas: int32(processCount),
Resources: processResources,
Volumes: processVolumes,
Volumes: processVolumesFor(baseVolumes, deployMountPairs, processType),
}
if processType == "web" {
@@ -589,7 +591,11 @@ func BuildAppChart(ctx context.Context, appName, imageTag string, opts BuildOpti
ProcessType: ProcessType_Cron,
Replicas: 1,
Resources: processResources,
Volumes: processVolumes,
// A cron ID is a hash of the app name, command and schedule, so it
// is not something an operator can name in --process-type and it
// churns whenever the task changes. Cron jobs therefore sit with
// `dokku run` and app.json tasks: default-scoped mounts only.
Volumes: processVolumesFor(baseVolumes, deployMountPairs, storage.DefaultProcessType),
}
values.Processes[cronTask.ID] = processValues
}

47
plugins/scheduler-k3s/storage.go
View File

@@ -275,18 +275,37 @@ func LoadAppMounts(appName string, phase string) ([]AppMountPair, error) {
return pairs, nil
}
// ToProcessVolumes converts each AppMountPair into a ProcessVolume. K3s
// app deployments reference the PVC by name; the PVC itself is owned by
// the storage entry's separate helm release. Any docker-local entries
// found here are an error - they cannot be mounted on a k3s app.
func ToProcessVolumes(pairs []AppMountPair) ([]ProcessVolume, error) {
volumes := []ProcessVolume{}
// ValidateMountPairs rejects any pair whose entry belongs to another
// scheduler. Kept separate from ToProcessVolumes so the chart builder can run
// it once over the whole set: folding it into the per-process conversion would
// make it reachable only for apps that have at least one scaled process or
// cron task, and an app scaled to zero would stop reporting the mismatch.
func ValidateMountPairs(pairs []AppMountPair) error {
for _, pair := range pairs {
if pair.Entry == nil || pair.Attachment == nil {
continue
}
if pair.Entry.Scheduler != storage.SchedulerK3s {
return nil, fmt.Errorf("storage entry %q is scheduler=%s but is mounted on a k3s app; recreate it with --scheduler k3s", pair.Entry.Name, pair.Entry.Scheduler)
return fmt.Errorf("storage entry %q is scheduler=%s but is mounted on a k3s app; recreate it with --scheduler k3s", pair.Entry.Name, pair.Entry.Scheduler)
}
}
return nil
}
// ToProcessVolumes converts the AppMountPairs that apply to processType into
// ProcessVolumes. K3s app deployments reference the PVC by name; the PVC
// itself is owned by the storage entry's separate helm release.
//
// An empty processType names a workload that belongs to no process type - a
// cron job - and so receives default-scoped attachments only.
func ToProcessVolumes(pairs []AppMountPair, processType string) []ProcessVolume {
volumes := []ProcessVolume{}
for _, pair := range pairs {
if pair.Entry == nil || pair.Attachment == nil {
continue
}
if !pair.Attachment.AppliesToProcessType(processType) {
continue
}
volumes = append(volumes, ProcessVolume{
Name: pair.Entry.Name,
@@ -298,7 +317,19 @@ func ToProcessVolumes(pairs []AppMountPair) ([]ProcessVolume, error) {
},
})
}
return volumes, nil
return volumes
}
// processVolumesFor returns the volumes one process type's pods receive: the
// volumes every workload in the app gets, plus the storage attachments scoped
// to that process type. The slice is freshly allocated rather than appended
// onto base, so one workload's volumes can never alias another's.
func processVolumesFor(base []ProcessVolume, pairs []AppMountPair, processType string) []ProcessVolume {
scoped := ToProcessVolumes(pairs, processType)
volumes := make([]ProcessVolume, 0, len(base)+len(scoped))
volumes = append(volumes, base...)
volumes = append(volumes, scoped...)
return volumes
}
// asK8sVolumeMount is a small helper used by tests / other callers that

115
plugins/scheduler-k3s/storage_test.go
View File

@@ -6,7 +6,7 @@ import (
"github.com/dokku/dokku/plugins/storage"
)
func TestToProcessVolumesRejectsDockerLocalEntry(t *testing.T) {
func TestValidateMountPairsRejectsDockerLocalEntry(t *testing.T) {
pairs := []AppMountPair{
{
Entry: &storage.Entry{
@@ -19,7 +19,7 @@ func TestToProcessVolumesRejectsDockerLocalEntry(t *testing.T) {
},
}
_, err := ToProcessVolumes(pairs)
err := ValidateMountPairs(pairs)
if err == nil {
t.Fatal("expected error for docker-local entry, got nil")
}
@@ -29,7 +29,7 @@ func TestToProcessVolumesRejectsDockerLocalEntry(t *testing.T) {
}
}
func TestToProcessVolumesRejectsNonK3sEntry(t *testing.T) {
func TestValidateMountPairsRejectsNonK3sEntry(t *testing.T) {
pairs := []AppMountPair{
{
Entry: &storage.Entry{
@@ -42,7 +42,7 @@ func TestToProcessVolumesRejectsNonK3sEntry(t *testing.T) {
},
}
_, err := ToProcessVolumes(pairs)
err := ValidateMountPairs(pairs)
if err == nil {
t.Fatal("expected error for non-k3s entry, got nil")
}
@@ -52,6 +52,28 @@ func TestToProcessVolumesRejectsNonK3sEntry(t *testing.T) {
}
}
// TestValidateMountPairsRejectsOutOfScopeEntry pins the reason validation is a
// separate pass: a mismatched entry scoped to a process type the caller is not
// currently building still has to fail the deploy.
func TestValidateMountPairsRejectsOutOfScopeEntry(t *testing.T) {
pairs := []AppMountPair{
{
Entry: &storage.Entry{
Name: "demo-data",
Scheduler: storage.SchedulerDockerLocal,
},
Attachment: &storage.Attachment{
ContainerPath: "/data",
ProcessType: "worker",
},
},
}
if err := ValidateMountPairs(pairs); err == nil {
t.Fatal("expected error for docker-local entry scoped to another process, got nil")
}
}
func TestToProcessVolumesAcceptsK3sEntry(t *testing.T) {
pairs := []AppMountPair{
{
@@ -67,10 +89,7 @@ func TestToProcessVolumesAcceptsK3sEntry(t *testing.T) {
},
}
volumes, err := ToProcessVolumes(pairs)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
volumes := ToProcessVolumes(pairs, "web")
if len(volumes) != 1 {
t.Fatalf("expected 1 volume, got %d", len(volumes))
}
@@ -99,11 +118,81 @@ func TestToProcessVolumesSkipsNilPairs(t *testing.T) {
{Entry: nil, Attachment: &storage.Attachment{ContainerPath: "/data"}},
}
volumes, err := ToProcessVolumes(pairs)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(volumes) != 0 {
if volumes := ToProcessVolumes(pairs, "web"); len(volumes) != 0 {
t.Fatalf("expected 0 volumes, got %d", len(volumes))
}
}
// TestToProcessVolumesScopesToProcessType is the k3s half of #9059: a mount
// scoped to a named process type reaches that process and no other, while a
// default-scoped mount reaches all of them.
func TestToProcessVolumesScopesToProcessType(t *testing.T) {
pairs := []AppMountPair{
{
Entry: &storage.Entry{Name: "shared", Scheduler: storage.SchedulerK3s},
Attachment: &storage.Attachment{ContainerPath: "/shared", ProcessType: storage.DefaultProcessType},
},
{
Entry: &storage.Entry{Name: "web-only", Scheduler: storage.SchedulerK3s},
Attachment: &storage.Attachment{ContainerPath: "/web", ProcessType: "web"},
},
{
Entry: &storage.Entry{Name: "legacy", Scheduler: storage.SchedulerK3s},
Attachment: &storage.Attachment{ContainerPath: "/legacy"},
},
}
for _, tc := range []struct {
processType string
expected []string
}{
{processType: "web", expected: []string{"shared", "web-only", "legacy"}},
{processType: "worker", expected: []string{"shared", "legacy"}},
{processType: "", expected: []string{"shared", "legacy"}},
} {
volumes := ToProcessVolumes(pairs, tc.processType)
names := []string{}
for _, volume := range volumes {
names = append(names, volume.Name)
}
if len(names) != len(tc.expected) {
t.Fatalf("process type %q: expected %v, got %v", tc.processType, tc.expected, names)
}
for i, name := range tc.expected {
if names[i] != name {
t.Fatalf("process type %q: expected %v, got %v", tc.processType, tc.expected, names)
}
}
}
}
// TestProcessVolumesForDoesNotAliasBase pins that each process type gets its
// own slice: appending onto a shared backing array would leak one process's
// storage mounts into another's pod spec.
func TestProcessVolumesForDoesNotAliasBase(t *testing.T) {
base := make([]ProcessVolume, 0, 8)
base = append(base, ProcessVolume{Name: "shmem", MountPath: "/dev/shm"})
pairs := []AppMountPair{
{
Entry: &storage.Entry{Name: "web-only", Scheduler: storage.SchedulerK3s},
Attachment: &storage.Attachment{ContainerPath: "/web", ProcessType: "web"},
},
}
web := processVolumesFor(base, pairs, "web")
worker := processVolumesFor(base, pairs, "worker")
if len(web) != 2 {
t.Fatalf("expected web to get 2 volumes, got %d", len(web))
}
if len(worker) != 1 {
t.Fatalf("expected worker to get 1 volume, got %d", len(worker))
}
if worker[0].Name != "shmem" {
t.Fatalf("expected worker to keep only the base volume, got %s", worker[0].Name)
}
if len(base) != 1 {
t.Fatalf("expected base to be left untouched, got %d volumes", len(base))
}
}

3
plugins/storage/.gitignore vendored
View File

@@ -1,4 +1,7 @@
/commands
/docker-args-deploy
/docker-args-process-deploy
/docker-args-run
/subcommands/*
/triggers/*
/triggers

2
plugins/storage/Makefile
View File

@@ -1,6 +1,6 @@
GOARCH ?= amd64
SUBCOMMANDS = subcommands/default subcommands/annotations:set subcommands/annotations:report subcommands/create subcommands/destroy subcommands/ensure-directory subcommands/exec subcommands/info subcommands/labels:set subcommands/labels:report subcommands/list subcommands/list-entries subcommands/migrate subcommands/mount subcommands/report subcommands/set subcommands/unmount subcommands/wait
TRIGGERS = triggers/install triggers/storage-list triggers/storage-app-mounts triggers/docker-args-deploy triggers/docker-args-run triggers/post-delete triggers/post-app-clone-setup triggers/post-app-rename-setup
TRIGGERS = triggers/install triggers/storage-list triggers/storage-app-mounts triggers/docker-args-process-deploy triggers/docker-args-run triggers/post-delete triggers/post-app-clone-setup triggers/post-app-rename-setup
BUILD = commands subcommands triggers
PLUGIN_NAME = storage

74
plugins/storage/attachment.go
View File

@@ -66,6 +66,38 @@ func (a *Attachment) Validate() error {
return nil
}
// EffectiveProcessType returns the process type an attachment is scoped to,
// treating an unset value as the default scope so attachments written before
// the field existed still sort into a scope.
func (a *Attachment) EffectiveProcessType() string {
if a == nil || a.ProcessType == "" {
return DefaultProcessType
}
return a.ProcessType
}
// AppliesToProcessType reports whether an attachment is mounted into the
// container being built for the given process type. The default scope applies
// to every process; a named scope applies only to itself. An empty processType
// names a container that belongs to no process at all - a `dokku run` one-off,
// an app.json deploy task, a k3s cron job - and so sees the default scope only.
func (a *Attachment) AppliesToProcessType(processType string) bool {
if processType == "" {
processType = DefaultProcessType
}
scope := a.EffectiveProcessType()
return scope == DefaultProcessType || scope == processType
}
// scopesOverlap reports whether two process-type scopes can both apply to one
// container. The default scope overlaps every named scope; two named scopes
// overlap only when they are equal.
func scopesOverlap(first string, second string) bool {
return first == DefaultProcessType || second == DefaultProcessType || first == second
}
// LoadAttachments returns every attachment registered against an app.
func LoadAttachments(appName string) ([]*Attachment, error) {
lines, err := common.PropertyListGet(PluginName, appName, AttachmentsProperty)
@@ -101,8 +133,29 @@ func SaveAttachments(appName string, attachments []*Attachment) error {
return common.PropertyListWrite(PluginName, appName, AttachmentsProperty, lines)
}
// ensureContainerPathFree rejects a candidate whose container path is already
// claimed by an attachment in an overlapping scope. Two attachments bound at
// one container path in overlapping scopes emit two -v flags for the same
// target, leaving which volume the process actually sees up to Docker.
func ensureContainerPathFree(appName string, attachments []*Attachment, candidate *Attachment) error {
for _, existing := range attachments {
if existing.ContainerPath != candidate.ContainerPath {
continue
}
if !scopesOverlap(existing.EffectiveProcessType(), candidate.EffectiveProcessType()) {
continue
}
return fmt.Errorf("Container path %s on app %s is already mounted by storage entry %s for process type %s",
candidate.ContainerPath, appName, existing.EntryName, existing.EffectiveProcessType())
}
return nil
}
// AddAttachment appends an attachment, rejecting duplicates of the same
// (entry_name, container_path, process_type) tuple.
// (entry_name, container_path, process_type) tuple as well as container paths
// already claimed by an attachment in an overlapping scope.
func AddAttachment(appName string, attachment *Attachment) error {
if err := attachment.Validate(); err != nil {
return err
@@ -116,12 +169,16 @@ func AddAttachment(appName string, attachment *Attachment) error {
for _, existing := range attachments {
if existing.EntryName == attachment.EntryName &&
existing.ContainerPath == attachment.ContainerPath &&
existing.ProcessType == attachment.ProcessType {
existing.EffectiveProcessType() == attachment.EffectiveProcessType() {
return fmt.Errorf("storage entry %q is already mounted at %q for process type %q on app %q",
attachment.EntryName, attachment.ContainerPath, attachment.ProcessType, appName)
attachment.EntryName, attachment.ContainerPath, attachment.EffectiveProcessType(), appName)
}
}
if err := ensureContainerPathFree(appName, attachments, attachment); err != nil {
return err
}
attachments = append(attachments, attachment)
return SaveAttachments(appName, attachments)
}
@@ -136,6 +193,9 @@ func AddAttachment(appName string, attachment *Attachment) error {
// false Readonly clears any previously-set value, mirroring the operator's
// "set the flags as I just typed them" intent and the same idempotent
// contract storage:create offers for entries.
//
// A new attachment is rejected when its container path is already claimed by
// an attachment in an overlapping scope.
func UpsertAttachment(appName string, attachment *Attachment) (bool, error) {
if err := attachment.Validate(); err != nil {
return false, err
@@ -149,7 +209,7 @@ func UpsertAttachment(appName string, attachment *Attachment) (bool, error) {
for _, existing := range attachments {
if existing.EntryName == attachment.EntryName &&
existing.ContainerPath == attachment.ContainerPath &&
existing.ProcessType == attachment.ProcessType {
existing.EffectiveProcessType() == attachment.EffectiveProcessType() {
existing.Phases = attachment.Phases
existing.Subpath = attachment.Subpath
existing.Readonly = attachment.Readonly
@@ -159,6 +219,12 @@ func UpsertAttachment(appName string, attachment *Attachment) (bool, error) {
}
}
// Only the append branch needs the overlap check: rewriting an existing
// tuple in place would otherwise trip over the attachment it is updating.
if err := ensureContainerPathFree(appName, attachments, attachment); err != nil {
return false, err
}
attachments = append(attachments, attachment)
return true, SaveAttachments(appName, attachments)
}

122
plugins/storage/attachment_test.go
View File

@@ -302,19 +302,135 @@ func TestUpsertAttachmentDistinctIdentity(t *testing.T) {
Expect(err).NotTo(HaveOccurred())
Expect(created).To(BeTrue())
// Different process type on the same (entry, container_path) is also distinct.
// Two named scopes never apply to the same container, so they can each
// claim one container path.
created, err = UpsertAttachment("demo", &Attachment{
EntryName: "demo-data",
ContainerPath: "/data",
ContainerPath: "/scoped",
Phases: []string{PhaseDeploy},
ProcessType: "web",
})
Expect(err).NotTo(HaveOccurred())
Expect(created).To(BeTrue())
created, err = UpsertAttachment("demo", &Attachment{
EntryName: "demo-data",
ContainerPath: "/scoped",
Phases: []string{PhaseDeploy},
ProcessType: "worker",
})
Expect(err).NotTo(HaveOccurred())
Expect(created).To(BeTrue())
attachments, err := LoadAttachments("demo")
Expect(err).NotTo(HaveOccurred())
Expect(attachments).To(HaveLen(3))
Expect(attachments).To(HaveLen(4))
}
// TestUpsertAttachmentRejectsOverlappingContainerPath covers the conflict the
// process-type scoping makes reachable: the default scope also applies to
// "web", so both attachments would emit a -v flag for /data on the same
// container and Docker would decide which one wins.
func TestUpsertAttachmentRejectsOverlappingContainerPath(t *testing.T) {
RegisterTestingT(t)
withTempLibRoot(t)
created, err := UpsertAttachment("demo", &Attachment{
EntryName: "demo-data",
ContainerPath: "/data",
Phases: []string{PhaseDeploy},
ProcessType: DefaultProcessType,
})
Expect(err).NotTo(HaveOccurred())
Expect(created).To(BeTrue())
_, err = UpsertAttachment("demo", &Attachment{
EntryName: "demo-cache",
ContainerPath: "/data",
Phases: []string{PhaseDeploy},
ProcessType: "web",
})
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("Container path /data on app demo is already mounted by storage entry demo-data for process type _default_"))
// The rejected write leaves the stored set exactly as it was.
attachments, err := LoadAttachments("demo")
Expect(err).NotTo(HaveOccurred())
Expect(attachments).To(HaveLen(1))
// The same collision the other way round: a default-scoped mount cannot
// claim a path a named scope already holds.
_, err = UpsertAttachment("demo", &Attachment{
EntryName: "demo-cache",
ContainerPath: "/cache",
Phases: []string{PhaseDeploy},
ProcessType: "worker",
})
Expect(err).NotTo(HaveOccurred())
_, err = UpsertAttachment("demo", &Attachment{
EntryName: "demo-logs",
ContainerPath: "/cache",
Phases: []string{PhaseDeploy},
ProcessType: DefaultProcessType,
})
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("for process type worker"))
}
// TestAttachmentAppliesToProcessType pins the scoping rule every deploy-time
// caller shares: the default scope reaches every process, a named scope reaches
// only itself, and a container with no process type of its own - a `dokku run`
// one-off, an app.json deploy task, a k3s cron job - sees the default scope.
func TestAttachmentAppliesToProcessType(t *testing.T) {
RegisterTestingT(t)
defaultScoped := &Attachment{ProcessType: DefaultProcessType}
legacyScoped := &Attachment{}
webScoped := &Attachment{ProcessType: "web"}
Expect(defaultScoped.EffectiveProcessType()).To(Equal(DefaultProcessType))
Expect(legacyScoped.EffectiveProcessType()).To(Equal(DefaultProcessType))
Expect(webScoped.EffectiveProcessType()).To(Equal("web"))
for _, processType := range []string{"web", "worker", "", DefaultProcessType} {
Expect(defaultScoped.AppliesToProcessType(processType)).To(BeTrue(), "default scope, process type %q", processType)
Expect(legacyScoped.AppliesToProcessType(processType)).To(BeTrue(), "legacy scope, process type %q", processType)
}
Expect(webScoped.AppliesToProcessType("web")).To(BeTrue())
Expect(webScoped.AppliesToProcessType("worker")).To(BeFalse())
Expect(webScoped.AppliesToProcessType("")).To(BeFalse())
Expect(webScoped.AppliesToProcessType(DefaultProcessType)).To(BeFalse())
}
// TestAddAttachmentTreatsLegacyScopeAsDefault covers attachments written before
// the process-type field existed: an empty value and an explicit _default_ name
// the same scope, so the pair is a duplicate rather than two mounts.
func TestAddAttachmentTreatsLegacyScopeAsDefault(t *testing.T) {
RegisterTestingT(t)
root := withTempLibRoot(t)
writeAttachmentsFile(t, root, "demo", []*Attachment{
{
EntryName: "demo-data",
ContainerPath: "/data",
Phases: []string{PhaseDeploy},
},
})
err := AddAttachment("demo", &Attachment{
EntryName: "demo-data",
ContainerPath: "/data",
Phases: []string{PhaseDeploy},
ProcessType: DefaultProcessType,
})
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("is already mounted at"))
attachments, err := LoadAttachments("demo")
Expect(err).NotTo(HaveOccurred())
Expect(attachments).To(HaveLen(1))
}
func TestUpsertAttachmentValidationFailureLeavesStoreUntouched(t *testing.T) {

112
plugins/storage/migrate.go
View File

@@ -19,6 +19,12 @@ import (
// filesystem flag file at `data/storage-registry/migrations/<app>`.
const MigratedProperty = "legacy-mounts-migrated"
// MigratedPropertyVersion is the value MigratedProperty carries once an app
// has been through the current migration. Earlier releases wrote "true" and
// drained only the default process-type scope, so an app still holding that
// value is rescanned once to pick up its process-scoped `-v` lines.
const MigratedPropertyVersion = "2"
// migrationFlagDir / migrationFlagFile remain for one release cycle so
// the upgrade-cycle helper convertLegacyMigrationFlag can drain any
// leftover flag files into the new property.
@@ -43,7 +49,7 @@ func MigrateApp(appName string) error {
if err := convertLegacyMigrationFlag(appName); err != nil {
return err
}
if err := migrateApp(appName); err != nil {
if err := migrateApp(appName, true); err != nil {
return fmt.Errorf("storage migration failed for app %q: %w", appName, err)
}
return nil
@@ -66,10 +72,7 @@ func MigrateLegacyMounts() error {
if err := convertLegacyMigrationFlag(app); err != nil {
return err
}
if common.PropertyExists(PluginName, app, MigratedProperty) {
continue
}
if err := migrateApp(app); err != nil {
if err := migrateApp(app, false); err != nil {
return fmt.Errorf("storage migration failed for app %q: %w", app, err)
}
}
@@ -97,18 +100,57 @@ func convertLegacyMigrationFlag(appName string) error {
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
// preserved on partial failure.
func migrateApp(appName string) error {
deployLines, err := dockeroptions.GetDockerOptionsForPhase(appName, PhaseDeploy)
// migrateApp performs the per-app migration across every process-type scope
// docker-options holds options for. The default scope is migrated first: the
// per-scope drain skips a named-scope mount that the default scope already
// covers, which only works once the default scope's attachments exist.
//
// force bypasses the version gate, which is what `dokku storage:migrate <app>`
// wants - it exists so an operator can rescan an app whose docker-options
// state changed after the bulk install-time pass.
func migrateApp(appName string, force bool) error {
migrated := common.PropertyGet(PluginName, appName, MigratedProperty)
if !force && migrated == MigratedPropertyVersion {
return nil
}
processTypes := []string{DefaultProcessType}
named, err := dockeroptions.ListProcessTypesWithOptions(appName)
if err != nil {
return err
}
runLines, err := dockeroptions.GetDockerOptionsForPhase(appName, PhaseRun)
processTypes = append(processTypes, named...)
drained := false
for _, processType := range processTypes {
scopeDrained, err := migrateProcessScope(appName, processType)
if err != nil {
return err
}
drained = drained || scopeDrained
}
// An app that never had legacy state keeps no marker at all, so
// `storage:report` (and future tooling) can still tell it apart from one
// that did and was drained. An app already carrying an older marker keeps
// one, advanced to the current version so it stops being rescanned.
if !drained && migrated == "" {
return nil
}
return common.PropertyWrite(PluginName, appName, MigratedProperty, MigratedPropertyVersion)
}
// migrateProcessScope drains one process-type scope's legacy `-v` lines into
// attachments. Reports whether anything was drained.
func migrateProcessScope(appName string, processType string) (bool, error) {
deployLines, err := dockeroptions.GetDockerOptionsForProcessPhase(appName, processType, PhaseDeploy)
if err != nil {
return err
return false, err
}
runLines, err := dockeroptions.GetDockerOptionsForProcessPhase(appName, processType, PhaseRun)
if err != nil {
return false, err
}
deployMounts := filterMountLines(deployLines)
@@ -130,22 +172,19 @@ 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
return false, nil
}
for _, mount := range mounts {
if err := migrateMount(appName, mount, phaseMap[mount]); err != nil {
return err
if err := migrateMount(appName, processType, mount, phaseMap[mount]); err != nil {
return false, err
}
}
return common.PropertyWrite(PluginName, appName, MigratedProperty, "true")
return true, nil
}
func migrateMount(appName string, mount string, phases []string) error {
func migrateMount(appName string, processType string, mount string, phases []string) error {
parsed := ParseMountPath(mount)
entry := LegacyMountToEntry(mount)
@@ -173,7 +212,7 @@ func migrateMount(appName string, mount string, phases []string) error {
EntryName: entry.Name,
ContainerPath: containerPath,
Phases: phases,
ProcessType: DefaultProcessType,
ProcessType: processType,
Readonly: parsed.Readonly,
VolumeOptions: parsed.VolumeOptions,
}
@@ -182,7 +221,11 @@ func migrateMount(appName string, mount string, phases []string) error {
if err != nil {
return err
}
if !attachmentExists(existing, attachment) {
// A named scope whose mount the default scope already covers needs no
// attachment of its own - the default one reaches that process anyway, and
// recording both would manufacture two -v flags for one container path.
// The docker-options line is still drained so it stops being emitted twice.
if !attachmentExists(existing, attachment) && !coveredByDefaultScope(existing, attachment) {
existing = append(existing, attachment)
if err := SaveAttachments(appName, existing); err != nil {
return err
@@ -193,12 +236,31 @@ func migrateMount(appName string, mount string, phases []string) error {
for _, phase := range phases {
mountedPhases = append(mountedPhases, phase)
}
if err := dockeroptions.RemoveDockerOptionFromPhases(appName, mountedPhases, fmt.Sprintf("-v %s", mount)); err != nil {
if err := dockeroptions.RemoveDockerOptionFromProcessPhases(appName, []string{processType}, mountedPhases, fmt.Sprintf("-v %s", mount)); err != nil {
return err
}
return nil
}
// coveredByDefaultScope reports whether a named-scope candidate binds the same
// entry at the same container path as an attachment already in the default
// scope. Always false for a default-scope candidate, which is its own cover.
func coveredByDefaultScope(attachments []*Attachment, candidate *Attachment) bool {
if candidate.EffectiveProcessType() == DefaultProcessType {
return false
}
for _, existing := range attachments {
if existing.EffectiveProcessType() != DefaultProcessType {
continue
}
if existing.EntryName == candidate.EntryName && existing.ContainerPath == candidate.ContainerPath {
return true
}
}
return false
}
func filterMountLines(lines []string) []string {
out := []string{}
for _, line := range lines {
@@ -222,7 +284,7 @@ func attachmentExists(attachments []*Attachment, candidate *Attachment) bool {
for _, existing := range attachments {
if existing.EntryName == candidate.EntryName &&
existing.ContainerPath == candidate.ContainerPath &&
existing.ProcessType == candidate.ProcessType {
existing.EffectiveProcessType() == candidate.EffectiveProcessType() {
return true
}
}

163
plugins/storage/migrate_test.go
View File

@@ -72,7 +72,7 @@ func TestMigrateAppSinglePhase(t *testing.T) {
"deploy": {"-v /var/log:/log", "--restart=on-failure:5"},
})
if err := migrateApp("alpha"); err != nil {
if err := migrateApp("alpha", false); err != nil {
t.Fatalf("migrateApp: %v", err)
}
@@ -126,7 +126,7 @@ func TestMigrateAppCrossPhaseGroupsIntoOneAttachment(t *testing.T) {
"run": {"-v /data:/d"},
})
if err := migrateApp("alpha"); err != nil {
if err := migrateApp("alpha", false); err != nil {
t.Fatalf("migrateApp: %v", err)
}
@@ -158,7 +158,7 @@ func TestMigrateAppPreservesVolumeOptions(t *testing.T) {
"deploy": {"-v /ro:/r:ro", "-v /label:/l:Z"},
})
if err := migrateApp("alpha"); err != nil {
if err := migrateApp("alpha", false); err != nil {
t.Fatalf("migrateApp: %v", err)
}
@@ -190,7 +190,7 @@ func TestMigrateAppIdempotent(t *testing.T) {
"deploy": {"-v /var/log:/log"},
})
if err := migrateApp("alpha"); err != nil {
if err := migrateApp("alpha", false); err != nil {
t.Fatalf("first migrateApp: %v", err)
}
@@ -202,7 +202,7 @@ func TestMigrateAppIdempotent(t *testing.T) {
if err := common.PropertyListWrite("docker-options", "alpha", "_default_.deploy", []string{"-v /var/log:/log"}); err != nil {
t.Fatalf("re-stage: %v", err)
}
if err := migrateApp("alpha"); err != nil {
if err := migrateApp("alpha", false); err != nil {
t.Fatalf("second migrateApp: %v", err)
}
@@ -280,7 +280,7 @@ func TestMigrateAppRefusesEntryNameCollision(t *testing.T) {
"deploy": {"-v /var/log:/log"},
})
if err := migrateApp("alpha"); err == nil {
if err := migrateApp("alpha", false); err == nil {
t.Fatalf("expected migrateApp to error on entry name collision")
}
@@ -301,7 +301,7 @@ func TestMigrateAppChownsLegacyEntryFiles(t *testing.T) {
"deploy": {"-v /var/log:/log"},
})
if err := migrateApp("alpha"); err != nil {
if err := migrateApp("alpha", false); err != nil {
t.Fatalf("migrateApp: %v", err)
}
@@ -334,7 +334,7 @@ func TestMigrateAppNoMountsDoesNotMarkMigrated(t *testing.T) {
"deploy": {"--restart=on-failure:5"},
})
if err := migrateApp("alpha"); err != nil {
if err := migrateApp("alpha", false); err != nil {
t.Fatalf("migrateApp: %v", err)
}
@@ -370,25 +370,28 @@ func TestMigrateLegacyMountsConvertsLegacyFlagFile(t *testing.T) {
t.Fatalf("MigrateLegacyMounts: %v", err)
}
// The flag file converts to the old "true" value, which the version gate
// then treats as stale and rescans, leaving the marker at the current
// version rather than at the value the flag file carried.
got := common.PropertyGetDefault(PluginName, "alpha", MigratedProperty, "")
if got != "true" {
t.Errorf("legacy-mounts-migrated = %q, want %q", got, "true")
if got != MigratedPropertyVersion {
t.Errorf("legacy-mounts-migrated = %q, want %q", got, MigratedPropertyVersion)
}
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) {
// TestMigrateLegacyMountsRespectsCurrentMarker confirms that the marker is the
// gate: an app with a `-v` line is skipped when it already carries the current
// version, leaving the legacy line in docker-options.
func TestMigrateLegacyMountsRespectsCurrentMarker(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 {
if err := common.PropertyWrite(PluginName, "alpha", MigratedProperty, MigratedPropertyVersion); err != nil {
t.Fatalf("seed property: %v", err)
}
@@ -405,7 +408,40 @@ func TestMigrateLegacyMountsRespectsExistingProperty(t *testing.T) {
t.Fatalf("LoadAttachments: %v", err)
}
if len(atts) != 0 {
t.Errorf("expected 0 attachments (property gated migration), got %d", len(atts))
t.Errorf("expected 0 attachments (marker gated migration), got %d", len(atts))
}
}
// TestMigrateLegacyMountsRescansStaleMarker is the other half of that gate.
// Releases before process-scoped migration wrote "true" and drained only the
// default scope, so an app still holding that value has to be rescanned once
// rather than skipped forever.
func TestMigrateLegacyMountsRescansStaleMarker(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)
}
if got := phaseOptions(t, "alpha", "deploy"); len(got) != 0 {
t.Errorf("expected -v line drained, got %v", got)
}
atts, err := LoadAttachments("alpha")
if err != nil {
t.Fatalf("LoadAttachments: %v", err)
}
if len(atts) != 1 {
t.Fatalf("expected 1 attachment, got %d", len(atts))
}
if got := common.PropertyGetDefault(PluginName, "alpha", MigratedProperty, ""); got != MigratedPropertyVersion {
t.Errorf("legacy-mounts-migrated = %q, want %q", got, MigratedPropertyVersion)
}
}
@@ -424,3 +460,98 @@ func equalSorted(a, b []string) bool {
}
return true
}
// stageProcessOptions seeds a named process-type scope's docker-options
// property file for one phase, the shape `docker-options:add --process worker`
// leaves behind.
func stageProcessOptions(t *testing.T, app string, processType string, phase string, lines []string) {
t.Helper()
key := processType + "." + phase
if err := common.PropertyListWrite("docker-options", app, key, lines); err != nil {
t.Fatalf("PropertyListWrite: %v", err)
}
}
func processPhaseOptions(t *testing.T, app string, processType string, phase string) []string {
t.Helper()
got, err := dockeroptions.GetDockerOptionsForProcessPhase(app, processType, phase)
if err != nil {
t.Fatalf("GetDockerOptionsForProcessPhase %s/%s/%s: %v", app, processType, phase, err)
}
return got
}
// TestMigrateAppDrainsProcessScopedMounts is the migration half of #9059. A
// `-v` line registered under `docker-options:add --process worker` used to be
// invisible to storage forever, because the drain only read the default scope.
func TestMigrateAppDrainsProcessScopedMounts(t *testing.T) {
_, dokkuRoot := setupMigrationEnv(t)
stageApp(t, dokkuRoot, "alpha", map[string][]string{
"deploy": {"-v /var/log:/log"},
})
stageProcessOptions(t, "alpha", "worker", "deploy", []string{"-v /var/spool:/spool", "--restart=on-failure:5"})
if err := migrateApp("alpha", false); err != nil {
t.Fatalf("migrateApp: %v", err)
}
attachments, err := LoadAttachments("alpha")
if err != nil {
t.Fatalf("LoadAttachments: %v", err)
}
if len(attachments) != 2 {
t.Fatalf("expected 2 attachments, got %d", len(attachments))
}
scopes := map[string]string{}
for _, attachment := range attachments {
scopes[attachment.ContainerPath] = attachment.EffectiveProcessType()
}
if scopes["/log"] != DefaultProcessType {
t.Errorf("expected /log in the default scope, got %q", scopes["/log"])
}
if scopes["/spool"] != "worker" {
t.Errorf("expected /spool scoped to worker, got %q", scopes["/spool"])
}
// The drained line goes, the non-mount option in the same scope stays.
if got := processPhaseOptions(t, "alpha", "worker", "deploy"); !equalSorted(got, []string{"--restart=on-failure:5"}) {
t.Errorf("expected only the non-mount option left, got %v", got)
}
}
// TestMigrateAppSkipsProcessScopeCoveredByDefault covers an app that declared
// the same mount twice, once globally and once for a process. The default
// attachment already reaches that process, so recording the second one would
// emit two -v flags for one container path.
func TestMigrateAppSkipsProcessScopeCoveredByDefault(t *testing.T) {
_, dokkuRoot := setupMigrationEnv(t)
stageApp(t, dokkuRoot, "alpha", map[string][]string{
"deploy": {"-v /var/log:/log"},
})
stageProcessOptions(t, "alpha", "worker", "deploy", []string{"-v /var/log:/log"})
if err := migrateApp("alpha", false); err != nil {
t.Fatalf("migrateApp: %v", err)
}
attachments, err := LoadAttachments("alpha")
if err != nil {
t.Fatalf("LoadAttachments: %v", err)
}
if len(attachments) != 1 {
t.Fatalf("expected 1 attachment, got %d", len(attachments))
}
if attachments[0].EffectiveProcessType() != DefaultProcessType {
t.Errorf("expected the surviving attachment in the default scope, got %q", attachments[0].EffectiveProcessType())
}
// Both lines are still drained, so docker-options stops emitting the
// duplicate alongside the storage attachment.
if got := processPhaseOptions(t, "alpha", "worker", "deploy"); len(got) != 0 {
t.Errorf("expected the worker scope drained, got %v", got)
}
if got := phaseOptions(t, "alpha", "deploy"); len(got) != 0 {
t.Errorf("expected the default scope drained, got %v", got)
}
}

60
plugins/storage/mounts_test.go
View File

@@ -178,6 +178,66 @@ func TestCommandMountReplaceReadonlyFlagAppliesToEverySpec(t *testing.T) {
Expect(attachments[1].Readonly).To(BeTrue())
}
// TestCommandMountReplaceRejectsPathHeldByAnotherScope covers the scopes a
// --replace call does not replace. The declared set is already unique on
// container path within its own scope, but a path held by a different process
// type would still collide at deploy time.
func TestCommandMountReplaceRejectsPathHeldByAnotherScope(t *testing.T) {
RegisterTestingT(t)
setupTestApp(t, "demo")
stageDockerLocalEntry(t, "demo-data")
stageDockerLocalEntry(t, "demo-cache")
Expect(CommandMount(CommandMountInput{
AppName: "demo",
NameOrPath: "demo-data",
ContainerDir: "/app/storage",
ProcessType: "web",
})).To(Succeed())
err := CommandMount(CommandMountInput{
AppName: "demo",
Replace: true,
Specs: []string{"demo-cache:/app/storage"},
})
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("Container path /app/storage on app demo is already mounted by storage entry demo-data for process type web"))
// The rejected call leaves the stored set exactly as it was.
attachments, err := LoadAttachments("demo")
Expect(err).NotTo(HaveOccurred())
Expect(attachments).To(HaveLen(1))
Expect(attachments[0].EntryName).To(Equal("demo-data"))
Expect(attachments[0].ProcessType).To(Equal("web"))
}
// TestCommandMountReplaceAllowsPathInAReplacedScope is the other side of it:
// the scope being replaced is dropped before the check, so re-declaring a path
// that scope already held is the ordinary idempotent case, not a conflict.
func TestCommandMountReplaceAllowsPathInAReplacedScope(t *testing.T) {
RegisterTestingT(t)
setupTestApp(t, "demo")
stageDockerLocalEntry(t, "demo-data")
stageDockerLocalEntry(t, "demo-cache")
Expect(CommandMount(CommandMountInput{
AppName: "demo",
NameOrPath: "demo-data",
ContainerDir: "/app/storage",
})).To(Succeed())
Expect(CommandMount(CommandMountInput{
AppName: "demo",
Replace: true,
Specs: []string{"demo-cache:/app/storage"},
})).To(Succeed())
attachments, err := LoadAttachments("demo")
Expect(err).NotTo(HaveOccurred())
Expect(attachments).To(HaveLen(1))
Expect(attachments[0].EntryName).To(Equal("demo-cache"))
}
func TestCommandMountReplaceIsIdempotent(t *testing.T) {
RegisterTestingT(t)
setupTestApp(t, "demo")

6
plugins/storage/report.go
View File

@@ -72,7 +72,9 @@ func reportRunMounts(appName string) string {
// registerAttachmentFlags adds one report flag per Attachment field for the
// given 1-based index. Host path is resolved from the referenced Entry
// (falling back to entry.Name when the entry has no host path, matching
// ListAppMountEntries).
// ListAppMountEntries). Process type is reported as the scope the attachment
// actually deploys under, so a row written before the field existed reads as
// _default_ rather than blank.
func registerAttachmentFlags(flags map[string]common.ReportFunc, index int, att *Attachment, entry *Entry) {
prefix := fmt.Sprintf("--storage-attachment.%d.", index)
captured := att
@@ -84,7 +86,7 @@ func registerAttachmentFlags(flags map[string]common.ReportFunc, index int, att
flags[prefix+"host-path"] = func(string) string { return host }
flags[prefix+"container-path"] = func(string) string { return captured.ContainerPath }
flags[prefix+"phases"] = func(string) string { return strings.Join(captured.Phases, ",") }
flags[prefix+"process-type"] = func(string) string { return captured.ProcessType }
flags[prefix+"process-type"] = func(string) string { return captured.EffectiveProcessType() }
flags[prefix+"subpath"] = func(string) string { return captured.Subpath }
flags[prefix+"readonly"] = func(string) string { return strconv.FormatBool(captured.Readonly) }
flags[prefix+"volume-options"] = func(string) string { return captured.VolumeOptions }

16
plugins/storage/src/triggers/triggers.go
View File

@@ -28,12 +28,22 @@ func main() {
appName := flag.Arg(0)
phase := flag.Arg(1)
err = storage.TriggerStorageAppMounts(appName, phase)
case "docker-args-process-deploy":
appName := flag.Arg(0)
processType := flag.Arg(3)
err = storage.TriggerDockerArgs(appName, storage.PhaseDeploy, processType)
case "docker-args-deploy":
appName := flag.Arg(0)
err = storage.TriggerDockerArgs(appName, storage.PhaseDeploy)
// No longer registered in the plugin Makefile; storage emits its
// deploy-phase flags through docker-args-process-deploy, which every
// caller of this trigger also invokes. The case remains so a symlink
// left behind by an older build resolves to a no-op rather than
// falling through to the error below and failing the deploy.
err = nil
case "docker-args-run":
// A `dokku run` container belongs to no process type, so only
// default-scoped attachments apply.
appName := flag.Arg(0)
err = storage.TriggerDockerArgs(appName, storage.PhaseRun)
err = storage.TriggerDockerArgs(appName, storage.PhaseRun, storage.DefaultProcessType)
case "post-delete":
appName := flag.Arg(0)
err = storage.TriggerPostDelete(appName)

32
plugins/storage/subcommands.go
View File

@@ -320,17 +320,6 @@ func resolveMountSpecEntry(appName string, appScheduler string, spec string, par
return entry, false, nil
}
// attachmentProcessType returns the process type an attachment is scoped to,
// treating an unset value as the default scope so attachments written before
// the field existed still sort into a scope.
func attachmentProcessType(attachment *Attachment) string {
if attachment.ProcessType == "" {
return DefaultProcessType
}
return attachment.ProcessType
}
// replaceMounts swaps one process type's entire mount set for the declared one
// in a single write, so a caller converging an app onto a declared set no
// longer issues one storage:mount or storage:unmount per difference and can no
@@ -417,13 +406,24 @@ func replaceMounts(input CommandMountInput) error {
return err
}
attachments := []*Attachment{}
retained := []*Attachment{}
for _, attachment := range existing {
if attachmentProcessType(attachment) != processType {
attachments = append(attachments, attachment)
if attachment.EffectiveProcessType() != processType {
retained = append(retained, attachment)
}
}
attachments = append(attachments, declared...)
// The declared set is already unique on container path within its own
// scope; what is left to check is the scopes this call does not replace,
// so a declared path cannot collide with one held by another process type.
// Checking before the write keeps the all-or-nothing contract.
for _, attachment := range declared {
if err := ensureContainerPathFree(input.AppName, retained, attachment); err != nil {
return err
}
}
attachments := append(retained, declared...)
if err := SaveAttachments(input.AppName, attachments); err != nil {
return err
@@ -578,7 +578,7 @@ func unmountAll(input CommandUnmountInput) error {
keep := []*Attachment{}
for _, attachment := range attachments {
if attachmentProcessType(attachment) != input.ProcessType {
if attachment.EffectiveProcessType() != input.ProcessType {
keep = append(keep, attachment)
}
}

48
plugins/storage/triggers.go
View File

@@ -236,31 +236,59 @@ func TriggerStorageAppMounts(appName string, phase string) error {
return nil
}
// TriggerDockerArgs emits `-v` flags for each docker-local attachment in
// the requested phase. Plugn concatenates this with docker-options'
// equivalent trigger output, so docker-local apps continue to receive
// their bind mounts through the standard pipeline.
func TriggerDockerArgs(appName string, phase string) error {
attachments, err := AttachmentsForPhase(appName, phase)
// TriggerDockerArgs emits `-v` flags for the docker-local attachments in the
// requested phase that apply to the given process type. Plugn concatenates
// this with docker-options' equivalent trigger output, so docker-local apps
// continue to receive their bind mounts through the standard pipeline.
//
// An empty processType names a container that belongs to no process - the
// app.json deploy-task path invokes the trigger without one - and so receives
// the default scope. Unlike docker-options, which emits its default scope from
// a separate docker-args-deploy handler, this is storage's only deploy-time
// emitter: returning early on an empty process type would drop every mount.
func TriggerDockerArgs(appName string, phase string, processType string) error {
flags, err := dockerVFlagsForProcess(appName, phase, processType)
if err != nil {
return err
}
for _, flag := range flags {
fmt.Printf(" %s", flag)
}
return nil
}
// dockerVFlagsForProcess returns the `-v` flags a single process type's
// container receives for the given phase.
//
// Every attachment in the phase is resolved and scheduler-checked, not just
// the ones in scope, so an entry created for the wrong scheduler still fails
// the deploy no matter which process type happens to be starting.
func dockerVFlagsForProcess(appName string, phase string, processType string) ([]string, error) {
attachments, err := AttachmentsForPhase(appName, phase)
if err != nil {
return nil, err
}
flags := []string{}
for _, attachment := range attachments {
entry, err := LoadEntry(attachment.EntryName)
if err != nil {
return fmt.Errorf("attachment on %q references missing entry %q: %w", appName, attachment.EntryName, err)
return nil, fmt.Errorf("attachment on %q references missing entry %q: %w", appName, attachment.EntryName, err)
}
if entry.Scheduler != SchedulerDockerLocal {
return fmt.Errorf("storage entry %q is scheduler=%s but is mounted on a docker-local app; recreate it with --scheduler docker-local", entry.Name, entry.Scheduler)
return nil, fmt.Errorf("storage entry %q is scheduler=%s but is mounted on a docker-local app; recreate it with --scheduler docker-local", entry.Name, entry.Scheduler)
}
if !attachment.AppliesToProcessType(processType) {
continue
}
flag := buildDockerVFlag(entry, attachment)
if flag == "" {
continue
}
fmt.Printf(" %s", flag)
flags = append(flags, flag)
}
return nil
return flags, nil
}
// buildDockerVFlag formats the Docker -v argument for a docker-local

80
plugins/storage/triggers_test.go
View File

@@ -78,7 +78,85 @@ func TestTriggerDockerArgsRejectsSchedulerMismatch(t *testing.T) {
},
})
err := TriggerDockerArgs("demo", PhaseDeploy)
err := TriggerDockerArgs("demo", PhaseDeploy, "web")
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(Equal(`storage entry "demo-pvc" is scheduler=k3s but is mounted on a docker-local app; recreate it with --scheduler docker-local`))
}
// TestTriggerDockerArgsRejectsSchedulerMismatchOutOfScope pins that the
// scheduler check runs over every attachment in the phase, not just the ones
// the process type being deployed would actually mount. An entry created for
// the wrong scheduler has to fail the deploy whichever process starts first.
func TestTriggerDockerArgsRejectsSchedulerMismatchOutOfScope(t *testing.T) {
RegisterTestingT(t)
root := withTempLibRoot(t)
Expect(SaveEntry(&Entry{
Name: "demo-pvc",
Scheduler: SchedulerK3s,
})).To(Succeed())
writeAttachmentsFile(t, root, "demo", []*Attachment{
{
EntryName: "demo-pvc",
ContainerPath: "/data",
Phases: []string{PhaseDeploy},
ProcessType: "worker",
},
})
_, err := dockerVFlagsForProcess("demo", PhaseDeploy, "web")
Expect(err).To(HaveOccurred())
}
// TestDockerVFlagsForProcessScopes is the docker-local half of #9059. An
// attachment scoped to a named process type reaches that process only; the
// default scope reaches every process, including containers that belong to no
// process at all - the app.json deploy-task path passes an empty process type.
func TestDockerVFlagsForProcessScopes(t *testing.T) {
RegisterTestingT(t)
root := withTempLibRoot(t)
for _, name := range []string{"shared", "web-only", "legacy"} {
Expect(SaveEntry(&Entry{
Name: name,
Scheduler: SchedulerDockerLocal,
HostPath: "/host/" + name,
})).To(Succeed())
}
writeAttachmentsFile(t, root, "demo", []*Attachment{
{
EntryName: "shared",
ContainerPath: "/shared",
Phases: []string{PhaseDeploy},
ProcessType: DefaultProcessType,
},
{
EntryName: "web-only",
ContainerPath: "/web",
Phases: []string{PhaseDeploy},
ProcessType: "web",
},
{
// Written before the process-type field existed.
EntryName: "legacy",
ContainerPath: "/legacy",
Phases: []string{PhaseDeploy},
},
})
for _, testCase := range []struct {
processType string
expected []string
}{
{processType: "web", expected: []string{"-v /host/legacy:/legacy", "-v /host/shared:/shared", "-v /host/web-only:/web"}},
{processType: "worker", expected: []string{"-v /host/legacy:/legacy", "-v /host/shared:/shared"}},
{processType: "", expected: []string{"-v /host/legacy:/legacy", "-v /host/shared:/shared"}},
{processType: DefaultProcessType, expected: []string{"-v /host/legacy:/legacy", "-v /host/shared:/shared"}},
} {
flags, err := dockerVFlagsForProcess("demo", PhaseDeploy, testCase.processType)
Expect(err).NotTo(HaveOccurred())
Expect(flags).To(Equal(testCase.expected), "process type %q", testCase.processType)
}
}

10
tests/unit/storage-1.bats
View File

@@ -670,12 +670,14 @@ teardown() {
assert_success
assert_output "1"
# Property marker is set after a successful migration with mounts.
# Property marker is set after a successful migration with mounts. It
# carries the migration version rather than a bare "true" so an app
# migrated by an older release is rescanned once for process-scoped 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"
assert_output "2"
# Cleanup: unmount + destroy the synthesized legacy entry. We need
# the entry name from list-entries since legacy-<hash> is content-derived.
@@ -727,11 +729,13 @@ teardown() {
echo "status: $status"
assert_success
# The flag file drains into the property, which the version gate then
# treats as stale and advances to the current migration version.
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"
assert_output "2"
run /bin/bash -c "sudo test -e $flag_dir/$TEST_APP"
assert_failure

239
tests/unit/storage-4.bats Normal file
View File

@@ -0,0 +1,239 @@
#!/usr/bin/env bats
load test_helper
setup() {
global_setup
create_app
rm -rf "$DOKKU_LIB_ROOT/data/storage/rdmtestapp*"
}
teardown() {
destroy_app
global_teardown
}
@test "(storage) [process-type] docker-args-process-deploy emits default and matching mounts" {
run /bin/bash -c "dokku storage:create rdmtest-shared"
echo "output: $output"
echo "status: $status"
assert_success
run /bin/bash -c "dokku storage:create rdmtest-web"
assert_success
run /bin/bash -c "dokku storage:mount $TEST_APP rdmtest-shared --container-dir /shared"
echo "output: $output"
echo "status: $status"
assert_success
run /bin/bash -c "dokku storage:mount $TEST_APP rdmtest-web --container-dir /web --process-type web"
echo "output: $output"
echo "status: $status"
assert_success
run /bin/bash -c "echo '' | dokku plugin:trigger docker-args-process-deploy $TEST_APP herokuish latest web | grep -o -- '-v [^ ]*' | cut -d: -f2 | sort | xargs"
echo "output: $output"
echo "status: $status"
assert_success
assert_output "/shared /web"
run /bin/bash -c "dokku storage:unmount --all $TEST_APP"
assert_success
run /bin/bash -c "dokku storage:destroy rdmtest-shared --force"
assert_success
run /bin/bash -c "dokku storage:destroy rdmtest-web --force"
assert_success
}
@test "(storage) [process-type] docker-args-process-deploy omits mounts scoped to another process" {
run /bin/bash -c "dokku storage:create rdmtest-shared"
assert_success
run /bin/bash -c "dokku storage:create rdmtest-web"
assert_success
run /bin/bash -c "dokku storage:mount $TEST_APP rdmtest-shared --container-dir /shared"
assert_success
run /bin/bash -c "dokku storage:mount $TEST_APP rdmtest-web --container-dir /web --process-type web"
assert_success
run /bin/bash -c "echo '' | dokku plugin:trigger docker-args-process-deploy $TEST_APP herokuish latest worker | grep -o -- '-v [^ ]*' | cut -d: -f2 | sort | xargs"
echo "output: $output"
echo "status: $status"
assert_success
assert_output "/shared"
# An ephemeral container with no process type of its own - the app.json
# deploy-task path invokes the trigger this way - still gets the default
# scope, and only the default scope.
run /bin/bash -c "echo '' | dokku plugin:trigger docker-args-process-deploy $TEST_APP herokuish latest | grep -o -- '-v [^ ]*' | cut -d: -f2 | sort | xargs"
echo "output: $output"
echo "status: $status"
assert_success
assert_output "/shared"
run /bin/bash -c "dokku storage:unmount --all $TEST_APP"
assert_success
run /bin/bash -c "dokku storage:destroy rdmtest-shared --force"
assert_success
run /bin/bash -c "dokku storage:destroy rdmtest-web --force"
assert_success
}
@test "(storage) [process-type] docker-args-run emits only default-scoped mounts" {
run /bin/bash -c "dokku storage:create rdmtest-shared"
assert_success
run /bin/bash -c "dokku storage:create rdmtest-web"
assert_success
run /bin/bash -c "dokku storage:mount $TEST_APP rdmtest-shared --container-dir /shared"
assert_success
run /bin/bash -c "dokku storage:mount $TEST_APP rdmtest-web --container-dir /web --process-type web"
assert_success
run /bin/bash -c "echo '' | dokku plugin:trigger docker-args-run $TEST_APP latest | grep -o -- '-v [^ ]*' | cut -d: -f2 | sort | xargs"
echo "output: $output"
echo "status: $status"
assert_success
assert_output "/shared"
run /bin/bash -c "dokku storage:unmount --all $TEST_APP"
assert_success
run /bin/bash -c "dokku storage:destroy rdmtest-shared --force"
assert_success
run /bin/bash -c "dokku storage:destroy rdmtest-web --force"
assert_success
}
@test "(storage:mount) rejects a container path already mounted in an overlapping process type" {
run /bin/bash -c "dokku storage:create rdmtest-data"
assert_success
run /bin/bash -c "dokku storage:create rdmtest-cache"
assert_success
run /bin/bash -c "dokku storage:mount $TEST_APP rdmtest-data --container-dir /app/storage"
echo "output: $output"
echo "status: $status"
assert_success
# The default scope also applies to web, so both mounts would target
# /app/storage on the same container.
run /bin/bash -c "dokku storage:mount $TEST_APP rdmtest-cache --container-dir /app/storage --process-type web"
echo "output: $output"
echo "status: $status"
assert_failure
assert_output_contains "is already mounted by storage entry rdmtest-data for process type _default_"
run /bin/bash -c "dokku storage:list $TEST_APP --format json | jq -r '. | length'"
echo "output: $output"
echo "status: $status"
assert_success
assert_output "1"
# Two named scopes never share a container, so they may each claim the path.
run /bin/bash -c "dokku storage:mount $TEST_APP rdmtest-data --container-dir /scoped --process-type web"
echo "output: $output"
echo "status: $status"
assert_success
run /bin/bash -c "dokku storage:mount $TEST_APP rdmtest-cache --container-dir /scoped --process-type worker"
echo "output: $output"
echo "status: $status"
assert_success
run /bin/bash -c "dokku storage:unmount --all $TEST_APP"
assert_success
run /bin/bash -c "dokku storage:destroy rdmtest-data --force"
assert_success
run /bin/bash -c "dokku storage:destroy rdmtest-cache --force"
assert_success
}
@test "(storage:migrate) drains process-scoped docker-options mounts" {
run /bin/bash -c "mkdir -p /tmp/rdmtest-spool"
assert_success
run /bin/bash -c "dokku docker-options:add --process worker $TEST_APP deploy '-v /tmp/rdmtest-spool:/spool'"
echo "output: $output"
echo "status: $status"
assert_success
run /bin/bash -c "dokku storage:migrate $TEST_APP"
echo "output: $output"
echo "status: $status"
assert_success
run /bin/bash -c "dokku storage:report $TEST_APP --format json | jq -r '.\"attachment.1.container-path\"'"
echo "output: $output"
echo "status: $status"
assert_success
assert_output "/spool"
run /bin/bash -c "dokku storage:report $TEST_APP --format json | jq -r '.\"attachment.1.process-type\"'"
echo "output: $output"
echo "status: $status"
assert_success
assert_output "worker"
run /bin/bash -c "dokku docker-options:report $TEST_APP --format json | jq -r '.. | strings' | grep -c 'rdmtest-spool' || true"
echo "output: $output"
echo "status: $status"
assert_success
assert_output "0"
run /bin/bash -c "dokku storage:unmount --all $TEST_APP"
assert_success
run /bin/bash -c "rm -rf /tmp/rdmtest-spool"
assert_success
}
@test "(storage) [process-type] a process-scoped mount reaches only its own process" {
run /bin/bash -c "dokku storage:create rdmtest-shared"
assert_success
run /bin/bash -c "dokku storage:create rdmtest-web"
assert_success
run /bin/bash -c "dokku storage:mount $TEST_APP rdmtest-shared --container-dir /shared"
assert_success
run /bin/bash -c "dokku storage:mount $TEST_APP rdmtest-web --container-dir /web --process-type web"
assert_success
run deploy_app python dokku@$DOKKU_DOMAIN:$TEST_APP web_worker_callback
echo "output: $output"
echo "status: $status"
assert_success
# Only web is scaled up by default, so the worker needs a container of its
# own before there is anything to compare against.
run /bin/bash -c "dokku ps:scale $TEST_APP worker=1"
echo "output: $output"
echo "status: $status"
assert_success
run /bin/bash -c "docker inspect -f '{{range .HostConfig.Binds}}{{println .}}{{end}}' \$(< $DOKKU_ROOT/$TEST_APP/CONTAINER.web.1) | cut -d: -f2 | sort | xargs"
echo "output: $output"
echo "status: $status"
assert_success
assert_output "/shared /web"
run /bin/bash -c "docker inspect -f '{{range .HostConfig.Binds}}{{println .}}{{end}}' \$(< $DOKKU_ROOT/$TEST_APP/CONTAINER.worker.1) | cut -d: -f2 | sort | xargs"
echo "output: $output"
echo "status: $status"
assert_success
assert_output "/shared"
run /bin/bash -c "dokku storage:unmount --all $TEST_APP"
assert_success
run /bin/bash -c "dokku storage:destroy rdmtest-shared --force"
assert_success
run /bin/bash -c "dokku storage:destroy rdmtest-web --force"
assert_success
}
web_worker_callback() {
local APP="$1"
local APP_REPO_DIR="$2"
cat >"$APP_REPO_DIR/Procfile" <<EOF
web: python3 -u web.py
worker: python3 -u worker.py
EOF
}