fix: treat empty ps restart-policy as an unset

`dokku ps:set <app> restart-policy` with no value erroneously returned `Invalid restart-policy specified` instead of unsetting the property like every other ps property. The restart policy is now managed as a normal app and global property surfaced through the `--ps-restart-policy`, `--ps-global-restart-policy`, and `--ps-computed-restart-policy` report flags, with the effective value applied at deploy time and existing values migrated on install. Because it is no longer stored as a Docker option it no longer appears in `docker-options:report`, and `--ps-restart-policy` now reports the raw value with the `on-failure:10` default available via `--ps-computed-restart-policy`.

docs/processes/process-management.md

@@ -367,6 +367,20 @@ dokku ps:set node-js-app restart-policy on-failure
 dokku ps:set node-js-app restart-policy on-failure:20
 ```
 
+The default policy (`on-failure:10`) may be restored by passing an empty value:
+
+```shell
+dokku ps:set node-js-app restart-policy
+```
+
+A global default may also be set, and is used by any app that does not have an app-specific restart policy:
+
+```shell
+dokku ps:set --global restart-policy always
+```
+
+The effective policy applied to an app's containers is resolved as the app-specific value, then the global value, then the built-in `on-failure:10` default. This computed value can be inspected via the `--ps-computed-restart-policy` report flag.
+
 Restart policies have no bearing on server reboot, and Dokku will always attempt to restart your apps at that point unless they were manually stopped.
 
 Dokku also runs `dokku-event-listener` in the background via the system's init service. This monitors container state, performing the following actions:
@@ -391,8 +405,10 @@ dokku ps:report
        Processes:                     0
        Ps can scale:                  true
        Ps computed procfile path:     Procfile2
+       Ps computed restart policy:    on-failure:10
        Ps global procfile path:       Procfile
-       Ps restart policy:             on-failure:10
+       Ps global restart policy:
+       Ps restart policy:
        Ps procfile path:              Procfile2
        Restore:                       true
        Running:                       false
@@ -401,8 +417,10 @@ dokku ps:report
        Processes:                     0
        Ps can scale:                  true
        Ps computed procfile path:     Procfile
+       Ps computed restart policy:    on-failure:10
        Ps global procfile path:       Procfile
-       Ps restart policy:             on-failure:10
+       Ps global restart policy:
+       Ps restart policy:
        Ps procfile path:
        Restore:                       true
        Running:                       false
@@ -411,8 +429,10 @@ dokku ps:report
        Processes:                     0
        Ps can scale:                  true
        Ps computed procfile path:     Procfile
+       Ps computed restart policy:    on-failure:10
        Ps global procfile path:       Procfile
-       Ps restart policy:             on-failure:10
+       Ps global restart policy:
+       Ps restart policy:
        Ps procfile path:
        Restore:                       true
        Running:                       false
@@ -429,7 +449,9 @@ dokku ps:report node-js-app
        Deployed:                      false
        Processes:                     0
        Ps can scale:                  true
-       Ps restart policy:             on-failure:10
+       Ps computed restart policy:    on-failure:10
+       Ps global restart policy:
+       Ps restart policy:
        Restore:                       true
        Running:                       false
 ```
@@ -475,7 +497,7 @@ dokku ps:set node-js-app restore
 |---|---|---|---|---|
 | `dockerfile-start-cmd` | app only | none | `--ps-dockerfile-start-cmd`, `--ps-computed-dockerfile-start-cmd` | Override `CMD` for Dockerfile-based apps |
 | `procfile-path` | app + global | `Procfile` | `--ps-procfile-path`, `--ps-global-procfile-path`, `--ps-computed-procfile-path` | Path to the app's Procfile, relative to the build root |
-| `restart-policy` | app only | `on-failure:10` | `--ps-restart-policy` | Docker restart policy applied to deployed containers (`no`, `always`, `unless-stopped`, `on-failure[:max-retries]`) |
+| `restart-policy` | app + global | `on-failure:10` | `--ps-restart-policy`, `--ps-global-restart-policy`, `--ps-computed-restart-policy` | Docker restart policy applied to deployed containers (`no`, `always`, `unless-stopped`, `on-failure[:max-retries]`) |
 | `restore` | app only | `true` | `--restore` | When `true`, the app is restarted automatically by `ps:retire` after a host reboot |
 | `skip-deploy` | app + global | `false` | `--ps-skip-deploy`, `--ps-global-skip-deploy`, `--ps-computed-skip-deploy` | When `true`, skips the deploy phase after a successful build |
 | `start-cmd` | app only | none | `--ps-start-cmd`, `--ps-computed-start-cmd` | Override start command for buildpack apps |

