From fceb412fb11422ea6f31b5d420ff3876b1c05cf6 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sun, 22 Nov 2020 17:12:51 -0500 Subject: [PATCH 1/3] fix: reference extracted Procfile Rather than attempting to always extract the Procfile, reference the one that was extracted by the last deploy. This fixes issues where 'docker container cp' may fail intermittently for lord knows what reason. Closes #4083 --- docs/development/plugin-triggers.md | 2 +- plugins/ps/functions.go | 6 +--- plugins/ps/src/triggers/triggers.go | 3 +- plugins/ps/subcommands.go | 13 +++++--- plugins/ps/triggers.go | 50 ++++++++++------------------- 5 files changed, 28 insertions(+), 46 deletions(-) diff --git a/docs/development/plugin-triggers.md b/docs/development/plugin-triggers.md index 5d96f61b7..e34c27842 100644 --- a/docs/development/plugin-triggers.md +++ b/docs/development/plugin-triggers.md @@ -1511,7 +1511,7 @@ set -eo pipefail; [[ $DOKKU_TRACE ]] && set -x - Description: Removes the extracted Procfile - Invoked by: `internally` -- Arguments: `$APP $PROCFILE_PATH` +- Arguments: `$APP` - Example: ```shell diff --git a/plugins/ps/functions.go b/plugins/ps/functions.go index e69dd52e6..f6a47bbe8 100644 --- a/plugins/ps/functions.go +++ b/plugins/ps/functions.go @@ -19,11 +19,7 @@ func canScaleApp(appName string) bool { return !common.FileExists(getScalefileExtractedPath(appName)) } -func extractProcfile(appName, image string, procfilePath string) error { - if err := removeProcfile(appName); err != nil { - return err - } - +func extractProcfile(appName, image string) error { destination := getProcfilePath(appName) common.CopyFromImage(appName, image, "Procfile", destination) if !common.FileExists(destination) { diff --git a/plugins/ps/src/triggers/triggers.go b/plugins/ps/src/triggers/triggers.go index d58084abe..2bd5e1b7e 100644 --- a/plugins/ps/src/triggers/triggers.go +++ b/plugins/ps/src/triggers/triggers.go @@ -62,8 +62,7 @@ func main() { err = ps.TriggerProcfileGetCommand(appName, processType, port) case "procfile-remove": appName := flag.Arg(0) - procfilePath := flag.Arg(1) - err = ps.TriggerProcfileRemove(appName, procfilePath) + err = ps.TriggerProcfileRemove(appName) case "report": appName := flag.Arg(0) err = ps.ReportSingleApp(appName, "") diff --git a/plugins/ps/subcommands.go b/plugins/ps/subcommands.go index ab085160d..90cd722f9 100644 --- a/plugins/ps/subcommands.go +++ b/plugins/ps/subcommands.go @@ -144,11 +144,14 @@ func CommandScale(appName string, skipDeploy bool, processTuples []string) error procfilePath := getProcfilePath(appName) if !common.FileExists(procfilePath) { - image := common.GetAppImageRepo(appName) - common.SuppressOutput(func() error { - extractProcfile(appName, image, procfilePath) - return nil - }) + extract := func() error { + image := common.GetDeployingAppImageName(appName, "", "") + return extractProcfile(appName, image) + } + + if err := common.SuppressOutput(extract); err != nil { + return err + } } if !hasScaleFile(appName) { diff --git a/plugins/ps/triggers.go b/plugins/ps/triggers.go index 6d2322f71..dcbfb9a63 100644 --- a/plugins/ps/triggers.go +++ b/plugins/ps/triggers.go @@ -17,13 +17,9 @@ func TriggerAppRestart(appName string) error { return Restart(appName) } -// TriggerCorePostDeploy removes extracted procfiles -// and sets a property to allow the app to be restored on boot +// TriggerCorePostDeploy sets a property to +// allow the app to be restored on boot func TriggerCorePostDeploy(appName string) error { - if err := removeProcfile(appName); err != nil { - return err - } - entries := map[string]string{ "DOKKU_APP_RESTORE": "1", } @@ -148,11 +144,12 @@ func TriggerPostStop(appName string) error { // TriggerPreDeploy ensures an app has an up to date scale file func TriggerPreDeploy(appName string, imageTag string) error { - image := common.GetAppImageRepo(appName) - removeProcfile(appName) + image := common.GetDeployingAppImageName(appName, imageTag, "") + if err := removeProcfile(appName); err != nil { + return err + } - procfilePath := getProcfilePath(appName) - if err := extractProcfile(appName, image, procfilePath); err != nil { + if err := extractProcfile(appName, image); err != nil { return err } @@ -174,27 +171,23 @@ func TriggerProcfileExtract(appName string, image string) error { return err } - procfilePath := getProcfilePath(appName) - - if common.FileExists(procfilePath) { - if err := common.PlugnTrigger("procfile-remove", []string{appName, procfilePath}...); err != nil { - return err - } + if err := removeProcfile(appName); err != nil { + return err } - return extractProcfile(appName, image, procfilePath) + 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) { - image := common.GetDeployingAppImageName(appName, "", "") - err := common.SuppressOutput(func() error { - return common.PlugnTrigger("procfile-extract", []string{appName, image}...) - }) + extract := func() error { + image := common.GetDeployingAppImageName(appName, "", "") + return extractProcfile(appName, image) + } - if err != nil { + if err := common.SuppressOutput(extract); err != nil { return err } } @@ -211,15 +204,6 @@ func TriggerProcfileGetCommand(appName string, processType string, port int) err } // TriggerProcfileRemove removes the procfile if it exists -func TriggerProcfileRemove(appName string, procfilePath string) error { - if procfilePath == "" { - procfilePath = getProcfilePath(appName) - } - - if !common.FileExists(procfilePath) { - return nil - } - - os.Remove(procfilePath) - return nil +func TriggerProcfileRemove(appName string) error { + return removeProcfile(appName) } From 97513dee918aa71647853582cf478366be82b816 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Mon, 23 Nov 2020 00:11:00 -0500 Subject: [PATCH 2/3] fix: handle case where deploying app image does not exist This happens if ps:scale is called before a deploy. Also refactor GetDeployingAppImageName() to return an error as the second return parameter. --- docs/appendices/0.22.0-migration-guide.md | 1 + plugins/app-json/appjson.go | 5 ++++- plugins/common/common.go | 14 +++++++------- plugins/ps/subcommands.go | 6 +++++- plugins/ps/triggers.go | 12 ++++++++++-- 5 files changed, 27 insertions(+), 11 deletions(-) diff --git a/docs/appendices/0.22.0-migration-guide.md b/docs/appendices/0.22.0-migration-guide.md index a4e4dc7f4..53c88b60d 100644 --- a/docs/appendices/0.22.0-migration-guide.md +++ b/docs/appendices/0.22.0-migration-guide.md @@ -5,6 +5,7 @@ - Underscores are no longer valid characters in app names. Please rename applications before upgrading. - Process type names specified in Procfile may no longer use characters not valid in DNS Label Names ([RFC 1123](https://tools.ietf.org/html/rfc1123)). - The minimum Docker version is now 17.05.0. +- The `common.GetDeployingAppImageName()` function now returns an `error` as the second return argument instead of calling `common.LogFail()` internally. ## Removals diff --git a/plugins/app-json/appjson.go b/plugins/app-json/appjson.go index bcb077ad4..3080d0fca 100644 --- a/plugins/app-json/appjson.go +++ b/plugins/app-json/appjson.go @@ -138,7 +138,10 @@ func getDokkuAppShell(appName string) string { } func executeScript(appName string, imageTag string, phase string) error { - image := common.GetDeployingAppImageName(appName, imageTag, "") + image, err := common.GetDeployingAppImageName(appName, imageTag, "") + if err != nil { + common.LogFail(err.Error()) + } command := "" phaseSource := "" if phase == "release" { diff --git a/plugins/common/common.go b/plugins/common/common.go index 74f89f85b..638448524 100644 --- a/plugins/common/common.go +++ b/plugins/common/common.go @@ -89,26 +89,26 @@ func GetGlobalScheduler() string { } // GetDeployingAppImageName returns deploying image identifier for a given app, tag tuple. validate if tag is presented -func GetDeployingAppImageName(appName, imageTag, imageRepo string) (imageName string) { +func GetDeployingAppImageName(appName, imageTag, imageRepo string) (string, error) { if appName == "" { LogFail("(GetDeployingAppImageName) APP must not be empty") } b, err := PlugnTriggerOutput("deployed-app-repository", []string{appName}...) if err != nil { - LogFail(err.Error()) + return "", err } imageRemoteRepository := string(b[:]) b, err = PlugnTriggerOutput("deployed-app-image-tag", []string{appName}...) if err != nil { - LogFail(err.Error()) + return "", err } newImageTag := string(b[:]) b, err = PlugnTriggerOutput("deployed-app-image-repo", []string{appName}...) if err != nil { - LogFail(err.Error()) + return "", err } newImageRepo := string(b[:]) @@ -125,11 +125,11 @@ func GetDeployingAppImageName(appName, imageTag, imageRepo string) (imageName st imageTag = "latest" } - imageName = fmt.Sprintf("%s%s:%s", imageRemoteRepository, imageRepo, imageTag) + imageName := fmt.Sprintf("%s%s:%s", imageRemoteRepository, imageRepo, imageTag) if !VerifyImage(imageName) { - LogFail(fmt.Sprintf("App image (%s) not found", imageName)) + return "", fmt.Errorf("App image (%s) not found", imageName) } - return + return imageName, nil } // GetAppImageRepo is the central definition of a dokku image repo pattern diff --git a/plugins/ps/subcommands.go b/plugins/ps/subcommands.go index 90cd722f9..01d7cf156 100644 --- a/plugins/ps/subcommands.go +++ b/plugins/ps/subcommands.go @@ -145,7 +145,11 @@ func CommandScale(appName string, skipDeploy bool, processTuples []string) error procfilePath := getProcfilePath(appName) if !common.FileExists(procfilePath) { extract := func() error { - image := common.GetDeployingAppImageName(appName, "", "") + image, err := common.GetDeployingAppImageName(appName, "", "") + if err != nil { + return nil + } + return extractProcfile(appName, image) } diff --git a/plugins/ps/triggers.go b/plugins/ps/triggers.go index dcbfb9a63..e72246aca 100644 --- a/plugins/ps/triggers.go +++ b/plugins/ps/triggers.go @@ -144,7 +144,11 @@ func TriggerPostStop(appName string) error { // TriggerPreDeploy ensures an app has an up to date scale file func TriggerPreDeploy(appName string, imageTag string) error { - image := common.GetDeployingAppImageName(appName, imageTag, "") + image, err := common.GetDeployingAppImageName(appName, imageTag, "") + if err != nil { + return err + } + if err := removeProcfile(appName); err != nil { return err } @@ -183,7 +187,11 @@ func TriggerProcfileGetCommand(appName string, processType string, port int) err procfilePath := getProcfilePath(appName) if !common.FileExists(procfilePath) { extract := func() error { - image := common.GetDeployingAppImageName(appName, "", "") + image, err := common.GetDeployingAppImageName(appName, "", "") + if err != nil { + return err + } + return extractProcfile(appName, image) } From bf170236399a94c6b9e8716e7b47ff49ceb2b32e Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Mon, 23 Nov 2020 00:11:10 -0500 Subject: [PATCH 3/3] refactor: simplify ps:scale setup --- plugins/ps/subcommands.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/plugins/ps/subcommands.go b/plugins/ps/subcommands.go index 01d7cf156..49a61b616 100644 --- a/plugins/ps/subcommands.go +++ b/plugins/ps/subcommands.go @@ -158,15 +158,11 @@ func CommandScale(appName string, skipDeploy bool, processTuples []string) error } } - if !hasScaleFile(appName) { - err := common.SuppressOutput(func() error { + if !hasScaleFile(appName) || common.FileExists(procfilePath) { + update := func() error { return updateScalefile(appName, make(map[string]int)) - }) - if err != nil { - return err } - } else if common.FileExists(procfilePath) { - if err := updateScalefile(appName, make(map[string]int)); err != nil { + if err := common.SuppressOutput(update); err != nil { return err } }