From e1f1531d4284a9d168644c40856b8f43f044960b Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sun, 8 Aug 2021 01:56:43 -0400 Subject: [PATCH 1/7] refactor: call golang version of docker cleanup from shell This ensures there is only a single implementation. --- plugins/common/functions | 47 +---------------------------- plugins/common/src/common/common.go | 4 +++ 2 files changed, 5 insertions(+), 46 deletions(-) diff --git a/plugins/common/functions b/plugins/common/functions index 739da92c2..b7fe733cf 100755 --- a/plugins/common/functions +++ b/plugins/common/functions @@ -728,53 +728,8 @@ docker_cleanup() { declare desc="cleans up all exited/dead containers and removes all dangling images" declare APP="$1" FORCE_CLEANUP="$2" local DOKKU_APP_SKIP_CLEANUP - source "$PLUGIN_AVAILABLE_PATH/config/functions" - if [[ "$FORCE_CLEANUP" != "true" ]]; then - [[ -n "$APP" ]] && DOKKU_APP_SKIP_CLEANUP="$(config_get "$APP" DOKKU_SKIP_CLEANUP || true)" - if [[ -n "$DOKKU_SKIP_CLEANUP" ]] || [[ -n "$DOKKU_APP_SKIP_CLEANUP" ]]; then - dokku_log_info1 "DOKKU_SKIP_CLEANUP set. Skipping dokku cleanup" - return - fi - fi - - dokku_log_info1 "Cleaning up..." - local DOKKU_SCHEDULER=$(get_app_scheduler "$APP") - - if [[ "$APP" == "--global" ]]; then - APP="" - fi - - plugn trigger scheduler-docker-cleanup "$DOKKU_SCHEDULER" "$APP" "$FORCE_CLEANUP" - - if [[ -n "$APP" ]]; then - # delete all non-running containers - # shellcheck disable=SC2046 - "$DOCKER_BIN" container rm $("$DOCKER_BIN" container list --all --filter "status=exited" --filter "label=$DOKKU_CONTAINER_LABEL" --filter "label=com.dokku.app-name=$APP" --quiet) &>/dev/null || true - - # delete all dead containers - # shellcheck disable=SC2046 - "$DOCKER_BIN" container rm $("$DOCKER_BIN" container list --all --filter "status=dead" --filter "label=$DOKKU_CONTAINER_LABEL" --filter "label=com.dokku.app-name=$APP" --quiet) &>/dev/null || true - - # delete danging images - # shellcheck disable=SC2046 - "$DOCKER_BIN" image rm $("$DOCKER_BIN" image list --filter 'dangling=true' --filter "label=com.dokku.app-name=$APP" --quiet) &>/dev/null || true - - # delete unused images - "$DOCKER_BIN" image prune --all --filter "label=com.dokku.app-name=$APP" --force &>/dev/null || true - else - # delete all non-running containers - # shellcheck disable=SC2046 - "$DOCKER_BIN" container rm $("$DOCKER_BIN" container list --all --filter "status=exited" --filter "label=$DOKKU_CONTAINER_LABEL" --quiet) &>/dev/null || true - - # delete all dead containers - # shellcheck disable=SC2046 - "$DOCKER_BIN" container rm $("$DOCKER_BIN" container list --all --filter "status=dead" --filter "label=$DOKKU_CONTAINER_LABEL" --quiet) &>/dev/null || true - - # delete danging images - # shellcheck disable=SC2046 - "$DOCKER_BIN" image rm $("$DOCKER_BIN" image list --filter 'dangling=true' --quiet) &>/dev/null || true - fi + "$PLUGIN_CORE_AVAILABLE_PATH/common/common" --quiet docker-cleanup "$APP" } get_available_port() { diff --git a/plugins/common/src/common/common.go b/plugins/common/src/common/common.go index 1562b1b2e..0a4275371 100644 --- a/plugins/common/src/common/common.go +++ b/plugins/common/src/common/common.go @@ -19,6 +19,10 @@ func main() { var err error switch cmd { + case "docker-cleanup": + appName := flag.Arg(1) + force := common.ToBool(flag.Arg(2)) + err = common.DockerCleanup(appName, force) case "is-deployed": appName := flag.Arg(1) if !common.IsDeployed(appName) { From e8d998233093342e813d82b31052456448c05811 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sun, 8 Aug 2021 01:57:44 -0400 Subject: [PATCH 2/7] chore: drop unnecessary scheduler-docker-cleanup hook All the logic was already called from docker-cleanup, so this just lengthened the cleanup time. --- docs/appendices/0.25.0-migration-guide.md | 1 + docs/development/plugin-triggers.md | 19 -------- plugins/common/docker.go | 10 ----- .../scheduler-docker-cleanup | 45 ------------------- 4 files changed, 1 insertion(+), 74 deletions(-) delete mode 100755 plugins/scheduler-docker-local/scheduler-docker-cleanup diff --git a/docs/appendices/0.25.0-migration-guide.md b/docs/appendices/0.25.0-migration-guide.md index 4a59b285b..31c7cc3f0 100644 --- a/docs/appendices/0.25.0-migration-guide.md +++ b/docs/appendices/0.25.0-migration-guide.md @@ -31,3 +31,4 @@ The [dokku-registry](https://github.com/dokku/dokku-registry) plugin is now buil - As of April 2021, Ubuntu 16.04 is no longer an LTS release, and support has been removed. Packages may still install on older versions of Ubuntu, but are no longer tested, and therefore no support will be provided. - The web installer has been removed. Users are now required to setup Dokku via cli commands. This change was made to combat potential security issues where users ignored the web installer, allowing unauthorized users to add their keys to the host. +- The `scheduler-docker-cleanup` plugin trigger has been removed from the scheduler interface, and is no longer invoked. diff --git a/docs/development/plugin-triggers.md b/docs/development/plugin-triggers.md index d2b0416d3..54bc763cc 100644 --- a/docs/development/plugin-triggers.md +++ b/docs/development/plugin-triggers.md @@ -2028,25 +2028,6 @@ DOKKU_SCHEDULER="$1"; APP="$2"; IMAGE_TAG="$3"; # TODO ``` -### `scheduler-docker-cleanup` - -> Warning: The scheduler plugin trigger apis are under development and may change -> between minor releases until the 1.0 release. - -- Description: Allows you to run scheduler commands when dokku cleanup is invoked -- Invoked by: `dokku deploy, dokku cleanup` -- Arguments: `$DOKKU_SCHEDULER $APP $FORCE_CLEANUP` -- Example: - -```shell -#!/usr/bin/env bash - -set -eo pipefail; [[ $DOKKU_TRACE ]] && set -x -DOKKU_SCHEDULER="$1"; APP="$2"; FORCE_CLEANUP="$3"; - -# TODO -``` - ### `scheduler-enter` > Warning: The scheduler plugin trigger apis are under development and may change diff --git a/plugins/common/docker.go b/plugins/common/docker.go index ca2a19340..50dcf8aff 100644 --- a/plugins/common/docker.go +++ b/plugins/common/docker.go @@ -186,20 +186,10 @@ func DockerCleanup(appName string, forceCleanup bool) error { } LogInfo1("Cleaning up...") - scheduler := GetAppScheduler(appName) if appName == "--global" { appName = "" } - forceCleanupArg := "false" - if forceCleanup { - forceCleanupArg = "true" - } - - if err := PlugnTrigger("scheduler-docker-cleanup", []string{scheduler, appName, forceCleanupArg}...); err != nil { - return fmt.Errorf("Failure while cleaning up app: %s", err) - } - // delete all non-running and dead containers exitedContainerIDs, _ := listContainers("exited", appName) deadContainerIDs, _ := listContainers("dead", appName) diff --git a/plugins/scheduler-docker-local/scheduler-docker-cleanup b/plugins/scheduler-docker-local/scheduler-docker-cleanup deleted file mode 100755 index 1496a0b37..000000000 --- a/plugins/scheduler-docker-local/scheduler-docker-cleanup +++ /dev/null @@ -1,45 +0,0 @@ -#!/usr/bin/env bash -set -eo pipefail -[[ $DOKKU_TRACE ]] && set -x -source "$PLUGIN_CORE_AVAILABLE_PATH/common/functions" - -trigger-scheduler-docker-local-scheduler-docker-cleanup() { - declare desc="scheduler-docker-local scheduler-docker-cleanup plugin trigger" - declare trigger="scheduler-docker-cleanup" - declare DOKKU_SCHEDULER="$1" APP="$2" FORCE_CLEANUP="$3" - - if [[ "$DOKKU_SCHEDULER" != "docker-local" ]]; then - return - fi - - if [[ -n "$APP" ]]; then - # delete all non-running containers - # shellcheck disable=SC2046 - "$DOCKER_BIN" container rm $("$DOCKER_BIN" container list --all --filter "status=exited" --filter "label=$DOKKU_CONTAINER_LABEL" --filter "label=com.dokku.app-name=$APP" --quiet) &>/dev/null || true - - # delete all dead containers - # shellcheck disable=SC2046 - "$DOCKER_BIN" container rm $("$DOCKER_BIN" container list --all --filter "status=dead" --filter "label=$DOKKU_CONTAINER_LABEL" --filter "label=com.dokku.app-name=$APP" --quiet) &>/dev/null || true - - # delete danging images - # shellcheck disable=SC2046 - "$DOCKER_BIN" image rm $("$DOCKER_BIN" image list --filter 'dangling=true' --filter "label=com.dokku.app-name=$APP" --quiet) &>/dev/null || true - - # delete unused images - "$DOCKER_BIN" image prune --all --filter "label=com.dokku.app-name=$APP" --force &>/dev/null || true - else - # delete all non-running containers - # shellcheck disable=SC2046 - "$DOCKER_BIN" container rm $("$DOCKER_BIN" container list --all --filter "status=exited" --filter "label=$DOKKU_CONTAINER_LABEL" --quiet) &>/dev/null || true - - # delete all dead containers - # shellcheck disable=SC2046 - "$DOCKER_BIN" container rm $("$DOCKER_BIN" container list --all --filter "status=dead" --filter "label=$DOKKU_CONTAINER_LABEL" --quiet) &>/dev/null || true - - # delete danging images - # shellcheck disable=SC2046 - "$DOCKER_BIN" image rm $("$DOCKER_BIN" image list --filter 'dangling=true' --quiet) &>/dev/null || true - fi -} - -trigger-scheduler-docker-local-scheduler-docker-cleanup "$@" From 7a564f318538afd84de18df644f115b0e98eaeb1 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sun, 8 Aug 2021 02:24:27 -0400 Subject: [PATCH 3/7] fix: do not retire the original image By removing the image cleanup registration, we avoid cases where we delete gliderlabs/herokuish, which may cause issues for users in airgapped installations (where the image may not be pullable from a registry). --- plugins/builder-herokuish/builder-build | 2 +- plugins/scheduler-docker-local/scheduler-register-retired | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/builder-herokuish/builder-build b/plugins/builder-herokuish/builder-build index 74d989cc7..f134583b8 100755 --- a/plugins/builder-herokuish/builder-build +++ b/plugins/builder-herokuish/builder-build @@ -41,7 +41,7 @@ trigger-builder-herokuish-builder-build() { fi "$DOCKER_BIN" container commit "${DOCKER_COMMIT_LABEL_ARGS[@]}" "$CID" "$IMAGE" >/dev/null - plugn trigger scheduler-register-retired "$APP" "$CID" + DOKKU_SKIP_IMAGE_CLEANUP_REGISTRATION=1 plugn trigger scheduler-register-retired "$APP" "$CID" [[ -d $DOKKU_APP_CACHE_DIR ]] || mkdir -p "$DOKKU_APP_CACHE_DIR" plugn trigger pre-build-buildpack "$APP" "$SOURCECODE_WORK_DIR" diff --git a/plugins/scheduler-docker-local/scheduler-register-retired b/plugins/scheduler-docker-local/scheduler-register-retired index eb26b2951..dd813729e 100755 --- a/plugins/scheduler-docker-local/scheduler-register-retired +++ b/plugins/scheduler-docker-local/scheduler-register-retired @@ -18,7 +18,7 @@ trigger-scheduler-docker-local-scheduler-register-retired() { fi fn-scheduler-docker-local-register-retired "container" "$APP" "$CONTAINER_ID" "$WAIT" - if [[ -n "$IMAGE_ID" ]]; then + if [[ -n "$IMAGE_ID" ]] && [[ -z "$DOKKU_SKIP_IMAGE_CLEANUP_REGISTRATION" ]]; then fn-scheduler-docker-local-register-retired "image" "$APP" "$IMAGE_ID" "$WAIT" fi } From 9e18736475c50ff260e0d1390fb942af013f9f08 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sun, 8 Aug 2021 03:04:25 -0400 Subject: [PATCH 4/7] feat: add ability to scope retire call to a single app Also cleanup the log output a bit for the ps:retire command. --- docs/development/plugin-triggers.md | 4 ++-- plugins/ps/src/subcommands/subcommands.go | 3 ++- plugins/ps/subcommands.go | 12 +++++++--- .../scheduler-docker-local/internal-functions | 22 ++++++++++++++----- .../scheduler-docker-local/scheduler-retire | 5 +++-- 5 files changed, 32 insertions(+), 14 deletions(-) diff --git a/docs/development/plugin-triggers.md b/docs/development/plugin-triggers.md index 54bc763cc..ef5372bb4 100644 --- a/docs/development/plugin-triggers.md +++ b/docs/development/plugin-triggers.md @@ -2169,14 +2169,14 @@ CONTAINER_ID="$2"; - Description: Allows you to run scheduler commands when containers should be force retired from the system - Invoked by: `dokku run` -- Arguments: `$DOKKU_SCHEDULER` +- Arguments: `$DOKKU_SCHEDULER $APP` - Example: ```shell #!/usr/bin/env bash set -eo pipefail; [[ $DOKKU_TRACE ]] && set -x -DOKKU_SCHEDULER="$1"; +DOKKU_SCHEDULER="$1"; APP="$2"; # TODO ``` diff --git a/plugins/ps/src/subcommands/subcommands.go b/plugins/ps/src/subcommands/subcommands.go index b77b0d7ba..306daea8a 100644 --- a/plugins/ps/src/subcommands/subcommands.go +++ b/plugins/ps/src/subcommands/subcommands.go @@ -56,7 +56,8 @@ func main() { case "retire": args := flag.NewFlagSet("ps:retire", flag.ExitOnError) args.Parse(os.Args[2:]) - err = ps.CommandRetire() + appName := args.Arg(0) + err = ps.CommandRetire(appName) case "scale": args := flag.NewFlagSet("ps:scale", flag.ExitOnError) skipDeploy := args.Bool("skip-deploy", false, "--skip-deploy: skip deploy of the app") diff --git a/plugins/ps/subcommands.go b/plugins/ps/subcommands.go index 1ba4703d2..e34698733 100644 --- a/plugins/ps/subcommands.go +++ b/plugins/ps/subcommands.go @@ -93,9 +93,14 @@ func CommandRestore(appName string, allApps bool, parallelCount int) error { } // CommandRetire ensures old containers are retired -func CommandRetire() error { +func CommandRetire(appName string) error { lockFile := filepath.Join(common.MustGetEnv("DOKKU_LIB_ROOT"), "data", "ps", "retire") - scheduler := common.GetGlobalScheduler() + scheduler := "" + if appName == "" { + scheduler = common.GetGlobalScheduler() + } else { + scheduler = common.GetAppScheduler(appName) + } fileLock := flock.New(lockFile) locked, err := fileLock.TryLock() @@ -108,7 +113,8 @@ func CommandRetire() error { return fmt.Errorf("Failed to acquire ps:retire lock") } - if err := common.PlugnTrigger("scheduler-retire", []string{scheduler}...); err != nil { + common.LogInfo1("Retiring old containers and images") + if err := common.PlugnTrigger("scheduler-retire", []string{scheduler, appName}...); err != nil { return err } diff --git a/plugins/scheduler-docker-local/internal-functions b/plugins/scheduler-docker-local/internal-functions index d11e966fb..d8355137a 100755 --- a/plugins/scheduler-docker-local/internal-functions +++ b/plugins/scheduler-docker-local/internal-functions @@ -67,10 +67,10 @@ cmd-scheduler-docker-local-report-single() { } fn-scheduler-docker-local-retire-container() { - declare APP="$1" CID="$2" DEAD_TIME="$3" + declare APP="$1" CID="$2" local STATE - dokku_log_info1_quiet "Attempting to retire $APP container $CID" + dokku_log_verbose_quiet "Attempting to retire $APP container $CID" STATE="$("$DOCKER_BIN" container inspect --format "{{ .State.Status }}" "$CID" 2>/dev/null || true)" if [[ -z "$STATE" ]]; then return @@ -108,6 +108,7 @@ fn-scheduler-docker-local-retire-container() { fn-scheduler-docker-local-retire-containers() { local DEAD_CONTAINER_FILE="${DOKKU_LIB_ROOT}/data/scheduler-docker-local/dead-containers" local APP CID CURRENT_TIME DEAD_TIME STATE + declare SCHEDULER="$1" APP="$2" if [[ ! -f "$DEAD_CONTAINER_FILE" ]]; then return @@ -117,15 +118,19 @@ fn-scheduler-docker-local-retire-containers() { while read line; do [[ -z "$line" ]] && continue CURRENT_TIME="$(date +%s)" - APP="$(echo "$line" | cut -d ' ' -f1)" + RETIRE_APP="$(echo "$line" | cut -d ' ' -f1)" CID="$(echo "$line" | cut -d ' ' -f2)" DEAD_TIME="$(echo "$line" | cut -d ' ' -f3)" + if [[ -n "$APP" ]] && [[ "$APP" != "$RETIRE_APP" ]]; then + continue + fi + if [[ "$CURRENT_TIME" -le "$DEAD_TIME" ]]; then continue fi - fn-scheduler-docker-local-retire-container "$APP" "$CID" "$DEAD_TIME" + fn-scheduler-docker-local-retire-container "$RETIRE_APP" "$CID" STATE="$("$DOCKER_BIN" container inspect --format "{{ .State.Status }}" "$CID" 2>/dev/null || true)" if [[ -z "$STATE" ]]; then DEAD_CONTAINERS+=("$CID") @@ -154,6 +159,7 @@ fn-scheduler-docker-local-retire-containers() { fn-scheduler-docker-local-retire-images() { local DEAD_IMAGE_FILE="${DOKKU_LIB_ROOT}/data/scheduler-docker-local/dead-images" local APP IMAGE_ID CURRENT_TIME DEAD_TIME STATE RM_OUTPUT + declare SCHEDULER="$1" APP="$2" if [[ ! -f "$DEAD_IMAGE_FILE" ]]; then return @@ -163,10 +169,14 @@ fn-scheduler-docker-local-retire-images() { while read line; do [[ -z "$line" ]] && continue CURRENT_TIME="$(date +%s)" - APP="$(echo "$line" | cut -d ' ' -f1)" + RETIRE_APP="$(echo "$line" | cut -d ' ' -f1)" IMAGE_ID="$(echo "$line" | cut -d ' ' -f2)" DEAD_TIME="$(echo "$line" | cut -d ' ' -f3)" + if [[ -n "$APP" ]] && [[ "$APP" != "$RETIRE_APP" ]]; then + continue + fi + if [[ "$CURRENT_TIME" -le "$DEAD_TIME" ]]; then continue fi @@ -177,7 +187,7 @@ fn-scheduler-docker-local-retire-images() { continue fi - dokku_log_info1_quiet "Attempting to retire $APP image $IMAGE_ID" + dokku_log_verbose_quiet "Attempting to retire $RETIRE_APP image $IMAGE_ID" if RM_OUTPUT="$("$DOCKER_BIN" image rm "$IMAGE_ID" 2>&1)"; then DEAD_IMAGES+=("$IMAGE_ID") continue diff --git a/plugins/scheduler-docker-local/scheduler-retire b/plugins/scheduler-docker-local/scheduler-retire index cf469956b..8f4b274dd 100755 --- a/plugins/scheduler-docker-local/scheduler-retire +++ b/plugins/scheduler-docker-local/scheduler-retire @@ -7,9 +7,10 @@ source "$PLUGIN_AVAILABLE_PATH/scheduler-docker-local/internal-functions" trigger-scheduler-docker-local-scheduler-retire() { declare desc="retires all old containers once they have aged out" declare trigger="scheduler-retire" + declare SCHEDULER="$1" APP="$2" - fn-scheduler-docker-local-retire-containers - fn-scheduler-docker-local-retire-images + fn-scheduler-docker-local-retire-containers "$SCHEDULER" "$APP" + fn-scheduler-docker-local-retire-images "$SCHEDULER" "$APP" } trigger-scheduler-docker-local-scheduler-retire "$@" From 72eb61d1fcb48c9b6681f5abecde197f8dbb99b1 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sun, 8 Aug 2021 03:04:50 -0400 Subject: [PATCH 5/7] feat: call scheduler-retire when destroying an app This helps ensure we don't keep around old containers related to a given app. --- plugins/apps/functions.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plugins/apps/functions.go b/plugins/apps/functions.go index 8ff1dc541..492ddb448 100644 --- a/plugins/apps/functions.go +++ b/plugins/apps/functions.go @@ -72,6 +72,11 @@ func destroyApp(appName string) error { forceCleanup := true common.DockerCleanup(appName, forceCleanup) + common.LogInfo1("Retiring old containers and images") + if err := common.PlugnTrigger("scheduler-retire", []string{scheduler, appName}...); err != nil { + return err + } + // remove contents for apps that are symlinks to other folders if err := os.RemoveAll(fmt.Sprintf("%v/", common.AppRoot(appName))); err != nil { common.LogWarn(err.Error()) From c58cb02689b743a5b8adf6397b032ec2da0f1485 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sun, 8 Aug 2021 03:31:53 -0400 Subject: [PATCH 6/7] fix: add flag to list --- plugins/common/src/common/common.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/plugins/common/src/common/common.go b/plugins/common/src/common/common.go index 0a4275371..29eb5f207 100644 --- a/plugins/common/src/common/common.go +++ b/plugins/common/src/common/common.go @@ -10,6 +10,7 @@ import ( func main() { quiet := flag.Bool("quiet", false, "--quiet: set DOKKU_QUIET_OUTPUT=1") + global := flag.Bool("global", false, "--global: Whether global or app-specific") flag.Parse() cmd := flag.Arg(0) @@ -22,6 +23,9 @@ func main() { case "docker-cleanup": appName := flag.Arg(1) force := common.ToBool(flag.Arg(2)) + if *global { + appName = "--global" + } err = common.DockerCleanup(appName, force) case "is-deployed": appName := flag.Arg(1) From 5103ee21aae18b1954b60a28b170d38fe51a5c3d Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sun, 8 Aug 2021 03:40:08 -0400 Subject: [PATCH 7/7] chore: drop redundant if guard --- plugins/ps/subcommands.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/plugins/ps/subcommands.go b/plugins/ps/subcommands.go index e34698733..3c4ea73a7 100644 --- a/plugins/ps/subcommands.go +++ b/plugins/ps/subcommands.go @@ -114,11 +114,7 @@ func CommandRetire(appName string) error { } common.LogInfo1("Retiring old containers and images") - if err := common.PlugnTrigger("scheduler-retire", []string{scheduler, appName}...); err != nil { - return err - } - - return nil + return common.PlugnTrigger("scheduler-retire", []string{scheduler, appName}...) } // CommandScale gets or sets how many instances of a given process to run