plugins/ps/Makefile

@@ -1,5 +1,5 @@
 SUBCOMMANDS = subcommands/inspect subcommands/rebuild subcommands/report subcommands/restart subcommands/restore subcommands/retire subcommands/scale subcommands/set subcommands/start subcommands/stop
-TRIGGERS = triggers/app-restart triggers/core-post-deploy triggers/core-post-extract triggers/install triggers/post-app-clone triggers/post-app-clone-setup triggers/post-app-rename triggers/post-app-rename-setup triggers/post-create triggers/post-delete triggers/post-release-builder triggers/post-stop triggers/procfile-get-command triggers/procfile-exists triggers/ps-can-scale triggers/ps-current-scale triggers/ps-get-property triggers/ps-set-scale triggers/report
+TRIGGERS = triggers/app-restart triggers/core-post-deploy triggers/core-post-extract triggers/docker-args-process-deploy triggers/install triggers/post-app-clone triggers/post-app-clone-setup triggers/post-app-rename triggers/post-app-rename-setup triggers/post-create triggers/post-delete triggers/post-release-builder triggers/post-stop triggers/procfile-get-command triggers/procfile-exists triggers/ps-can-scale triggers/ps-current-scale triggers/ps-get-property triggers/ps-set-scale triggers/report
 BUILD = commands subcommands triggers
 PLUGIN_NAME = ps
 

plugins/ps/ps.go

