diff --git a/docs/development/plugin-triggers.md b/docs/development/plugin-triggers.md index 76e2f1543..b5cc0445f 100644 --- a/docs/development/plugin-triggers.md +++ b/docs/development/plugin-triggers.md @@ -875,7 +875,7 @@ APP="$1"; IMAGE_SOURCE_TYPE="$2" ### `docker-args-process-deploy` -- Description: emits docker arguments scoped to a specific Procfile process type. The `docker-options` plugin implements this trigger to surface options registered via `docker-options:add --process `, and the `storage` plugin implements it to emit the `-v` flags for mounts made with `storage:mount --process-type `. `$PROC_TYPE` may be empty (or set to the magic `_default_` value) to signify default-scope docker deploy options. Note that the two plugins split the work differently: `docker-options` emits only its process-specific scope here and its default scope through `docker-args-deploy`, while `storage` emits both scopes here and does not implement `docker-args-deploy` at all. +- Description: emits docker arguments scoped to a specific Procfile process type. The `docker-options` plugin implements this trigger to surface options registered via `docker-options:add --process `, and the `storage` plugin implements it to emit the `-v` flags for mounts made with `storage:mount --process-type `. `$PROC_TYPE` may be empty (or set to the magic `_default_` value) to signify default-scope docker deploy options. Note that the two plugins split the work differently: `docker-options` emits only its process-specific scope here and its default scope through `docker-args-deploy`, while `storage` emits both scopes here and does not implement `docker-args-deploy` at all. A handler that exits non-zero aborts the deploy, including the ephemeral containers that run `app.json` deploy tasks, so a handler that has nothing to contribute for an app should emit nothing and exit zero rather than fail. - Invoked by: `dokku deploy` - Arguments: `$APP $IMAGE_SOURCE_TYPE $IMAGE_TAG [$PROC_TYPE $CONTAINER_INDEX]` - Example: diff --git a/plugins/app-json/functions.go b/plugins/app-json/functions.go index 1422c37f5..8fa148b51 100644 --- a/plugins/app-json/functions.go +++ b/plugins/app-json/functions.go @@ -182,34 +182,39 @@ func executeScript(appName string, image string, imageTag string, phase string) imageSourceType = "pack" } + // Both triggers are fatal here, the same way they are on the scheduler's + // own deploy path. Swallowing the error meant a plugin refusing to describe + // the container - a storage entry belonging to another scheduler, say - + // silently produced a deploy task running without the arguments it asked + // for, while the identical failure aborted the deploy a moment later. var dockerArgs []string results, err := common.CallPlugnTrigger(common.PlugnTriggerInput{ Trigger: "docker-args-deploy", Args: []string{appName, imageTag}, Stdin: strings.NewReader(""), }) - if err == nil { - words, err := shellquote.Split(results.StdoutContents()) - if err != nil { - return err - } - - dockerArgs = append(dockerArgs, words...) + if err != nil { + return fmt.Errorf("Error getting deploy docker args for %s task: %w", phase, err) } + words, err := shellquote.Split(results.StdoutContents()) + if err != nil { + return err + } + dockerArgs = append(dockerArgs, words...) results, err = common.CallPlugnTrigger(common.PlugnTriggerInput{ Trigger: "docker-args-process-deploy", Args: []string{appName, imageSourceType, imageTag}, Stdin: strings.NewReader(""), }) - if err == nil { - words, err := shellquote.Split(results.StdoutContents()) - if err != nil { - return err - } - - dockerArgs = append(dockerArgs, words...) + if err != nil { + return fmt.Errorf("Error getting process deploy docker args for %s task: %w", phase, err) } + words, err = shellquote.Split(results.StdoutContents()) + if err != nil { + return err + } + dockerArgs = append(dockerArgs, words...) filteredArgs := []string{ "--cpus", diff --git a/plugins/storage/entry_test.go b/plugins/storage/entry_test.go index edbb4caa5..f53806efc 100644 --- a/plugins/storage/entry_test.go +++ b/plugins/storage/entry_test.go @@ -20,6 +20,10 @@ func withTempLibRoot(t *testing.T) string { t.Helper() dir := t.TempDir() t.Setenv("DOKKU_LIB_ROOT", dir) + // Point plugn at an empty tree so anything resolving a plugin trigger in + // these tests falls back to its default instead of reading the host's + // real dokku install. + t.Setenv("PLUGIN_PATH", filepath.Join(dir, "plugins")) current, err := user.Current() if err != nil { diff --git a/plugins/storage/triggers.go b/plugins/storage/triggers.go index f92c76fee..144986331 100644 --- a/plugins/storage/triggers.go +++ b/plugins/storage/triggers.go @@ -264,7 +264,17 @@ func TriggerDockerArgs(appName string, phase string, processType string) error { // Every attachment in the phase is resolved and scheduler-checked, not just // the ones in scope, so an entry created for the wrong scheduler still fails // the deploy no matter which process type happens to be starting. +// +// Apps on another scheduler emit nothing rather than failing: the docker-args +// triggers are invoked for every app, whatever its scheduler, because the +// app.json deploy-task path builds its ephemeral container with docker +// directly. A k3s app's volumes are PersistentVolumeClaims mounted by its own +// scheduler and there is nothing to bind-mount here. func dockerVFlagsForProcess(appName string, phase string, processType string) ([]string, error) { + if appSchedulerFor(appName) != SchedulerDockerLocal { + return nil, nil + } + attachments, err := AttachmentsForPhase(appName, phase) if err != nil { return nil, err @@ -291,6 +301,10 @@ func dockerVFlagsForProcess(appName string, phase string, processType string) ([ return flags, nil } +// appSchedulerFor resolves the scheduler an app deploys with. It is a variable +// so tests can exercise the non-docker-local path without a plugn install. +var appSchedulerFor = common.GetAppScheduler + // buildDockerVFlag formats the Docker -v argument for a docker-local // attachment. func buildDockerVFlag(entry *Entry, attachment *Attachment) string { diff --git a/plugins/storage/triggers_test.go b/plugins/storage/triggers_test.go index 41843bec9..29b4b311e 100644 --- a/plugins/storage/triggers_test.go +++ b/plugins/storage/triggers_test.go @@ -109,6 +109,38 @@ func TestTriggerDockerArgsRejectsSchedulerMismatchOutOfScope(t *testing.T) { Expect(err).To(HaveOccurred()) } +// TestDockerVFlagsForProcessSkipsOtherSchedulers covers apps that do not +// deploy with docker-local. The docker-args triggers are fired for every app - +// the app.json deploy-task path builds its ephemeral container with docker +// whatever the app's scheduler is - so a k3s app reaches this code with k3s +// entries attached. Those are PersistentVolumeClaims its own scheduler mounts, +// so the right answer is no flags, not an error. +func TestDockerVFlagsForProcessSkipsOtherSchedulers(t *testing.T) { + RegisterTestingT(t) + root := withTempLibRoot(t) + + previous := appSchedulerFor + appSchedulerFor = func(string) string { return SchedulerK3s } + t.Cleanup(func() { appSchedulerFor = previous }) + + Expect(SaveEntry(&Entry{ + Name: "demo-pvc", + Scheduler: SchedulerK3s, + })).To(Succeed()) + + writeAttachmentsFile(t, root, "demo", []*Attachment{ + { + EntryName: "demo-pvc", + ContainerPath: "/data", + Phases: []string{PhaseDeploy}, + }, + }) + + flags, err := dockerVFlagsForProcess("demo", PhaseDeploy, "web") + Expect(err).NotTo(HaveOccurred()) + Expect(flags).To(BeEmpty()) +} + // TestDockerVFlagsForProcessScopes is the docker-local half of #9059. An // attachment scoped to a named process type reaches that process only; the // default scope reaches every process, including containers that belong to no