From 0786954d34aca76c49c7d8550ad9788f79fc97fd Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Thu, 26 Nov 2020 03:21:45 -0500 Subject: [PATCH 1/3] feat: schedule related images for cleanup when retiring containers If the containers no longer exist and the images are unused, we should remove the images as well. Note that images may sometimes stick around for a little while, depending on the retire schedule and the wait time specified for the container. Closes #4104 --- docs/development/plugin-triggers.md | 2 +- .../scheduler-docker-local/internal-functions | 110 +++++++++++++++++- .../scheduler-docker-local/scheduler-deploy | 2 +- .../scheduler-register-retired | 6 +- .../scheduler-docker-local/scheduler-retire | 44 +------ 5 files changed, 116 insertions(+), 48 deletions(-) diff --git a/docs/development/plugin-triggers.md b/docs/development/plugin-triggers.md index 53ea488ec..fab7d8406 100644 --- a/docs/development/plugin-triggers.md +++ b/docs/development/plugin-triggers.md @@ -1914,7 +1914,7 @@ DOKKU_SCHEDULER="$1"; APP="$2"; CONTAINER_ID="$3"; > Warning: The scheduler plugin trigger apis are under development and may change > between minor releases until the 1.0 release. -- Description: Allows scheduling retiring a local container +- Description: Allows scheduling retiring a local container and any related images - Invoked by: `internally` - Arguments: `$APP $CONTAINER_ID` - Example: diff --git a/plugins/scheduler-docker-local/internal-functions b/plugins/scheduler-docker-local/internal-functions index 030714df3..9f01be4e7 100755 --- a/plugins/scheduler-docker-local/internal-functions +++ b/plugins/scheduler-docker-local/internal-functions @@ -105,12 +105,116 @@ fn-scheduler-docker-local-retire-container() { fi } -fn-scheduler-docker-local-register-retired-container() { - declare APP="$1" CID="$2" WAIT="$3" +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 + + if [[ ! -f "$DEAD_CONTAINER_FILE" ]]; then + return + fi + + DEAD_CONTAINERS=() + while read line; do + [[ -z "$line" ]] && continue + CURRENT_TIME="$(date +%s)" + APP="$(echo "$line" | cut -d ' ' -f1)" + CID="$(echo "$line" | cut -d ' ' -f2)" + DEAD_TIME="$(echo "$line" | cut -d ' ' -f3)" + + if [[ "$CURRENT_TIME" -le "$DEAD_TIME" ]]; then + continue + fi + + fn-scheduler-docker-local-retire-container "$APP" "$CID" "$DEAD_TIME" + STATE="$("$DOCKER_BIN" container inspect --format "{{ .State.Status }}" "$CID" 2>/dev/null || true)" + if [[ -z "$STATE" ]]; then + DEAD_CONTAINERS+=("$CID") + continue + fi + + if [[ "$STATE" == "running" ]]; then + dokku_log_warn "Container ${CID} still running" + continue + fi + + "$DOCKER_BIN" container rm --force "$CID" >/dev/null 2>&1 || true + if "$DOCKER_BIN" container inspect "${CID}" >/dev/null 2>&1; then + dokku_log_warn "Container ${CID} still running" + continue + fi + + DEAD_CONTAINERS+=("$CID") + done <"$DEAD_CONTAINER_FILE" + + for CID in "${DEAD_CONTAINERS[@]}"; do + sed -i "/${CID}/d" "$DEAD_CONTAINER_FILE" + done +} + +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 + + if [[ ! -f "$DEAD_IMAGE_FILE" ]]; then + return + fi + + DEAD_IMAGES=() + while read line; do + [[ -z "$line" ]] && continue + CURRENT_TIME="$(date +%s)" + APP="$(echo "$line" | cut -d ' ' -f1)" + IMAGE_ID="$(echo "$line" | cut -d ' ' -f2)" + DEAD_TIME="$(echo "$line" | cut -d ' ' -f3)" + + if [[ "$CURRENT_TIME" -le "$DEAD_TIME" ]]; then + continue + fi + + STATE="$("$DOCKER_BIN" image inspect --format "{{ .Id }}" "$IMAGE_ID" 2>/dev/null || true)" + if [[ -z "$STATE" ]]; then + DEAD_IMAGES+=("$IMAGE_ID") + continue + fi + + dokku_log_info1_quiet "Attempting to retire $APP image $IMAGE_ID" + if RM_OUTPUT="$("$DOCKER_BIN" image rm "$IMAGE_ID" 2>&1)"; then + DEAD_IMAGES+=("$IMAGE_ID") + continue + fi + + if echo "$RM_OUTPUT" | grep -q "image has dependent child images"; then + dokku_log_warn "Image ${IMAGE_ID} has children images, skipping rm and marking dead" + DEAD_IMAGES+=("$IMAGE_ID") + continue + fi + + if echo "$RM_OUTPUT" | grep -q "image is being used by running container"; then + dokku_log_warn "Image ${IMAGE_ID} has running containers, skipping rm" + continue + fi + + dokku_log_warn "Image ${IMAGE_ID} still running" + done <"$DEAD_IMAGE_FILE" + + for IMAGE_ID in "${DEAD_IMAGES[@]}"; do + sed -i "/${IMAGE_ID}/d" "$DEAD_IMAGE_FILE" + done + + sort -o "$DEAD_IMAGE_FILE" -r "$DEAD_IMAGE_FILE" +} + +fn-scheduler-docker-local-register-retired() { + declare TYPE="$1" APP="$2" DOCKER_ID="$3" WAIT="$4" + local DEAD_FILE="${DOKKU_LIB_ROOT}/data/scheduler-docker-local/dead-containers" + if [[ "$TYPE" == "image" ]]; then + local DEAD_FILE="${DOKKU_LIB_ROOT}/data/scheduler-docker-local/dead-images" + fi local CURRENT_TIME DEAD_TIME CURRENT_TIME="$(date +%s)" DEAD_TIME=$((CURRENT_TIME + WAIT)) - echo "${APP} ${CID} ${DEAD_TIME}" >>"${DEAD_CONTAINER_FILE}" + if ! grep -q "$DOCKER_ID" "$DEAD_FILE"; then + echo "${APP} ${DOCKER_ID} ${DEAD_TIME}" >>"${DEAD_FILE}" + fi } diff --git a/plugins/scheduler-docker-local/scheduler-deploy b/plugins/scheduler-docker-local/scheduler-deploy index cabfc8146..9613ae12d 100755 --- a/plugins/scheduler-docker-local/scheduler-deploy +++ b/plugins/scheduler-docker-local/scheduler-deploy @@ -180,7 +180,7 @@ trigger-scheduler-docker-local-scheduler-deploy() { local oldid for oldid in $oldids; do dokku_log_verbose_quiet "$oldid" - fn-scheduler-docker-local-register-retired-container "$APP" "$oldid" "$WAIT" + plugn trigger scheduler-register-retired "$APP" "$oldid" "$WAIT" done ( exec >/dev/null 2>/dev/null /dev/null || true)" - if [[ -z "$STATE" ]]; then - DEAD_CONTAINERS+=("$CID") - continue - fi - - if [[ "$STATE" == "running" ]]; then - dokku_log_warn "Container ${CID} still running" - continue - fi - - "$DOCKER_BIN" container rm --force "$CID" >/dev/null 2>&1 || true - if "$DOCKER_BIN" container inspect "${CID}" >/dev/null 2>&1; then - dokku_log_warn "Container ${CID} still running" - continue - fi - - DEAD_CONTAINERS+=("$CID") - done <"$DEAD_CONTAINER_FILE" - - for CID in "${DEAD_CONTAINERS[@]}"; do - sed -i "/${CID}/d" "$DEAD_CONTAINER_FILE" - done + fn-scheduler-docker-local-retire-containers + fn-scheduler-docker-local-retire-images } trigger-scheduler-docker-local-scheduler-retire "$@" From e8a532645ffb253dcaded435f9315942866adfcc Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sat, 28 Nov 2020 00:38:28 -0500 Subject: [PATCH 2/3] fix: handle case where container does not exist during retire step This ma happen during app renames, as the old versions of the containers will no longer be running. --- plugins/scheduler-docker-local/scheduler-register-retired | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/plugins/scheduler-docker-local/scheduler-register-retired b/plugins/scheduler-docker-local/scheduler-register-retired index f0e520d26..14e355b9b 100755 --- a/plugins/scheduler-docker-local/scheduler-register-retired +++ b/plugins/scheduler-docker-local/scheduler-register-retired @@ -9,10 +9,12 @@ trigger-scheduler-docker-local-scheduler-register-retired() { declare APP="$1" CONTAINER_ID="$2" WAIT="${3:-60}" local IMAGE_ID - IMAGE_ID="$("$DOCKER_BIN" container inspect "$CONTAINER_ID" --format "{{.Image}}" | cut -d: -f2)" - fn-scheduler-docker-local-register-retired "image" "$APP" "$IMAGE_ID" "$WAIT" + IMAGE_ID="$("$DOCKER_BIN" container inspect "$CONTAINER_ID" --format "{{.Image}}" 2>/dev/null | cut -d: -f2 || true)" fn-scheduler-docker-local-register-retired "container" "$APP" "$CONTAINER_ID" "$WAIT" + if [[ -z "$IMAGE_ID" ]]; then + fn-scheduler-docker-local-register-retired "image" "$APP" "$IMAGE_ID" "$WAIT" + fi } trigger-scheduler-docker-local-scheduler-register-retired "$@" From f7863c8391d5cf828a83c41c474f912b62646e9d Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sat, 28 Nov 2020 00:39:28 -0500 Subject: [PATCH 3/3] fix: handle clone and rename properly --- plugins/ps/src/triggers/triggers.go | 8 ++++++++ plugins/ps/triggers.go | 27 +++++++++++++++++++++++++-- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/plugins/ps/src/triggers/triggers.go b/plugins/ps/src/triggers/triggers.go index 2bd5e1b7e..53d87bbb2 100644 --- a/plugins/ps/src/triggers/triggers.go +++ b/plugins/ps/src/triggers/triggers.go @@ -30,10 +30,18 @@ func main() { oldAppName := flag.Arg(0) newAppName := flag.Arg(1) err = ps.TriggerPostAppClone(oldAppName, newAppName) + case "post-app-clone-setup": + oldAppName := flag.Arg(0) + newAppName := flag.Arg(1) + err = ps.TriggerPostAppCloneSetup(oldAppName, newAppName) case "post-app-rename": oldAppName := flag.Arg(0) newAppName := flag.Arg(1) err = ps.TriggerPostAppRename(oldAppName, newAppName) + case "post-app-rename-setup": + oldAppName := flag.Arg(0) + newAppName := flag.Arg(1) + err = ps.TriggerPostAppRenameSetup(oldAppName, newAppName) case "post-create": appName := flag.Arg(0) err = ps.TriggerPostCreate(appName) diff --git a/plugins/ps/triggers.go b/plugins/ps/triggers.go index e72246aca..452b417a7 100644 --- a/plugins/ps/triggers.go +++ b/plugins/ps/triggers.go @@ -31,8 +31,8 @@ func TriggerCorePostDeploy(appName string) error { // TriggerInstall initializes app restart policies func TriggerInstall() error { - if err := common.PropertySetup("buildpacks"); err != nil { - return fmt.Errorf("Unable to install the buildpacks plugin: %s", err.Error()) + if err := common.PropertySetup("ps"); err != nil { + return fmt.Errorf("Unable to install the ps plugin: %s", err.Error()) } directory := filepath.Join(common.MustGetEnv("DOKKU_LIB_ROOT"), "data", "ps") @@ -76,6 +76,16 @@ func TriggerPostAppClone(oldAppName string, newAppName string) error { return Rebuild(newAppName) } +// TriggerPostAppCloneSetup creates new ps files +func TriggerPostAppCloneSetup(oldAppName string, newAppName string) error { + err := common.PropertyClone("ps", oldAppName, newAppName) + if err != nil { + return err + } + + return nil +} + // TriggerPostAppRename rebuilds the renamed app func TriggerPostAppRename(oldAppName string, newAppName string) error { if os.Getenv("SKIP_REBUILD") == "true" { @@ -85,6 +95,19 @@ func TriggerPostAppRename(oldAppName string, newAppName string) error { return Rebuild(newAppName) } +// TriggerPostAppRenameSetup renames ps files +func TriggerPostAppRenameSetup(oldAppName string, newAppName string) error { + if err := common.PropertyClone("ps", oldAppName, newAppName); err != nil { + return err + } + + if err := common.PropertyDestroy("ps", oldAppName); err != nil { + return err + } + + return nil +} + // TriggerPostCreate ensures apps have a default restart policy // and scale value for web func TriggerPostCreate(appName string) error {