From fe4d80a82b7d4d4d8a92d6b3e3f6584b8f2268ba Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Fri, 25 Nov 2022 00:41:55 -0500 Subject: [PATCH] refactor: cleanup zero'd out processes when a Procfile omitting those process types is set This change also moves the referenced Procfile out to a host path once on deploy vs potentially several times, which should speed up deploys a small amount and simplify reasoning about the file. Closes #5112 --- docs/appendices/0.29.0-migration-guide.md | 3 +- docs/development/plugin-triggers.md | 30 ------ plugins/app-json/functions.go | 8 -- plugins/common/io.go | 18 +++- plugins/ps/Makefile | 2 +- plugins/ps/functions.go | 75 +++++++-------- plugins/ps/src/triggers/triggers.go | 11 --- plugins/ps/triggers.go | 111 ++++++++-------------- tests/unit/ps-general-2.bats | 47 +++++++++ 9 files changed, 139 insertions(+), 166 deletions(-) diff --git a/docs/appendices/0.29.0-migration-guide.md b/docs/appendices/0.29.0-migration-guide.md index a8a1ec347..b02e83673 100644 --- a/docs/appendices/0.29.0-migration-guide.md +++ b/docs/appendices/0.29.0-migration-guide.md @@ -8,4 +8,5 @@ ## Removals -- The `DOKKU_WAIT_TO_RETIRE` environment variable has been migrated to a `checks` property named `wait-to-retire` and will be ignored if set as an environment variable. \ No newline at end of file +- The `DOKKU_WAIT_TO_RETIRE` environment variable has been migrated to a `checks` property named `wait-to-retire` and will be ignored if set as an environment variable. +- The `Procfile` is now extracted when source code is extracted for a build and not from the built image. Users can specify alternative paths via the `procfile-path` property of the `ps` plugin. See the [process management documentation](/docs/processes/process-management.md#changing-the-procfile-location) for more information on how to configure the `Procfile` path for your application. diff --git a/docs/development/plugin-triggers.md b/docs/development/plugin-triggers.md index 74f5cd255..72293c3a3 100644 --- a/docs/development/plugin-triggers.md +++ b/docs/development/plugin-triggers.md @@ -1783,21 +1783,6 @@ APP="$1"; curl "https://dokku.me/starting/${APP}" || true ``` -### `procfile-extract` - -- Description: Extracts a Procfile from a given image to a path -- Invoked by: `internally` -- Arguments: `$APP $IMAGE` -- Example: - -```shell -#!/usr/bin/env bash - -set -eo pipefail; [[ $DOKKU_TRACE ]] && set -x - -# TODO -``` - ### `procfile-get-command` - Description: Fetches the command for a specific process type @@ -1813,21 +1798,6 @@ set -eo pipefail; [[ $DOKKU_TRACE ]] && set -x # TODO ``` -### `procfile-remove` - -- Description: Removes the extracted Procfile -- Invoked by: `internally` -- Arguments: `$APP` -- Example: - -```shell -#!/usr/bin/env bash - -set -eo pipefail; [[ $DOKKU_TRACE ]] && set -x - -# TODO -``` - ### `proxy-build-config` - Description: Builds the proxy implementation configuration for a given app diff --git a/plugins/app-json/functions.go b/plugins/app-json/functions.go index 97e97cb64..fe8c47c9b 100644 --- a/plugins/app-json/functions.go +++ b/plugins/app-json/functions.go @@ -122,14 +122,6 @@ func getPhaseScript(appName string, phase string) (string, error) { // getReleaseCommand extracts the release command from a given app's procfile func getReleaseCommand(appName string, image string) string { - err := common.SuppressOutput(func() error { - return common.PlugnTrigger("procfile-extract", []string{appName, image}...) - }) - - if err != nil { - return "" - } - processType := "release" port := "5000" b, _ := common.PlugnTriggerOutput("procfile-get-command", []string{appName, processType, port}...) diff --git a/plugins/common/io.go b/plugins/common/io.go index 8bf1bbe4d..46b8a3ae7 100644 --- a/plugins/common/io.go +++ b/plugins/common/io.go @@ -15,12 +15,12 @@ import ( func CatFile(filename string) { slice, err := FileToSlice(filename) if err != nil { - LogDebug(fmt.Sprintf("Error cat'ing file %s: %s", filename, err.Error())) + LogWarn(fmt.Sprintf("Error cat'ing file %s: %s", filename, err.Error())) return } for _, line := range slice { - LogDebug(fmt.Sprintf("line: '%s'", line)) + LogWarn(fmt.Sprintf("line: '%s'", line)) } } @@ -209,6 +209,20 @@ func SetPermissions(path string, fileMode os.FileMode) error { return os.Chown(path, uid, gid) } +// TouchFile creates an empty file at the specified path +func TouchFile(filename string) error { + mode := os.FileMode(0600) + file, err := os.OpenFile(filename, os.O_RDWR|os.O_CREATE|os.O_TRUNC, mode) + if err != nil { + return err + } + defer file.Close() + + file.Chmod(mode) + SetPermissions(filename, mode) + return nil +} + // WriteSliceToFile writes a slice of strings to a file func WriteSliceToFile(filename string, lines []string) error { mode := os.FileMode(0600) diff --git a/plugins/ps/Makefile b/plugins/ps/Makefile index c8610988d..dbf6e3221 100644 --- a/plugins/ps/Makefile +++ b/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-extract triggers/post-stop triggers/pre-deploy triggers/procfile-extract triggers/procfile-get-command triggers/procfile-remove triggers/ps-can-scale triggers/ps-current-scale triggers/ps-set-scale triggers/report +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-stop triggers/pre-deploy triggers/procfile-get-command triggers/ps-can-scale triggers/ps-current-scale triggers/ps-set-scale triggers/report BUILD = commands subcommands triggers PLUGIN_NAME = ps diff --git a/plugins/ps/functions.go b/plugins/ps/functions.go index ea9aed9b5..e4f9e5bf9 100644 --- a/plugins/ps/functions.go +++ b/plugins/ps/functions.go @@ -20,33 +20,6 @@ func canScaleApp(appName string) bool { return common.ToBool(canScale) } -func extractProcfile(appName, image string) error { - destination := getProcfilePath(appName) - common.CopyFromImage(appName, image, "Procfile", destination) - if !common.FileExists(destination) { - common.LogInfo1Quiet("No Procfile found in app image") - return nil - } - - common.LogInfo1Quiet("App Procfile file found") - checkCmd := common.NewShellCmd(strings.Join([]string{ - "procfile-util", - "check", - "--procfile", - destination, - }, " ")) - var stderr bytes.Buffer - checkCmd.ShowOutput = false - checkCmd.Command.Stderr = &stderr - _, err := checkCmd.Output() - - if err != nil { - return fmt.Errorf(strings.TrimSpace(stderr.String())) - } - - return nil -} - func getProcessStatus(appName string) map[string]string { statuses := make(map[string]string) containerFiles := common.ListFilesWithPrefix(common.AppRoot(appName), "CONTAINER.") @@ -127,6 +100,19 @@ func getRunningState(appName string) string { func hasProcfile(appName string) bool { procfilePath := getProcfilePath(appName) + common.LogWarn("Checking if missing Procfile") + if common.FileExists(fmt.Sprintf("%s.%s.missing", procfilePath, os.Getenv("DOKKU_PID"))) { + common.LogWarn("Procfile is missing") + return false + } + + common.LogWarn("Checking for process-specific Procfile") + if common.FileExists(fmt.Sprintf("%s.%s", procfilePath, os.Getenv("DOKKU_PID"))) { + common.LogWarn("Process-specific Procfile exists") + return true + } + + common.LogWarn("Checking for default Procfile") return common.FileExists(procfilePath) } @@ -208,15 +194,6 @@ func getFormations(appName string) (FormationSlice, error) { return parseProcessTuples(processTuples) } -func removeProcfile(appName string) error { - procfile := getProcfilePath(appName) - if !common.FileExists(procfile) { - return nil - } - - return os.Remove(procfile) -} - func restorePrep() error { if err := common.PlugnTrigger("proxy-clear-config", []string{"--all"}...); err != nil { return fmt.Errorf("Error clearing proxy config: %s", err) @@ -288,6 +265,16 @@ func scaleSet(appName string, skipDeploy bool, clearExisting bool, processTuples return nil } +func getProcessSpecificProcfile(appName string) string { + existingProcfile := getProcfilePath(appName) + processSpecificProcfile := fmt.Sprintf("%s.%s", existingProcfile, os.Getenv("DOKKU_PID")) + if common.FileExists(processSpecificProcfile) { + return processSpecificProcfile + } + + return existingProcfile +} + func updateScale(appName string, clearExisting bool, formationUpdates FormationSlice) error { formations := FormationSlice{} if !clearExisting { @@ -302,21 +289,23 @@ func updateScale(appName string, clearExisting bool, formationUpdates FormationS } } - procfilePath := getProcfilePath(appName) - procfileExists := hasProcfile(appName) validProcessTypes := make(map[string]bool) - if procfileExists { + if hasProcfile(appName) { var err error - validProcessTypes, err = processesInProcfile(procfilePath) + validProcessTypes, err = processesInProcfile(getProcessSpecificProcfile(appName)) if err != nil { return err } } + if common.FileExists(getProcessSpecificProcfile(appName)) { + common.CatFile(getProcessSpecificProcfile(appName)) + } + foundProcessTypes := map[string]bool{} updatedFormation := FormationSlice{} for _, formation := range formationUpdates { - if procfileExists && !validProcessTypes[formation.ProcessType] && formation.Quantity != 0 { + if hasProcfile(appName) && !validProcessTypes[formation.ProcessType] && formation.Quantity != 0 { return fmt.Errorf("%s is not a valid process name to scale up", formation.ProcessType) } @@ -352,6 +341,10 @@ func updateScale(appName string, clearExisting bool, formationUpdates FormationS values := []string{} for _, formation := range updatedFormation { + if !validProcessTypes[formation.ProcessType] && formation.Quantity == 0 { + continue + } + values = append(values, fmt.Sprintf("%s=%d", formation.ProcessType, formation.Quantity)) } diff --git a/plugins/ps/src/triggers/triggers.go b/plugins/ps/src/triggers/triggers.go index 9593c7d6e..c4170d297 100644 --- a/plugins/ps/src/triggers/triggers.go +++ b/plugins/ps/src/triggers/triggers.go @@ -52,10 +52,6 @@ func main() { case "post-delete": appName := flag.Arg(0) err = ps.TriggerPostDelete(appName) - case "post-extract": - appName := flag.Arg(0) - tmpWorkDir := flag.Arg(1) - err = ps.TriggerPostExtract(appName, tmpWorkDir) case "post-stop": appName := flag.Arg(0) err = ps.TriggerPostStop(appName) @@ -63,18 +59,11 @@ func main() { appName := flag.Arg(0) imageTag := flag.Arg(1) err = ps.TriggerPreDeploy(appName, imageTag) - case "procfile-extract": - appName := flag.Arg(0) - image := flag.Arg(1) - err = ps.TriggerProcfileExtract(appName, image) case "procfile-get-command": appName := flag.Arg(0) processType := flag.Arg(1) port := common.ToInt(flag.Arg(2), 5000) err = ps.TriggerProcfileGetCommand(appName, processType, port) - case "procfile-remove": - appName := flag.Arg(0) - err = ps.TriggerProcfileRemove(appName) case "ps-can-scale": appName := flag.Arg(0) canScale := common.ToBool(flag.Arg(1)) diff --git a/plugins/ps/triggers.go b/plugins/ps/triggers.go index 918f447ae..ace70ad7c 100644 --- a/plugins/ps/triggers.go +++ b/plugins/ps/triggers.go @@ -24,6 +24,22 @@ func TriggerAppRestart(appName string) error { // TriggerCorePostDeploy sets a property to // allow the app to be restored on boot func TriggerCorePostDeploy(appName string) error { + existingProcfile := getProcfilePath(appName) + processSpecificProcfile := fmt.Sprintf("%s.%s", existingProcfile, os.Getenv("DOKKU_PID")) + if common.FileExists(processSpecificProcfile) { + if err := os.Rename(processSpecificProcfile, existingProcfile); err != nil { + return err + } + } else if common.FileExists(fmt.Sprintf("%s.missing", existingProcfile)) { + if err := os.Remove(fmt.Sprintf("%s.missing", existingProcfile)); err != nil { + return err + } + + if err := os.Remove(existingProcfile); err != nil { + return err + } + } + entries := map[string]string{ "DOKKU_APP_RESTORE": "1", } @@ -36,26 +52,36 @@ func TriggerCorePostDeploy(appName string) error { // TriggerCorePostExtract ensures that the main Procfile is the one specified by procfile-path func TriggerCorePostExtract(appName string, sourceWorkDir string) error { procfilePath := strings.Trim(reportComputedProcfilePath(appName), "/") - if procfilePath == "" || procfilePath == "Procfile" { - return nil + if procfilePath == "" { + procfilePath = "Procfile" } - defaultProcfilePath := path.Join(sourceWorkDir, "Procfile") - if common.FileExists(defaultProcfilePath) { - if err := os.Remove(defaultProcfilePath); err != nil { - return fmt.Errorf("Unable to remove existing Procfile: %v", err.Error()) + existingProcfile := getProcfilePath(appName) + + files, err := filepath.Glob(fmt.Sprintf("%s.*", existingProcfile)) + if err != nil { + return err + } + for _, f := range files { + if err := os.Remove(f); err != nil { + return err } } - fullProcfilePath := path.Join(sourceWorkDir, procfilePath) - if !common.FileExists(fullProcfilePath) { - return nil + repoProcfilePath := path.Join(sourceWorkDir, procfilePath) + processSpecificProcfile := fmt.Sprintf("%s.%s", existingProcfile, os.Getenv("DOKKU_PID")) + if !common.FileExists(repoProcfilePath) { + return common.TouchFile(fmt.Sprintf("%s.missing", processSpecificProcfile)) } - if err := copy.Copy(fullProcfilePath, path.Join(sourceWorkDir, "Procfile")); err != nil { - return fmt.Errorf("Unable to move specified Procfile into place: %v", err.Error()) + if err := copy.Copy(repoProcfilePath, processSpecificProcfile); err != nil { + return fmt.Errorf("Unable to extract Procfile: %v", err.Error()) } + b, err := sh.Command("procfile-util", "check", "-P", processSpecificProcfile).CombinedOutput() + if err != nil { + return fmt.Errorf(strings.TrimSpace(string(b[:]))) + } return nil } @@ -137,6 +163,7 @@ func TriggerPostAppCloneSetup(oldAppName string, newAppName string) error { return err } + // TODO: Copy data dir return nil } @@ -159,6 +186,7 @@ func TriggerPostAppRenameSetup(oldAppName string, newAppName string) error { return err } + // TODO: Move data dir return nil } @@ -200,20 +228,6 @@ func TriggerPostDelete(appName string) error { return propertyErr } -// TriggerPostExtract validates a procfile -func TriggerPostExtract(appName string, tempWorkDir string) error { - procfile := filepath.Join(tempWorkDir, "Procfile") - if !common.FileExists(procfile) { - return nil - } - - b, err := sh.Command("procfile-util", "check", "-P", procfile).CombinedOutput() - if err != nil { - return fmt.Errorf(strings.TrimSpace(string(b[:]))) - } - return nil -} - // TriggerPostStop sets the restore property to false func TriggerPostStop(appName string) error { entries := map[string]string{ @@ -227,16 +241,6 @@ func TriggerPostStop(appName string) error { // TriggerPreDeploy ensures an app has an up to date scale parameters func TriggerPreDeploy(appName string, imageTag string) error { - image := common.GetAppImageName(appName, imageTag, "") - - if err := removeProcfile(appName); err != nil { - return err - } - - if err := extractProcfile(appName, image); err != nil { - return err - } - if err := updateScale(appName, false, FormationSlice{}); err != nil { common.LogDebug(fmt.Sprintf("Error generating scale file: %s", err.Error())) return err @@ -245,41 +249,9 @@ func TriggerPreDeploy(appName string, imageTag string) error { return nil } -// TriggerProcfileExtract extracted the procfile -func TriggerProcfileExtract(appName string, image string) error { - directory := filepath.Join(common.MustGetEnv("DOKKU_LIB_ROOT"), "data", "ps", appName) - if err := os.MkdirAll(directory, 0755); err != nil { - return err - } - - if err := common.SetPermissions(directory, 0755); err != nil { - return err - } - - if err := removeProcfile(appName); err != nil { - return err - } - - return extractProcfile(appName, image) -} - // TriggerProcfileGetCommand fetches a command from the procfile func TriggerProcfileGetCommand(appName string, processType string, port int) error { procfilePath := getProcfilePath(appName) - if !common.FileExists(procfilePath) { - extract := func() error { - image, err := common.GetDeployingAppImageName(appName, "", "") - if err != nil { - return err - } - - return extractProcfile(appName, image) - } - - if err := common.SuppressOutput(extract); err != nil { - return err - } - } command, err := getProcfileCommand(procfilePath, processType, port) if err != nil { return err @@ -292,11 +264,6 @@ func TriggerProcfileGetCommand(appName string, processType string, port int) err return nil } -// TriggerProcfileRemove removes the procfile if it exists -func TriggerProcfileRemove(appName string) error { - return removeProcfile(appName) -} - // TriggerPsCanScale sets whether or not a user can scale an app with ps:scale func TriggerPsCanScale(appName string, canScale bool) error { return common.PropertyWrite("ps", appName, "can-scale", strconv.FormatBool(canScale)) diff --git a/tests/unit/ps-general-2.bats b/tests/unit/ps-general-2.bats index 8ec18ed00..0fc167f5b 100644 --- a/tests/unit/ps-general-2.bats +++ b/tests/unit/ps-general-2.bats @@ -128,3 +128,50 @@ teardown() { assert_success assert_output_contains 'SECRET_KEY:' } + +@test "(ps:scale) remove zerod processes" { + run /bin/bash -c "dokku builder-herokuish:set $TEST_APP allowed true" + echo "output: $output" + echo "status: $status" + assert_success + + run deploy_app python + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku ps:scale $TEST_APP worker=1" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku --quiet ps:scale $TEST_APP" + output=$(echo "$output" | tr -s " ") + echo "output: ($output)" + assert_output $'cron: 0\ncustom: 0\nrelease: 0\nweb: 1\nworker: 1' + + run /bin/bash -c "dokku ps:scale $TEST_APP worker=0" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku --quiet ps:scale $TEST_APP" + output=$(echo "$output" | tr -s " ") + echo "output: ($output)" + assert_output $'cron: 0\ncustom: 0\nrelease: 0\nweb: 1\nworker: 0' + + run /bin/bash -c "dokku ps:set $TEST_APP procfile-path second.Procfile" + 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 + + run /bin/bash -c "dokku --quiet ps:scale $TEST_APP" + output=$(echo "$output" | tr -s " ") + echo "output: ($output)" + assert_output 'web: 1' +}