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"