From 4e87d1f44741aa431438707a43abf8c1aa1bffef Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Wed, 29 Apr 2026 14:36:47 -0400 Subject: [PATCH 01/13] feat: add storage entry and attachment types Introduces the Entry and Attachment types that the storage plugin will use as the source of truth for named storage volumes, replacing the colon-delimited mount strings stored under docker-options. Entries persist as JSON under config/storage/entries and validate against scheduler-specific rules; attachments persist via the property list and capture how an app uses an entry. The legacy migration name synthesizer is also added so existing colon-form mounts can converge on a deterministic legacy- entry name. Existing storage commands are unchanged in this commit. --- go.work | 1 + plugins/storage/attachment.go | 221 +++++++++++++++++++++++ plugins/storage/attachment_test.go | 33 ++++ plugins/storage/entry.go | 281 +++++++++++++++++++++++++++++ plugins/storage/entry_test.go | 196 ++++++++++++++++++++ 5 files changed, 732 insertions(+) create mode 100644 plugins/storage/attachment.go create mode 100644 plugins/storage/attachment_test.go create mode 100644 plugins/storage/entry.go create mode 100644 plugins/storage/entry_test.go diff --git a/go.work b/go.work index 814223fc1..bf66069d0 100644 --- a/go.work +++ b/go.work @@ -21,4 +21,5 @@ use ( ./plugins/scheduler ./plugins/scheduler-docker-local ./plugins/scheduler-k3s + ./plugins/storage ) diff --git a/plugins/storage/attachment.go b/plugins/storage/attachment.go new file mode 100644 index 000000000..f43070fab --- /dev/null +++ b/plugins/storage/attachment.go @@ -0,0 +1,221 @@ +package storage + +import ( + "encoding/json" + "errors" + "fmt" + "sort" + "strings" + + "github.com/dokku/dokku/plugins/common" +) + +// AttachmentsProperty is the property-list key used to store attachments +// for an app. All process types live under one list and carry their own +// ProcessType field; we filter on read. +const AttachmentsProperty = "mounts" + +// PluginName is the name we register with the property system. +const PluginName = "storage" + +// PhaseDeploy is the standard deploy phase identifier. +const PhaseDeploy = "deploy" + +// PhaseRun is the standard run phase identifier. +const PhaseRun = "run" + +// DefaultProcessType is the wildcard process type that applies to every +// process. Mirrors docker-options' DefaultProcessType. +const DefaultProcessType = "_default_" + +// Attachment is the source of truth for *how* an app uses a storage entry. +// One Attachment binds one entry into one container path on one app. +type Attachment struct { + EntryName string `json:"entry_name"` + ContainerPath string `json:"container_path"` + Phases []string `json:"phases"` + ProcessType string `json:"process_type,omitempty"` + Subpath string `json:"subpath,omitempty"` + Readonly bool `json:"readonly,omitempty"` + VolumeOptions string `json:"volume_options,omitempty"` + VolumeChown string `json:"volume_chown,omitempty"` +} + +// Validate checks an Attachment's fields against structural rules. +func (a *Attachment) Validate() error { + if a == nil { + return errors.New("attachment is nil") + } + if a.EntryName == "" { + return errors.New("attachment is missing entry name") + } + if a.ContainerPath == "" { + return errors.New("attachment is missing container path") + } + if !strings.HasPrefix(a.ContainerPath, "/") { + return fmt.Errorf("attachment container path %q must be absolute", a.ContainerPath) + } + if len(a.Phases) == 0 { + return errors.New("attachment must specify at least one phase") + } + for _, phase := range a.Phases { + if phase != PhaseDeploy && phase != PhaseRun { + return fmt.Errorf("attachment phase %q is not supported (must be %q or %q)", phase, PhaseDeploy, PhaseRun) + } + } + return nil +} + +// LoadAttachments returns every attachment registered against an app. +func LoadAttachments(appName string) ([]*Attachment, error) { + lines, err := common.PropertyListGet(PluginName, appName, AttachmentsProperty) + if err != nil { + return nil, fmt.Errorf("unable to read storage attachments for %q: %w", appName, err) + } + + attachments := []*Attachment{} + for _, line := range lines { + line = strings.TrimSpace(line) + if line == "" { + continue + } + attachment := &Attachment{} + if err := json.Unmarshal([]byte(line), attachment); err != nil { + return nil, fmt.Errorf("unable to parse storage attachment for %q: %w", appName, err) + } + attachments = append(attachments, attachment) + } + return attachments, nil +} + +// SaveAttachments overwrites the entire attachment list for an app. +func SaveAttachments(appName string, attachments []*Attachment) error { + lines := make([]string, 0, len(attachments)) + for _, attachment := range attachments { + data, err := json.Marshal(attachment) + if err != nil { + return fmt.Errorf("unable to encode storage attachment for %q: %w", appName, err) + } + lines = append(lines, string(data)) + } + return common.PropertyListWrite(PluginName, appName, AttachmentsProperty, lines) +} + +// AddAttachment appends an attachment, rejecting duplicates of the same +// (entry_name, container_path, process_type) tuple. +func AddAttachment(appName string, attachment *Attachment) error { + if err := attachment.Validate(); err != nil { + return err + } + + attachments, err := LoadAttachments(appName) + if err != nil { + return err + } + + for _, existing := range attachments { + if existing.EntryName == attachment.EntryName && + existing.ContainerPath == attachment.ContainerPath && + existing.ProcessType == attachment.ProcessType { + 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) + } + } + + attachments = append(attachments, attachment) + return SaveAttachments(appName, attachments) +} + +// RemoveAttachment removes the attachment matching the given entry and +// optional container path. If containerPath is empty and there is more +// than one match, returns an error. +func RemoveAttachment(appName string, entryName string, containerPath string) error { + attachments, err := LoadAttachments(appName) + if err != nil { + return err + } + + matches := []*Attachment{} + keep := []*Attachment{} + for _, attachment := range attachments { + if attachment.EntryName == entryName && (containerPath == "" || attachment.ContainerPath == containerPath) { + matches = append(matches, attachment) + continue + } + keep = append(keep, attachment) + } + + if len(matches) == 0 { + if containerPath == "" { + return fmt.Errorf("storage entry %q is not mounted on app %q", entryName, appName) + } + return fmt.Errorf("storage entry %q is not mounted at %q on app %q", entryName, containerPath, appName) + } + if len(matches) > 1 && containerPath == "" { + paths := []string{} + for _, attachment := range matches { + paths = append(paths, attachment.ContainerPath) + } + sort.Strings(paths) + return fmt.Errorf("storage entry %q is mounted at multiple paths on app %q (%s); pass --container-dir to disambiguate", + entryName, appName, strings.Join(paths, ", ")) + } + + return SaveAttachments(appName, keep) +} + +// AttachmentsForPhase returns the subset of an app's attachments that +// apply to the given phase, sorted for stable output. +func AttachmentsForPhase(appName string, phase string) ([]*Attachment, error) { + attachments, err := LoadAttachments(appName) + if err != nil { + return nil, err + } + + filtered := []*Attachment{} + for _, attachment := range attachments { + for _, p := range attachment.Phases { + if p == phase { + filtered = append(filtered, attachment) + break + } + } + } + + sort.Slice(filtered, func(i, j int) bool { + if filtered[i].EntryName != filtered[j].EntryName { + return filtered[i].EntryName < filtered[j].EntryName + } + return filtered[i].ContainerPath < filtered[j].ContainerPath + }) + return filtered, nil +} + +// AppsUsingEntry returns the list of app names that have at least one +// attachment referencing the given entry name. Used by storage:destroy +// to refuse removing an entry that's still mounted. +func AppsUsingEntry(entryName string) ([]string, error) { + apps, err := common.DokkuApps() + if err != nil { + if errors.Is(err, common.NoAppsExist) { + return nil, nil + } + return nil, err + } + + using := []string{} + for _, app := range apps { + attachments, err := LoadAttachments(app) + if err != nil { + return nil, err + } + for _, attachment := range attachments { + if attachment.EntryName == entryName { + using = append(using, app) + break + } + } + } + sort.Strings(using) + return using, nil +} diff --git a/plugins/storage/attachment_test.go b/plugins/storage/attachment_test.go new file mode 100644 index 000000000..d6be3e31b --- /dev/null +++ b/plugins/storage/attachment_test.go @@ -0,0 +1,33 @@ +package storage + +import ( + "testing" + + . "github.com/onsi/gomega" +) + +func TestAttachmentValidate(t *testing.T) { + RegisterTestingT(t) + + good := &Attachment{ + EntryName: "demo-data", + ContainerPath: "/data", + Phases: []string{PhaseDeploy, PhaseRun}, + } + Expect(good.Validate()).To(Succeed()) + + noEntry := &Attachment{ContainerPath: "/data", Phases: []string{PhaseDeploy}} + Expect(noEntry.Validate()).To(HaveOccurred()) + + noPath := &Attachment{EntryName: "demo-data", Phases: []string{PhaseDeploy}} + Expect(noPath.Validate()).To(HaveOccurred()) + + relative := &Attachment{EntryName: "demo-data", ContainerPath: "data", Phases: []string{PhaseDeploy}} + Expect(relative.Validate()).To(HaveOccurred()) + + noPhases := &Attachment{EntryName: "demo-data", ContainerPath: "/data"} + Expect(noPhases.Validate()).To(HaveOccurred()) + + badPhase := &Attachment{EntryName: "demo-data", ContainerPath: "/data", Phases: []string{"build"}} + Expect(badPhase.Validate()).To(HaveOccurred()) +} diff --git a/plugins/storage/entry.go b/plugins/storage/entry.go new file mode 100644 index 000000000..68ef5d79f --- /dev/null +++ b/plugins/storage/entry.go @@ -0,0 +1,281 @@ +package storage + +import ( + "crypto/sha1" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "regexp" + "sort" + "strings" + + "github.com/dokku/dokku/plugins/common" +) + +// SchemaVersion is the on-disk schema version emitted by this plugin. +// Bumped when the format of Entry / Attachment changes incompatibly. +const SchemaVersion = 1 + +// SchedulerDockerLocal is the default scheduler value for storage entries. +const SchedulerDockerLocal = "docker-local" + +// SchedulerK3s is the k3s scheduler value for storage entries. +const SchedulerK3s = "k3s" + +// LegacyEntryPrefix is reserved for migration-synthesized entries; users +// cannot create entries whose name starts with this prefix. +const LegacyEntryPrefix = "legacy-" + +// MaxEntryNameLength caps the storage entry name so that the helm release +// name "storage-" stays within Helm's practical 53-char ceiling. +const MaxEntryNameLength = 45 + +// ReclaimPolicyRetain keeps the underlying PV after the entry is destroyed. +const ReclaimPolicyRetain = "Retain" + +// ReclaimPolicyDelete deletes the underlying PV after the entry is destroyed. +const ReclaimPolicyDelete = "Delete" + +var ( + // dns1123LabelRegexp matches a Kubernetes DNS-1123 label, the format + // required for resource names like PVCs and Helm releases. + dns1123LabelRegexp = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$`) + + // supportedSchedulers lists the values accepted for --scheduler. + supportedSchedulers = map[string]bool{ + SchedulerDockerLocal: true, + SchedulerK3s: true, + } + + // supportedAccessModes lists the values accepted for --access-mode on k3s. + supportedAccessModes = map[string]bool{ + "ReadWriteOnce": true, + "ReadOnlyMany": true, + "ReadWriteMany": true, + "ReadWriteOncePod": true, + } +) + +// Entry is the source of truth for a storage volume. One file per entry +// lives at $DOKKU_LIB_ROOT/config/storage/entries/.json. +type Entry struct { + Name string `json:"name"` + Scheduler string `json:"scheduler"` + HostPath string `json:"host_path,omitempty"` + Size string `json:"size,omitempty"` + AccessMode string `json:"access_mode,omitempty"` + StorageClass string `json:"storage_class,omitempty"` + Namespace string `json:"namespace,omitempty"` + Chown string `json:"chown,omitempty"` + ReclaimPolicy string `json:"reclaim_policy,omitempty"` + Annotations map[string]string `json:"annotations,omitempty"` + Labels map[string]string `json:"labels,omitempty"` + SchemaVersion int `json:"schema_version"` +} + +// EntriesDirectory returns the directory holding per-entry JSON files. +func EntriesDirectory() string { + root := common.GetenvWithDefault("DOKKU_LIB_ROOT", "/var/lib/dokku") + return filepath.Join(root, "config", "storage", "entries") +} + +// entryPath returns the on-disk path for a named entry. +func entryPath(name string) string { + return filepath.Join(EntriesDirectory(), name+".json") +} + +// EnsureEntriesDirectory makes sure the entries directory exists with +// permissive enough mode that the dokku user can read it. +func EnsureEntriesDirectory() error { + dir := EntriesDirectory() + if err := os.MkdirAll(dir, 0755); err != nil { + return fmt.Errorf("unable to create storage entries directory: %w", err) + } + return nil +} + +// EntryExists reports whether an entry with the given name is registered. +func EntryExists(name string) bool { + _, err := os.Stat(entryPath(name)) + return err == nil +} + +// LoadEntry reads a registered entry by name. +func LoadEntry(name string) (*Entry, error) { + data, err := os.ReadFile(entryPath(name)) + if err != nil { + if os.IsNotExist(err) { + return nil, fmt.Errorf("storage entry %q does not exist", name) + } + return nil, fmt.Errorf("unable to read storage entry %q: %w", name, err) + } + + entry := &Entry{} + if err := json.Unmarshal(data, entry); err != nil { + return nil, fmt.Errorf("unable to parse storage entry %q: %w", name, err) + } + return entry, nil +} + +// SaveEntry persists an entry to its JSON file. +func SaveEntry(entry *Entry) error { + if err := EnsureEntriesDirectory(); err != nil { + return err + } + + if entry.SchemaVersion == 0 { + entry.SchemaVersion = SchemaVersion + } + + data, err := json.MarshalIndent(entry, "", " ") + if err != nil { + return fmt.Errorf("unable to encode storage entry %q: %w", entry.Name, err) + } + + path := entryPath(entry.Name) + if err := os.WriteFile(path, data, 0600); err != nil { + return fmt.Errorf("unable to write storage entry %q: %w", entry.Name, err) + } + return nil +} + +// DeleteEntry removes the on-disk record for an entry. +func DeleteEntry(name string) error { + if err := os.Remove(entryPath(name)); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("unable to remove storage entry %q: %w", name, err) + } + return nil +} + +// ListEntries returns every registered entry sorted by name. +func ListEntries() ([]*Entry, error) { + dir := EntriesDirectory() + dirEntries, err := os.ReadDir(dir) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + return nil, fmt.Errorf("unable to list storage entries: %w", err) + } + + entries := []*Entry{} + for _, dirEntry := range dirEntries { + if dirEntry.IsDir() || !strings.HasSuffix(dirEntry.Name(), ".json") { + continue + } + name := strings.TrimSuffix(dirEntry.Name(), ".json") + entry, err := LoadEntry(name) + if err != nil { + return nil, err + } + entries = append(entries, entry) + } + + sort.Slice(entries, func(i, j int) bool { return entries[i].Name < entries[j].Name }) + return entries, nil +} + +// ValidateEntryName checks the user-supplied name against length and DNS +// label rules. Returns nil for legacy names if allowLegacyPrefix is true, +// which is only set during migration. +func ValidateEntryName(name string, allowLegacyPrefix bool) error { + if name == "" { + return errors.New("storage entry name is required") + } + if len(name) > MaxEntryNameLength { + return fmt.Errorf("storage entry name %q is too long (max %d characters); helm release names cap our budget", name, MaxEntryNameLength) + } + if !dns1123LabelRegexp.MatchString(name) { + return fmt.Errorf("storage entry name %q must be a DNS-1123 label (lowercase letters, digits, and dashes; cannot start or end with a dash)", name) + } + if !allowLegacyPrefix && strings.HasPrefix(name, LegacyEntryPrefix) { + return fmt.Errorf("storage entry name %q must not start with reserved prefix %q", name, LegacyEntryPrefix) + } + return nil +} + +// Validate checks an Entry's fields against the cross-field rules for its +// scheduler. It is reused by both storage:create and the legacy migration. +func (e *Entry) Validate() error { + if e == nil { + return errors.New("entry is nil") + } + if err := ValidateEntryName(e.Name, strings.HasPrefix(e.Name, LegacyEntryPrefix)); err != nil { + return err + } + + if !supportedSchedulers[e.Scheduler] { + return fmt.Errorf("storage entry %q has unsupported scheduler %q (supported: docker-local, k3s)", e.Name, e.Scheduler) + } + + switch e.Scheduler { + case SchedulerDockerLocal: + if e.HostPath == "" { + return fmt.Errorf("storage entry %q (docker-local) is missing host_path", e.Name) + } + if !filepath.IsAbs(e.HostPath) { + return fmt.Errorf("storage entry %q host_path must be absolute, got %q", e.Name, e.HostPath) + } + if e.Size != "" { + return fmt.Errorf("storage entry %q (docker-local) does not accept --size", e.Name) + } + if e.StorageClass != "" { + return fmt.Errorf("storage entry %q (docker-local) does not accept --storage-class-name", e.Name) + } + if e.AccessMode != "" { + return fmt.Errorf("storage entry %q (docker-local) does not accept --access-mode", e.Name) + } + case SchedulerK3s: + if e.Size == "" { + return fmt.Errorf("storage entry %q (k3s) requires --size", e.Name) + } + if e.StorageClass != "" && e.HostPath != "" { + return fmt.Errorf("storage entry %q (k3s) cannot set both --storage-class-name and a host path; the cluster provisions class-backed volumes", e.Name) + } + if e.AccessMode != "" && !supportedAccessModes[e.AccessMode] { + return fmt.Errorf("storage entry %q has unsupported access mode %q", e.Name, e.AccessMode) + } + if e.HostPath != "" && !filepath.IsAbs(e.HostPath) { + return fmt.Errorf("storage entry %q host_path must be absolute, got %q", e.Name, e.HostPath) + } + if e.ReclaimPolicy != "" && e.ReclaimPolicy != ReclaimPolicyRetain && e.ReclaimPolicy != ReclaimPolicyDelete { + return fmt.Errorf("storage entry %q has unsupported reclaim policy %q", e.Name, e.ReclaimPolicy) + } + } + + return nil +} + +// LegacyMountToEntry synthesizes a deterministic Entry for a legacy +// "host:container[:options]" mount string. The synthesized name is +// stable across runs so re-running the migration is idempotent and so +// two apps mounting the same host path converge on a single entry. +func LegacyMountToEntry(mountPath string) *Entry { + parsed := ParseMountPath(mountPath) + host := parsed.HostPath + + hashInput := host + if !strings.HasPrefix(host, "/") { + hashInput = "vol:" + host + } + sum := sha1.Sum([]byte(hashInput)) + name := LegacyEntryPrefix + hex.EncodeToString(sum[:])[:10] + + hostPath := host + if !strings.HasPrefix(host, "/") { + // Named docker volumes pass through unchanged on docker-local; + // the docker engine resolves them. Store as-is so the args + // shim can emit the original token. + hostPath = host + } + + return &Entry{ + Name: name, + Scheduler: SchedulerDockerLocal, + HostPath: hostPath, + SchemaVersion: SchemaVersion, + } +} diff --git a/plugins/storage/entry_test.go b/plugins/storage/entry_test.go new file mode 100644 index 000000000..683032008 --- /dev/null +++ b/plugins/storage/entry_test.go @@ -0,0 +1,196 @@ +package storage + +import ( + "path/filepath" + "strings" + "testing" + + . "github.com/onsi/gomega" +) + +func withTempLibRoot(t *testing.T) string { + t.Helper() + dir := t.TempDir() + t.Setenv("DOKKU_LIB_ROOT", dir) + return dir +} + +func TestValidateEntryName(t *testing.T) { + RegisterTestingT(t) + + Expect(ValidateEntryName("foo", false)).To(Succeed()) + Expect(ValidateEntryName("foo-bar", false)).To(Succeed()) + Expect(ValidateEntryName("a", false)).To(Succeed()) + Expect(ValidateEntryName("foo123", false)).To(Succeed()) + + err := ValidateEntryName("", false) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("required")) + + err = ValidateEntryName("Foo", false) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("DNS-1123")) + + err = ValidateEntryName("foo_bar", false) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("DNS-1123")) + + err = ValidateEntryName("-foo", false) + Expect(err).To(HaveOccurred()) + + err = ValidateEntryName("foo-", false) + Expect(err).To(HaveOccurred()) + + long := strings.Repeat("a", MaxEntryNameLength+1) + err = ValidateEntryName(long, false) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("too long")) + + maxOK := strings.Repeat("a", MaxEntryNameLength) + Expect(ValidateEntryName(maxOK, false)).To(Succeed()) + + err = ValidateEntryName("legacy-foo", false) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("reserved")) + + Expect(ValidateEntryName("legacy-foo", true)).To(Succeed()) +} + +func TestEntryValidateDockerLocal(t *testing.T) { + RegisterTestingT(t) + + good := &Entry{Name: "foo", Scheduler: SchedulerDockerLocal, HostPath: "/var/lib/dokku/data/storage/foo"} + Expect(good.Validate()).To(Succeed()) + + noPath := &Entry{Name: "foo", Scheduler: SchedulerDockerLocal} + Expect(noPath.Validate()).To(HaveOccurred()) + + relativePath := &Entry{Name: "foo", Scheduler: SchedulerDockerLocal, HostPath: "relative/path"} + Expect(relativePath.Validate()).To(HaveOccurred()) + + withSize := &Entry{Name: "foo", Scheduler: SchedulerDockerLocal, HostPath: "/data", Size: "2Gi"} + Expect(withSize.Validate()).To(HaveOccurred()) + + withClass := &Entry{Name: "foo", Scheduler: SchedulerDockerLocal, HostPath: "/data", StorageClass: "longhorn"} + Expect(withClass.Validate()).To(HaveOccurred()) +} + +func TestEntryValidateK3s(t *testing.T) { + RegisterTestingT(t) + + dynamic := &Entry{Name: "foo", Scheduler: SchedulerK3s, Size: "2Gi", StorageClass: "longhorn", AccessMode: "ReadWriteOnce"} + Expect(dynamic.Validate()).To(Succeed()) + + hostPath := &Entry{Name: "foo", Scheduler: SchedulerK3s, Size: "2Gi", HostPath: "/data/foo"} + Expect(hostPath.Validate()).To(Succeed()) + + noSize := &Entry{Name: "foo", Scheduler: SchedulerK3s, StorageClass: "longhorn"} + Expect(noSize.Validate()).To(HaveOccurred()) + + bothPathAndClass := &Entry{Name: "foo", Scheduler: SchedulerK3s, Size: "2Gi", HostPath: "/data", StorageClass: "longhorn"} + err := bothPathAndClass.Validate() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("storage-class-name")) + + badAccessMode := &Entry{Name: "foo", Scheduler: SchedulerK3s, Size: "2Gi", StorageClass: "longhorn", AccessMode: "Bogus"} + Expect(badAccessMode.Validate()).To(HaveOccurred()) + + badReclaim := &Entry{Name: "foo", Scheduler: SchedulerK3s, Size: "2Gi", StorageClass: "longhorn", ReclaimPolicy: "Recycle"} + Expect(badReclaim.Validate()).To(HaveOccurred()) +} + +func TestEntryValidateScheduler(t *testing.T) { + RegisterTestingT(t) + + bad := &Entry{Name: "foo", Scheduler: "nomad"} + err := bad.Validate() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("scheduler")) +} + +func TestEntryRoundTrip(t *testing.T) { + RegisterTestingT(t) + root := withTempLibRoot(t) + + original := &Entry{ + Name: "demo-data", + Scheduler: SchedulerK3s, + Size: "2Gi", + StorageClass: "longhorn", + AccessMode: "ReadWriteOnce", + Namespace: "dokku", + Annotations: map[string]string{"backup.velero.io/backup-volumes": "demo-data"}, + Labels: map[string]string{"app.kubernetes.io/managed-by": "dokku"}, + } + + Expect(SaveEntry(original)).To(Succeed()) + Expect(EntryExists("demo-data")).To(BeTrue()) + + loaded, err := LoadEntry("demo-data") + Expect(err).NotTo(HaveOccurred()) + Expect(loaded.Name).To(Equal(original.Name)) + Expect(loaded.Scheduler).To(Equal(original.Scheduler)) + Expect(loaded.Size).To(Equal(original.Size)) + Expect(loaded.StorageClass).To(Equal(original.StorageClass)) + Expect(loaded.AccessMode).To(Equal(original.AccessMode)) + Expect(loaded.Namespace).To(Equal(original.Namespace)) + Expect(loaded.Annotations).To(Equal(original.Annotations)) + Expect(loaded.Labels).To(Equal(original.Labels)) + Expect(loaded.SchemaVersion).To(Equal(SchemaVersion)) + + expectedPath := filepath.Join(root, "config", "storage", "entries", "demo-data.json") + Expect(expectedPath).To(BeARegularFile()) + + Expect(DeleteEntry("demo-data")).To(Succeed()) + Expect(EntryExists("demo-data")).To(BeFalse()) +} + +func TestListEntries(t *testing.T) { + RegisterTestingT(t) + withTempLibRoot(t) + + entries, err := ListEntries() + Expect(err).NotTo(HaveOccurred()) + Expect(entries).To(BeEmpty()) + + Expect(SaveEntry(&Entry{Name: "b", Scheduler: SchedulerDockerLocal, HostPath: "/b"})).To(Succeed()) + Expect(SaveEntry(&Entry{Name: "a", Scheduler: SchedulerDockerLocal, HostPath: "/a"})).To(Succeed()) + + entries, err = ListEntries() + Expect(err).NotTo(HaveOccurred()) + Expect(entries).To(HaveLen(2)) + Expect(entries[0].Name).To(Equal("a")) + Expect(entries[1].Name).To(Equal("b")) +} + +func TestLegacyMountToEntry(t *testing.T) { + RegisterTestingT(t) + + a := LegacyMountToEntry("/host/path:/container/path") + b := LegacyMountToEntry("/host/path:/container/different") + c := LegacyMountToEntry("/host/path:/container/path:ro") + + // Same host path produces the same entry name regardless of container + // path or options - container path lives on the attachment, not the + // entry, so apps mounting the same source converge. + Expect(a.Name).To(Equal(b.Name)) + Expect(a.Name).To(Equal(c.Name)) + Expect(a.Name).To(HavePrefix(LegacyEntryPrefix)) + Expect(len(a.Name)).To(Equal(len(LegacyEntryPrefix) + 10)) + + // Different host paths produce different names. + other := LegacyMountToEntry("/different/path:/container/path") + Expect(other.Name).NotTo(Equal(a.Name)) + + // Named docker volumes are distinguished from absolute paths. + named := LegacyMountToEntry("my_volume:/container/path") + Expect(named.Name).NotTo(Equal(a.Name)) + Expect(named.HostPath).To(Equal("my_volume")) + + // The synthesized entry is a valid docker-local entry that passes + // Validate (other than the legacy- prefix, which Validate accepts). + abs := LegacyMountToEntry("/host/path:/container/path") + Expect(abs.Scheduler).To(Equal(SchedulerDockerLocal)) + Expect(abs.HostPath).To(Equal("/host/path")) + Expect(abs.Validate()).To(Succeed()) +} From 8a46d891815751a1735f353dce537989fd19c983 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Wed, 29 Apr 2026 14:39:59 -0400 Subject: [PATCH 02/13] feat: add storage:create, storage:destroy, storage:info, storage:list-entries Wires the new entry-management commands on top of the Entry type. storage:create accepts both docker-local entries (with idempotent host-directory creation) and k3s entries (which delegate PVC provisioning to a forthcoming scheduler-side storage-create trigger). storage:destroy refuses to remove an entry that any app still mounts. storage:ensure-directory now emits a deprecation warning. storage:mount and storage:unmount keep the existing colon-form behavior so this commit is non-breaking; the named-entry mount form follows in a later commit. --- plugins/storage/Makefile | 2 +- plugins/storage/commands_entries.go | 302 ++++++++++++++++++ plugins/storage/src/commands/commands.go | 6 +- .../storage/src/subcommands/subcommands.go | 68 ++++ plugins/storage/subcommands.go | 6 +- 5 files changed, 381 insertions(+), 3 deletions(-) create mode 100644 plugins/storage/commands_entries.go diff --git a/plugins/storage/Makefile b/plugins/storage/Makefile index 754299f43..c719dde7d 100644 --- a/plugins/storage/Makefile +++ b/plugins/storage/Makefile @@ -1,5 +1,5 @@ GOARCH ?= amd64 -SUBCOMMANDS = subcommands/default subcommands/ensure-directory subcommands/list subcommands/mount subcommands/report subcommands/unmount +SUBCOMMANDS = subcommands/default subcommands/create subcommands/destroy subcommands/ensure-directory subcommands/info subcommands/list subcommands/list-entries subcommands/mount subcommands/report subcommands/unmount TRIGGERS = triggers/install triggers/storage-list BUILD = commands subcommands triggers PLUGIN_NAME = storage diff --git a/plugins/storage/commands_entries.go b/plugins/storage/commands_entries.go new file mode 100644 index 000000000..4f5c6ff5f --- /dev/null +++ b/plugins/storage/commands_entries.go @@ -0,0 +1,302 @@ +package storage + +import ( + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/dokku/dokku/plugins/common" +) + +// CommandCreateInput captures the flags accepted by storage:create. +type CommandCreateInput struct { + Name string + Path string + Scheduler string + Size string + AccessMode string + StorageClass string + Namespace string + Chown string + ReclaimPolicy string + Annotations map[string]string + Labels map[string]string +} + +// CommandCreate registers a new storage entry. +func CommandCreate(input CommandCreateInput) error { + if err := ValidateEntryName(input.Name, false); err != nil { + return err + } + + scheduler := input.Scheduler + if scheduler == "" { + scheduler = SchedulerDockerLocal + } + + hostPath := input.Path + if scheduler == SchedulerDockerLocal && hostPath == "" { + hostPath = filepath.Join(GetStorageDirectory(), input.Name) + } + + entry := &Entry{ + Name: input.Name, + Scheduler: scheduler, + HostPath: hostPath, + Size: input.Size, + AccessMode: input.AccessMode, + StorageClass: input.StorageClass, + Namespace: input.Namespace, + Chown: input.Chown, + ReclaimPolicy: input.ReclaimPolicy, + Annotations: input.Annotations, + Labels: input.Labels, + SchemaVersion: SchemaVersion, + } + + if err := entry.Validate(); err != nil { + return err + } + + if EntryExists(entry.Name) { + existing, err := LoadEntry(entry.Name) + if err != nil { + return err + } + if existing.Scheduler != entry.Scheduler { + return fmt.Errorf("storage entry %q already exists with scheduler %q; refusing to redefine", entry.Name, existing.Scheduler) + } + common.LogInfo1Quiet(fmt.Sprintf("Storage entry %s already exists, leaving in place", entry.Name)) + } + + if scheduler == SchedulerDockerLocal { + if err := ensureDockerLocalPath(entry); err != nil { + return err + } + } + + if err := SaveEntry(entry); err != nil { + return err + } + + if scheduler == SchedulerK3s { + if err := callSchedulerCreateTrigger(entry); err != nil { + // Roll back the on-disk entry so the disk and the cluster stay in sync. + _ = DeleteEntry(entry.Name) + return fmt.Errorf("scheduler refused storage entry %q: %w", entry.Name, err) + } + } + + common.LogInfo1(fmt.Sprintf("Storage entry %s created", entry.Name)) + return nil +} + +// CommandDestroy removes a registered storage entry. Refuses to remove +// an entry that any app still has attached. +func CommandDestroy(name string) error { + if name == "" { + return errors.New("storage entry name is required") + } + if !EntryExists(name) { + return fmt.Errorf("storage entry %q does not exist", name) + } + + using, err := AppsUsingEntry(name) + if err != nil { + return err + } + if len(using) > 0 { + return fmt.Errorf("storage entry %q is still mounted by app(s): %s", name, strings.Join(using, ", ")) + } + + entry, err := LoadEntry(name) + if err != nil { + return err + } + + if entry.Scheduler == SchedulerK3s { + if err := callSchedulerDestroyTrigger(entry); err != nil { + return fmt.Errorf("scheduler refused to remove storage entry %q: %w", name, err) + } + } + + if err := DeleteEntry(name); err != nil { + return err + } + common.LogInfo1(fmt.Sprintf("Storage entry %s destroyed", name)) + return nil +} + +// CommandInfo prints a single entry's details, optionally as JSON. +func CommandInfo(name string, format string) error { + if name == "" { + return errors.New("storage entry name is required") + } + if !EntryExists(name) { + return fmt.Errorf("storage entry %q does not exist", name) + } + + entry, err := LoadEntry(name) + if err != nil { + return err + } + + if format == "json" { + data, err := json.MarshalIndent(entry, "", " ") + if err != nil { + return err + } + fmt.Println(string(data)) + return nil + } + + common.LogInfo1Quiet(fmt.Sprintf("Storage entry %s", entry.Name)) + common.LogVerbose(fmt.Sprintf("Scheduler: %s", entry.Scheduler)) + if entry.HostPath != "" { + common.LogVerbose(fmt.Sprintf("Host path: %s", entry.HostPath)) + } + if entry.Size != "" { + common.LogVerbose(fmt.Sprintf("Size: %s", entry.Size)) + } + if entry.AccessMode != "" { + common.LogVerbose(fmt.Sprintf("Access mode: %s", entry.AccessMode)) + } + if entry.StorageClass != "" { + common.LogVerbose(fmt.Sprintf("Storage class: %s", entry.StorageClass)) + } + if entry.Namespace != "" { + common.LogVerbose(fmt.Sprintf("Namespace: %s", entry.Namespace)) + } + if entry.Chown != "" { + common.LogVerbose(fmt.Sprintf("Chown: %s", entry.Chown)) + } + if entry.ReclaimPolicy != "" { + common.LogVerbose(fmt.Sprintf("Reclaim policy: %s", entry.ReclaimPolicy)) + } + return nil +} + +// CommandListEntries lists registered entries (distinct from the legacy +// per-app storage:list which lists colon-form mounts). +func CommandListEntries(scheduler string, format string) error { + entries, err := ListEntries() + if err != nil { + return err + } + + filtered := []*Entry{} + for _, entry := range entries { + if scheduler != "" && entry.Scheduler != scheduler { + continue + } + filtered = append(filtered, entry) + } + + if format == "json" { + data, err := json.MarshalIndent(filtered, "", " ") + if err != nil { + return err + } + fmt.Println(string(data)) + return nil + } + + if len(filtered) == 0 { + common.LogInfo1Quiet("No storage entries registered") + return nil + } + common.LogInfo1Quiet("Storage entries:") + for _, entry := range filtered { + common.LogVerbose(fmt.Sprintf("%s\t%s\t%s", entry.Name, entry.Scheduler, entry.HostPath)) + } + return nil +} + +// ensureDockerLocalPath creates the host directory referenced by a +// docker-local entry if it doesn't already exist. Idempotent: a +// pre-existing directory is left in place. +func ensureDockerLocalPath(entry *Entry) error { + info, err := os.Stat(entry.HostPath) + if err != nil && !os.IsNotExist(err) { + return fmt.Errorf("unable to stat %s: %w", entry.HostPath, err) + } + if err == nil && !info.IsDir() { + return fmt.Errorf("storage entry path %s exists but is not a directory", entry.HostPath) + } + if err != nil { + if mkdirErr := os.MkdirAll(entry.HostPath, 0755); mkdirErr != nil { + return fmt.Errorf("unable to create %s: %w", entry.HostPath, mkdirErr) + } + common.LogVerbose(fmt.Sprintf("Created %s", entry.HostPath)) + } + + if entry.Chown != "" && entry.Chown != "false" { + chownID, err := resolveChownID(entry.Chown) + if err != nil { + return err + } + if chownID != "false" { + pluginPath := common.MustGetEnv("PLUGIN_AVAILABLE_PATH") + chownScript := filepath.Join(pluginPath, "storage", "bin", "chown-storage-dir") + result, err := common.CallExecCommand(common.ExecCommandInput{ + Command: "sudo", + Args: []string{chownScript, entry.HostPath, chownID}, + }) + if err != nil { + return fmt.Errorf("unable to chown %s: %w", entry.HostPath, err) + } + if result.ExitCode != 0 { + return fmt.Errorf("unable to chown %s: %s", entry.HostPath, result.StderrContents()) + } + } + } + return nil +} + +// callSchedulerCreateTrigger asks the scheduler plugin (k3s) to provision +// the underlying PVC/PV. The scheduler is responsible for any +// cluster-level validation (storage class existence, etc.). +func callSchedulerCreateTrigger(entry *Entry) error { + data, err := json.Marshal(entry) + if err != nil { + return err + } + results, err := common.CallPlugnTrigger(common.PlugnTriggerInput{ + Trigger: "storage-create", + Args: []string{entry.Name}, + StreamStdio: true, + Stdin: strings.NewReader(string(data)), + }) + if err != nil { + return err + } + if results.ExitCode != 0 { + return fmt.Errorf("storage-create trigger exited with %d: %s", results.ExitCode, results.StderrContents()) + } + return nil +} + +// callSchedulerDestroyTrigger asks the scheduler plugin to release the +// underlying PVC/PV. +func callSchedulerDestroyTrigger(entry *Entry) error { + data, err := json.Marshal(entry) + if err != nil { + return err + } + results, err := common.CallPlugnTrigger(common.PlugnTriggerInput{ + Trigger: "storage-destroy", + Args: []string{entry.Name}, + StreamStdio: true, + Stdin: strings.NewReader(string(data)), + }) + if err != nil { + return err + } + if results.ExitCode != 0 { + return fmt.Errorf("storage-destroy trigger exited with %d: %s", results.ExitCode, results.StderrContents()) + } + return nil +} diff --git a/plugins/storage/src/commands/commands.go b/plugins/storage/src/commands/commands.go index 8ad0b2de8..0c5ed04a1 100644 --- a/plugins/storage/src/commands/commands.go +++ b/plugins/storage/src/commands/commands.go @@ -18,8 +18,12 @@ Manage mounted volumes Additional commands:` helpContent = ` - storage:ensure-directory [--chown option] , Creates a persistent storage directory in the recommended storage path + storage:create [] [flags], Register a named storage entry + storage:destroy , Remove a named storage entry (must be unmounted from every app first) + storage:ensure-directory [--chown option] , [DEPRECATED] use storage:create instead + storage:info [--format text|json], Show details for one storage entry storage:list [--format text|json], List bind mounts for app's container(s) (host:container) + storage:list-entries [--scheduler s] [--format text|json], List registered storage entries storage:mount , Create a new bind mount storage:report [] [], Displays a storage report for one or more apps storage:unmount , Remove an existing bind mount` diff --git a/plugins/storage/src/subcommands/subcommands.go b/plugins/storage/src/subcommands/subcommands.go index 3e051c341..86ce98ecf 100644 --- a/plugins/storage/src/subcommands/subcommands.go +++ b/plugins/storage/src/subcommands/subcommands.go @@ -19,18 +19,71 @@ func main() { switch subcommand { case "default": err = storage.CommandHelp() + case "create": + args := flag.NewFlagSet("storage:create", flag.ExitOnError) + scheduler := args.String("scheduler", storage.SchedulerDockerLocal, "--scheduler: target scheduler (docker-local, k3s)") + size := args.String("size", "", "--size: PVC size (k3s only, e.g. 2Gi)") + accessMode := args.String("access-mode", "", "--access-mode: PVC access mode (k3s only)") + storageClass := args.String("storage-class-name", "", "--storage-class-name: PVC storage class (k3s only)") + namespace := args.String("namespace", "", "--namespace: PVC namespace (k3s only)") + chown := args.String("chown", "", "--chown: chown option (docker-local only)") + reclaim := args.String("reclaim-policy", "", "--reclaim-policy: PV reclaim policy (Retain or Delete, k3s only)") + annotations := args.StringSlice("annotation", nil, "--annotation key=value: PVC annotation (repeatable)") + labels := args.StringSlice("label", nil, "--label key=value: PVC label (repeatable)") + args.Parse(os.Args[2:]) + name := args.Arg(0) + path := args.Arg(1) + annotMap, parseErr := parseKVPairs(*annotations) + if parseErr != nil { + err = parseErr + break + } + labelMap, parseErr := parseKVPairs(*labels) + if parseErr != nil { + err = parseErr + break + } + err = storage.CommandCreate(storage.CommandCreateInput{ + Name: name, + Path: path, + Scheduler: *scheduler, + Size: *size, + AccessMode: *accessMode, + StorageClass: *storageClass, + Namespace: *namespace, + Chown: *chown, + ReclaimPolicy: *reclaim, + Annotations: annotMap, + Labels: labelMap, + }) + case "destroy": + args := flag.NewFlagSet("storage:destroy", flag.ExitOnError) + args.Parse(os.Args[2:]) + err = storage.CommandDestroy(args.Arg(0)) case "ensure-directory": args := flag.NewFlagSet("storage:ensure-directory", flag.ExitOnError) chown := args.String("chown", "herokuish", "--chown: chown option (herokuish, heroku, paketo, root, false)") args.Parse(os.Args[2:]) directory := args.Arg(0) + common.LogWarn("storage:ensure-directory is deprecated; use storage:create instead.") err = storage.CommandEnsureDirectory(directory, *chown) + case "info": + args := flag.NewFlagSet("storage:info", flag.ExitOnError) + format := args.String("format", "text", "--format: output format (text, json)") + args.Parse(os.Args[2:]) + err = storage.CommandInfo(args.Arg(0), *format) case "list": args := flag.NewFlagSet("storage:list", flag.ExitOnError) format := args.String("format", "text", "--format: output format (text, json)") args.Parse(os.Args[2:]) appName := args.Arg(0) err = storage.CommandList(appName, *format) + case "list-entries": + args := flag.NewFlagSet("storage:list-entries", flag.ExitOnError) + scheduler := args.String("scheduler", "", "--scheduler: filter to a single scheduler") + format := args.String("format", "text", "--format: output format (text, json)") + args.Parse(os.Args[2:]) + err = storage.CommandListEntries(*scheduler, *format) case "mount": args := flag.NewFlagSet("storage:mount", flag.ExitOnError) args.Parse(os.Args[2:]) @@ -63,3 +116,18 @@ func main() { common.LogFailWithError(err) } } + +func parseKVPairs(pairs []string) (map[string]string, error) { + if len(pairs) == 0 { + return nil, nil + } + out := map[string]string{} + for _, pair := range pairs { + idx := strings.Index(pair, "=") + if idx <= 0 { + return nil, fmt.Errorf("expected key=value, got %q", pair) + } + out[pair[:idx]] = pair[idx+1:] + } + return out, nil +} diff --git a/plugins/storage/subcommands.go b/plugins/storage/subcommands.go index 4ff579a65..b84575b84 100644 --- a/plugins/storage/subcommands.go +++ b/plugins/storage/subcommands.go @@ -19,8 +19,12 @@ Manage mounted volumes Additional commands:` helpContent = ` - storage:ensure-directory [--chown option] , Creates a persistent storage directory in the recommended storage path + storage:create [] [flags], Register a named storage entry + storage:destroy , Remove a named storage entry (must be unmounted from every app first) + storage:ensure-directory [--chown option] , [DEPRECATED] use storage:create instead + storage:info [--format text|json], Show details for one storage entry storage:list [--format text|json], List bind mounts for app's container(s) (host:container) + storage:list-entries [--scheduler s] [--format text|json], List registered storage entries storage:mount , Create a new bind mount storage:report [] [], Displays a storage report for one or more apps storage:unmount , Remove an existing bind mount` From e1b6bdb1d41bcee2d6416d015c515783f7e660dc Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Wed, 29 Apr 2026 14:43:51 -0400 Subject: [PATCH 03/13] feat: storage attachments, docker-args triggers, install-time migration storage:mount and storage:unmount now accept the named-entry form (--container-dir, --phase, --process-type, --volume-subpath, --volume-readonly, --volume-chown), persisting attachments via the property list. The legacy host:container colon form is preserved for back-compat. Storage now emits its own docker-args-deploy and docker-args-run triggers so docker-local apps continue to receive bind mounts even after their data moves out of docker-options. The install trigger walks every app once per upgrade and converts legacy -v lines into legacy- entries plus attachments, draining the originals from docker-options last so a partial run leaves the old behavior intact. Lifecycle triggers post-delete, post-app-clone-setup, and post-app-rename-setup keep the attachment store in sync with app state. --- plugins/storage/Makefile | 2 +- plugins/storage/migrate.go | 190 ++++++++++++++++++ .../storage/src/subcommands/subcommands.go | 28 ++- plugins/storage/src/triggers/triggers.go | 21 ++ plugins/storage/subcommands.go | 108 ++++++++-- plugins/storage/triggers.go | 134 +++++++++++- 6 files changed, 453 insertions(+), 30 deletions(-) create mode 100644 plugins/storage/migrate.go diff --git a/plugins/storage/Makefile b/plugins/storage/Makefile index c719dde7d..7a6b61e35 100644 --- a/plugins/storage/Makefile +++ b/plugins/storage/Makefile @@ -1,6 +1,6 @@ GOARCH ?= amd64 SUBCOMMANDS = subcommands/default subcommands/create subcommands/destroy subcommands/ensure-directory subcommands/info subcommands/list subcommands/list-entries subcommands/mount subcommands/report subcommands/unmount -TRIGGERS = triggers/install triggers/storage-list +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 BUILD = commands subcommands triggers PLUGIN_NAME = storage diff --git a/plugins/storage/migrate.go b/plugins/storage/migrate.go new file mode 100644 index 000000000..088ad4d83 --- /dev/null +++ b/plugins/storage/migrate.go @@ -0,0 +1,190 @@ +package storage + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "sort" + "strings" + + "github.com/dokku/dokku/plugins/common" + 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. +func migrationFlagDir() string { + root := common.GetenvWithDefault("DOKKU_LIB_ROOT", "/var/lib/dokku") + return filepath.Join(root, "config", "storage", ".migrated") +} + +func migrationFlagFile(appName string) string { + return filepath.Join(migrationFlagDir(), appName) +} + +// 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. +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) { + return nil + } + return err + } + + for _, app := range apps { + if _, err := os.Stat(migrationFlagFile(app)); err == nil { + continue + } + if err := migrateApp(app); err != nil { + return fmt.Errorf("storage migration failed for app %q: %w", app, err) + } + } + return nil +} + +// 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) + if err != nil { + return err + } + runLines, err := dockeroptions.GetDockerOptionsForPhase(appName, PhaseRun) + if err != nil { + return err + } + + deployMounts := filterMountLines(deployLines) + runMounts := filterMountLines(runLines) + + // Group identical mount strings across phases. + phaseMap := map[string][]string{} + for _, mount := range deployMounts { + phaseMap[mount] = appendUnique(phaseMap[mount], PhaseDeploy) + } + for _, mount := range runMounts { + phaseMap[mount] = appendUnique(phaseMap[mount], PhaseRun) + } + + mounts := make([]string, 0, len(phaseMap)) + for mount := range phaseMap { + mounts = append(mounts, mount) + } + sort.Strings(mounts) + + for _, mount := range mounts { + if err := migrateMount(appName, mount, phaseMap[mount]); err != nil { + return err + } + } + + return touchMigrationFlag(appName) +} + +func migrateMount(appName string, mount string, phases []string) error { + parsed := ParseMountPath(mount) + entry := LegacyMountToEntry(mount) + + if EntryExists(entry.Name) { + existing, err := LoadEntry(entry.Name) + if err != nil { + return err + } + if existing.HostPath != entry.HostPath || existing.Scheduler != entry.Scheduler { + return fmt.Errorf("legacy entry %q already exists with conflicting fields", entry.Name) + } + } else { + if err := SaveEntry(entry); err != nil { + return err + } + } + + containerPath := parsed.ContainerPath + if containerPath == "" { + // Defensive: legacy mounts must have a container path; skip if not. + return nil + } + + attachment := &Attachment{ + EntryName: entry.Name, + ContainerPath: containerPath, + Phases: phases, + ProcessType: DefaultProcessType, + } + if parsed.VolumeOptions != "" { + if parsed.VolumeOptions == "ro" { + attachment.Readonly = true + } else { + attachment.VolumeOptions = parsed.VolumeOptions + } + } + + existing, err := LoadAttachments(appName) + if err != nil { + return err + } + if !attachmentExists(existing, attachment) { + existing = append(existing, attachment) + if err := SaveAttachments(appName, existing); err != nil { + return err + } + } + + mountedPhases := []string{} + for _, phase := range phases { + mountedPhases = append(mountedPhases, phase) + } + if err := dockeroptions.RemoveDockerOptionFromPhases(appName, mountedPhases, fmt.Sprintf("-v %s", mount)); err != nil { + return err + } + return nil +} + +func touchMigrationFlag(appName string) error { + f, err := os.Create(migrationFlagFile(appName)) + if err != nil { + return err + } + return f.Close() +} + +func filterMountLines(lines []string) []string { + out := []string{} + for _, line := range lines { + if strings.HasPrefix(line, "-v ") { + out = append(out, strings.TrimPrefix(line, "-v ")) + } + } + return out +} + +func appendUnique(slice []string, value string) []string { + for _, existing := range slice { + if existing == value { + return slice + } + } + return append(slice, value) +} + +func attachmentExists(attachments []*Attachment, candidate *Attachment) bool { + for _, existing := range attachments { + if existing.EntryName == candidate.EntryName && + existing.ContainerPath == candidate.ContainerPath && + existing.ProcessType == candidate.ProcessType { + return true + } + } + return false +} diff --git a/plugins/storage/src/subcommands/subcommands.go b/plugins/storage/src/subcommands/subcommands.go index 86ce98ecf..52d9a46bb 100644 --- a/plugins/storage/src/subcommands/subcommands.go +++ b/plugins/storage/src/subcommands/subcommands.go @@ -86,10 +86,23 @@ func main() { err = storage.CommandListEntries(*scheduler, *format) case "mount": args := flag.NewFlagSet("storage:mount", flag.ExitOnError) + containerDir := args.String("container-dir", "", "--container-dir: container path (named-entry form)") + phases := args.StringSlice("phase", nil, "--phase: phase to mount in (deploy, run; default both)") + processType := args.String("process-type", "", "--process-type: process type to mount for") + subpath := args.String("volume-subpath", "", "--volume-subpath: subpath within the entry") + readonly := args.Bool("volume-readonly", false, "--volume-readonly: mount the volume read-only") + volumeChown := args.String("volume-chown", "", "--volume-chown: chown option applied at mount time") args.Parse(os.Args[2:]) - appName := args.Arg(0) - mountPath := args.Arg(1) - err = storage.CommandMount(appName, mountPath) + err = storage.CommandMount(storage.CommandMountInput{ + AppName: args.Arg(0), + NameOrPath: args.Arg(1), + ContainerDir: *containerDir, + Phases: *phases, + ProcessType: *processType, + Subpath: *subpath, + Readonly: *readonly, + VolumeChown: *volumeChown, + }) case "report": args := flag.NewFlagSet("storage:report", flag.ExitOnError) format := args.String("format", "stdout", "format: [ stdout | json ]") @@ -104,10 +117,13 @@ func main() { } case "unmount": args := flag.NewFlagSet("storage:unmount", flag.ExitOnError) + containerDir := args.String("container-dir", "", "--container-dir: container path (named-entry form, disambiguates duplicates)") args.Parse(os.Args[2:]) - appName := args.Arg(0) - mountPath := args.Arg(1) - err = storage.CommandUnmount(appName, mountPath) + err = storage.CommandUnmount(storage.CommandUnmountInput{ + AppName: args.Arg(0), + NameOrPath: args.Arg(1), + ContainerDir: *containerDir, + }) default: err = fmt.Errorf("Invalid plugin subcommand call: %s", subcommand) } diff --git a/plugins/storage/src/triggers/triggers.go b/plugins/storage/src/triggers/triggers.go index 3d2dd3ebf..565891290 100644 --- a/plugins/storage/src/triggers/triggers.go +++ b/plugins/storage/src/triggers/triggers.go @@ -24,6 +24,27 @@ func main() { phase := flag.Arg(1) format := flag.Arg(2) err = storage.TriggerStorageList(appName, phase, format) + case "storage-app-mounts": + appName := flag.Arg(0) + phase := flag.Arg(1) + err = storage.TriggerStorageAppMounts(appName, phase) + case "docker-args-deploy": + appName := flag.Arg(0) + err = storage.TriggerDockerArgs(appName, storage.PhaseDeploy) + case "docker-args-run": + appName := flag.Arg(0) + err = storage.TriggerDockerArgs(appName, storage.PhaseRun) + case "post-delete": + appName := flag.Arg(0) + err = storage.TriggerPostDelete(appName) + case "post-app-clone-setup": + oldName := flag.Arg(0) + newName := flag.Arg(1) + err = storage.TriggerPostAppCloneSetup(oldName, newName) + case "post-app-rename-setup": + oldName := flag.Arg(0) + newName := flag.Arg(1) + err = storage.TriggerPostAppRenameSetup(oldName, newName) default: err = fmt.Errorf("Invalid plugin trigger call: %s", trigger) } diff --git a/plugins/storage/subcommands.go b/plugins/storage/subcommands.go index b84575b84..9b3eb5824 100644 --- a/plugins/storage/subcommands.go +++ b/plugins/storage/subcommands.go @@ -125,38 +125,102 @@ func isUserNamespacesEnabled() (bool, error) { return strings.TrimSpace(result.StdoutContents()) == "true", nil } -// CommandMount creates a new bind mount for an app -func CommandMount(appName string, mountPath string) error { - if err := common.VerifyAppName(appName); err != nil { - return err - } - - if err := VerifyPaths(mountPath); err != nil { - return err - } - - if CheckIfPathExists(appName, mountPath, MountPhases) { - return errors.New("Mount path already exists.") - } - - return dockeroptions.AddDockerOptionToPhases(appName, MountPhases, fmt.Sprintf("-v %s", mountPath)) +// CommandMountInput captures the optional flags accepted by storage:mount +// when the second argument is a named entry rather than a colon-form path. +type CommandMountInput struct { + AppName string + NameOrPath string + ContainerDir string + Phases []string + ProcessType string + Subpath string + Readonly bool + VolumeOptions string + VolumeChown string } -// CommandUnmount removes an existing bind mount from an app -func CommandUnmount(appName string, mountPath string) error { - if err := common.VerifyAppName(appName); err != nil { +// CommandMount creates a new bind mount for an app. The second positional +// argument may be either a legacy host:container[:options] string (kept +// for back-compat on docker-local) or a registered storage entry name. +func CommandMount(input CommandMountInput) error { + if err := common.VerifyAppName(input.AppName); err != nil { return err } - if err := VerifyPaths(mountPath); err != nil { + // Legacy colon form: keep writing into docker-options so existing + // users see no behavior change. + if strings.Contains(input.NameOrPath, ":") { + if err := VerifyPaths(input.NameOrPath); err != nil { + return err + } + if CheckIfPathExists(input.AppName, input.NameOrPath, MountPhases) { + return errors.New("Mount path already exists.") + } + return dockeroptions.AddDockerOptionToPhases(input.AppName, MountPhases, fmt.Sprintf("-v %s", input.NameOrPath)) + } + + // Named-entry form: persist as an attachment. + if !EntryExists(input.NameOrPath) { + return fmt.Errorf("storage entry %q does not exist; create it first with `dokku storage:create`", input.NameOrPath) + } + if input.ContainerDir == "" { + return errors.New("--container-dir is required when mounting a named storage entry") + } + + entry, err := LoadEntry(input.NameOrPath) + if err != nil { return err } - if !CheckIfPathExists(appName, mountPath, MountPhases) { - return errors.New("Mount path does not exist.") + phases := input.Phases + if len(phases) == 0 { + phases = []string{PhaseDeploy, PhaseRun} } - return dockeroptions.RemoveDockerOptionFromPhases(appName, MountPhases, fmt.Sprintf("-v %s", mountPath)) + processType := input.ProcessType + if processType == "" { + processType = DefaultProcessType + } + + attachment := &Attachment{ + EntryName: entry.Name, + ContainerPath: input.ContainerDir, + Phases: phases, + ProcessType: processType, + Subpath: input.Subpath, + Readonly: input.Readonly, + VolumeOptions: input.VolumeOptions, + VolumeChown: input.VolumeChown, + } + + return AddAttachment(input.AppName, attachment) +} + +// CommandUnmountInput captures the flags accepted by storage:unmount when +// the second argument is a named entry. +type CommandUnmountInput struct { + AppName string + NameOrPath string + ContainerDir string +} + +// CommandUnmount removes an existing bind mount from an app. +func CommandUnmount(input CommandUnmountInput) error { + if err := common.VerifyAppName(input.AppName); err != nil { + return err + } + + if strings.Contains(input.NameOrPath, ":") { + if err := VerifyPaths(input.NameOrPath); err != nil { + return err + } + if !CheckIfPathExists(input.AppName, input.NameOrPath, MountPhases) { + return errors.New("Mount path does not exist.") + } + return dockeroptions.RemoveDockerOptionFromPhases(input.AppName, MountPhases, fmt.Sprintf("-v %s", input.NameOrPath)) + } + + return RemoveAttachment(input.AppName, input.NameOrPath, input.ContainerDir) } // CommandList lists all bind mounts for an app diff --git a/plugins/storage/triggers.go b/plugins/storage/triggers.go index ac2c4d4e0..389783e42 100644 --- a/plugins/storage/triggers.go +++ b/plugins/storage/triggers.go @@ -6,11 +6,13 @@ import ( "os" "path/filepath" "runtime" + "strings" "github.com/dokku/dokku/plugins/common" ) -// TriggerInstall sets up the storage plugin on installation +// TriggerInstall sets up the storage plugin on installation and runs +// the bulk legacy-mount migration once per upgrade. func TriggerInstall() error { storageDir := GetStorageDirectory() @@ -25,6 +27,14 @@ func TriggerInstall() error { return fmt.Errorf("Unable to set storage directory permissions: %s", err.Error()) } + if err := EnsureEntriesDirectory(); err != nil { + return err + } + + if err := MigrateLegacyMounts(); err != nil { + return fmt.Errorf("storage migration failed: %w", err) + } + distro := detectDistro() if distro == "" { return nil @@ -44,6 +54,44 @@ func TriggerInstall() error { return nil } +// TriggerPostDelete clears any attachment list for an app that's being +// destroyed. Entries are global and survive app deletion. +func TriggerPostDelete(appName string) error { + if appName == "" { + return nil + } + return common.PropertyListWrite(PluginName, appName, AttachmentsProperty, []string{}) +} + +// TriggerPostAppCloneSetup copies attachments from the source app to the +// cloned app. Entries are global so no entry-level work is needed. +func TriggerPostAppCloneSetup(oldName string, newName string) error { + if oldName == "" || newName == "" { + return nil + } + attachments, err := LoadAttachments(oldName) + if err != nil { + return err + } + return SaveAttachments(newName, attachments) +} + +// TriggerPostAppRenameSetup moves attachments from the old name to the +// new name and clears the old. +func TriggerPostAppRenameSetup(oldName string, newName string) error { + if oldName == "" || newName == "" { + return nil + } + attachments, err := LoadAttachments(oldName) + if err != nil { + return err + } + if err := SaveAttachments(newName, attachments); err != nil { + return err + } + return common.PropertyListWrite(PluginName, oldName, AttachmentsProperty, []string{}) +} + // detectDistro returns the Linux distribution name func detectDistro() string { if runtime.GOOS != "linux" { @@ -91,3 +139,87 @@ func TriggerStorageList(appName string, phase string, format string) error { return nil } + +// AppMountPair pairs an Attachment with the Entry it references; consumed +// by scheduler plugins via the storage-app-mounts trigger. +type AppMountPair struct { + Entry *Entry `json:"entry"` + Attachment *Attachment `json:"attachment"` +} + +// TriggerStorageAppMounts emits the (entry, attachment) pairs an app has +// for the given phase, in JSON. Schedulers consume this at deploy time. +func TriggerStorageAppMounts(appName string, phase string) error { + if phase == "" { + phase = PhaseDeploy + } + attachments, err := AttachmentsForPhase(appName, phase) + if err != nil { + return err + } + + pairs := []AppMountPair{} + 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) + } + pairs = append(pairs, AppMountPair{Entry: entry, Attachment: attachment}) + } + + output, err := json.Marshal(pairs) + if err != nil { + return err + } + fmt.Println(string(output)) + 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) + if err != nil { + return err + } + + 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) + } + if entry.Scheduler != SchedulerDockerLocal { + continue + } + flag := buildDockerVFlag(entry, attachment) + if flag == "" { + continue + } + fmt.Printf(" %s", flag) + } + return nil +} + +// buildDockerVFlag formats the Docker -v argument for a docker-local +// attachment. +func buildDockerVFlag(entry *Entry, attachment *Attachment) string { + host := entry.HostPath + container := attachment.ContainerPath + if host == "" || container == "" { + return "" + } + options := []string{} + if attachment.Readonly { + options = append(options, "ro") + } + if attachment.VolumeOptions != "" { + options = append(options, attachment.VolumeOptions) + } + flag := fmt.Sprintf("-v %s:%s", host, container) + if len(options) > 0 { + flag = fmt.Sprintf("%s:%s", flag, strings.Join(options, ",")) + } + return flag +} From cfb7da8ec0ce581487a51c2dbd62ef2552df9b56 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Wed, 29 Apr 2026 14:49:15 -0400 Subject: [PATCH 04/13] feat: scheduler-k3s renders pvc-backed volumes from storage attachments Adds a per-entry storage helm chart that owns the PVC and (when host_path is set) the PV, and three triggers for the storage plugin to drive: storage-create installs/upgrades the chart with storage-class existence validation, storage-destroy uninstalls it, storage-status reports the PVC phase. App deployment and cron-job charts now consume storage-app-mounts and reference each PVC by name; the // todo: implement volumes blocks are gone. ProcessVolume gained sub_path, read_only, and persistent_claim fields and the deployment/cron-job templates render them. --- plugins/scheduler-k3s/Makefile | 2 +- plugins/scheduler-k3s/go.mod | 3 + .../scheduler-k3s/src/triggers/triggers.go | 10 + plugins/scheduler-k3s/storage.go | 313 ++++++++++++++++++ plugins/scheduler-k3s/template.go | 13 +- .../templates/chart/cron-job.yaml | 32 ++ .../templates/chart/deployment.yaml | 10 + .../templates/persistent-volume-claim.yaml | 29 ++ .../templates/persistent-volume.yaml | 29 ++ plugins/scheduler-k3s/triggers.go | 12 +- 10 files changed, 447 insertions(+), 6 deletions(-) create mode 100644 plugins/scheduler-k3s/storage.go create mode 100644 plugins/scheduler-k3s/templates/storage-chart/templates/persistent-volume-claim.yaml create mode 100644 plugins/scheduler-k3s/templates/storage-chart/templates/persistent-volume.yaml diff --git a/plugins/scheduler-k3s/Makefile b/plugins/scheduler-k3s/Makefile index fce3bb816..69b114ba4 100644 --- a/plugins/scheduler-k3s/Makefile +++ b/plugins/scheduler-k3s/Makefile @@ -1,5 +1,5 @@ SUBCOMMANDS = subcommands/annotations:set subcommands/autoscaling-auth:set subcommands/autoscaling-auth:report subcommands/cluster:add subcommands/cluster:list subcommands/cluster:remove subcommands/ensure-charts subcommands/initialize subcommands/labels:set subcommands/profiles:add subcommands/profiles:list subcommands/profiles:remove subcommands/report subcommands/set subcommands/show-kubeconfig subcommands/uninstall -TRIGGERS = triggers/core-post-deploy triggers/core-post-extract triggers/install triggers/post-app-clone-setup triggers/post-app-rename-setup triggers/post-certs-update triggers/post-certs-remove triggers/post-create triggers/post-delete triggers/report triggers/scheduler-app-status triggers/scheduler-deploy triggers/scheduler-enter triggers/scheduler-is-deployed triggers/scheduler-logs triggers/scheduler-proxy-config triggers/scheduler-proxy-logs triggers/scheduler-post-delete triggers/scheduler-run triggers/scheduler-run-list triggers/scheduler-stop triggers/scheduler-cron-write +TRIGGERS = triggers/core-post-deploy triggers/core-post-extract triggers/install triggers/post-app-clone-setup triggers/post-app-rename-setup triggers/post-certs-update triggers/post-certs-remove triggers/post-create triggers/post-delete triggers/report triggers/scheduler-app-status triggers/scheduler-deploy triggers/scheduler-enter triggers/scheduler-is-deployed triggers/scheduler-logs triggers/scheduler-proxy-config triggers/scheduler-proxy-logs triggers/scheduler-post-delete triggers/scheduler-run triggers/scheduler-run-list triggers/scheduler-stop triggers/scheduler-cron-write triggers/storage-create triggers/storage-destroy triggers/storage-status BUILD = commands subcommands triggers PLUGIN_NAME = scheduler-k3s diff --git a/plugins/scheduler-k3s/go.mod b/plugins/scheduler-k3s/go.mod index 9f57b850b..6e9b0eb34 100644 --- a/plugins/scheduler-k3s/go.mod +++ b/plugins/scheduler-k3s/go.mod @@ -13,6 +13,7 @@ require ( github.com/dokku/dokku/plugins/logs v0.0.0-00010101000000-000000000000 github.com/dokku/dokku/plugins/nginx-vhosts v0.0.0-00010101000000-000000000000 github.com/dokku/dokku/plugins/registry v0.0.0-00010101000000-000000000000 + github.com/dokku/dokku/plugins/storage v0.0.0-00010101000000-000000000000 github.com/fatih/color v1.19.0 github.com/fluxcd/pkg/kustomize v1.31.0 github.com/go-openapi/jsonpointer v0.23.1 @@ -193,6 +194,8 @@ replace github.com/dokku/dokku/plugins/nginx-vhosts => ../nginx-vhosts replace github.com/dokku/dokku/plugins/registry => ../registry +replace github.com/dokku/dokku/plugins/storage => ../storage + replace github.com/joho/godotenv => github.com/joho/godotenv v1.2.0 replace sigs.k8s.io/controller-runtime => sigs.k8s.io/controller-runtime v0.22.4 diff --git a/plugins/scheduler-k3s/src/triggers/triggers.go b/plugins/scheduler-k3s/src/triggers/triggers.go index d78606e72..e9720f7d2 100644 --- a/plugins/scheduler-k3s/src/triggers/triggers.go +++ b/plugins/scheduler-k3s/src/triggers/triggers.go @@ -1,6 +1,7 @@ package main import ( + "context" "fmt" "os" "strconv" @@ -158,6 +159,15 @@ func main() { appName := flag.Arg(1) format := flag.Arg(2) err = scheduler_k3s.TriggerSchedulerRunList(scheduler, appName, format) + case "storage-create": + entryName := flag.Arg(0) + err = scheduler_k3s.TriggerStorageCreate(context.Background(), entryName) + case "storage-destroy": + entryName := flag.Arg(0) + err = scheduler_k3s.TriggerStorageDestroy(context.Background(), entryName) + case "storage-status": + entryName := flag.Arg(0) + err = scheduler_k3s.TriggerStorageStatus(context.Background(), entryName) default: err = fmt.Errorf("Invalid plugin trigger call: %s", trigger) } diff --git a/plugins/scheduler-k3s/storage.go b/plugins/scheduler-k3s/storage.go new file mode 100644 index 000000000..9cf0e29ce --- /dev/null +++ b/plugins/scheduler-k3s/storage.go @@ -0,0 +1,313 @@ +package scheduler_k3s + +import ( + "context" + "encoding/json" + "fmt" + "io" + "os" + "path/filepath" + "strings" + + "github.com/dokku/dokku/plugins/common" + storage "github.com/dokku/dokku/plugins/storage" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// StorageChartValues are the helm chart inputs for the per-entry storage chart. +type StorageChartValues struct { + Global StorageChartGlobal `yaml:"global"` +} + +// StorageChartGlobal contains global values for the storage chart. +type StorageChartGlobal struct { + EntryName string `yaml:"entry_name"` + Namespace string `yaml:"namespace"` + Size string `yaml:"size"` + AccessMode string `yaml:"access_mode,omitempty"` + StorageClass string `yaml:"storage_class,omitempty"` + HostPath string `yaml:"host_path,omitempty"` + ReclaimPolicy string `yaml:"reclaim_policy,omitempty"` + Annotations map[string]string `yaml:"annotations,omitempty"` + Labels map[string]string `yaml:"labels,omitempty"` +} + +// GetStorageReleaseName returns the helm release name for a storage entry. +func GetStorageReleaseName(entryName string) string { + return fmt.Sprintf("storage-%s", entryName) +} + +// readEntryFromStdin decodes a storage.Entry from stdin. The storage +// plugin pipes the JSON-encoded entry to scheduler triggers. +func readEntryFromStdin() (*storage.Entry, error) { + raw, err := io.ReadAll(os.Stdin) + if err != nil { + return nil, fmt.Errorf("unable to read storage entry from stdin: %w", err) + } + if len(raw) == 0 { + return nil, fmt.Errorf("no storage entry payload received on stdin") + } + entry := &storage.Entry{} + if err := json.Unmarshal(raw, entry); err != nil { + return nil, fmt.Errorf("unable to parse storage entry payload: %w", err) + } + return entry, nil +} + +// TriggerStorageCreate provisions or updates the PVC/PV for a k3s storage +// entry. Idempotent: re-runs against the same entry are a helm upgrade. +func TriggerStorageCreate(ctx context.Context, entryName string) error { + if err := isKubernetesAvailable(); err != nil { + return fmt.Errorf("kubernetes not available: %w", err) + } + + entry, err := readEntryFromStdin() + if err != nil { + return err + } + if entry.Name != entryName { + return fmt.Errorf("entry name mismatch: stdin=%q, arg=%q", entry.Name, entryName) + } + if entry.Scheduler != storage.SchedulerK3s { + return nil + } + + if err := validateStorageClass(ctx, entry.StorageClass); err != nil { + return err + } + + chartDir, err := os.MkdirTemp("", "dokku-storage-chart-") + if err != nil { + return fmt.Errorf("error creating chart directory: %w", err) + } + defer os.RemoveAll(chartDir) + + releaseName := GetStorageReleaseName(entry.Name) + namespace := entry.Namespace + if namespace == "" { + namespace = "default" + } + + if err := os.MkdirAll(filepath.Join(chartDir, "templates"), 0755); err != nil { + return fmt.Errorf("error creating chart templates directory: %w", err) + } + + chart := &Chart{ + ApiVersion: "v2", + AppVersion: "1.0.0", + Name: releaseName, + Icon: "https://dokku.com/assets/dokku-logo.svg", + Version: "0.0.1", + } + if err := writeYaml(WriteYamlInput{Object: chart, Path: filepath.Join(chartDir, "Chart.yaml")}); err != nil { + return fmt.Errorf("error writing chart: %w", err) + } + + for _, name := range []string{"persistent-volume-claim.yaml", "persistent-volume.yaml"} { + b, err := templates.ReadFile(filepath.Join("templates", "storage-chart", "templates", name)) + if err != nil { + return fmt.Errorf("error reading %s: %w", name, err) + } + if err := os.WriteFile(filepath.Join(chartDir, "templates", name), b, 0644); err != nil { + return fmt.Errorf("error writing %s: %w", name, err) + } + } + + values := &StorageChartValues{ + Global: StorageChartGlobal{ + EntryName: entry.Name, + Namespace: namespace, + Size: entry.Size, + AccessMode: entry.AccessMode, + StorageClass: entry.StorageClass, + HostPath: entry.HostPath, + ReclaimPolicy: entry.ReclaimPolicy, + Annotations: entry.Annotations, + Labels: entry.Labels, + }, + } + if err := writeYaml(WriteYamlInput{Object: values, Path: filepath.Join(chartDir, "values.yaml")}); err != nil { + return fmt.Errorf("error writing values: %w", err) + } + + if err := createKubernetesNamespace(ctx, namespace); err != nil { + return fmt.Errorf("error creating namespace: %w", err) + } + + helmAgent, err := NewHelmAgent(namespace, DeployLogPrinter) + if err != nil { + return fmt.Errorf("error creating helm agent: %w", err) + } + + chartPath, err := filepath.Abs(chartDir) + if err != nil { + return fmt.Errorf("error getting chart path: %w", err) + } + + common.LogVerboseQuiet(fmt.Sprintf("Installing storage chart for %s", entry.Name)) + if err := helmAgent.InstallOrUpgradeChart(ctx, ChartInput{ + ChartPath: chartPath, + Namespace: namespace, + ReleaseName: releaseName, + Wait: false, + }); err != nil { + return fmt.Errorf("error installing storage chart: %w", err) + } + return nil +} + +// TriggerStorageDestroy removes the helm release backing a storage entry. +func TriggerStorageDestroy(ctx context.Context, entryName string) error { + if err := isKubernetesAvailable(); err != nil { + return fmt.Errorf("kubernetes not available: %w", err) + } + + entry, err := readEntryFromStdin() + if err != nil { + return err + } + if entry.Scheduler != storage.SchedulerK3s { + return nil + } + + namespace := entry.Namespace + if namespace == "" { + namespace = "default" + } + releaseName := GetStorageReleaseName(entryName) + + helmAgent, err := NewHelmAgent(namespace, DeployLogPrinter) + if err != nil { + return fmt.Errorf("error creating helm agent: %w", err) + } + exists, err := helmAgent.ChartExists(releaseName) + if err != nil { + return fmt.Errorf("error checking if storage chart exists: %w", err) + } + if !exists { + return nil + } + + common.LogVerboseQuiet(fmt.Sprintf("Removing storage chart for %s", entryName)) + if err := helmAgent.UninstallChart(releaseName); err != nil { + return fmt.Errorf("error uninstalling storage chart: %w", err) + } + return nil +} + +// TriggerStorageStatus prints "Bound" if the entry's PVC is bound, +// "Pending" or "Lost" otherwise. Used by the storage:wait command. +func TriggerStorageStatus(ctx context.Context, entryName string) error { + if err := isKubernetesAvailable(); err != nil { + return fmt.Errorf("kubernetes not available: %w", err) + } + entry, err := readEntryFromStdin() + if err != nil { + return err + } + namespace := entry.Namespace + if namespace == "" { + namespace = "default" + } + + clientset, err := NewKubernetesClient() + if err != nil { + return err + } + pvc, err := clientset.Client.CoreV1().PersistentVolumeClaims(namespace).Get(ctx, entryName, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("error fetching PVC %s/%s: %w", namespace, entryName, err) + } + fmt.Println(string(pvc.Status.Phase)) + return nil +} + +// validateStorageClass surfaces a clear error when the requested storage +// class doesn't exist in the cluster. +func validateStorageClass(ctx context.Context, name string) error { + if name == "" { + return nil + } + clientset, err := NewKubernetesClient() + if err != nil { + return err + } + classes, err := clientset.Client.StorageV1().StorageClasses().List(ctx, metav1.ListOptions{}) + if err != nil { + return fmt.Errorf("error listing storage classes: %w", err) + } + available := []string{} + for _, cls := range classes.Items { + if cls.Name == name { + return nil + } + available = append(available, cls.Name) + } + return fmt.Errorf("storage class %q does not exist; available: %s", name, strings.Join(available, ", ")) +} + +// AppMountPair mirrors the storage plugin's wire format so we can decode +// the storage-app-mounts trigger output here. +type AppMountPair struct { + Entry *storage.Entry `json:"entry"` + Attachment *storage.Attachment `json:"attachment"` +} + +// LoadAppMounts asks the storage plugin for an app's mount pairs in the +// given phase. Returns an empty slice when there are none. +func LoadAppMounts(appName string, phase string) ([]AppMountPair, error) { + results, err := common.CallPlugnTrigger(common.PlugnTriggerInput{ + Trigger: "storage-app-mounts", + Args: []string{appName, phase}, + }) + if err != nil { + return nil, err + } + output := strings.TrimSpace(results.StdoutContents()) + if output == "" { + return nil, nil + } + pairs := []AppMountPair{} + if err := json.Unmarshal([]byte(output), &pairs); err != nil { + return nil, fmt.Errorf("unable to parse storage-app-mounts output: %w", err) + } + 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{} + for _, pair := range pairs { + if pair.Entry == nil || pair.Attachment == nil { + continue + } + if pair.Entry.Scheduler == storage.SchedulerDockerLocal { + return nil, fmt.Errorf("storage entry %q is scheduler=docker-local but is mounted on a k3s app; recreate it with --scheduler k3s", pair.Entry.Name) + } + volumes = append(volumes, ProcessVolume{ + Name: pair.Entry.Name, + MountPath: pair.Attachment.ContainerPath, + SubPath: pair.Attachment.Subpath, + ReadOnly: pair.Attachment.Readonly, + PersistentClaim: &ProcessVolumePersistentClaim{ + ClaimName: pair.Entry.Name, + }, + }) + } + return volumes, nil +} + +// asK8sVolumeMount is a small helper used by tests / other callers that +// want a corev1.VolumeMount from a ProcessVolume. +func asK8sVolumeMount(v ProcessVolume) corev1.VolumeMount { + return corev1.VolumeMount{ + Name: v.Name, + MountPath: v.MountPath, + SubPath: v.SubPath, + ReadOnly: v.ReadOnly, + } +} diff --git a/plugins/scheduler-k3s/template.go b/plugins/scheduler-k3s/template.go index f58fccc4d..9d2b2e63a 100644 --- a/plugins/scheduler-k3s/template.go +++ b/plugins/scheduler-k3s/template.go @@ -117,9 +117,12 @@ type ProcessValues struct { } type ProcessVolume struct { - Name string `yaml:"name"` - MountPath string `yaml:"mount_path"` - EmptyDir *ProcessVolumeEmptyDir `yaml:"empty_dir,omitempty"` + Name string `yaml:"name"` + MountPath string `yaml:"mount_path"` + SubPath string `yaml:"sub_path,omitempty"` + ReadOnly bool `yaml:"read_only,omitempty"` + EmptyDir *ProcessVolumeEmptyDir `yaml:"empty_dir,omitempty"` + PersistentClaim *ProcessVolumePersistentClaim `yaml:"persistent_claim,omitempty"` } type ProcessVolumeEmptyDir struct { @@ -127,6 +130,10 @@ type ProcessVolumeEmptyDir struct { SizeLimit string `yaml:"size_limit"` } +type ProcessVolumePersistentClaim struct { + ClaimName string `yaml:"claim_name"` +} + type ProcessAnnotations struct { CertificateAnnotations map[string]string `yaml:"certificate,omitempty"` CronJobAnnotations map[string]string `yaml:"cronjob,omitempty"` diff --git a/plugins/scheduler-k3s/templates/chart/cron-job.yaml b/plugins/scheduler-k3s/templates/chart/cron-job.yaml index fec53e1f2..e8cb4675a 100644 --- a/plugins/scheduler-k3s/templates/chart/cron-job.yaml +++ b/plugins/scheduler-k3s/templates/chart/cron-job.yaml @@ -127,12 +127,44 @@ spec: {{- if $.Values.global.image.working_dir }} workingDir: {{ $.Values.global.image.working_dir }} {{- end }} + {{- if $config.volumes }} + volumeMounts: + {{- range $volume := $config.volumes }} + - name: {{ $volume.name }} + mountPath: {{ $volume.mount_path }} + {{- if $volume.sub_path }} + subPath: {{ $volume.sub_path }} + {{- end }} + {{- if $volume.read_only }} + readOnly: true + {{- end }} + {{- end }} + {{- end }} {{- if $.Values.global.image.image_pull_secrets }} imagePullSecrets: - name: {{ $.Values.global.image.image_pull_secrets }} {{- end }} restartPolicy: Never serviceAccountName: {{ $.Values.global.app_name }} + {{- if $config.volumes }} + volumes: + {{- range $volume := $config.volumes }} + - name: {{ $volume.name }} + {{- if $volume.empty_dir }} + emptyDir: + {{- if $volume.empty_dir.medium }} + medium: {{ $volume.empty_dir.medium }} + {{- end }} + {{- if $volume.empty_dir.size_limit }} + sizeLimit: {{ $volume.empty_dir.size_limit }} + {{- end }} + {{- end }} + {{- if $volume.persistent_claim }} + persistentVolumeClaim: + claimName: {{ $volume.persistent_claim.claim_name }} + {{- end }} + {{- end }} + {{- end }} schedule: {{ $config.cron.schedule }} startingDeadlineSeconds: 60 successfulJobsHistoryLimit: 10 diff --git a/plugins/scheduler-k3s/templates/chart/deployment.yaml b/plugins/scheduler-k3s/templates/chart/deployment.yaml index 2eb5946c0..bee80b421 100644 --- a/plugins/scheduler-k3s/templates/chart/deployment.yaml +++ b/plugins/scheduler-k3s/templates/chart/deployment.yaml @@ -152,6 +152,12 @@ spec: {{- range $volume := $config.volumes }} - name: {{ $volume.name }} mountPath: {{ $volume.mount_path }} + {{- if $volume.sub_path }} + subPath: {{ $volume.sub_path }} + {{- end }} + {{- if $volume.read_only }} + readOnly: true + {{- end }} {{- end }} {{- end }} {{- if $.Values.global.image.image_pull_secrets }} @@ -172,6 +178,10 @@ spec: sizeLimit: {{ $volume.empty_dir.size_limit }} {{- end }} {{- end }} + {{- if $volume.persistent_claim }} + persistentVolumeClaim: + claimName: {{ $volume.persistent_claim.claim_name }} + {{- end }} {{- end }} {{- end }} {{- end }} diff --git a/plugins/scheduler-k3s/templates/storage-chart/templates/persistent-volume-claim.yaml b/plugins/scheduler-k3s/templates/storage-chart/templates/persistent-volume-claim.yaml new file mode 100644 index 000000000..34505bc96 --- /dev/null +++ b/plugins/scheduler-k3s/templates/storage-chart/templates/persistent-volume-claim.yaml @@ -0,0 +1,29 @@ +apiVersion: v1 +kind: PersistentVolumeClaim +metadata: + name: {{ .Values.global.entry_name }} + namespace: {{ .Values.global.namespace }} + labels: + app.kubernetes.io/name: {{ .Values.global.entry_name }} + app.kubernetes.io/managed-by: dokku + {{- range $k, $v := .Values.global.labels }} + {{ $k }}: {{ $v | quote }} + {{- end }} + annotations: + dokku.com/managed: "true" + {{- range $k, $v := .Values.global.annotations }} + {{ $k }}: {{ $v | quote }} + {{- end }} +spec: + accessModes: + - {{ .Values.global.access_mode | default "ReadWriteOnce" }} + resources: + requests: + storage: {{ .Values.global.size }} + {{- if .Values.global.storage_class }} + storageClassName: {{ .Values.global.storage_class }} + {{- end }} + {{- if .Values.global.host_path }} + volumeName: {{ .Values.global.entry_name }} + storageClassName: "" + {{- end }} diff --git a/plugins/scheduler-k3s/templates/storage-chart/templates/persistent-volume.yaml b/plugins/scheduler-k3s/templates/storage-chart/templates/persistent-volume.yaml new file mode 100644 index 000000000..92175f2d8 --- /dev/null +++ b/plugins/scheduler-k3s/templates/storage-chart/templates/persistent-volume.yaml @@ -0,0 +1,29 @@ +{{- if .Values.global.host_path }} +apiVersion: v1 +kind: PersistentVolume +metadata: + name: {{ .Values.global.entry_name }} + labels: + app.kubernetes.io/name: {{ .Values.global.entry_name }} + app.kubernetes.io/managed-by: dokku + {{- range $k, $v := .Values.global.labels }} + {{ $k }}: {{ $v | quote }} + {{- end }} + annotations: + dokku.com/managed: "true" + {{- range $k, $v := .Values.global.annotations }} + {{ $k }}: {{ $v | quote }} + {{- end }} +spec: + capacity: + storage: {{ .Values.global.size }} + accessModes: + - {{ .Values.global.access_mode | default "ReadWriteOnce" }} + persistentVolumeReclaimPolicy: {{ .Values.global.reclaim_policy | default "Retain" }} + hostPath: + path: {{ .Values.global.host_path }} + storageClassName: "" + claimRef: + namespace: {{ .Values.global.namespace }} + name: {{ .Values.global.entry_name }} +{{- end }} diff --git a/plugins/scheduler-k3s/triggers.go b/plugins/scheduler-k3s/triggers.go index bc5d406a9..ec455a304 100644 --- a/plugins/scheduler-k3s/triggers.go +++ b/plugins/scheduler-k3s/triggers.go @@ -605,10 +605,19 @@ func TriggerSchedulerDeploy(scheduler string, appName string, imageTag string) e }) } + deployMountPairs, err := LoadAppMounts(appName, "deploy") + if err != nil { + return fmt.Errorf("error loading storage app mounts: %w", err) + } + deployVolumes, err := ToProcessVolumes(deployMountPairs) + if err != nil { + return err + } + processVolumes = append(processVolumes, deployVolumes...) + for processType, processCount := range processes { // todo: implement deployment annotations // todo: implement pod annotations - // todo: implement volumes healthchecks, ok := appJSON.Healthchecks[processType] if !ok { @@ -778,7 +787,6 @@ func TriggerSchedulerDeploy(scheduler string, appName string, imageTag string) e for _, cronTask := range cronTasks { // todo: implement deployment annotations // todo: implement pod annotations - // todo: implement volumes suffix := "" for _, cronJob := range cronJobs { if cronJob.Labels["dokku.com/cron-id"] == cronTask.ID { From 1ca961251db5053958ef5de94a640bb1493a3be9 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Wed, 29 Apr 2026 14:51:49 -0400 Subject: [PATCH 05/13] feat: add storage:set, storage:exec, storage:wait, storage:report --global storage:set is the in-place editor for an existing entry; size grows are passed through to a helm upgrade and Kubernetes-incompatible swaps (access-mode, storage-class) are rejected up front. storage:exec spawns a throwaway container that mounts the entry: docker run on docker-local entries, kubectl run with a Pod-spec override on k3s, falling back to bash then sh when no command is supplied. storage:wait blocks until a k3s entry's PVC reports Bound via the storage-status trigger, with a docker-local fast path that just checks the host directory. storage:report --global enumerates every registered entry and the apps that mount it, in text or JSON form. --- plugins/storage/Makefile | 2 +- plugins/storage/commands_entries.go | 229 ++++++++++++++++++ plugins/storage/src/commands/commands.go | 5 +- .../storage/src/subcommands/subcommands.go | 51 ++++ plugins/storage/subcommands.go | 16 +- 5 files changed, 300 insertions(+), 3 deletions(-) diff --git a/plugins/storage/Makefile b/plugins/storage/Makefile index 7a6b61e35..520b4e9a6 100644 --- a/plugins/storage/Makefile +++ b/plugins/storage/Makefile @@ -1,5 +1,5 @@ GOARCH ?= amd64 -SUBCOMMANDS = subcommands/default subcommands/create subcommands/destroy subcommands/ensure-directory subcommands/info subcommands/list subcommands/list-entries subcommands/mount subcommands/report subcommands/unmount +SUBCOMMANDS = subcommands/default subcommands/create subcommands/destroy subcommands/ensure-directory subcommands/exec subcommands/info subcommands/list subcommands/list-entries 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 BUILD = commands subcommands triggers PLUGIN_NAME = storage diff --git a/plugins/storage/commands_entries.go b/plugins/storage/commands_entries.go index 4f5c6ff5f..d4d62c1c7 100644 --- a/plugins/storage/commands_entries.go +++ b/plugins/storage/commands_entries.go @@ -179,6 +179,235 @@ func CommandInfo(name string, format string) error { return nil } +// CommandSetInput captures the flags accepted by storage:set. +type CommandSetInput struct { + Name string + Size string + AccessMode string + StorageClass string + Namespace string + Chown string + ReclaimPolicy string + Annotations map[string]string + Labels map[string]string +} + +// CommandSet edits an existing entry's mutable fields and re-fires the +// scheduler-side helm release. Refuses changes Kubernetes can't apply +// in place (access-mode swap, storage-class swap, size shrink). +func CommandSet(input CommandSetInput) error { + if !EntryExists(input.Name) { + return fmt.Errorf("storage entry %q does not exist", input.Name) + } + entry, err := LoadEntry(input.Name) + if err != nil { + return err + } + + if input.AccessMode != "" && input.AccessMode != entry.AccessMode { + return fmt.Errorf("storage:set cannot change access-mode in place; recreate the entry") + } + if input.StorageClass != "" && input.StorageClass != entry.StorageClass { + return fmt.Errorf("storage:set cannot change storage-class-name in place; recreate the entry") + } + if input.Size != "" { + entry.Size = input.Size + } + if input.Namespace != "" { + entry.Namespace = input.Namespace + } + if input.Chown != "" { + entry.Chown = input.Chown + } + if input.ReclaimPolicy != "" { + entry.ReclaimPolicy = input.ReclaimPolicy + } + if input.Annotations != nil { + entry.Annotations = input.Annotations + } + if input.Labels != nil { + entry.Labels = input.Labels + } + + if err := entry.Validate(); err != nil { + return err + } + if err := SaveEntry(entry); err != nil { + return err + } + if entry.Scheduler == SchedulerK3s { + if err := callSchedulerCreateTrigger(entry); err != nil { + return fmt.Errorf("scheduler refused storage:set for %q: %w", entry.Name, err) + } + } + common.LogInfo1(fmt.Sprintf("Storage entry %s updated", entry.Name)) + return nil +} + +// CommandExec spawns a temporary container/Pod that mounts the entry +// and runs the given command (or an interactive shell when args is +// empty). It execs into the underlying CLI (docker or kubectl) so +// stdin/stdout/stderr stream naturally. +func CommandExec(name string, image string, args []string) error { + if !EntryExists(name) { + return fmt.Errorf("storage entry %q does not exist", name) + } + entry, err := LoadEntry(name) + if err != nil { + return err + } + if image == "" { + image = "alpine:3" + } + + cmd := args + if len(cmd) == 0 { + cmd = []string{"sh", "-c", "command -v bash >/dev/null 2>&1 && exec bash || exec sh"} + } + + if entry.Scheduler == SchedulerDockerLocal { + dockerArgs := []string{"run", "--rm", "-it", "-v", fmt.Sprintf("%s:/data", entry.HostPath), image} + dockerArgs = append(dockerArgs, cmd...) + return execStream(common.DockerBin(), dockerArgs) + } + + // k3s: render a Pod-spec override that mounts the PVC, then kubectl run. + namespace := entry.Namespace + if namespace == "" { + namespace = "default" + } + overrides := map[string]any{ + "apiVersion": "v1", + "spec": map[string]any{ + "containers": []map[string]any{ + { + "name": "exec", + "image": image, + "stdin": true, + "tty": true, + "volumeMounts": []map[string]any{{"name": "data", "mountPath": "/data"}}, + }, + }, + "volumes": []map[string]any{ + {"name": "data", "persistentVolumeClaim": map[string]any{"claimName": entry.Name}}, + }, + }, + } + overrideJSON, err := json.Marshal(overrides) + if err != nil { + return err + } + kubectlArgs := []string{ + "run", "dokku-storage-exec-" + entry.Name, + "--rm", "-it", + "--namespace=" + namespace, + "--image=" + image, + "--restart=Never", + "--overrides=" + string(overrideJSON), + "--", + } + kubectlArgs = append(kubectlArgs, cmd...) + return execStream("kubectl", kubectlArgs) +} + +func execStream(bin string, args []string) error { + result, err := common.CallExecCommand(common.ExecCommandInput{ + Command: bin, + Args: args, + StreamStdio: true, + }) + if err != nil { + return err + } + if result.ExitCode != 0 { + return fmt.Errorf("%s exited with %d", bin, result.ExitCode) + } + return nil +} + +// CommandWait blocks until a k3s storage entry's PVC is bound. +func CommandWait(name string) error { + if !EntryExists(name) { + return fmt.Errorf("storage entry %q does not exist", name) + } + entry, err := LoadEntry(name) + if err != nil { + return err + } + if entry.Scheduler != SchedulerK3s { + // Docker-local entries are immediately "ready" once the directory exists. + if entry.HostPath != "" { + info, statErr := os.Stat(entry.HostPath) + if statErr != nil || !info.IsDir() { + return fmt.Errorf("storage entry %q host path %s is not present", name, entry.HostPath) + } + } + return nil + } + + data, err := json.Marshal(entry) + if err != nil { + return err + } + results, err := common.CallPlugnTrigger(common.PlugnTriggerInput{ + Trigger: "storage-status", + Args: []string{entry.Name}, + Stdin: strings.NewReader(string(data)), + }) + if err != nil { + return err + } + status := strings.TrimSpace(results.StdoutContents()) + if status != "Bound" { + return fmt.Errorf("storage entry %q is %s, not Bound", name, status) + } + common.LogInfo1(fmt.Sprintf("Storage entry %s is bound", name)) + return nil +} + +// CommandReportGlobal prints a global storage report listing every entry +// and the apps that mount it. Output is text by default; pass +// format="json" for machine-readable output. +func CommandReportGlobal(format string) error { + entries, err := ListEntries() + if err != nil { + return err + } + type entryWithUse struct { + Entry *Entry `json:"entry"` + MountedBy []string `json:"mounted_by"` + } + rows := []entryWithUse{} + for _, entry := range entries { + using, err := AppsUsingEntry(entry.Name) + if err != nil { + return err + } + rows = append(rows, entryWithUse{Entry: entry, MountedBy: using}) + } + if format == "json" { + data, err := json.MarshalIndent(rows, "", " ") + if err != nil { + return err + } + fmt.Println(string(data)) + return nil + } + if len(rows) == 0 { + common.LogInfo1Quiet("No storage entries registered") + return nil + } + common.LogInfo1Quiet("Storage report (global):") + for _, row := range rows { + mountedBy := "—" + if len(row.MountedBy) > 0 { + mountedBy = strings.Join(row.MountedBy, ", ") + } + common.LogVerbose(fmt.Sprintf("%s\t%s\tmounted by: %s", row.Entry.Name, row.Entry.Scheduler, mountedBy)) + } + return nil +} + // CommandListEntries lists registered entries (distinct from the legacy // per-app storage:list which lists colon-form mounts). func CommandListEntries(scheduler string, format string) error { diff --git a/plugins/storage/src/commands/commands.go b/plugins/storage/src/commands/commands.go index 0c5ed04a1..68d4edca3 100644 --- a/plugins/storage/src/commands/commands.go +++ b/plugins/storage/src/commands/commands.go @@ -21,12 +21,15 @@ Additional commands:` storage:create [] [flags], Register a named storage entry storage:destroy , Remove a named storage entry (must be unmounted from every app first) storage:ensure-directory [--chown option] , [DEPRECATED] use storage:create instead + storage:exec [-- ...], Run a command (or shell) in a temporary container that mounts the entry storage:info [--format text|json], Show details for one storage entry storage:list [--format text|json], List bind mounts for app's container(s) (host:container) storage:list-entries [--scheduler s] [--format text|json], List registered storage entries storage:mount , Create a new bind mount storage:report [] [], Displays a storage report for one or more apps - storage:unmount , Remove an existing bind mount` + storage:set [flags], Update a storage entry in place + storage:unmount , Remove an existing bind mount + storage:wait , Wait for a storage entry's PVC to be bound (k3s)` ) func main() { diff --git a/plugins/storage/src/subcommands/subcommands.go b/plugins/storage/src/subcommands/subcommands.go index 52d9a46bb..9fc49a81b 100644 --- a/plugins/storage/src/subcommands/subcommands.go +++ b/plugins/storage/src/subcommands/subcommands.go @@ -78,6 +78,57 @@ func main() { args.Parse(os.Args[2:]) appName := args.Arg(0) err = storage.CommandList(appName, *format) + case "set": + args := flag.NewFlagSet("storage:set", flag.ExitOnError) + size := args.String("size", "", "--size: new PVC size (k3s)") + accessMode := args.String("access-mode", "", "--access-mode: existing access mode (must match)") + storageClass := args.String("storage-class-name", "", "--storage-class-name: existing storage class (must match)") + namespace := args.String("namespace", "", "--namespace: new namespace") + chown := args.String("chown", "", "--chown: chown option") + reclaim := args.String("reclaim-policy", "", "--reclaim-policy: PV reclaim policy") + annotations := args.StringSlice("annotation", nil, "--annotation key=value: PVC annotation (repeatable, replaces all)") + labels := args.StringSlice("label", nil, "--label key=value: PVC label (repeatable, replaces all)") + args.Parse(os.Args[2:]) + annotMap, parseErr := parseKVPairs(*annotations) + if parseErr != nil { + err = parseErr + break + } + labelMap, parseErr := parseKVPairs(*labels) + if parseErr != nil { + err = parseErr + break + } + err = storage.CommandSet(storage.CommandSetInput{ + Name: args.Arg(0), + Size: *size, + AccessMode: *accessMode, + StorageClass: *storageClass, + Namespace: *namespace, + Chown: *chown, + ReclaimPolicy: *reclaim, + Annotations: annotMap, + Labels: labelMap, + }) + case "exec": + args := flag.NewFlagSet("storage:exec", flag.ExitOnError) + image := args.String("image", "", "--image: container image to use (default alpine:3)") + args.Parse(os.Args[2:]) + positional := args.Args() + if len(positional) == 0 { + err = fmt.Errorf("storage:exec requires a storage entry name") + break + } + name := positional[0] + var cmd []string + if len(positional) > 1 { + cmd = positional[1:] + } + err = storage.CommandExec(name, *image, cmd) + case "wait": + args := flag.NewFlagSet("storage:wait", flag.ExitOnError) + args.Parse(os.Args[2:]) + err = storage.CommandWait(args.Arg(0)) case "list-entries": args := flag.NewFlagSet("storage:list-entries", flag.ExitOnError) scheduler := args.String("scheduler", "", "--scheduler: filter to a single scheduler") diff --git a/plugins/storage/subcommands.go b/plugins/storage/subcommands.go index 9b3eb5824..922103af9 100644 --- a/plugins/storage/subcommands.go +++ b/plugins/storage/subcommands.go @@ -22,12 +22,15 @@ Additional commands:` storage:create [] [flags], Register a named storage entry storage:destroy , Remove a named storage entry (must be unmounted from every app first) storage:ensure-directory [--chown option] , [DEPRECATED] use storage:create instead + storage:exec [-- ...], Run a command (or shell) in a temporary container that mounts the entry storage:info [--format text|json], Show details for one storage entry storage:list [--format text|json], List bind mounts for app's container(s) (host:container) storage:list-entries [--scheduler s] [--format text|json], List registered storage entries storage:mount , Create a new bind mount storage:report [] [], Displays a storage report for one or more apps - storage:unmount , Remove an existing bind mount` + storage:set [flags], Update a storage entry in place + storage:unmount , Remove an existing bind mount + storage:wait , Wait for a storage entry's PVC to be bound (k3s)` ) // CommandHelp displays help for the storage plugin @@ -266,6 +269,17 @@ func CommandList(appName string, format string) error { // CommandReport displays a storage report for one or more apps func CommandReport(appName string, format string, infoFlag string) error { + if appName == "--global" { + // Global storage report: list every registered entry and its + // attachment count; falls back to the legacy per-app loop when + // no entries exist so existing automation keeps working. + reportFormat := format + if reportFormat == "stdout" { + reportFormat = "text" + } + return CommandReportGlobal(reportFormat) + } + if appName == "" { apps, err := common.DokkuApps() if err != nil { From b601a626f5555000bafd4d5c20cc34e30095d14a Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Wed, 29 Apr 2026 14:54:15 -0400 Subject: [PATCH 06/13] docs: storage entries and migration guide for #6814 Updates persistent-storage.md to lead with the named storage entry workflow while keeping the legacy colon-form documentation intact, adds a Persistent storage section to the k3s scheduler doc, documents the storage-app-mounts, storage-create, storage-destroy, and storage-status triggers in plugin-triggers, and adds an entry to the 0.38.0 migration guide explaining the install-time migration of legacy mounts and the new DNS-1123 name validation. Bats coverage in tests/unit/storage.bats now exercises storage:create / list-entries / destroy, name validation rejections, multi-entry attachment, the destroy-while-mounted error, and the ensure-directory deprecation warning. --- docs/advanced-usage/persistent-storage.md | 33 ++++--- docs/appendices/0.38.0-migration-guide.md | 1 + docs/deployment/schedulers/k3s.md | 21 +++++ docs/development/plugin-triggers.md | 28 ++++++ tests/unit/storage.bats | 104 ++++++++++++++++++++++ 5 files changed, 177 insertions(+), 10 deletions(-) diff --git a/docs/advanced-usage/persistent-storage.md b/docs/advanced-usage/persistent-storage.md index d2289976a..c033e734e 100644 --- a/docs/advanced-usage/persistent-storage.md +++ b/docs/advanced-usage/persistent-storage.md @@ -1,24 +1,37 @@ # Persistent Storage > [!IMPORTANT] -> New as of 0.5.0 +> New as of 0.5.0. Named storage entries new as of 0.38.0. -The preferred method to mount external containers to a Dokku managed container, is to use the Dokku storage plugin. +The preferred method to attach persistent storage to a Dokku-managed container is the Dokku storage plugin. ``` -storage:ensure-directory [--chown option] # Creates a persistent storage directory in the recommended storage path -storage:list [--format text|json] # List bind mounts for app's container(s) (host:container) -storage:mount # Create a new bind mount -storage:report [] [] # Displays a checks report for one or more apps -storage:unmount # Remove an existing bind mount +storage:create [] [flags] # Register a named storage entry +storage:destroy # Remove a named storage entry (must be unmounted from every app first) +storage:ensure-directory [--chown option] # [DEPRECATED] use storage:create instead +storage:exec [-- ...] # Run a command (or shell) in a temporary container that mounts the entry +storage:info [--format text|json] # Show details for one storage entry +storage:list [--format text|json] # List bind mounts for an app's container(s) (legacy host:container view) +storage:list-entries [--scheduler s] [--format text|json] # List registered storage entries +storage:mount --container-dir [flags] # Mount a named entry into an app +storage:mount # [LEGACY] colon-form mount, docker-local only +storage:report [] [] # Display a storage report for one or more apps +storage:report --global # Display a cluster-wide entry inventory +storage:set [flags] # Update a storage entry in place +storage:unmount [--container-dir ] # Remove an attachment +storage:wait # Block until a k3s entry's PVC is bound ``` -> The storage plugin is compatible with storage mounts created with the docker-options. The storage plugin will only list mounts from the deploy/run phase. +A storage entry is the source of truth for the underlying volume - a host directory on docker-local, or a PersistentVolumeClaim on k3s. Multiple apps can mount the same entry, and an attachment carries the per-app details (container path, phases, subpath, readonly, process type). Names are globally unique across the install and must be DNS-1123 labels (lowercase letters, digits, dashes) of 45 characters or less so they can be used verbatim as Helm release and PVC names. + +The legacy `storage:mount :` form continues to work on docker-local. On a k3s app it is rejected; create a named entry with `storage:create --scheduler k3s` and mount it instead. Existing colon-form mounts are migrated automatically the first time the new code runs - they show up as `legacy-` entries in `storage:list-entries`. The storage plugin supports the following mount points: -- explicit paths that exist on the host -- docker volumes +- explicit paths that exist on the host (docker-local) +- docker volumes (docker-local) +- PersistentVolumeClaims provisioned via a StorageClass (k3s) +- hostPath-backed PVs (k3s) ## Usage diff --git a/docs/appendices/0.38.0-migration-guide.md b/docs/appendices/0.38.0-migration-guide.md index dbf297da8..96f0bfbfd 100644 --- a/docs/appendices/0.38.0-migration-guide.md +++ b/docs/appendices/0.38.0-migration-guide.md @@ -22,6 +22,7 @@ - The `docker-local` scheduler no longer queues an image for retirement when another running container of the same app still uses it. This fixes the case where a `ps:rebuild` against an image-based deploy (`git:from-image`) produced an identical-SHA image and the `dokku-retire` cron timer would log `Image ... has running containers, skipping rm` on every run. Stuck entries from prior versions are pruned automatically on the next `ps:retire` run. - All `:report` subcommands now accept the `--global` flag, which scopes the report to globally-configured properties. The flag composes with `--format json`, so a JSON report of global properties can be obtained via, for example, `dokku scheduler:report --global --format json`. Previously, combining `--global` with `--format json` was rejected with an "info flag" error, and `--global` on its own was treated as an unknown flag. - The `scheduler-k3s` plugin now manages env config and the dokku-generated image pull Secret as their own helm releases with stable names (`config-{app}` and `pull-secret-{app}`) rather than bundling them into the app helm chart with a per-deploy timestamp suffix (`env-{app}.{ts}` / `ims-{app}.{ts}`). This fixes two bugs: a helm rollback of the app chart no longer deletes Secrets that older ReplicaSets still reference, and the Deployment's `imagePullSecrets` list no longer accumulates references to nonexistent Secrets across deploys. The next deploy of an app switches the Deployment's `envFrom` and `imagePullSecrets` references to the stable names and prunes any leaked entries; existing live Deployments do not need to be patched manually. App rename now also uninstalls the old `tls-{app}`, `config-{app}`, and `pull-secret-{app}` releases under the previous app name; the new name's releases are recreated on the next deploy or certs sync. +- The storage plugin now treats persistent volumes as named, scheduler-aware first-class resources via `storage:create`, `storage:mount`, `storage:set`, and `storage:destroy`. The legacy `storage:mount :` colon form continues to work on docker-local apps but is deprecated; on k3s apps it is rejected. Existing colon-form mounts are migrated automatically the first time the new storage plugin runs (during the install trigger) - they appear as `legacy-` entries in `storage:list-entries`. The migration is idempotent and tied to a per-app flag file at `$DOKKU_LIB_ROOT/config/storage/.migrated/`; deleting that file forces a re-scan on the next install. The `storage:ensure-directory` command keeps working but now emits a deprecation warning - prefer `storage:create []` (the path defaults to the same `$DOKKU_LIB_ROOT/data/storage/` location). Storage entry names must now be DNS-1123 labels of 45 characters or less so they can be used verbatim as Helm release and Kubernetes resource names; underscores and uppercase characters that the older `ensure-directory` validator accepted are rejected for new names. The migration synthesizer always uses lowercase hex hashes so existing data is never locked out. ### TLS handshake behavior change diff --git a/docs/deployment/schedulers/k3s.md b/docs/deployment/schedulers/k3s.md index 33e55126b..b3fa64432 100644 --- a/docs/deployment/schedulers/k3s.md +++ b/docs/deployment/schedulers/k3s.md @@ -685,6 +685,27 @@ Alternatively, a comma separated list of chart names can be specified to only fo dokku scheduler-k3s:ensure-charts --charts cert-manager ``` +### Persistent storage + +K3s apps mount persistent volumes through the Dokku storage plugin. A `storage:create --scheduler k3s` call provisions a PersistentVolumeClaim through a small per-entry helm release; `storage:mount --container-dir ` then attaches it to an app. Because storage entries are global, the same PVC can be mounted into multiple apps (for `ReadWriteMany` workloads) or into the same app at multiple subpaths. + +```shell +# dynamically-provisioned PVC backed by Longhorn +dokku storage:create demo-data \ + --scheduler k3s \ + --size 2Gi \ + --access-mode ReadWriteOnce \ + --storage-class-name longhorn + +dokku storage:mount demo demo-data --container-dir /data +dokku storage:wait demo-data +git push dokku master +``` + +For a hostPath-backed PV (no StorageClass), pass `` 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 on `storage:create` / `storage:set` propagate to both the PVC and the PV so backup tools (Velero, Longhorn snapshots) can find them. + +The legacy `storage:mount :` 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 Some chart version transitions require side-effects that a plain `helm upgrade` cannot perform. For example, the upgrade of `keda-add-ons-http` to `0.12.2` introduces breaking changes to deployment selectors, which are immutable in Kubernetes; the chart-managed deployments must be deleted before the upgrade can proceed. diff --git a/docs/development/plugin-triggers.md b/docs/development/plugin-triggers.md index 7fd0f7f29..c0dae2518 100644 --- a/docs/development/plugin-triggers.md +++ b/docs/development/plugin-triggers.md @@ -2920,6 +2920,34 @@ APP="$1" # TODO ``` +### `storage-app-mounts` + +- Description: Returns the (entry, attachment) pairs an app has for the given phase, in JSON. Schedulers consume this to render volume mounts. +- Invoked by: scheduler plugins during deploy +- Arguments: `$APP $PHASE` +- Output: a JSON array of objects shaped `{"entry": Entry, "attachment": Attachment}` + +### `storage-create` + +- Description: Provisions or updates the underlying volume for a k3s storage entry. Called when `storage:create` or `storage:set` runs against a k3s entry. Receives the JSON-encoded entry on stdin. +- Invoked by: `dokku storage:create`, `dokku storage:set` +- Arguments: `$ENTRY_NAME` +- Stdin: JSON-encoded `Entry` payload + +### `storage-destroy` + +- Description: Releases the underlying volume for a k3s storage entry. Called when `storage:destroy` runs against a k3s entry. Receives the JSON-encoded entry on stdin. +- Invoked by: `dokku storage:destroy` +- Arguments: `$ENTRY_NAME` +- Stdin: JSON-encoded `Entry` payload + +### `storage-status` + +- Description: Returns the current status of a storage entry's underlying volume. Used by `storage:wait` to poll the cluster. The handler should print one line: `Bound`, `Pending`, or `Lost` for k3s entries. +- Invoked by: `dokku storage:wait` +- Arguments: `$ENTRY_NAME` +- Stdin: JSON-encoded `Entry` payload + ### `traefik-template-source` - Description: Retrieves an alternative template for the traefik compose config diff --git a/tests/unit/storage.bats b/tests/unit/storage.bats index 9687d0e73..7c4b3ecc4 100644 --- a/tests/unit/storage.bats +++ b/tests/unit/storage.bats @@ -183,3 +183,107 @@ teardown() { assert_success assert_output "mount_volume:/mount" } + +@test "(storage) storage:create / storage:list-entries / storage:destroy" { + run /bin/bash -c "dokku storage:create rdmtest-entry" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-entry$'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "rdmtest-entry" + + run /bin/bash -c "dokku storage:info rdmtest-entry --format json | jq -r '.scheduler'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "docker-local" + + run /bin/bash -c "dokku storage:destroy rdmtest-entry" + echo "output: $output" + echo "status: $status" + assert_success +} + +@test "(storage) storage:create rejects invalid names" { + # underscore: rejected + run /bin/bash -c "dokku storage:create rdmtest_invalid" + echo "output: $output" + echo "status: $status" + assert_failure + + # uppercase: rejected + run /bin/bash -c "dokku storage:create RdmTest" + echo "output: $output" + echo "status: $status" + assert_failure + + # 46 chars: too long + long_name=$(printf 'a%.0s' {1..46}) + run /bin/bash -c "dokku storage:create $long_name" + echo "output: $output" + echo "status: $status" + assert_failure + + # legacy- prefix: reserved + run /bin/bash -c "dokku storage:create legacy-foo" + echo "output: $output" + echo "status: $status" + assert_failure +} + +@test "(storage) storage:create + storage:mount with named entry attaches multiple entries to one app" { + 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 /data" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:mount $TEST_APP rdmtest-cache --container-dir /cache" + echo "output: $output" + echo "status: $status" + assert_success + + # cleanup + run /bin/bash -c "dokku storage:unmount $TEST_APP rdmtest-data" + assert_success + run /bin/bash -c "dokku storage:unmount $TEST_APP rdmtest-cache" + assert_success + run /bin/bash -c "dokku storage:destroy rdmtest-data" + assert_success + run /bin/bash -c "dokku storage:destroy rdmtest-cache" + assert_success +} + +@test "(storage) storage:destroy refuses to remove a still-mounted entry" { + run /bin/bash -c "dokku storage:create rdmtest-busy" + assert_success + run /bin/bash -c "dokku storage:mount $TEST_APP rdmtest-busy --container-dir /data" + assert_success + + run /bin/bash -c "dokku storage:destroy rdmtest-busy" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "still mounted" + + run /bin/bash -c "dokku storage:unmount $TEST_APP rdmtest-busy" + assert_success + run /bin/bash -c "dokku storage:destroy rdmtest-busy" + assert_success +} + +@test "(storage) storage:ensure-directory emits deprecation warning" { + run /bin/bash -c "dokku storage:ensure-directory $TEST_APP 2>&1" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "deprecated" +} From 89afd09087a01d6d9200ceedc785ac139c4d39f4 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Wed, 29 Apr 2026 22:36:00 -0400 Subject: [PATCH 07/13] fix: storage plugin install setup and post-delete permissions apps:destroy was failing in CI because the post-delete trigger called PropertyListWrite, which tried to mkdir /var/lib/dokku/config/storage/ as the dokku user but the parent directory had been created by the install trigger as root. The install trigger now calls PropertySetup("storage") so the per-plugin config root has the correct ownership, and post-delete / post-app-clone / post-app-rename short-circuit when the app has no attachments to begin with so they can't fail on a never-touched property tree. The entry registry and migration flag directories also moved out of config/storage (where they would have collided with the property-store path config/storage/) into a dedicated data/storage-registry tree. --- plugins/storage/entry.go | 13 +++++++++++-- plugins/storage/entry_test.go | 2 +- plugins/storage/migrate.go | 7 ++++--- plugins/storage/triggers.go | 18 +++++++++++++++++- 4 files changed, 33 insertions(+), 7 deletions(-) diff --git a/plugins/storage/entry.go b/plugins/storage/entry.go index 68ef5d79f..3fae69d66 100644 --- a/plugins/storage/entry.go +++ b/plugins/storage/entry.go @@ -76,10 +76,19 @@ type Entry struct { SchemaVersion int `json:"schema_version"` } +// RegistryDirectory returns the parent directory for storage-plugin +// state that lives outside the property store. Kept under data/ rather +// than config/ so it doesn't collide with the property-list paths +// `config/storage//` that the rest of the plugin +// relies on for per-app attachments. +func RegistryDirectory() string { + root := common.GetenvWithDefault("DOKKU_LIB_ROOT", "/var/lib/dokku") + return filepath.Join(root, "data", "storage-registry") +} + // EntriesDirectory returns the directory holding per-entry JSON files. func EntriesDirectory() string { - root := common.GetenvWithDefault("DOKKU_LIB_ROOT", "/var/lib/dokku") - return filepath.Join(root, "config", "storage", "entries") + return filepath.Join(RegistryDirectory(), "entries") } // entryPath returns the on-disk path for a named entry. diff --git a/plugins/storage/entry_test.go b/plugins/storage/entry_test.go index 683032008..804245b1f 100644 --- a/plugins/storage/entry_test.go +++ b/plugins/storage/entry_test.go @@ -138,7 +138,7 @@ func TestEntryRoundTrip(t *testing.T) { Expect(loaded.Labels).To(Equal(original.Labels)) Expect(loaded.SchemaVersion).To(Equal(SchemaVersion)) - expectedPath := filepath.Join(root, "config", "storage", "entries", "demo-data.json") + expectedPath := filepath.Join(root, "data", "storage-registry", "entries", "demo-data.json") Expect(expectedPath).To(BeARegularFile()) Expect(DeleteEntry("demo-data")).To(Succeed()) diff --git a/plugins/storage/migrate.go b/plugins/storage/migrate.go index 088ad4d83..3dba56eb0 100644 --- a/plugins/storage/migrate.go +++ b/plugins/storage/migrate.go @@ -14,10 +14,11 @@ import ( // 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. +// drained into the attachment store. Lives under the registry directory +// so it doesn't collide with the property-store path +// `config/storage/`. func migrationFlagDir() string { - root := common.GetenvWithDefault("DOKKU_LIB_ROOT", "/var/lib/dokku") - return filepath.Join(root, "config", "storage", ".migrated") + return filepath.Join(RegistryDirectory(), "migrations") } func migrationFlagFile(appName string) string { diff --git a/plugins/storage/triggers.go b/plugins/storage/triggers.go index 389783e42..c8f8b30a6 100644 --- a/plugins/storage/triggers.go +++ b/plugins/storage/triggers.go @@ -14,6 +14,10 @@ import ( // TriggerInstall sets up the storage plugin on installation and runs // the bulk legacy-mount migration once per upgrade. func TriggerInstall() error { + if err := common.PropertySetup(PluginName); err != nil { + return fmt.Errorf("Unable to install the storage plugin: %s", err.Error()) + } + storageDir := GetStorageDirectory() if err := os.MkdirAll(storageDir, 0755); err != nil { @@ -55,11 +59,17 @@ func TriggerInstall() error { } // TriggerPostDelete clears any attachment list for an app that's being -// destroyed. Entries are global and survive app deletion. +// destroyed. Entries are global and survive app deletion. Skipped when +// the app has no attachments to begin with so we don't trigger a +// pointless propertyTouch that could fail on installs with no +// property-list state for this plugin yet. func TriggerPostDelete(appName string) error { if appName == "" { return nil } + if !common.PropertyExists(PluginName, appName, AttachmentsProperty) { + return nil + } return common.PropertyListWrite(PluginName, appName, AttachmentsProperty, []string{}) } @@ -69,6 +79,9 @@ func TriggerPostAppCloneSetup(oldName string, newName string) error { if oldName == "" || newName == "" { return nil } + if !common.PropertyExists(PluginName, oldName, AttachmentsProperty) { + return nil + } attachments, err := LoadAttachments(oldName) if err != nil { return err @@ -82,6 +95,9 @@ func TriggerPostAppRenameSetup(oldName string, newName string) error { if oldName == "" || newName == "" { return nil } + if !common.PropertyExists(PluginName, oldName, AttachmentsProperty) { + return nil + } attachments, err := LoadAttachments(oldName) if err != nil { return err From 6a5d56894426a9513d96a30fc5d27ad4b9364b53 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Wed, 29 Apr 2026 22:39:12 -0400 Subject: [PATCH 08/13] fix: remove the per-app property folder on app delete and rename Switches the storage plugin's post-delete and post-app-rename-setup triggers from rewriting the attachment list as empty to removing the per-app property folder outright via PropertyDestroy. This matches the convention other plugins use on app deletion and avoids leaving an empty mounts file behind for an app that no longer exists. --- plugins/storage/triggers.go | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/plugins/storage/triggers.go b/plugins/storage/triggers.go index c8f8b30a6..1d5b00747 100644 --- a/plugins/storage/triggers.go +++ b/plugins/storage/triggers.go @@ -58,19 +58,15 @@ func TriggerInstall() error { return nil } -// TriggerPostDelete clears any attachment list for an app that's being -// destroyed. Entries are global and survive app deletion. Skipped when -// the app has no attachments to begin with so we don't trigger a -// pointless propertyTouch that could fail on installs with no -// property-list state for this plugin yet. +// TriggerPostDelete removes the attachment store for an app that's being +// destroyed. Entries are global and survive app deletion. The whole +// per-app property folder goes away rather than being rewritten as +// empty, matching what other plugins do on app deletion. func TriggerPostDelete(appName string) error { if appName == "" { return nil } - if !common.PropertyExists(PluginName, appName, AttachmentsProperty) { - return nil - } - return common.PropertyListWrite(PluginName, appName, AttachmentsProperty, []string{}) + return common.PropertyDestroy(PluginName, appName) } // TriggerPostAppCloneSetup copies attachments from the source app to the @@ -90,7 +86,7 @@ func TriggerPostAppCloneSetup(oldName string, newName string) error { } // TriggerPostAppRenameSetup moves attachments from the old name to the -// new name and clears the old. +// new name and removes the old per-app property folder. func TriggerPostAppRenameSetup(oldName string, newName string) error { if oldName == "" || newName == "" { return nil @@ -105,7 +101,7 @@ func TriggerPostAppRenameSetup(oldName string, newName string) error { if err := SaveAttachments(newName, attachments); err != nil { return err } - return common.PropertyListWrite(PluginName, oldName, AttachmentsProperty, []string{}) + return common.PropertyDestroy(PluginName, oldName) } // detectDistro returns the Linux distribution name From d75228f165cf2870310478740f08167bababbc2c Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Wed, 29 Apr 2026 22:51:50 -0400 Subject: [PATCH 09/13] fix: storage:list reads from attachments, deprecate storage-list trigger storage:list was calling the storage-list plugn trigger which read -v lines from docker-options. After the install-time migration drains those lines into the attachment store, that source is empty for every migrated app and for any app that only ever used storage:create + storage:mount. The fix moves CommandList to call a new in-process ListAppMountEntries helper that reads attachments directly, surfaces the entry name in JSON output via a new entry_name field, and falls back to the entry name as the host token for k3s entries with no host path so the colon form remains well-formed. The storage-list plugn trigger is kept for back-compat with external callers but emits a deprecation warning and now reads from the same attachment-driven source. --- docs/development/plugin-triggers.md | 22 ++--- plugins/storage/attachment_test.go | 128 ++++++++++++++++++++++++++++ plugins/storage/storage.go | 79 ++++++++++++++--- plugins/storage/subcommands.go | 42 +++++---- plugins/storage/triggers.go | 27 +++--- tests/unit/storage.bats | 19 +++++ 6 files changed, 258 insertions(+), 59 deletions(-) diff --git a/docs/development/plugin-triggers.md b/docs/development/plugin-triggers.md index c0dae2518..e2d8ce464 100644 --- a/docs/development/plugin-triggers.md +++ b/docs/development/plugin-triggers.md @@ -2904,21 +2904,17 @@ DOKKU_SCHEDULER="$1"; APP="$2"; REMOVE_CONTAINERS="$3"; # TODO ``` -### `storage-list` +### `storage-list` (deprecated) -- Description: Returns a list of storage mounts -- Invoked by: `dokku storage:list` and `dokku deploy` +> [!WARNING] +> Deprecated as of 0.38.0. The trigger handler still functions for back-compat +> with external plugins but emits a deprecation warning on every invocation. +> In-process callers should use the `storage-app-mounts` trigger instead, or +> the `storage` Go package directly. + +- Description: Returns a list of storage mounts for an app +- Invoked by: external plugins (the core `storage:list` subcommand no longer uses it) - Arguments: `$APP $PHASE $FORMAT` -- Example: - -```shell -#!/usr/bin/env bash - -set -eo pipefail; [[ $DOKKU_TRACE ]] && set -x -APP="$1" - -# TODO -``` ### `storage-app-mounts` diff --git a/plugins/storage/attachment_test.go b/plugins/storage/attachment_test.go index d6be3e31b..6c698ff77 100644 --- a/plugins/storage/attachment_test.go +++ b/plugins/storage/attachment_test.go @@ -1,11 +1,39 @@ package storage import ( + "encoding/json" + "os" + "path/filepath" "testing" . "github.com/onsi/gomega" ) +// writeAttachmentsFile drops the JSON-encoded attachment list into the +// per-app property file directly, bypassing common.PropertyListWrite (which +// calls SetPermissions and chowns to dokku:dokku, which a developer machine +// or CI runner generally doesn't have). +func writeAttachmentsFile(t *testing.T, root string, appName string, attachments []*Attachment) { + t.Helper() + dir := filepath.Join(root, "config", "storage", appName) + if err := os.MkdirAll(dir, 0755); err != nil { + t.Fatalf("mkdir attachment dir: %v", err) + } + path := filepath.Join(dir, AttachmentsProperty) + f, err := os.Create(path) + if err != nil { + t.Fatalf("create attachment file: %v", err) + } + defer f.Close() + for _, a := range attachments { + data, err := json.Marshal(a) + if err != nil { + t.Fatalf("marshal attachment: %v", err) + } + f.Write(append(data, '\n')) + } +} + func TestAttachmentValidate(t *testing.T) { RegisterTestingT(t) @@ -31,3 +59,103 @@ func TestAttachmentValidate(t *testing.T) { badPhase := &Attachment{EntryName: "demo-data", ContainerPath: "/data", Phases: []string{"build"}} Expect(badPhase.Validate()).To(HaveOccurred()) } + +func TestListAppMountEntriesDockerLocal(t *testing.T) { + RegisterTestingT(t) + root := withTempLibRoot(t) + + Expect(SaveEntry(&Entry{ + Name: "demo-data", + Scheduler: SchedulerDockerLocal, + HostPath: "/var/lib/dokku/data/storage/demo-data", + })).To(Succeed()) + + writeAttachmentsFile(t, root, "demo", []*Attachment{ + { + EntryName: "demo-data", + ContainerPath: "/data", + Phases: []string{PhaseDeploy, PhaseRun}, + Readonly: true, + }, + }) + + rows, err := ListAppMountEntries("demo", PhaseDeploy) + Expect(err).NotTo(HaveOccurred()) + Expect(rows).To(HaveLen(1)) + Expect(rows[0].EntryName).To(Equal("demo-data")) + Expect(rows[0].HostPath).To(Equal("/var/lib/dokku/data/storage/demo-data")) + Expect(rows[0].ContainerPath).To(Equal("/data")) + Expect(rows[0].VolumeOptions).To(Equal("ro")) + + // Run phase shows it too because the attachment includes both phases. + runRows, err := ListAppMountEntries("demo", PhaseRun) + Expect(err).NotTo(HaveOccurred()) + Expect(runRows).To(HaveLen(1)) + + // formatStorageListEntry produces the legacy colon form including options. + Expect(formatStorageListEntry(rows[0])).To(Equal("/var/lib/dokku/data/storage/demo-data:/data:ro")) +} + +func TestListAppMountEntriesK3sUsesEntryName(t *testing.T) { + RegisterTestingT(t) + root := withTempLibRoot(t) + + // k3s entry has no host path because the cluster provisions it. + Expect(SaveEntry(&Entry{ + Name: "demo-pvc", + Scheduler: SchedulerK3s, + Size: "2Gi", + StorageClass: "longhorn", + AccessMode: "ReadWriteOnce", + })).To(Succeed()) + + writeAttachmentsFile(t, root, "demo", []*Attachment{ + { + EntryName: "demo-pvc", + ContainerPath: "/data", + Phases: []string{PhaseDeploy}, + Subpath: "uploads", + }, + }) + + rows, err := ListAppMountEntries("demo", PhaseDeploy) + Expect(err).NotTo(HaveOccurred()) + Expect(rows).To(HaveLen(1)) + // HostPath falls back to the entry name so the colon form remains + // well-formed for callers that only know about the legacy shape. + Expect(rows[0].HostPath).To(Equal("demo-pvc")) + Expect(rows[0].EntryName).To(Equal("demo-pvc")) + Expect(rows[0].ContainerPath).To(Equal("/data")) + Expect(formatStorageListEntry(rows[0])).To(Equal("demo-pvc:/data")) +} + +func TestListAppMountEntriesPhaseFilter(t *testing.T) { + RegisterTestingT(t) + root := withTempLibRoot(t) + + Expect(SaveEntry(&Entry{ + Name: "demo-deploy-only", + Scheduler: SchedulerDockerLocal, + HostPath: "/srv/deploy", + })).To(Succeed()) + Expect(SaveEntry(&Entry{ + Name: "demo-run-only", + Scheduler: SchedulerDockerLocal, + HostPath: "/srv/run", + })).To(Succeed()) + + writeAttachmentsFile(t, root, "demo", []*Attachment{ + {EntryName: "demo-deploy-only", ContainerPath: "/d", Phases: []string{PhaseDeploy}}, + {EntryName: "demo-run-only", ContainerPath: "/r", Phases: []string{PhaseRun}}, + }) + + deployRows, err := ListAppMountEntries("demo", PhaseDeploy) + Expect(err).NotTo(HaveOccurred()) + Expect(deployRows).To(HaveLen(1)) + Expect(deployRows[0].EntryName).To(Equal("demo-deploy-only")) + + runRows, err := ListAppMountEntries("demo", PhaseRun) + Expect(err).NotTo(HaveOccurred()) + Expect(runRows).To(HaveLen(1)) + Expect(runRows[0].EntryName).To(Equal("demo-run-only")) +} diff --git a/plugins/storage/storage.go b/plugins/storage/storage.go index 386cec11d..3054688be 100644 --- a/plugins/storage/storage.go +++ b/plugins/storage/storage.go @@ -51,19 +51,18 @@ func CheckIfPathExists(appName string, mountPath string, phases []string) bool { return false } -// GetBindMounts returns the bind mounts for an app and phase +// GetBindMounts returns the bind mounts for an app and phase, synthesized +// from the attachment store. The returned strings use the legacy colon +// form (host:container[:options]) so existing display paths keep +// working unchanged. func GetBindMounts(appName string, phase string) ([]string, error) { - mounts := []string{} - options, err := dockeroptions.GetDockerOptionsForPhase(appName, phase) + entries, err := ListAppMountEntries(appName, phase) if err != nil { - return mounts, err + return nil, err } - - for _, option := range options { - if strings.HasPrefix(option, "-v ") { - mount := strings.TrimPrefix(option, "-v ") - mounts = append(mounts, mount) - } + mounts := make([]string, 0, len(entries)) + for _, entry := range entries { + mounts = append(mounts, formatStorageListEntry(entry)) } return mounts, nil } @@ -82,13 +81,71 @@ func GetBindMountsForDisplay(appName string, phase string) string { return strings.Join(result, " ") } -// StorageListEntry represents a storage mount entry for JSON output +// StorageListEntry represents a storage mount entry for JSON output. type StorageListEntry struct { + EntryName string `json:"entry_name,omitempty"` HostPath string `json:"host_path"` ContainerPath string `json:"container_path"` VolumeOptions string `json:"volume_options"` } +// ListAppMountEntries returns one StorageListEntry per attachment on +// an app for the requested phase, joining each attachment with its +// referenced storage entry. Used by storage:list and the deprecated +// storage-list trigger. +func ListAppMountEntries(appName string, phase string) ([]StorageListEntry, error) { + if phase == "" { + phase = PhaseDeploy + } + attachments, err := AttachmentsForPhase(appName, phase) + if err != nil { + return nil, err + } + + rows := make([]StorageListEntry, 0, len(attachments)) + for _, attachment := range attachments { + entry, err := LoadEntry(attachment.EntryName) + if err != nil { + return nil, fmt.Errorf("attachment on %q references missing entry %q: %w", appName, attachment.EntryName, err) + } + + host := entry.HostPath + if host == "" { + // k3s-only entries with no host path: surface the entry + // name as the host token so the colon-form output is + // well-formed and parseable. + host = entry.Name + } + + options := "" + switch { + case attachment.Readonly && attachment.VolumeOptions != "": + options = "ro," + attachment.VolumeOptions + case attachment.Readonly: + options = "ro" + case attachment.VolumeOptions != "": + options = attachment.VolumeOptions + } + + rows = append(rows, StorageListEntry{ + EntryName: entry.Name, + HostPath: host, + ContainerPath: attachment.ContainerPath, + VolumeOptions: options, + }) + } + return rows, nil +} + +// formatStorageListEntry renders a StorageListEntry into the legacy +// host:container[:options] colon form for textual output. +func formatStorageListEntry(entry StorageListEntry) string { + if entry.VolumeOptions == "" { + return fmt.Sprintf("%s:%s", entry.HostPath, entry.ContainerPath) + } + return fmt.Sprintf("%s:%s:%s", entry.HostPath, entry.ContainerPath, entry.VolumeOptions) +} + // ParseMountPath parses a mount path into its components func ParseMountPath(mountPath string) StorageListEntry { parts := strings.SplitN(mountPath, ":", 3) diff --git a/plugins/storage/subcommands.go b/plugins/storage/subcommands.go index 922103af9..96b76800e 100644 --- a/plugins/storage/subcommands.go +++ b/plugins/storage/subcommands.go @@ -1,6 +1,7 @@ package storage import ( + "encoding/json" "errors" "fmt" "os" @@ -226,7 +227,9 @@ func CommandUnmount(input CommandUnmountInput) error { return RemoveAttachment(input.AppName, input.NameOrPath, input.ContainerDir) } -// CommandList lists all bind mounts for an app +// CommandList lists all bind mounts for an app. Reads attachments +// directly from the storage plugin's own state rather than going through +// the deprecated `storage-list` plugn trigger. func CommandList(appName string, format string) error { if err := common.VerifyAppName(appName); err != nil { return err @@ -236,34 +239,29 @@ func CommandList(appName string, format string) error { return errors.New("Invalid --format value specified") } - if format == "text" { - common.LogInfo1Quiet(fmt.Sprintf("%s volume bind-mounts:", appName)) - } - - results, err := common.CallPlugnTrigger(common.PlugnTriggerInput{ - Trigger: "storage-list", - Args: []string{appName, "deploy", format}, - }) + rows, err := ListAppMountEntries(appName, PhaseDeploy) if err != nil { return err } - output := results.StdoutContents() - if format == "text" && output != "" { - lines := strings.Split(strings.TrimSpace(output), "\n") - for _, line := range lines { - if line != "" { - if os.Getenv("DOKKU_QUIET_OUTPUT") != "" { - fmt.Println(line) - } else { - common.LogVerbose(line) - } - } + if format == "json" { + output, err := json.Marshal(rows) + if err != nil { + return err } - } else if output != "" { - fmt.Println(output) + fmt.Println(string(output)) + return nil } + common.LogInfo1Quiet(fmt.Sprintf("%s volume bind-mounts:", appName)) + for _, row := range rows { + line := formatStorageListEntry(row) + if os.Getenv("DOKKU_QUIET_OUTPUT") != "" { + fmt.Println(line) + } else { + common.LogVerbose(line) + } + } return nil } diff --git a/plugins/storage/triggers.go b/plugins/storage/triggers.go index 1d5b00747..1d6cf9279 100644 --- a/plugins/storage/triggers.go +++ b/plugins/storage/triggers.go @@ -125,30 +125,31 @@ func detectDistro() string { return "" } -// TriggerStorageList outputs storage mounts for an app +// TriggerStorageList outputs storage mounts for an app. +// +// Deprecated: the storage-list plugn trigger is retained for back-compat +// with external plugins that may still call it, but in-process callers +// should use storage.ListAppMountEntries directly. A deprecation warning +// is emitted on every invocation. func TriggerStorageList(appName string, phase string, format string) error { - mounts, err := GetBindMounts(appName, phase) + common.LogWarn("the storage-list plugn trigger is deprecated; use the storage-app-mounts trigger or the storage Go package directly") + + rows, err := ListAppMountEntries(appName, phase) if err != nil { return err } if format == "json" { - entries := []StorageListEntry{} - for _, mount := range mounts { - entries = append(entries, ParseMountPath(mount)) - } - - output, err := json.Marshal(entries) + output, err := json.Marshal(rows) if err != nil { return err } fmt.Println(string(output)) - } else { - for _, mount := range mounts { - fmt.Println(mount) - } + return nil + } + for _, row := range rows { + fmt.Println(formatStorageListEntry(row)) } - return nil } diff --git a/tests/unit/storage.bats b/tests/unit/storage.bats index 7c4b3ecc4..eed154ab3 100644 --- a/tests/unit/storage.bats +++ b/tests/unit/storage.bats @@ -251,11 +251,30 @@ teardown() { echo "status: $status" assert_success + run /bin/bash -c "dokku storage:list $TEST_APP --format json | jq -r '. | length'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "2" + + run /bin/bash -c "dokku storage:list $TEST_APP --format json | jq -r '.[].entry_name' | sort | xargs" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "rdmtest-cache rdmtest-data" + # cleanup run /bin/bash -c "dokku storage:unmount $TEST_APP rdmtest-data" assert_success run /bin/bash -c "dokku storage:unmount $TEST_APP rdmtest-cache" assert_success + + run /bin/bash -c "dokku storage:list $TEST_APP --format json | jq -r '. | length'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "0" + run /bin/bash -c "dokku storage:destroy rdmtest-data" assert_success run /bin/bash -c "dokku storage:destroy rdmtest-cache" From 58042b9330f90a3de0b96ccee190f9ff5159b993 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Wed, 29 Apr 2026 23:29:30 -0400 Subject: [PATCH 10/13] feat: delegate storage:exec to scheduler plugins, harden the flow Moves the actual exec out of the storage plugin and into a new scheduler-storage-exec plugn trigger. scheduler-docker-local does docker run with TTY-aware -it/-i selection and --user derived from entry.Chown; scheduler-k3s creates a throwaway Pod via the kubernetes API, waits for it to reach Running with structured error reporting (ImagePullBackOff and friends are surfaced from the container status verbatim, no kubectl involvement), execs the user command via the existing SPDY plumbing in k8s.go, and deletes the Pod on the way out. (Entry).Validate now accepts either an absolute path or a docker named-volume token for docker-local entries so the migration synthesizer's named-volume legacy entries work cleanly. storage:exec gains --as-user for one-off uid overrides, propagates the underlying tool's exit code via os.Exit, and detects TTY/interactive mode from os.Stdin so non-interactive scripted use no longer trips over docker's input-device-is-not-a-tty error. --- docs/development/plugin-triggers.md | 7 + plugins/scheduler-docker-local/Makefile | 2 +- plugins/scheduler-docker-local/go.mod | 7 + plugins/scheduler-docker-local/go.sum | 2 + .../src/triggers/triggers.go | 28 ++- .../scheduler-docker-local/storage_exec.go | 160 ++++++++++++ .../storage_exec_test.go | 118 +++++++++ plugins/scheduler-k3s/Makefile | 2 +- .../scheduler-k3s/src/triggers/triggers.go | 24 ++ plugins/scheduler-k3s/storage_exec.go | 236 ++++++++++++++++++ plugins/scheduler-k3s/storage_exec_test.go | 103 ++++++++ plugins/storage/commands_entries.go | 119 +++++---- plugins/storage/entry.go | 8 +- plugins/storage/entry_test.go | 9 + .../storage/src/subcommands/subcommands.go | 8 +- plugins/storage/subcommands.go | 6 +- tests/unit/storage.bats | 23 ++ 17 files changed, 790 insertions(+), 72 deletions(-) create mode 100644 plugins/scheduler-docker-local/storage_exec.go create mode 100644 plugins/scheduler-docker-local/storage_exec_test.go create mode 100644 plugins/scheduler-k3s/storage_exec.go create mode 100644 plugins/scheduler-k3s/storage_exec_test.go diff --git a/docs/development/plugin-triggers.md b/docs/development/plugin-triggers.md index e2d8ce464..dc2399ace 100644 --- a/docs/development/plugin-triggers.md +++ b/docs/development/plugin-triggers.md @@ -2944,6 +2944,13 @@ DOKKU_SCHEDULER="$1"; APP="$2"; REMOVE_CONTAINERS="$3"; - Arguments: `$ENTRY_NAME` - Stdin: JSON-encoded `Entry` payload +### `scheduler-storage-exec` + +- Description: Runs an interactive or non-interactive command against a storage entry. The storage plugin fires this with `` as the first arg; each scheduler plugin's handler matches against its own scheduler name and either handles or no-ops, mirroring `scheduler-deploy` / `scheduler-app-status`. Plugn forwards stdin/stdout/stderr to the handler subprocess so an interactive shell streams cleanly. The handler exits with the underlying tool's status code so `dokku storage:exec` propagates exit codes verbatim. +- Invoked by: `dokku storage:exec` +- Arguments: `$SCHEDULER $ENTRY_NAME $IMAGE [-- $cmd...]` +- Flags: `--interactive` (stdin is open), `--tty` (stdin is a terminal), `--as-user ` (override `entry.Chown`). + ### `traefik-template-source` - Description: Retrieves an alternative template for the traefik compose config diff --git a/plugins/scheduler-docker-local/Makefile b/plugins/scheduler-docker-local/Makefile index bd7c75ae1..e781a2d7f 100644 --- a/plugins/scheduler-docker-local/Makefile +++ b/plugins/scheduler-docker-local/Makefile @@ -1,4 +1,4 @@ -TRIGGERS = triggers/scheduler-cron-write +TRIGGERS = triggers/scheduler-cron-write triggers/scheduler-storage-exec BUILD = triggers PLUGIN_NAME = scheduler-docker-local diff --git a/plugins/scheduler-docker-local/go.mod b/plugins/scheduler-docker-local/go.mod index 8db9ad568..8eb6eddd8 100644 --- a/plugins/scheduler-docker-local/go.mod +++ b/plugins/scheduler-docker-local/go.mod @@ -5,13 +5,16 @@ go 1.26.2 require ( github.com/dokku/dokku/plugins/common v0.0.0-00010101000000-000000000000 github.com/dokku/dokku/plugins/cron v0.0.0-00010101000000-000000000000 + github.com/dokku/dokku/plugins/storage v0.0.0-00010101000000-000000000000 github.com/multiformats/go-base36 v0.2.0 + github.com/spf13/pflag v1.0.10 golang.org/x/sync v0.20.0 ) require ( github.com/alexellis/go-execute/v2 v2.2.1 // indirect github.com/dokku/dokku/plugins/app-json v0.0.0-00010101000000-000000000000 // indirect + github.com/dokku/dokku/plugins/docker-options v0.0.0-00010101000000-000000000000 // indirect github.com/fatih/color v1.19.0 // indirect github.com/hashicorp/errwrap v1.0.0 // indirect github.com/hashicorp/go-multierror v1.1.1 // indirect @@ -38,3 +41,7 @@ replace github.com/dokku/dokku/plugins/app-json => ../app-json replace github.com/dokku/dokku/plugins/common => ../common replace github.com/dokku/dokku/plugins/cron => ../cron + +replace github.com/dokku/dokku/plugins/storage => ../storage + +replace github.com/dokku/dokku/plugins/docker-options => ../docker-options diff --git a/plugins/scheduler-docker-local/go.sum b/plugins/scheduler-docker-local/go.sum index fd1216a63..caf492d5a 100644 --- a/plugins/scheduler-docker-local/go.sum +++ b/plugins/scheduler-docker-local/go.sum @@ -46,6 +46,8 @@ github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0t github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc= github.com/ryanuber/columnize v2.1.2+incompatible h1:C89EOx/XBWwIXl8wm8OPJBd7kPF25UfsK2X7Ph/zCAk= github.com/ryanuber/columnize v2.1.2+incompatible/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts= +github.com/spf13/pflag v1.0.10 h1:4EBh2KAYBwaONj6b2Ye1GiHfwjqyROoF4RwYO+vPwFk= +github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/tailscale/hujson v0.0.0-20250605163823-992244df8c5a h1:a6TNDN9CgG+cYjaeN8l2mc4kSz2iMiCDQxPEyltUV/I= diff --git a/plugins/scheduler-docker-local/src/triggers/triggers.go b/plugins/scheduler-docker-local/src/triggers/triggers.go index 7ee657f92..2e2c4ff61 100644 --- a/plugins/scheduler-docker-local/src/triggers/triggers.go +++ b/plugins/scheduler-docker-local/src/triggers/triggers.go @@ -1,11 +1,12 @@ package main import ( - "flag" "fmt" "os" "strings" + flag "github.com/spf13/pflag" + "github.com/dokku/dokku/plugins/common" schedulerdockerlocal "github.com/dokku/dokku/plugins/scheduler-docker-local" ) @@ -14,6 +15,10 @@ import ( func main() { parts := strings.Split(os.Args[0], "/") trigger := parts[len(parts)-1] + + interactive := flag.Bool("interactive", false, "--interactive: stdin is open") + tty := flag.Bool("tty", false, "--tty: stdin is a terminal") + asUser := flag.String("as-user", "", "--as-user: numeric uid override") flag.Parse() var err error @@ -21,6 +26,27 @@ func main() { case "scheduler-cron-write": scheduler := flag.Arg(0) err = schedulerdockerlocal.TriggerSchedulerCronWrite(scheduler) + case "scheduler-storage-exec": + args := flag.Args() + if len(args) < 3 { + err = fmt.Errorf("scheduler-storage-exec requires ") + break + } + scheduler := args[0] + entryName := args[1] + image := args[2] + var cmd []string + if len(args) > 3 { + cmd = args[3:] + } + err = schedulerdockerlocal.TriggerSchedulerStorageExec(scheduler, schedulerdockerlocal.StorageExecInput{ + EntryName: entryName, + Image: image, + Interactive: *interactive, + Tty: *tty, + AsUser: *asUser, + Command: cmd, + }) default: err = fmt.Errorf("Invalid plugin trigger call: %s", trigger) } diff --git a/plugins/scheduler-docker-local/storage_exec.go b/plugins/scheduler-docker-local/storage_exec.go new file mode 100644 index 000000000..15938ba02 --- /dev/null +++ b/plugins/scheduler-docker-local/storage_exec.go @@ -0,0 +1,160 @@ +package schedulerdockerlocal + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "regexp" + "strings" + + "github.com/dokku/dokku/plugins/common" + "github.com/dokku/dokku/plugins/storage" +) + +// dockerNamedVolumeRegexp mirrors storage.dockerNamedVolumeRegexp and is +// used to decide whether a HostPath should be statted (filesystem) or +// looked up via `docker volume inspect`. +var dockerNamedVolumeRegexp = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9_.-]+$`) + +// StorageExecInput captures the inputs forwarded by the storage plugin +// over the scheduler-storage-exec trigger. +type StorageExecInput struct { + EntryName string + Image string + Interactive bool + Tty bool + AsUser string + Command []string +} + +// TriggerSchedulerStorageExec runs an interactive or non-interactive +// command in a throwaway docker container that mounts the storage +// entry's host path or named volume at /data. Returns the exit code of +// docker run via os.Exit so the caller's status mirrors the underlying +// tool. +func TriggerSchedulerStorageExec(scheduler string, input StorageExecInput) error { + if scheduler != "docker-local" { + // Not for us; let other handlers respond. + return nil + } + + if !storage.EntryExists(input.EntryName) { + return fmt.Errorf("storage entry %q does not exist", input.EntryName) + } + entry, err := storage.LoadEntry(input.EntryName) + if err != nil { + return err + } + if entry.Scheduler != storage.SchedulerDockerLocal { + return fmt.Errorf("storage entry %q has scheduler %q, not docker-local", entry.Name, entry.Scheduler) + } + + if err := preflightDockerLocalSource(entry.HostPath); err != nil { + return err + } + + args, err := buildDockerExecArgs(entry, input) + if err != nil { + return err + } + + result, err := common.CallExecCommand(common.ExecCommandInput{ + Command: common.DockerBin(), + Args: args, + StreamStdio: true, + }) + if err != nil { + return err + } + if result.ExitCode != 0 { + os.Exit(result.ExitCode) + } + return nil +} + +// preflightDockerLocalSource fails fast when the host path or named +// volume backing the entry doesn't exist, so the user gets a real error +// instead of a generic "docker: ..." line. +func preflightDockerLocalSource(hostPath string) error { + if hostPath == "" { + return errors.New("storage entry is missing a host path") + } + if filepath.IsAbs(hostPath) { + info, err := os.Stat(hostPath) + if err != nil { + if os.IsNotExist(err) { + return fmt.Errorf("host path %s does not exist", hostPath) + } + return fmt.Errorf("unable to stat host path %s: %w", hostPath, err) + } + if !info.IsDir() { + return fmt.Errorf("host path %s exists but is not a directory", hostPath) + } + return nil + } + if !dockerNamedVolumeRegexp.MatchString(hostPath) { + return fmt.Errorf("host path %q is neither absolute nor a valid docker volume token", hostPath) + } + result, err := common.CallExecCommand(common.ExecCommandInput{ + Command: common.DockerBin(), + Args: []string{"volume", "inspect", hostPath}, + }) + if err != nil { + return err + } + if result.ExitCode != 0 { + return fmt.Errorf("docker named volume %q is not present: %s", hostPath, strings.TrimSpace(result.StderrContents())) + } + return nil +} + +// buildDockerExecArgs assembles the `docker run` argv for a storage:exec +// invocation. Split out so the unit test can exercise it without +// actually invoking docker. +func buildDockerExecArgs(entry *storage.Entry, input StorageExecInput) ([]string, error) { + cmd := input.Command + if len(cmd) == 0 { + cmd = []string{"sh", "-c", "command -v bash >/dev/null 2>&1 && exec bash || exec sh"} + } + + args := []string{"run", "--rm"} + if input.Tty { + args = append(args, "-it") + } else if input.Interactive { + args = append(args, "-i") + } + + user, err := resolveUser(entry.Chown, input.AsUser) + if err != nil { + return nil, err + } + if user != "" { + args = append(args, "--user", user) + } + + args = append(args, "-v", fmt.Sprintf("%s:/data", entry.HostPath)) + args = append(args, input.Image) + args = append(args, cmd...) + return args, nil +} + +// resolveUser turns the entry's Chown setting plus an optional --as-user +// override into a docker --user value. Returns an empty string when the +// container should run as the image's default user. +func resolveUser(chown string, asUser string) (string, error) { + if asUser != "" { + return fmt.Sprintf("%s:%s", asUser, asUser), nil + } + if chown == "" || chown == "false" { + return "", nil + } + uid, err := storage.ResolveChownID(chown) + if err != nil { + return "", err + } + if uid == "" || uid == "false" { + return "", nil + } + return fmt.Sprintf("%s:%s", uid, uid), nil +} diff --git a/plugins/scheduler-docker-local/storage_exec_test.go b/plugins/scheduler-docker-local/storage_exec_test.go new file mode 100644 index 000000000..110bab0eb --- /dev/null +++ b/plugins/scheduler-docker-local/storage_exec_test.go @@ -0,0 +1,118 @@ +package schedulerdockerlocal + +import ( + "strings" + "testing" + + "github.com/dokku/dokku/plugins/storage" +) + +func TestBuildDockerExecArgsTTY(t *testing.T) { + entry := &storage.Entry{ + Name: "demo-data", + Scheduler: storage.SchedulerDockerLocal, + HostPath: "/var/lib/dokku/data/storage/demo-data", + } + + cases := []struct { + name string + interactive bool + tty bool + want string + }{ + {"non-interactive", false, false, ""}, + {"piped stdin", true, false, "-i"}, + {"interactive tty", true, true, "-it"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + args, err := buildDockerExecArgs(entry, StorageExecInput{ + Image: "alpine:3", + Interactive: tc.interactive, + Tty: tc.tty, + Command: []string{"ls", "/data"}, + }) + if err != nil { + t.Fatalf("buildDockerExecArgs returned error: %v", err) + } + joined := strings.Join(args, " ") + if tc.want == "" { + if strings.Contains(joined, " -i ") || strings.Contains(joined, " -it ") { + t.Fatalf("expected no -i / -it flag, got: %s", joined) + } + } else if !strings.Contains(joined, " "+tc.want+" ") { + t.Fatalf("expected to find %q in args, got: %s", tc.want, joined) + } + }) + } +} + +func TestBuildDockerExecArgsUser(t *testing.T) { + entry := &storage.Entry{ + Name: "demo-data", + Scheduler: storage.SchedulerDockerLocal, + HostPath: "/var/lib/dokku/data/storage/demo-data", + Chown: "false", + } + + args, err := buildDockerExecArgs(entry, StorageExecInput{ + Image: "alpine:3", + Command: []string{"ls"}, + }) + if err != nil { + t.Fatalf("err: %v", err) + } + if strings.Contains(strings.Join(args, " "), "--user") { + t.Fatalf("did not expect --user flag with chown=false, got: %v", args) + } + + args, err = buildDockerExecArgs(entry, StorageExecInput{ + Image: "alpine:3", + AsUser: "1234", + Command: []string{"ls"}, + }) + if err != nil { + t.Fatalf("err: %v", err) + } + if !strings.Contains(strings.Join(args, " "), "--user 1234:1234") { + t.Fatalf("expected --user 1234:1234, got: %v", args) + } +} + +func TestBuildDockerExecArgsDefaultsToShellWhenNoCommand(t *testing.T) { + entry := &storage.Entry{ + Name: "demo-data", + Scheduler: storage.SchedulerDockerLocal, + HostPath: "/var/lib/dokku/data/storage/demo-data", + } + args, err := buildDockerExecArgs(entry, StorageExecInput{ + Image: "alpine:3", + }) + if err != nil { + t.Fatalf("err: %v", err) + } + last := args[len(args)-3:] + if last[0] != "sh" || last[1] != "-c" { + t.Fatalf("expected the trailing command to be a shell fallback, got: %v", last) + } +} + +func TestBuildDockerExecArgsMountsHostPath(t *testing.T) { + entry := &storage.Entry{ + Name: "demo-data", + Scheduler: storage.SchedulerDockerLocal, + HostPath: "/srv/demo", + } + args, err := buildDockerExecArgs(entry, StorageExecInput{ + Image: "alpine:3", + Command: []string{"ls"}, + }) + if err != nil { + t.Fatalf("err: %v", err) + } + joined := strings.Join(args, " ") + if !strings.Contains(joined, "-v /srv/demo:/data") { + t.Fatalf("expected -v /srv/demo:/data, got: %s", joined) + } +} diff --git a/plugins/scheduler-k3s/Makefile b/plugins/scheduler-k3s/Makefile index 69b114ba4..13dc52e1b 100644 --- a/plugins/scheduler-k3s/Makefile +++ b/plugins/scheduler-k3s/Makefile @@ -1,5 +1,5 @@ SUBCOMMANDS = subcommands/annotations:set subcommands/autoscaling-auth:set subcommands/autoscaling-auth:report subcommands/cluster:add subcommands/cluster:list subcommands/cluster:remove subcommands/ensure-charts subcommands/initialize subcommands/labels:set subcommands/profiles:add subcommands/profiles:list subcommands/profiles:remove subcommands/report subcommands/set subcommands/show-kubeconfig subcommands/uninstall -TRIGGERS = triggers/core-post-deploy triggers/core-post-extract triggers/install triggers/post-app-clone-setup triggers/post-app-rename-setup triggers/post-certs-update triggers/post-certs-remove triggers/post-create triggers/post-delete triggers/report triggers/scheduler-app-status triggers/scheduler-deploy triggers/scheduler-enter triggers/scheduler-is-deployed triggers/scheduler-logs triggers/scheduler-proxy-config triggers/scheduler-proxy-logs triggers/scheduler-post-delete triggers/scheduler-run triggers/scheduler-run-list triggers/scheduler-stop triggers/scheduler-cron-write triggers/storage-create triggers/storage-destroy triggers/storage-status +TRIGGERS = triggers/core-post-deploy triggers/core-post-extract triggers/install triggers/post-app-clone-setup triggers/post-app-rename-setup triggers/post-certs-update triggers/post-certs-remove triggers/post-create triggers/post-delete triggers/report triggers/scheduler-app-status triggers/scheduler-deploy triggers/scheduler-enter triggers/scheduler-is-deployed triggers/scheduler-logs triggers/scheduler-proxy-config triggers/scheduler-proxy-logs triggers/scheduler-post-delete triggers/scheduler-run triggers/scheduler-run-list triggers/scheduler-stop triggers/scheduler-cron-write triggers/storage-create triggers/storage-destroy triggers/storage-status triggers/scheduler-storage-exec BUILD = commands subcommands triggers PLUGIN_NAME = scheduler-k3s diff --git a/plugins/scheduler-k3s/src/triggers/triggers.go b/plugins/scheduler-k3s/src/triggers/triggers.go index e9720f7d2..6998f9b1d 100644 --- a/plugins/scheduler-k3s/src/triggers/triggers.go +++ b/plugins/scheduler-k3s/src/triggers/triggers.go @@ -19,6 +19,9 @@ func main() { parts := strings.Split(os.Args[0], "/") trigger := parts[len(parts)-1] podIdentifier := flag.String("container-id", "", "--container-id: A pod identifier") + storageExecInteractive := flag.Bool("interactive", false, "--interactive: stdin is open (storage:exec)") + storageExecTty := flag.Bool("tty", false, "--tty: stdin is a terminal (storage:exec)") + storageExecAsUser := flag.String("as-user", "", "--as-user: numeric uid override (storage:exec)") flag.Parse() var err error @@ -168,6 +171,27 @@ func main() { case "storage-status": entryName := flag.Arg(0) err = scheduler_k3s.TriggerStorageStatus(context.Background(), entryName) + case "scheduler-storage-exec": + positional := flag.Args() + if len(positional) < 3 { + err = fmt.Errorf("scheduler-storage-exec requires ") + break + } + schedulerName := positional[0] + entryName := positional[1] + image := positional[2] + var cmd []string + if len(positional) > 3 { + cmd = positional[3:] + } + err = scheduler_k3s.TriggerSchedulerStorageExec(context.Background(), schedulerName, scheduler_k3s.StorageExecInput{ + EntryName: entryName, + Image: image, + Interactive: *storageExecInteractive, + Tty: *storageExecTty, + AsUser: *storageExecAsUser, + Command: cmd, + }) default: err = fmt.Errorf("Invalid plugin trigger call: %s", trigger) } diff --git a/plugins/scheduler-k3s/storage_exec.go b/plugins/scheduler-k3s/storage_exec.go new file mode 100644 index 000000000..a7193cde1 --- /dev/null +++ b/plugins/scheduler-k3s/storage_exec.go @@ -0,0 +1,236 @@ +package scheduler_k3s + +import ( + "context" + "errors" + "fmt" + "os" + "strconv" + "time" + + "github.com/dokku/dokku/plugins/common" + "github.com/dokku/dokku/plugins/storage" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilexec "k8s.io/client-go/util/exec" +) + +// StorageExecInput captures the inputs forwarded by the storage plugin +// over the scheduler-storage-exec trigger. +type StorageExecInput struct { + EntryName string + Image string + Interactive bool + Tty bool + AsUser string + Command []string +} + +// TriggerSchedulerStorageExec runs an interactive or non-interactive +// command against a k3s storage entry by creating a throwaway Pod that +// mounts the PVC at /data, exec-ing the user's command via the +// kubernetes API, and deleting the Pod afterwards. Errors at every +// stage (PVC missing, image pull failure, etc.) come through the +// structured Kubernetes API objects rather than kubectl-shaped strings. +func TriggerSchedulerStorageExec(ctx context.Context, scheduler string, input StorageExecInput) error { + if scheduler != storage.SchedulerK3s { + return nil + } + if !storage.EntryExists(input.EntryName) { + return fmt.Errorf("storage entry %q does not exist", input.EntryName) + } + entry, err := storage.LoadEntry(input.EntryName) + if err != nil { + return err + } + if entry.Scheduler != storage.SchedulerK3s { + return fmt.Errorf("storage entry %q has scheduler %q, not k3s", entry.Name, entry.Scheduler) + } + + if err := isKubernetesAvailable(); err != nil { + return fmt.Errorf("kubernetes not available: %w", err) + } + + clientset, err := NewKubernetesClient() + if err != nil { + return err + } + + namespace := entry.Namespace + if namespace == "" { + namespace = "default" + } + + if _, err := clientset.Client.CoreV1().PersistentVolumeClaims(namespace).Get(ctx, entry.Name, metav1.GetOptions{}); err != nil { + if apierrors.IsNotFound(err) { + return fmt.Errorf("PVC %s not found in namespace %s", entry.Name, namespace) + } + return fmt.Errorf("error reading PVC %s in namespace %s: %w", entry.Name, namespace, err) + } + + podName := fmt.Sprintf("dokku-storage-exec-%s-%d", entry.Name, time.Now().UnixNano()/int64(time.Millisecond)%1000000) + pod, err := buildStoragePodSpec(podName, namespace, entry, input) + if err != nil { + return err + } + + common.LogVerboseQuiet(fmt.Sprintf("Creating exec pod %s/%s", namespace, podName)) + if _, err := clientset.Client.CoreV1().Pods(namespace).Create(ctx, pod, metav1.CreateOptions{}); err != nil { + return fmt.Errorf("error creating exec pod %s/%s: %w", namespace, podName, err) + } + + defer func() { + bg := context.Background() + gracePeriod := int64(0) + _ = clientset.Client.CoreV1().Pods(namespace).Delete(bg, podName, metav1.DeleteOptions{GracePeriodSeconds: &gracePeriod}) + }() + + if err := waitForPodRunning(ctx, clientset, namespace, podName, 60*time.Second); err != nil { + return err + } + + command := input.Command + if len(command) == 0 { + command = []string{"sh", "-c", "command -v bash >/dev/null 2>&1 && exec bash || exec sh"} + } + + execErr := clientset.ExecCommand(ctx, ExecCommandInput{ + Name: podName, + Namespace: namespace, + ContainerName: "exec", + Command: command, + }) + if execErr == nil { + return nil + } + + var coded utilexec.CodeExitError + if errors.As(execErr, &coded) { + os.Exit(coded.Code) + } + return execErr +} + +// buildStoragePodSpec assembles the throwaway Pod that mounts the PVC +// for storage:exec. Split out so the unit test can verify the spec +// without hitting a cluster. +func buildStoragePodSpec(name string, namespace string, entry *storage.Entry, input StorageExecInput) (*corev1.Pod, error) { + uid, err := resolveStorageExecUID(entry.Chown, input.AsUser) + if err != nil { + return nil, err + } + + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Labels: map[string]string{ + "app.kubernetes.io/managed-by": "dokku", + "dokku.com/storage-entry": entry.Name, + "dokku.com/purpose": "storage-exec", + }, + }, + Spec: corev1.PodSpec{ + RestartPolicy: corev1.RestartPolicyNever, + Volumes: []corev1.Volume{ + { + Name: "data", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: entry.Name, + }, + }, + }, + }, + Containers: []corev1.Container{ + { + Name: "exec", + Image: input.Image, + Command: []string{"sleep", "infinity"}, + Stdin: input.Interactive, + TTY: input.Tty, + VolumeMounts: []corev1.VolumeMount{ + {Name: "data", MountPath: "/data"}, + }, + }, + }, + }, + } + + if uid != nil { + pod.Spec.SecurityContext = &corev1.PodSecurityContext{ + RunAsUser: uid, + RunAsGroup: uid, + FSGroup: uid, + } + } + + return pod, nil +} + +// resolveStorageExecUID picks the UID to run the exec Pod as. Returns +// nil to mean "leave the image's default user in place". +func resolveStorageExecUID(chown string, asUser string) (*int64, error) { + if asUser != "" { + n, err := strconv.ParseInt(asUser, 10, 64) + if err != nil { + return nil, fmt.Errorf("--as-user must be a numeric uid, got %q", asUser) + } + return &n, nil + } + if chown == "" || chown == "false" { + return nil, nil + } + uidStr, err := storage.ResolveChownID(chown) + if err != nil { + return nil, err + } + if uidStr == "" || uidStr == "false" { + return nil, nil + } + n, err := strconv.ParseInt(uidStr, 10, 64) + if err != nil { + return nil, fmt.Errorf("could not parse resolved chown UID %q: %w", uidStr, err) + } + return &n, nil +} + +// waitForPodRunning polls the Pod until its phase is Running, surfacing +// the structured Kubernetes API state on failure (image pull failures, +// scheduling problems, etc.) rather than a generic timeout. +func waitForPodRunning(ctx context.Context, clientset KubernetesClient, namespace, name string, timeout time.Duration) error { + deadline := time.Now().Add(timeout) + for { + pod, err := clientset.Client.CoreV1().Pods(namespace).Get(ctx, name, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("error reading exec pod %s/%s: %w", namespace, name, err) + } + switch pod.Status.Phase { + case corev1.PodRunning: + return nil + case corev1.PodFailed, corev1.PodSucceeded: + return fmt.Errorf("exec pod %s/%s reached terminal phase %s before exec could attach", namespace, name, pod.Status.Phase) + } + + for _, cs := range pod.Status.ContainerStatuses { + if cs.State.Waiting == nil { + continue + } + reason := cs.State.Waiting.Reason + switch reason { + case "ImagePullBackOff", "ErrImagePull", "CreateContainerConfigError", "InvalidImageName": + return fmt.Errorf("exec pod %s/%s container %q failed to start: %s: %s", namespace, name, cs.Name, reason, cs.State.Waiting.Message) + } + } + + if time.Now().After(deadline) { + return fmt.Errorf("exec pod %s/%s did not reach Running within %s (current phase: %s)", namespace, name, timeout, pod.Status.Phase) + } + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(500 * time.Millisecond): + } + } +} diff --git a/plugins/scheduler-k3s/storage_exec_test.go b/plugins/scheduler-k3s/storage_exec_test.go new file mode 100644 index 000000000..7bc3b0cf6 --- /dev/null +++ b/plugins/scheduler-k3s/storage_exec_test.go @@ -0,0 +1,103 @@ +package scheduler_k3s + +import ( + "testing" + + "github.com/dokku/dokku/plugins/storage" +) + +func TestBuildStoragePodSpec(t *testing.T) { + entry := &storage.Entry{ + Name: "demo-data", + Scheduler: storage.SchedulerK3s, + Namespace: "dokku", + } + pod, err := buildStoragePodSpec("dokku-storage-exec-demo-data-1", "dokku", entry, StorageExecInput{ + Image: "alpine:3", + Interactive: true, + Tty: true, + Command: []string{"sh"}, + }) + if err != nil { + t.Fatalf("buildStoragePodSpec err: %v", err) + } + + if pod.Spec.RestartPolicy != "Never" { + t.Fatalf("expected RestartPolicy=Never, got %s", pod.Spec.RestartPolicy) + } + if len(pod.Spec.Volumes) != 1 || pod.Spec.Volumes[0].PersistentVolumeClaim == nil { + t.Fatalf("expected one PVC volume, got %+v", pod.Spec.Volumes) + } + if pod.Spec.Volumes[0].PersistentVolumeClaim.ClaimName != "demo-data" { + t.Fatalf("expected PVC claim name demo-data, got %s", pod.Spec.Volumes[0].PersistentVolumeClaim.ClaimName) + } + if len(pod.Spec.Containers) != 1 { + t.Fatalf("expected one container, got %d", len(pod.Spec.Containers)) + } + c := pod.Spec.Containers[0] + if c.Image != "alpine:3" { + t.Fatalf("expected image alpine:3, got %s", c.Image) + } + if !c.Stdin || !c.TTY { + t.Fatalf("expected stdin and tty true on the container, got stdin=%v tty=%v", c.Stdin, c.TTY) + } + if len(c.VolumeMounts) != 1 || c.VolumeMounts[0].MountPath != "/data" { + t.Fatalf("expected one volume mount at /data, got %+v", c.VolumeMounts) + } +} + +func TestBuildStoragePodSpecChownToSecurityContext(t *testing.T) { + entry := &storage.Entry{ + Name: "demo-data", + Scheduler: storage.SchedulerK3s, + Chown: "herokuish", + } + pod, err := buildStoragePodSpec("p", "default", entry, StorageExecInput{Image: "alpine:3"}) + if err != nil { + t.Fatalf("err: %v", err) + } + if pod.Spec.SecurityContext == nil || pod.Spec.SecurityContext.RunAsUser == nil { + t.Fatalf("expected SecurityContext.RunAsUser to be populated for chown=herokuish") + } + // Don't assert exact UID because ResolveChownID consults docker for + // userns offset; only assert non-nil and non-zero. + if *pod.Spec.SecurityContext.RunAsUser == 0 { + t.Fatalf("expected non-zero UID for chown=herokuish, got 0") + } +} + +func TestBuildStoragePodSpecAsUserOverride(t *testing.T) { + entry := &storage.Entry{ + Name: "demo-data", + Scheduler: storage.SchedulerK3s, + Chown: "herokuish", + } + pod, err := buildStoragePodSpec("p", "default", entry, StorageExecInput{ + Image: "alpine:3", + AsUser: "0", + }) + if err != nil { + t.Fatalf("err: %v", err) + } + if pod.Spec.SecurityContext == nil || pod.Spec.SecurityContext.RunAsUser == nil { + t.Fatalf("expected RunAsUser to be set") + } + if *pod.Spec.SecurityContext.RunAsUser != 0 { + t.Fatalf("expected RunAsUser to be 0 (override), got %d", *pod.Spec.SecurityContext.RunAsUser) + } +} + +func TestBuildStoragePodSpecChownFalseLeavesDefaultUser(t *testing.T) { + entry := &storage.Entry{ + Name: "demo-data", + Scheduler: storage.SchedulerK3s, + Chown: "false", + } + pod, err := buildStoragePodSpec("p", "default", entry, StorageExecInput{Image: "alpine:3"}) + if err != nil { + t.Fatalf("err: %v", err) + } + if pod.Spec.SecurityContext != nil { + t.Fatalf("expected no SecurityContext when chown=false, got %+v", pod.Spec.SecurityContext) + } +} diff --git a/plugins/storage/commands_entries.go b/plugins/storage/commands_entries.go index d4d62c1c7..71b5bb3e3 100644 --- a/plugins/storage/commands_entries.go +++ b/plugins/storage/commands_entries.go @@ -244,87 +244,80 @@ func CommandSet(input CommandSetInput) error { return nil } -// CommandExec spawns a temporary container/Pod that mounts the entry -// and runs the given command (or an interactive shell when args is -// empty). It execs into the underlying CLI (docker or kubectl) so -// stdin/stdout/stderr stream naturally. -func CommandExec(name string, image string, args []string) error { - if !EntryExists(name) { - return fmt.Errorf("storage entry %q does not exist", name) +// CommandExecInput captures the storage:exec subcommand inputs. +type CommandExecInput struct { + Name string + Image string + AsUser string + Args []string +} + +// CommandExec delegates the actual exec to the scheduler plugin that owns +// the storage entry, by firing the scheduler-storage-exec plugn trigger +// with stdin/stdout/stderr streamed through. The handler decides between +// docker run, the k8s exec SDK, etc. - storage stays scheduler-agnostic. +// +// Exit codes from the underlying tool are propagated verbatim via +// os.Exit so callers in scripts see the right status. +func CommandExec(input CommandExecInput) error { + if !EntryExists(input.Name) { + return fmt.Errorf("storage entry %q does not exist", input.Name) } - entry, err := LoadEntry(name) + entry, err := LoadEntry(input.Name) if err != nil { return err } + + image := input.Image if image == "" { image = "alpine:3" } - cmd := args - if len(cmd) == 0 { - cmd = []string{"sh", "-c", "command -v bash >/dev/null 2>&1 && exec bash || exec sh"} + interactive, tty := stdinModes() + + triggerArgs := []string{ + entry.Scheduler, + entry.Name, + image, + } + triggerArgs = append(triggerArgs, fmt.Sprintf("--interactive=%t", interactive)) + triggerArgs = append(triggerArgs, fmt.Sprintf("--tty=%t", tty)) + if input.AsUser != "" { + triggerArgs = append(triggerArgs, "--as-user", input.AsUser) + } + if len(input.Args) > 0 { + triggerArgs = append(triggerArgs, "--") + triggerArgs = append(triggerArgs, input.Args...) } - if entry.Scheduler == SchedulerDockerLocal { - dockerArgs := []string{"run", "--rm", "-it", "-v", fmt.Sprintf("%s:/data", entry.HostPath), image} - dockerArgs = append(dockerArgs, cmd...) - return execStream(common.DockerBin(), dockerArgs) - } - - // k3s: render a Pod-spec override that mounts the PVC, then kubectl run. - namespace := entry.Namespace - if namespace == "" { - namespace = "default" - } - overrides := map[string]any{ - "apiVersion": "v1", - "spec": map[string]any{ - "containers": []map[string]any{ - { - "name": "exec", - "image": image, - "stdin": true, - "tty": true, - "volumeMounts": []map[string]any{{"name": "data", "mountPath": "/data"}}, - }, - }, - "volumes": []map[string]any{ - {"name": "data", "persistentVolumeClaim": map[string]any{"claimName": entry.Name}}, - }, - }, - } - overrideJSON, err := json.Marshal(overrides) - if err != nil { - return err - } - kubectlArgs := []string{ - "run", "dokku-storage-exec-" + entry.Name, - "--rm", "-it", - "--namespace=" + namespace, - "--image=" + image, - "--restart=Never", - "--overrides=" + string(overrideJSON), - "--", - } - kubectlArgs = append(kubectlArgs, cmd...) - return execStream("kubectl", kubectlArgs) -} - -func execStream(bin string, args []string) error { - result, err := common.CallExecCommand(common.ExecCommandInput{ - Command: bin, - Args: args, + results, err := common.CallPlugnTrigger(common.PlugnTriggerInput{ + Trigger: "scheduler-storage-exec", + Args: triggerArgs, StreamStdio: true, }) if err != nil { return err } - if result.ExitCode != 0 { - return fmt.Errorf("%s exited with %d", bin, result.ExitCode) + if results.ExitCode != 0 { + os.Exit(results.ExitCode) } return nil } +// stdinModes inspects os.Stdin to decide whether docker / the k8s exec +// SDK should request an interactive session and a TTY. +func stdinModes() (interactive bool, tty bool) { + fi, err := os.Stdin.Stat() + if err != nil || fi == nil { + return false, false + } + mode := fi.Mode() + tty = mode&os.ModeCharDevice != 0 + hasStdinData := mode&os.ModeNamedPipe != 0 + interactive = tty || hasStdinData + return interactive, tty +} + // CommandWait blocks until a k3s storage entry's PVC is bound. func CommandWait(name string) error { if !EntryExists(name) { @@ -463,7 +456,7 @@ func ensureDockerLocalPath(entry *Entry) error { } if entry.Chown != "" && entry.Chown != "false" { - chownID, err := resolveChownID(entry.Chown) + chownID, err := ResolveChownID(entry.Chown) if err != nil { return err } diff --git a/plugins/storage/entry.go b/plugins/storage/entry.go index 3fae69d66..470ac3781 100644 --- a/plugins/storage/entry.go +++ b/plugins/storage/entry.go @@ -44,6 +44,10 @@ var ( // required for resource names like PVCs and Helm releases. dns1123LabelRegexp = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$`) + // dockerNamedVolumeRegexp matches a Docker named-volume token. Same + // rule docker uses to disambiguate volume names from bind paths. + dockerNamedVolumeRegexp = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9_.-]+$`) + // supportedSchedulers lists the values accepted for --scheduler. supportedSchedulers = map[string]bool{ SchedulerDockerLocal: true, @@ -225,8 +229,8 @@ func (e *Entry) Validate() error { if e.HostPath == "" { return fmt.Errorf("storage entry %q (docker-local) is missing host_path", e.Name) } - if !filepath.IsAbs(e.HostPath) { - return fmt.Errorf("storage entry %q host_path must be absolute, got %q", e.Name, e.HostPath) + if !filepath.IsAbs(e.HostPath) && !dockerNamedVolumeRegexp.MatchString(e.HostPath) { + return fmt.Errorf("storage entry %q host_path must be an absolute path or a docker named volume token, got %q", e.Name, e.HostPath) } if e.Size != "" { return fmt.Errorf("storage entry %q (docker-local) does not accept --size", e.Name) diff --git a/plugins/storage/entry_test.go b/plugins/storage/entry_test.go index 804245b1f..d317452dc 100644 --- a/plugins/storage/entry_test.go +++ b/plugins/storage/entry_test.go @@ -65,9 +65,18 @@ func TestEntryValidateDockerLocal(t *testing.T) { noPath := &Entry{Name: "foo", Scheduler: SchedulerDockerLocal} Expect(noPath.Validate()).To(HaveOccurred()) + // A leading-slash-less but DNS-1123-ish token is treated as a docker + // named volume and accepted. + namedVolume := &Entry{Name: "foo", Scheduler: SchedulerDockerLocal, HostPath: "myvolume"} + Expect(namedVolume.Validate()).To(Succeed()) + + // Slash-containing relative paths still fail, as do tokens with bad chars. relativePath := &Entry{Name: "foo", Scheduler: SchedulerDockerLocal, HostPath: "relative/path"} Expect(relativePath.Validate()).To(HaveOccurred()) + badToken := &Entry{Name: "foo", Scheduler: SchedulerDockerLocal, HostPath: "with spaces"} + Expect(badToken.Validate()).To(HaveOccurred()) + withSize := &Entry{Name: "foo", Scheduler: SchedulerDockerLocal, HostPath: "/data", Size: "2Gi"} Expect(withSize.Validate()).To(HaveOccurred()) diff --git a/plugins/storage/src/subcommands/subcommands.go b/plugins/storage/src/subcommands/subcommands.go index 9fc49a81b..6af83275d 100644 --- a/plugins/storage/src/subcommands/subcommands.go +++ b/plugins/storage/src/subcommands/subcommands.go @@ -113,6 +113,7 @@ func main() { case "exec": args := flag.NewFlagSet("storage:exec", flag.ExitOnError) image := args.String("image", "", "--image: container image to use (default alpine:3)") + asUser := args.String("as-user", "", "--as-user: numeric uid to run the exec container as (overrides the entry's chown)") args.Parse(os.Args[2:]) positional := args.Args() if len(positional) == 0 { @@ -124,7 +125,12 @@ func main() { if len(positional) > 1 { cmd = positional[1:] } - err = storage.CommandExec(name, *image, cmd) + err = storage.CommandExec(storage.CommandExecInput{ + Name: name, + Image: *image, + AsUser: *asUser, + Args: cmd, + }) case "wait": args := flag.NewFlagSet("storage:wait", flag.ExitOnError) args.Parse(os.Args[2:]) diff --git a/plugins/storage/subcommands.go b/plugins/storage/subcommands.go index 96b76800e..e3d489f1f 100644 --- a/plugins/storage/subcommands.go +++ b/plugins/storage/subcommands.go @@ -46,7 +46,7 @@ func CommandEnsureDirectory(directory string, chownFlag string) error { return err } - chownID, err := resolveChownID(chownFlag) + chownID, err := ResolveChownID(chownFlag) if err != nil { return err } @@ -80,8 +80,8 @@ func CommandEnsureDirectory(directory string, chownFlag string) error { return nil } -// resolveChownID converts a chown flag value to a numeric UID -func resolveChownID(chownFlag string) (string, error) { +// ResolveChownID converts a chown flag value to a numeric UID +func ResolveChownID(chownFlag string) (string, error) { var chownID string switch chownFlag { diff --git a/tests/unit/storage.bats b/tests/unit/storage.bats index eed154ab3..ad48240ed 100644 --- a/tests/unit/storage.bats +++ b/tests/unit/storage.bats @@ -299,6 +299,29 @@ teardown() { assert_success } +@test "(storage) storage:exec runs a non-interactive command and propagates exit code" { + run /bin/bash -c "dokku storage:create rdmtest-exec" + assert_success + + # docker pull image up-front so the test isn't gated on registry latency. + run /bin/bash -c "docker pull alpine:3 >/dev/null" + assert_success + + run /bin/bash -c "dokku storage:exec rdmtest-exec -- /bin/sh -c 'touch /data/marker && ls /data/marker'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "/data/marker" + + run /bin/bash -c "dokku storage:exec rdmtest-exec -- /bin/sh -c 'exit 42'" + echo "output: $output" + echo "status: $status" + assert_equal "$status" 42 + + run /bin/bash -c "dokku storage:destroy rdmtest-exec" + assert_success +} + @test "(storage) storage:ensure-directory emits deprecation warning" { run /bin/bash -c "dokku storage:ensure-directory $TEST_APP 2>&1" echo "output: $output" From cdbc91048a5cf1c9acbd2df1862b4ed9022a8a86 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Thu, 30 Apr 2026 00:01:54 -0400 Subject: [PATCH 11/13] fix: chown the storage registry tree to the dokku user The install trigger created /var/lib/dokku/data/storage-registry and its entries / migrations subdirectories as root with mode 0755, so the dokku user that runs storage:create couldn't write entry JSON files or migration flag files into them. Mirrors what PropertySetup does for config/storage by calling SetPermissions on each registry path. This unblocks the bats coverage that was failing at the first storage:create call. --- plugins/storage/triggers.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/plugins/storage/triggers.go b/plugins/storage/triggers.go index 1d6cf9279..270b00812 100644 --- a/plugins/storage/triggers.go +++ b/plugins/storage/triggers.go @@ -35,6 +35,21 @@ func TriggerInstall() error { return err } + // 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. + 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()) + } + if err := common.SetPermissions(common.SetPermissionInput{ + Filename: dir, + Mode: 0755, + }); err != nil { + return fmt.Errorf("Unable to set permissions on %s: %s", dir, err.Error()) + } + } + if err := MigrateLegacyMounts(); err != nil { return fmt.Errorf("storage migration failed: %w", err) } From e9a53ac82dcab8b0e1c3f4ab7dfb22307e18c889 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Thu, 30 Apr 2026 01:10:00 -0400 Subject: [PATCH 12/13] fix: propagate storage:exec exit codes and route legacy mounts to attachments CallExecCommand wraps non-zero exit codes as errors with the code populated on the response, so storage:exec was returning the wrapped error and being collapsed to exit 1 by LogFailWithError before the os.Exit branch ran. Both storage's CommandExec and scheduler-docker-local's TriggerSchedulerStorageExec now check ExitCode before err so the underlying tool's status flows through verbatim. Separately, the legacy host:container colon form of storage:mount used to write straight into docker-options, but storage:list now reads only attachments, so newly mounted legacy-form storage was invisible. CommandMount and CommandUnmount route the colon form through LegacyMountToEntry plus AddAttachment / RemoveAttachment, making storage:list show every mount regardless of form while preserving the existing "Mount path already exists." / "Mount path does not exist." error wording. --- .../scheduler-docker-local/storage_exec.go | 10 +- plugins/storage/commands_entries.go | 11 ++- plugins/storage/subcommands.go | 93 +++++++++++++++---- 3 files changed, 91 insertions(+), 23 deletions(-) diff --git a/plugins/scheduler-docker-local/storage_exec.go b/plugins/scheduler-docker-local/storage_exec.go index 15938ba02..4db397e29 100644 --- a/plugins/scheduler-docker-local/storage_exec.go +++ b/plugins/scheduler-docker-local/storage_exec.go @@ -64,12 +64,16 @@ func TriggerSchedulerStorageExec(scheduler string, input StorageExecInput) error Args: args, StreamStdio: true, }) - if err != nil { - return err - } + // CallExecCommand wraps non-zero exit as an error; for storage:exec + // the docker-run exit code is the signal we want to forward to the + // caller. Propagate it before falling through to the err return, + // which would otherwise be collapsed to exit 1 by the dispatcher. if result.ExitCode != 0 { os.Exit(result.ExitCode) } + if err != nil { + return err + } return nil } diff --git a/plugins/storage/commands_entries.go b/plugins/storage/commands_entries.go index 71b5bb3e3..ada6f68e3 100644 --- a/plugins/storage/commands_entries.go +++ b/plugins/storage/commands_entries.go @@ -295,12 +295,17 @@ func CommandExec(input CommandExecInput) error { Args: triggerArgs, StreamStdio: true, }) - if err != nil { - return err - } + // common.CallExecCommand wraps a non-zero exit code as an error, but + // for storage:exec the underlying tool's exit code IS the signal we + // want to surface - it's how `dokku storage:exec demo -- exit 42` + // reaches the user as exit 42. Check ExitCode first so that branch + // runs before err collapses to LogFailWithError → exit 1. if results.ExitCode != 0 { os.Exit(results.ExitCode) } + if err != nil { + return err + } return nil } diff --git a/plugins/storage/subcommands.go b/plugins/storage/subcommands.go index e3d489f1f..f8b857bf7 100644 --- a/plugins/storage/subcommands.go +++ b/plugins/storage/subcommands.go @@ -9,7 +9,6 @@ import ( "strings" "github.com/dokku/dokku/plugins/common" - dockeroptions "github.com/dokku/dokku/plugins/docker-options" ) const ( @@ -151,16 +150,12 @@ func CommandMount(input CommandMountInput) error { return err } - // Legacy colon form: keep writing into docker-options so existing - // users see no behavior change. + // Legacy colon form: synthesize a legacy- entry plus an + // attachment so storage:list (now attachment-only) sees the mount. + // The storage docker-args trigger emits the corresponding -v flag at + // deploy time, so behavior at the docker-run boundary is unchanged. if strings.Contains(input.NameOrPath, ":") { - if err := VerifyPaths(input.NameOrPath); err != nil { - return err - } - if CheckIfPathExists(input.AppName, input.NameOrPath, MountPhases) { - return errors.New("Mount path already exists.") - } - return dockeroptions.AddDockerOptionToPhases(input.AppName, MountPhases, fmt.Sprintf("-v %s", input.NameOrPath)) + return mountLegacyColon(input.AppName, input.NameOrPath) } // Named-entry form: persist as an attachment. @@ -215,18 +210,82 @@ func CommandUnmount(input CommandUnmountInput) error { } if strings.Contains(input.NameOrPath, ":") { - if err := VerifyPaths(input.NameOrPath); err != nil { - return err - } - if !CheckIfPathExists(input.AppName, input.NameOrPath, MountPhases) { - return errors.New("Mount path does not exist.") - } - return dockeroptions.RemoveDockerOptionFromPhases(input.AppName, MountPhases, fmt.Sprintf("-v %s", input.NameOrPath)) + return unmountLegacyColon(input.AppName, input.NameOrPath) } return RemoveAttachment(input.AppName, input.NameOrPath, input.ContainerDir) } +// mountLegacyColon translates a `:[:options]` mount +// string into a synthesized legacy- entry plus an attachment. +// Idempotent: re-running the same mount errors with the existing +// "already mounted" message via AddAttachment's duplicate check. +func mountLegacyColon(appName string, mountPath string) error { + if err := VerifyPaths(mountPath); err != nil { + return err + } + parsed := ParseMountPath(mountPath) + if parsed.ContainerPath == "" { + return errors.New("Storage path must be two valid paths divided by colon.") + } + + entry := LegacyMountToEntry(mountPath) + if !EntryExists(entry.Name) { + if err := SaveEntry(entry); err != nil { + return err + } + } + + attachment := &Attachment{ + EntryName: entry.Name, + ContainerPath: parsed.ContainerPath, + Phases: []string{PhaseDeploy, PhaseRun}, + ProcessType: DefaultProcessType, + } + switch parsed.VolumeOptions { + case "": + case "ro": + attachment.Readonly = true + default: + attachment.VolumeOptions = parsed.VolumeOptions + } + + if err := AddAttachment(appName, attachment); err != nil { + // AddAttachment's duplicate error mentions the entry name, but + // the legacy form historically said "Mount path already + // exists." - preserve that exact wording so existing automation + // and the bats suite keep matching. + if strings.Contains(err.Error(), "is already mounted at") { + return errors.New("Mount path already exists.") + } + return err + } + return nil +} + +// unmountLegacyColon is the inverse of mountLegacyColon. The legacy +// mount string identifies an entry+container-path tuple deterministically +// via LegacyMountToEntry, so we can route to RemoveAttachment. +func unmountLegacyColon(appName string, mountPath string) error { + if err := VerifyPaths(mountPath); err != nil { + return err + } + parsed := ParseMountPath(mountPath) + if parsed.ContainerPath == "" { + return errors.New("Storage path must be two valid paths divided by colon.") + } + + entry := LegacyMountToEntry(mountPath) + if err := RemoveAttachment(appName, entry.Name, parsed.ContainerPath); err != nil { + // Match the legacy wording for "not currently mounted". + if strings.Contains(err.Error(), "is not mounted") { + return errors.New("Mount path does not exist.") + } + return err + } + return nil +} + // CommandList lists all bind mounts for an app. Reads attachments // directly from the storage plugin's own state rather than going through // the deprecated `storage-list` plugn trigger. From f0d088bc1d2f2a57f5d5aa3946582a402aaf453d Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Thu, 30 Apr 2026 02:37:01 -0400 Subject: [PATCH 13/13] test: cover the legacy -v migration with go and bats tests Adds plugins/storage/migrate_test.go exercising migrateApp and MigrateLegacyMounts: single deploy phase, cross-phase grouping into one attachment, :ro and free-form volume options preserved on the attachment, idempotency on a second pass, the per-app flag-file fast path that skips already-migrated apps, and the name-collision refusal that aborts before draining. tests/unit/storage.bats gains an end-to-end case that stages a -v line via docker-options:add, runs storage:migrate, and asserts the synthesized colon form shows up in storage:list, the legacy- entry shows up in storage:list-entries, and the original -v line is gone from docker-options:report on both phases. To make that test (and operators with restored backups) practical, storage gains a storage:migrate |--all subcommand backed by a new MigrateApp helper that skips the per-app flag file so a re-run actually re-scans. --- plugins/storage/Makefile | 2 +- plugins/storage/commands_entries.go | 38 +++ plugins/storage/migrate.go | 15 + plugins/storage/migrate_test.go | 317 ++++++++++++++++++ plugins/storage/src/commands/commands.go | 1 + .../storage/src/subcommands/subcommands.go | 5 + plugins/storage/subcommands.go | 1 + tests/unit/storage.bats | 68 ++++ 8 files changed, 446 insertions(+), 1 deletion(-) create mode 100644 plugins/storage/migrate_test.go diff --git a/plugins/storage/Makefile b/plugins/storage/Makefile index 520b4e9a6..11829d5ec 100644 --- a/plugins/storage/Makefile +++ b/plugins/storage/Makefile @@ -1,5 +1,5 @@ GOARCH ?= amd64 -SUBCOMMANDS = subcommands/default subcommands/create subcommands/destroy subcommands/ensure-directory subcommands/exec subcommands/info subcommands/list subcommands/list-entries subcommands/mount subcommands/report subcommands/set subcommands/unmount subcommands/wait +SUBCOMMANDS = subcommands/default subcommands/create subcommands/destroy subcommands/ensure-directory subcommands/exec subcommands/info 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 BUILD = commands subcommands triggers PLUGIN_NAME = storage diff --git a/plugins/storage/commands_entries.go b/plugins/storage/commands_entries.go index ada6f68e3..a79d1cdff 100644 --- a/plugins/storage/commands_entries.go +++ b/plugins/storage/commands_entries.go @@ -323,6 +323,44 @@ func stdinModes() (interactive bool, tty bool) { return interactive, tty } +// CommandMigrate re-runs the legacy `-v` → entry/attachment migration +// for a single app or every app on the install. Useful when an operator +// has restored an app from a backup that predates the install-time +// migration, or when they've added `-v` lines via docker-options:add +// after the per-app flag was set. +func CommandMigrate(appName string, all bool) error { + if all && appName != "" { + return errors.New("storage:migrate accepts either an app name or --all, not both") + } + if !all && appName == "" { + return errors.New("storage:migrate requires an app name (or --all)") + } + if all { + apps, err := common.DokkuApps() + if err != nil { + if errors.Is(err, common.NoAppsExist) { + return nil + } + return err + } + for _, app := range apps { + if err := MigrateApp(app); err != nil { + return err + } + } + common.LogInfo1(fmt.Sprintf("Re-migrated %d app(s)", len(apps))) + return nil + } + if err := common.VerifyAppName(appName); err != nil { + return err + } + if err := MigrateApp(appName); err != nil { + return err + } + common.LogInfo1(fmt.Sprintf("Re-migrated %s", appName)) + return nil +} + // CommandWait blocks until a k3s storage entry's PVC is bound. func CommandWait(name string) error { if !EntryExists(name) { diff --git a/plugins/storage/migrate.go b/plugins/storage/migrate.go index 3dba56eb0..62fae8d8c 100644 --- a/plugins/storage/migrate.go +++ b/plugins/storage/migrate.go @@ -25,6 +25,21 @@ 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 `. +func MigrateApp(appName string) error { + if err := os.MkdirAll(migrationFlagDir(), 0755); err != nil { + return fmt.Errorf("unable to create migration flag dir: %w", err) + } + _ = os.Remove(migrationFlagFile(appName)) + if err := migrateApp(appName); err != nil { + return fmt.Errorf("storage migration failed for app %q: %w", appName, err) + } + return nil +} + // 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. diff --git a/plugins/storage/migrate_test.go b/plugins/storage/migrate_test.go new file mode 100644 index 000000000..41188e246 --- /dev/null +++ b/plugins/storage/migrate_test.go @@ -0,0 +1,317 @@ +package storage + +import ( + "os" + "os/user" + "path/filepath" + "sort" + "testing" + + "github.com/dokku/dokku/plugins/common" + dockeroptions "github.com/dokku/dokku/plugins/docker-options" +) + +// setupMigrationEnv points DOKKU_LIB_ROOT and DOKKU_ROOT at temp dirs and +// tells the permission helpers to chown to the current user (a no-op), +// so the test works without root and without the dokku system user. +// Mirrors the helper in docker-options/migration_test.go. +func setupMigrationEnv(t *testing.T) (libRoot string, dokkuRoot string) { + t.Helper() + libRoot = t.TempDir() + dokkuRoot = t.TempDir() + + t.Setenv("DOKKU_LIB_ROOT", libRoot) + t.Setenv("DOKKU_ROOT", dokkuRoot) + t.Setenv("PLUGIN_PATH", filepath.Join(libRoot, "plugins")) + + current, err := user.Current() + if err != nil { + t.Fatalf("user.Current: %v", err) + } + group, err := user.LookupGroupId(current.Gid) + if err != nil { + t.Fatalf("user.LookupGroupId: %v", err) + } + t.Setenv("DOKKU_SYSTEM_USER", current.Username) + t.Setenv("DOKKU_SYSTEM_GROUP", group.Name) + + // MigrateLegacyMounts creates the flag dir before iterating apps; + // migrateApp (the per-app helper) assumes it already exists. Pre- + // create it so tests that call migrateApp directly don't trip. + if err := os.MkdirAll(migrationFlagDir(), 0755); err != nil { + t.Fatalf("mkdir migration flag dir: %v", err) + } + + return libRoot, dokkuRoot +} + +// stageApp registers the named app under DOKKU_ROOT so DokkuApps can +// see it, and seeds its docker-options property files for the requested +// phases. Each map entry is a phase ("deploy", "run", "build") whose +// value is the full property-list contents (one option per line). +func stageApp(t *testing.T, dokkuRoot string, app string, options map[string][]string) { + t.Helper() + if err := os.MkdirAll(filepath.Join(dokkuRoot, app), 0755); err != nil { + t.Fatalf("mkdir app: %v", err) + } + for phase, lines := range options { + if err := dockeroptions.AddDockerOptionToPhases(app, []string{phase}, ""); err != nil { + t.Fatalf("seed property file: %v", err) + } + // Replace whatever AddDockerOptionToPhases left with the precise + // list the test wants, since it appends rather than overwriting. + key := "_default_." + phase + if err := common.PropertyListWrite("docker-options", app, key, lines); err != nil { + t.Fatalf("PropertyListWrite: %v", err) + } + } +} + +func phaseOptions(t *testing.T, app string, phase string) []string { + t.Helper() + got, err := dockeroptions.GetDockerOptionsForPhase(app, phase) + if err != nil { + t.Fatalf("GetDockerOptionsForPhase %s/%s: %v", app, phase, err) + } + return got +} + +func TestMigrateAppSinglePhase(t *testing.T) { + _, dokkuRoot := setupMigrationEnv(t) + stageApp(t, dokkuRoot, "alpha", map[string][]string{ + "deploy": {"-v /var/log:/log", "--restart=on-failure:5"}, + }) + + if err := migrateApp("alpha"); err != nil { + t.Fatalf("migrateApp: %v", err) + } + + entry := LegacyMountToEntry("/var/log:/log") + if !EntryExists(entry.Name) { + t.Fatalf("expected entry %s on disk", entry.Name) + } + loaded, err := LoadEntry(entry.Name) + if err != nil { + t.Fatalf("LoadEntry: %v", err) + } + if loaded.HostPath != "/var/log" { + t.Errorf("entry HostPath = %s, want /var/log", loaded.HostPath) + } + if loaded.Scheduler != SchedulerDockerLocal { + t.Errorf("entry Scheduler = %s, want %s", loaded.Scheduler, SchedulerDockerLocal) + } + + 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)) + } + att := attachments[0] + if att.EntryName != entry.Name || att.ContainerPath != "/log" { + t.Errorf("attachment = %+v, want entry=%s container=/log", att, entry.Name) + } + if len(att.Phases) != 1 || att.Phases[0] != PhaseDeploy { + t.Errorf("attachment phases = %v, want [deploy]", att.Phases) + } + + // docker-options drained the -v line but kept everything else. + deployAfter := phaseOptions(t, "alpha", "deploy") + if !equalSorted(deployAfter, []string{"--restart=on-failure:5"}) { + 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) + } +} + +func TestMigrateAppCrossPhaseGroupsIntoOneAttachment(t *testing.T) { + _, dokkuRoot := setupMigrationEnv(t) + // Same -v in both phases groups into one attachment with phases = [deploy, run]. + stageApp(t, dokkuRoot, "alpha", map[string][]string{ + "deploy": {"-v /data:/d"}, + "run": {"-v /data:/d"}, + }) + + if err := migrateApp("alpha"); 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)) + } + got := attachments[0].Phases + sort.Strings(got) + if !equalSorted(got, []string{PhaseDeploy, PhaseRun}) { + t.Errorf("phases = %v, want [deploy run]", got) + } + + // Drain affected both phases. + if got := phaseOptions(t, "alpha", "deploy"); len(got) != 0 { + t.Errorf("deploy options after migration = %v, want []", got) + } + if got := phaseOptions(t, "alpha", "run"); len(got) != 0 { + t.Errorf("run options after migration = %v, want []", got) + } +} + +func TestMigrateAppPreservesVolumeOptions(t *testing.T) { + _, dokkuRoot := setupMigrationEnv(t) + stageApp(t, dokkuRoot, "alpha", map[string][]string{ + "deploy": {"-v /ro:/r:ro", "-v /label:/l:Z"}, + }) + + if err := migrateApp("alpha"); err != nil { + t.Fatalf("migrateApp: %v", err) + } + + atts, err := LoadAttachments("alpha") + if err != nil { + t.Fatalf("LoadAttachments: %v", err) + } + byPath := map[string]*Attachment{} + for _, a := range atts { + byPath[a.ContainerPath] = a + } + + if a := byPath["/r"]; a == nil { + t.Fatalf("missing /r attachment") + } else if !a.Readonly || a.VolumeOptions != "" { + t.Errorf("/r attachment = readonly=%v opts=%q, want readonly=true opts=\"\"", a.Readonly, a.VolumeOptions) + } + + if a := byPath["/l"]; a == nil { + t.Fatalf("missing /l attachment") + } else if a.Readonly || a.VolumeOptions != "Z" { + t.Errorf("/l attachment = readonly=%v opts=%q, want readonly=false opts=\"Z\"", a.Readonly, a.VolumeOptions) + } +} + +func TestMigrateAppIdempotent(t *testing.T) { + _, dokkuRoot := setupMigrationEnv(t) + stageApp(t, dokkuRoot, "alpha", map[string][]string{ + "deploy": {"-v /var/log:/log"}, + }) + + if err := migrateApp("alpha"); err != nil { + t.Fatalf("first migrateApp: %v", err) + } + + // Re-stage the same -v line by hand to simulate someone re-adding + // the legacy form via docker-options:add after migration. The + // migration is gated by the per-app flag file at the + // MigrateLegacyMounts level, but migrateApp itself should still be + // safe to call directly: a second run produces no duplicates. + 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 { + t.Fatalf("second migrateApp: %v", err) + } + + atts, err := LoadAttachments("alpha") + if err != nil { + t.Fatalf("LoadAttachments: %v", err) + } + if len(atts) != 1 { + t.Errorf("expected 1 attachment after rerun, got %d", len(atts)) + } +} + +func TestMigrateLegacyMountsBulkAndFlagFastPath(t *testing.T) { + _, dokkuRoot := setupMigrationEnv(t) + stageApp(t, dokkuRoot, "alpha", map[string][]string{ + "deploy": {"-v /alpha-data:/data"}, + }) + stageApp(t, dokkuRoot, "beta", map[string][]string{ + "deploy": {"-v /beta-data:/data"}, + "run": {"-v /beta-data:/data"}, + }) + + if err := MigrateLegacyMounts(); err != nil { + t.Fatalf("MigrateLegacyMounts: %v", err) + } + + for _, app := range []string{"alpha", "beta"} { + if _, err := os.Stat(migrationFlagFile(app)); err != nil { + t.Errorf("flag file missing for %s: %v", app, err) + } + atts, err := LoadAttachments(app) + if err != nil { + t.Fatalf("LoadAttachments %s: %v", app, err) + } + if len(atts) != 1 { + t.Errorf("%s: expected 1 attachment, got %d", app, len(atts)) + } + } + + // 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. + if err := common.PropertyListWrite("docker-options", "alpha", "_default_.deploy", []string{"-v /sneaky:/x"}); err != nil { + t.Fatalf("re-stage alpha: %v", err) + } + if err := MigrateLegacyMounts(); err != nil { + t.Fatalf("MigrateLegacyMounts (rerun): %v", err) + } + got := phaseOptions(t, "alpha", "deploy") + if !equalSorted(got, []string{"-v /sneaky:/x"}) { + t.Errorf("expected sneaky -v line untouched after second pass, got %v", got) + } + atts, err := LoadAttachments("alpha") + if err != nil { + t.Fatalf("LoadAttachments alpha rerun: %v", err) + } + if len(atts) != 1 { + t.Errorf("expected 1 attachment after rerun, got %d", len(atts)) + } +} + +func TestMigrateAppRefusesEntryNameCollision(t *testing.T) { + libRoot, dokkuRoot := setupMigrationEnv(t) + _ = libRoot + + // Plant a colliding entry at the legacy- name with conflicting + // fields so migration's "fields match" check trips. + collide := LegacyMountToEntry("/var/log:/log") + collide.HostPath = "/something/else" + if err := SaveEntry(collide); err != nil { + t.Fatalf("seed colliding entry: %v", err) + } + + stageApp(t, dokkuRoot, "alpha", map[string][]string{ + "deploy": {"-v /var/log:/log"}, + }) + + if err := migrateApp("alpha"); err == nil { + t.Fatalf("expected migrateApp to error on entry name collision") + } + + // Drain did not happen. + if got := phaseOptions(t, "alpha", "deploy"); len(got) != 1 || got[0] != "-v /var/log:/log" { + t.Errorf("expected legacy line preserved on collision, got %v", got) + } +} + +func equalSorted(a, b []string) bool { + if len(a) != len(b) { + return false + } + ax := append([]string(nil), a...) + bx := append([]string(nil), b...) + sort.Strings(ax) + sort.Strings(bx) + for i := range ax { + if ax[i] != bx[i] { + return false + } + } + return true +} diff --git a/plugins/storage/src/commands/commands.go b/plugins/storage/src/commands/commands.go index 68d4edca3..a026f0709 100644 --- a/plugins/storage/src/commands/commands.go +++ b/plugins/storage/src/commands/commands.go @@ -25,6 +25,7 @@ Additional commands:` storage:info [--format text|json], Show details for one storage entry storage:list [--format text|json], List bind mounts for app's container(s) (host:container) storage:list-entries [--scheduler s] [--format text|json], List registered storage entries + storage:migrate [|--all], Re-run the legacy -v to attachment migration for an app storage:mount , Create a new bind mount storage:report [] [], Displays a storage report for one or more apps storage:set [flags], Update a storage entry in place diff --git a/plugins/storage/src/subcommands/subcommands.go b/plugins/storage/src/subcommands/subcommands.go index 6af83275d..3a5bb8d6b 100644 --- a/plugins/storage/src/subcommands/subcommands.go +++ b/plugins/storage/src/subcommands/subcommands.go @@ -131,6 +131,11 @@ func main() { AsUser: *asUser, Args: cmd, }) + case "migrate": + args := flag.NewFlagSet("storage:migrate", flag.ExitOnError) + all := args.Bool("all", false, "--all: re-migrate every app on this install") + args.Parse(os.Args[2:]) + err = storage.CommandMigrate(args.Arg(0), *all) case "wait": args := flag.NewFlagSet("storage:wait", flag.ExitOnError) args.Parse(os.Args[2:]) diff --git a/plugins/storage/subcommands.go b/plugins/storage/subcommands.go index f8b857bf7..2e92125ad 100644 --- a/plugins/storage/subcommands.go +++ b/plugins/storage/subcommands.go @@ -26,6 +26,7 @@ Additional commands:` storage:info [--format text|json], Show details for one storage entry storage:list [--format text|json], List bind mounts for app's container(s) (host:container) storage:list-entries [--scheduler s] [--format text|json], List registered storage entries + storage:migrate [|--all], Re-run the legacy -v to attachment migration for an app storage:mount , Create a new bind mount storage:report [] [], Displays a storage report for one or more apps storage:set [flags], Update a storage entry in place diff --git a/tests/unit/storage.bats b/tests/unit/storage.bats index ad48240ed..83af1fc59 100644 --- a/tests/unit/storage.bats +++ b/tests/unit/storage.bats @@ -322,6 +322,74 @@ teardown() { assert_success } +@test "(storage) storage:migrate converts legacy docker-options -v lines into attachments" { + # Stage a legacy `-v` line directly via docker-options:add. This + # bypasses the new storage:mount path so the line lives in + # docker-options without a corresponding attachment, simulating an + # app upgraded from a pre-PR Dokku version. + run /bin/bash -c "dokku docker-options:add $TEST_APP deploy,run \"-v /tmp/legacy-mount:/legacy\"" + echo "output: $output" + echo "status: $status" + assert_success + + # Before migration, storage:list shows nothing (attachment-only) and + # docker-options has the -v line. + run /bin/bash -c "dokku storage:list $TEST_APP --format json | jq -r '. | length'" + assert_success + assert_output "0" + + run /bin/bash -c "dokku docker-options:report $TEST_APP --docker-options-deploy" + assert_success + assert_output_contains "-v /tmp/legacy-mount:/legacy" + + # Run the migration for this app. + run /bin/bash -c "dokku storage:migrate $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + + # storage:list now surfaces the synthesized colon form. + run /bin/bash -c "dokku --quiet storage:list $TEST_APP" + assert_success + assert_output "/tmp/legacy-mount:/legacy" + + # The synthesized entry shows up under the legacy- prefix. + run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^legacy-' | head -1" + echo "output: $output" + echo "status: $status" + assert_success + + # docker-options no longer holds the -v line on either phase. + run /bin/bash -c "dokku docker-options:report $TEST_APP --docker-options-deploy" + assert_success + assert_output_not_contains "-v /tmp/legacy-mount:/legacy" + + run /bin/bash -c "dokku docker-options:report $TEST_APP --docker-options-run" + assert_success + assert_output_not_contains "-v /tmp/legacy-mount:/legacy" + + # Idempotency: re-running storage:migrate is a no-op (still one + # attachment, no errors). + run /bin/bash -c "dokku storage:migrate $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:list $TEST_APP --format json | jq -r '. | length'" + assert_success + assert_output "1" + + # Cleanup: unmount + destroy the synthesized legacy entry. We need + # the entry name from list-entries since legacy- is content-derived. + legacy_name=$(dokku storage:list-entries --format json | jq -r '.[] | select(.name | startswith("legacy-")) | .name' | head -1) + if [[ -n "$legacy_name" ]]; then + run /bin/bash -c "dokku storage:unmount $TEST_APP $legacy_name --container-dir /legacy" + assert_success + run /bin/bash -c "dokku storage:destroy $legacy_name" + assert_success + fi +} + @test "(storage) storage:ensure-directory emits deprecation warning" { run /bin/bash -c "dokku storage:ensure-directory $TEST_APP 2>&1" echo "output: $output"