@@ -25,6 +25,7 @@ var (
 	// GlobalProperties is a map of all valid global ps properties
 	GlobalProperties = map[string]bool{
 		"procfile-path":        true,
+		"restart-policy":       true,
 		"skip-deploy":          true,
 		"stop-timeout-seconds": true,
 	}

plugins/ps/report.go

@@ -20,9 +20,11 @@ func ReportSingleApp(appName string, format string, infoFlag string) error {
 	if appName == "--global" {
 		flags = map[string]common.ReportFunc{
 			"--ps-computed-procfile-path":        reportComputedProcfilePath,
+			"--ps-computed-restart-policy":       reportComputedRestartPolicy,
 			"--ps-computed-skip-deploy":          reportComputedSkipDeploy,
 			"--ps-computed-stop-timeout-seconds": reportComputedStopTimeoutSeconds,
 			"--ps-global-procfile-path":          reportGlobalProcfilePath,
+			"--ps-global-restart-policy":         reportGlobalRestartPolicy,
 			"--ps-global-skip-deploy":            reportGlobalSkipDeploy,
 			"--ps-global-stop-timeout-seconds":   reportGlobalStopTimeoutSeconds,
 		}
@@ -33,11 +35,13 @@ func ReportSingleApp(appName string, format string, infoFlag string) error {
 			"--ps-can-scale":                       reportCanScale,
 			"--ps-computed-dockerfile-start-cmd":   reportComputedDockerfileStartCmd,
 			"--ps-computed-procfile-path":          reportComputedProcfilePath,
+			"--ps-computed-restart-policy":         reportComputedRestartPolicy,
 			"--ps-computed-skip-deploy":            reportComputedSkipDeploy,
 			"--ps-computed-start-cmd":              reportComputedStartCmd,
 			"--ps-computed-stop-timeout-seconds":   reportComputedStopTimeoutSeconds,
 			"--ps-dockerfile-start-cmd":            reportDockerfileStartCmd,
 			"--ps-global-procfile-path":            reportGlobalProcfilePath,
+			"--ps-global-restart-policy":           reportGlobalRestartPolicy,
 			"--ps-global-skip-deploy":              reportGlobalSkipDeploy,
 			"--ps-global-stop-timeout-seconds":     reportGlobalStopTimeoutSeconds,
 			"--ps-procfile-path":                   reportProcfilePath,
@@ -162,13 +166,24 @@ func reportProcesses(appName string) string {
 	return strconv.Itoa(count)
 }
 
-func reportRestartPolicy(appName string) string {
-	policy, _ := getRestartPolicy(appName)
-	if policy == "" {
-		policy = DefaultProperties["restart-policy"]
+func reportComputedRestartPolicy(appName string) string {
+	value := reportRestartPolicy(appName)
+	if value == "" {
+		value = reportGlobalRestartPolicy(appName)
+	}
+	if value == "" {
+		value = DefaultProperties["restart-policy"]
 	}
 
-	return policy
+	return value
+}
+
+func reportGlobalRestartPolicy(appName string) string {
+	return common.PropertyGet("ps", "--global", "restart-policy")
+}
+
+func reportRestartPolicy(appName string) string {
+	return common.PropertyGet("ps", appName, "restart-policy")
 }
 
 func reportRestore(appName string) string {

plugins/ps/src/triggers/triggers.go

@@ -28,6 +28,9 @@ func main() {
 		appName := flag.Arg(0)
 		sourceWorkDir := flag.Arg(1)
 		err = ps.TriggerCorePostExtract(appName, sourceWorkDir)
+	case "docker-args-process-deploy":
+		appName := flag.Arg(0)
+		err = ps.TriggerDockerArgsProcessDeploy(appName)
 	case "install":
 		err = ps.TriggerInstall()
 	case "post-app-clone":

plugins/ps/subcommands.go

@@ -7,7 +7,6 @@ import (
 	"strings"
 
 	"github.com/dokku/dokku/plugins/common"
-	dockeroptions "github.com/dokku/dokku/plugins/docker-options"
 	"github.com/gofrs/flock"
 )
 
@@ -184,13 +183,8 @@ func CommandScale(appName string, skipDeploy bool, processTuples []string) error
 
 // CommandSet sets or clears a ps property for an app
 func CommandSet(appName string, property string, value string) error {
-	if property == "restart-policy" {
-		if !isValidRestartPolicy(value) {
-			return errors.New("Invalid restart-policy specified")
-		}
-
-		common.LogInfo2Quiet(fmt.Sprintf("Setting %s to %s", property, value))
-		return dockeroptions.SetDockerOptionForPhases(appName, []string{"deploy"}, "restart", value)
+	if property == "restart-policy" && value != "" && !isValidRestartPolicy(value) {
+		return errors.New("Invalid restart-policy specified")
 	}
 
 	common.CommandPropertySet("ps", appName, property, value, DefaultProperties, GlobalProperties)

plugins/ps/triggers.go

@@ -3,6 +3,7 @@ package ps
 import (
 	"errors"
 	"fmt"
+	"io"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -39,6 +40,24 @@ func TriggerCorePostDeploy(appName string) error {
 	return common.PropertyWrite("ps", appName, "restore", "true")
 }
 
+// TriggerDockerArgsProcessDeploy injects the computed restart policy as a
+// `--restart=` docker option at deploy time. The value is no longer persisted
+// in the docker-options store; it is derived from the app/global restart-policy
+// property on every deploy.
+func TriggerDockerArgsProcessDeploy(appName string) error {
+	stdin, err := io.ReadAll(os.Stdin)
+	if err != nil {
+		return err
+	}
+
+	if _, err := os.Stdout.Write(stdin); err != nil {
+		return err
+	}
+
+	fmt.Printf(" --restart=%s", reportComputedRestartPolicy(appName))
+	return nil
+}
+
 // TriggerCorePostExtract ensures that the main Procfile is the one specified by procfile-path
 func TriggerCorePostExtract(appName string, sourceWorkDir string) error {
 	destination := common.GetAppDataDirectory("ps", appName)
@@ -104,16 +123,22 @@ func TriggerInstall() error {
 	}
 
 	for _, appName := range apps {
-		policies, err := getRestartPolicy(appName)
+		policy, err := getRestartPolicy(appName)
 		if err != nil {
 			return err
 		}
 
-		if len(policies) != 0 {
+		if policy == "" {
 			continue
 		}
 
-		if err := dockeroptions.AddDockerOptionToPhases(appName, []string{"deploy"}, "--restart=on-failure:10"); err != nil {
+		if policy != DefaultProperties["restart-policy"] {
+			if err := common.PropertyWrite("ps", appName, "restart-policy", policy); err != nil {
+				common.LogWarn(err.Error())
+			}
+		}
+
+		if err := dockeroptions.RemoveDockerOptionFromPhases(appName, []string{"deploy"}, fmt.Sprintf("--restart=%s", policy)); err != nil {
 			common.LogWarn(err.Error())
 		}
 	}
@@ -210,13 +235,8 @@ func TriggerPostAppRenameSetup(oldAppName string, newAppName string) error {
 	return common.CloneAppData("ps", oldAppName, newAppName)
 }
 
-// TriggerPostCreate ensures apps have a default restart policy
-// and scale value for web
+// TriggerPostCreate ensures apps have a default scale value for web
 func TriggerPostCreate(appName string) error {
-	if err := dockeroptions.AddDockerOptionToPhases(appName, []string{"deploy"}, "--restart=on-failure:10"); err != nil {
-		return err
-	}
-
 	if err := common.CreateAppDataDirectory("ps", appName); err != nil {
 		return err
 	}
@@ -319,6 +339,7 @@ func TriggerPsSetScale(appName string, skipDeploy bool, clearExisting bool, proc
 
 func TriggerPsGetProperty(appName string, property string) error {
 	computedValueMap := map[string]common.ReportFunc{
+		"restart-policy":       reportComputedRestartPolicy,
 		"restore":              reportRestore,
 		"skip-deploy":          reportComputedSkipDeploy,
 		"stop-timeout-seconds": reportComputedStopTimeoutSeconds,

tests/unit/docker-options.bats

@@ -193,7 +193,6 @@ teardown() {
   echo "status: $status"
   assert_success
   assert_output_contains "-v /tmp" 0
-  assert_output_contains "Docker options deploy:         --restart=on-failure:10"
 }
 
 @test "(docker-options) docker-options:remove (build phase)" {

tests/unit/ps-general-1.bats

@@ -434,6 +434,11 @@ web
   run /bin/bash -c "dokku ps:report $TEST_APP --ps-restart-policy"
   echo "output: $output"
   echo "status: $status"
+  assert_output ""
+
+  run /bin/bash -c "dokku ps:report $TEST_APP --ps-computed-restart-policy"
+  echo "output: $output"
+  echo "status: $status"
   assert_output "on-failure:10"
 }
 
@@ -448,9 +453,86 @@ web
     echo "output: $output"
     echo "status: $status"
     assert_output "$policy"
+
+    run /bin/bash -c "dokku ps:report $TEST_APP --ps-computed-restart-policy"
+    echo "output: $output"
+    echo "status: $status"
+    assert_output "$policy"
   done
 }
 
+@test "(ps:restart-policy) ps:set restart-policy unset" {
+  run /bin/bash -c "dokku ps:set $TEST_APP restart-policy on-failure:5"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku ps:report $TEST_APP --ps-restart-policy"
+  echo "output: $output"
+  echo "status: $status"
+  assert_output "on-failure:5"
+
+  run /bin/bash -c "dokku ps:set $TEST_APP restart-policy"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku ps:report $TEST_APP --ps-restart-policy"
+  echo "output: $output"
+  echo "status: $status"
+  assert_output ""
+
+  run /bin/bash -c "dokku ps:report $TEST_APP --ps-computed-restart-policy"
+  echo "output: $output"
+  echo "status: $status"
+  assert_output "on-failure:10"
+}
+
+@test "(ps:restart-policy) invalid policy" {
+  run /bin/bash -c "dokku ps:set $TEST_APP restart-policy bogus"
+  echo "output: $output"
+  echo "status: $status"
+  assert_failure
+  assert_output_contains "Invalid restart-policy specified"
+}
+
+@test "(ps:restart-policy) global policy" {
+  run /bin/bash -c "dokku ps:set --global restart-policy always"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku ps:report $TEST_APP --ps-global-restart-policy"
+  echo "output: $output"
+  echo "status: $status"
+  assert_output "always"
+
+  run /bin/bash -c "dokku ps:report $TEST_APP --ps-restart-policy"
+  echo "output: $output"
+  echo "status: $status"
+  assert_output ""
+
+  run /bin/bash -c "dokku ps:report $TEST_APP --ps-computed-restart-policy"
+  echo "output: $output"
+  echo "status: $status"
+  assert_output "always"
+
+  run /bin/bash -c "dokku ps:set $TEST_APP restart-policy no"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku ps:report $TEST_APP --ps-computed-restart-policy"
+  echo "output: $output"
+  echo "status: $status"
+  assert_output "no"
+
+  run /bin/bash -c "dokku ps:set --global restart-policy"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+}
+
 @test "(ps:restart-policy) deployed policy" {
   test_restart_policy="on-failure:20"
   run /bin/bash -c "dokku ps:set $TEST_APP restart-policy $test_restart_policy"
@@ -478,6 +560,22 @@ web
   echo "output: $output"
   echo "status: $status"
   assert_output "$test_restart_policy"
+
+  run /bin/bash -c "dokku ps:set $TEST_APP restart-policy"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku ps:rebuild $TEST_APP"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  CID=$(<$DOKKU_ROOT/$TEST_APP/CONTAINER.web.1)
+  run /bin/bash -c "docker inspect -f '{{ .HostConfig.RestartPolicy.Name }}:{{ .HostConfig.RestartPolicy.MaximumRetryCount }}' $CID"
+  echo "output: $output"
+  echo "status: $status"
+  assert_output "on-failure:10"
 }
 
 procfile_line_endings_to_windows() {