From f29fdae40967936daa87dc4d723210116dde03dc Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Wed, 30 Sep 2026 01:25:13 -0400 Subject: [PATCH] fix: keep previous image restartable after failed deploy When a deploy failed, the old containers kept serving while `dokku/:latest` still pointed at the failed image, so `ps:restart`, `ps:start`, and `ps:restore` would redeploy the broken release or fail with `App image not found` once that image was retired. The running image is now kept available during a build and deploy, the `latest` tag is restored to it when the build, release, or deploy fails, and `ps:retire` no longer removes the image an app's `latest` tag points at. Failed containers are also now registered for retirement under the correct app so they and their images are cleaned up. --- docs/deployment/schedulers/docker-local.md | 10 ++ plugins/common/functions | 92 ++++++++++- .../bin/scheduler-deploy-process-container | 4 +- .../scheduler-docker-local/internal-functions | 14 ++ tests/apps/python/app-failing-check.json | 15 ++ tests/unit/builder-dockerfile.bats | 55 ++++++ tests/unit/scheduler-docker-local.bats | 156 ++++++++++++++++++ 7 files changed, 338 insertions(+), 8 deletions(-) create mode 100644 tests/apps/python/app-failing-check.json diff --git a/docs/deployment/schedulers/docker-local.md b/docs/deployment/schedulers/docker-local.md index a6b403fb7..7039ed4ef 100644 --- a/docs/deployment/schedulers/docker-local.md +++ b/docs/deployment/schedulers/docker-local.md @@ -60,6 +60,16 @@ dokku scheduler-docker-local:set --global init-process All image containers with the label `org.opencontainers.image.vendor=linuxserver.io` will have the automatic init process injection force-disabled without further intervention when neither an app-level nor a global value is set. +### Failed deploys + +When a deploy fails - for example, because a new container does not pass its [zero downtime checks](/docs/deployment/zero-downtime-deploys.md) - the previously running containers are left in place. The app's `latest` image tag is restored to the image the previously running containers use, so commands such as `ps:restart`, `ps:start`, and `ps:restore` bring back the previous release instead of the failed one. The image of the failed release is then removed by `ps:retire`. + +To support this, the image an app is running is tagged as `dokku/:dokku-previous` while a build and deploy are in progress. This tag is removed once the deploy completes. + +The image currently tagged as the app's `latest` image is never removed by `ps:retire`. This allows an app whose first deploy failed to be started once the cause of the failure has been corrected without requiring a rebuild. + +When the `registry` plugin's `push-on-release` property is enabled, the incremented image tag is kept after a failed deploy, and restarting the app will deploy that tag. + ### Deploying Process Types in Parallel > [!IMPORTANT] diff --git a/plugins/common/functions b/plugins/common/functions index ba0a9ee73..d03b8f708 100755 --- a/plugins/common/functions +++ b/plugins/common/functions @@ -675,24 +675,97 @@ is_container_status() { return 1 } +fn-app-previous-image-id() { + declare desc="returns the image id the app is currently running, if it can be unambiguously determined" + declare APP="$1" + local CID IMAGE_ID + local -a IMAGE_IDS=() + + for CID in $(get_app_container_ids "$APP"); do + IMAGE_ID="$("$DOCKER_BIN" container inspect --format "{{.Image}}" "$CID" 2>/dev/null || true)" + [[ -z "$IMAGE_ID" ]] && continue + fn-in-array "$IMAGE_ID" "${IMAGE_IDS[@]}" && continue + IMAGE_IDS+=("$IMAGE_ID") + done + + if [[ "${#IMAGE_IDS[@]}" -eq 0 ]]; then + while read -r IMAGE_ID; do + [[ -z "$IMAGE_ID" ]] && continue + fn-in-array "$IMAGE_ID" "${IMAGE_IDS[@]}" && continue + IMAGE_IDS+=("$IMAGE_ID") + done < <("$DOCKER_BIN" image ls --no-trunc --filter "label=com.dokku.image-stage=release" --filter "label=com.dokku.app-name=$APP" --format "{{.ID}}" 2>/dev/null || true) + fi + + if [[ "${#IMAGE_IDS[@]}" -eq 1 ]]; then + echo "${IMAGE_IDS[0]}" + fi +} + +fn-app-previous-image-pin() { + declare desc="returns the image tag used to keep the previous app image available during a build and deploy" + declare APP="$1" + + echo "$(get_app_image_repo "$APP"):dokku-previous" +} + +fn-app-pin-previous-image() { + declare desc="tags the image the app is currently running so it survives the build moving the latest tag" + declare APP="$1" + local PREVIOUS_IMAGE_ID + + fn-app-unpin-previous-image "$APP" + PREVIOUS_IMAGE_ID="$(fn-app-previous-image-id "$APP")" + if [[ -z "$PREVIOUS_IMAGE_ID" ]]; then + return + fi + + "$DOCKER_BIN" image tag "$PREVIOUS_IMAGE_ID" "$(fn-app-previous-image-pin "$APP")" &>/dev/null || true +} + +fn-app-unpin-previous-image() { + declare desc="removes the tag keeping the previous app image available" + declare APP="$1" + + "$DOCKER_BIN" image remove --force --no-prune "$(fn-app-previous-image-pin "$APP")" &>/dev/null || true +} + +fn-app-restore-previous-image() { + declare desc="retags the app image to the pinned previous image, returning 1 if there is no pinned image" + declare APP="$1" + local IMAGE PIN_IMAGE PREVIOUS_IMAGE_ID CURRENT_IMAGE_ID + + IMAGE="$(get_app_image_repo "$APP"):latest" + PIN_IMAGE="$(fn-app-previous-image-pin "$APP")" + PREVIOUS_IMAGE_ID="$("$DOCKER_BIN" image inspect --format "{{.Id}}" "$PIN_IMAGE" 2>/dev/null || true)" + if [[ -z "$PREVIOUS_IMAGE_ID" ]]; then + return 1 + fi + + CURRENT_IMAGE_ID="$("$DOCKER_BIN" image inspect --format "{{.Id}}" "$IMAGE" 2>/dev/null || true)" + if [[ "$CURRENT_IMAGE_ID" == "$PREVIOUS_IMAGE_ID" ]]; then + return 0 + fi + + dokku_log_warn "Retagging old image $PREVIOUS_IMAGE_ID as $IMAGE" + "$DOCKER_BIN" image tag "$PIN_IMAGE" "$IMAGE" 2>/dev/null +} + dokku_build() { declare desc="build phase" declare APP="$1" IMAGE_SOURCE_TYPE="$2" SOURCECODE_WORK_DIR="$3" local IMAGE=$(get_app_image_name "$APP") - local RELEASED_IMAGE_ID="$(docker image ls --filter "label=com.dokku.image-stage=release" --filter "label=com.dokku.app-name=$APP" --format "{{.ID}}")" + fn-app-pin-previous-image "$APP" if plugn trigger builder-build "$IMAGE_SOURCE_TYPE" "$APP" "$SOURCECODE_WORK_DIR"; then return fi - if [[ -n "$RELEASED_IMAGE_ID" ]]; then - dokku_log_warn "Retagging old image $RELEASED_IMAGE_ID as $IMAGE" - "$DOCKER_BIN" image tag "$RELEASED_IMAGE_ID" "$IMAGE" 2>/dev/null || true - else + if ! fn-app-restore-previous-image "$APP"; then dokku_log_warn "Removing invalid image tag $IMAGE" "$DOCKER_BIN" image remove --force --no-prune "$IMAGE" &>/dev/null || true fi + fn-app-unpin-previous-image "$APP" dokku_log_fail "App build failed" } @@ -781,7 +854,14 @@ dokku_receive() { if ! dokku_build "$APP" "$IMAGE_SOURCE_TYPE" "$TMP_WORK_DIR"; then return 1 fi - plugn trigger release-and-deploy "$APP" + + if ! plugn trigger release-and-deploy "$APP"; then + fn-app-restore-previous-image "$APP" || true + fn-app-unpin-previous-image "$APP" + return 1 + fi + + fn-app-unpin-previous-image "$APP" } docker_cleanup() { diff --git a/plugins/scheduler-docker-local/bin/scheduler-deploy-process-container b/plugins/scheduler-docker-local/bin/scheduler-deploy-process-container index ded30d045..354e82ab6 100755 --- a/plugins/scheduler-docker-local/bin/scheduler-deploy-process-container +++ b/plugins/scheduler-docker-local/bin/scheduler-deploy-process-container @@ -83,7 +83,7 @@ main() { kill_new() { declare desc="wrapper function to kill newly started app container" - declare CID="$1" PROC_TYPE="$2" CONTAINER_INDEX="$3" + declare APP="$1" CID="$2" PROC_TYPE="$3" CONTAINER_INDEX="$4" plugn trigger scheduler-register-retired "$APP" "$CID" "$DOKKU_WAIT_TO_RETIRE" mkdir -p "${DOKKU_LIB_ROOT}/data/scheduler-docker-local/$APP" @@ -98,7 +98,7 @@ main() { } # run checks first, then post-deploy hooks, which switches proxy traffic - trap "kill_new $cid $PROC_TYPE $CONTAINER_INDEX" INT TERM EXIT + trap "kill_new $APP $cid $PROC_TYPE $CONTAINER_INDEX" INT TERM EXIT if [[ "$DOKKU_CHECKS_DISABLED" == "false" ]]; then dokku_log_verbose "Attempting pre-flight checks ($PROC_TYPE.$CONTAINER_INDEX)" plugn trigger check-deploy "$APP" "$cid" "$PROC_TYPE" "$DOKKU_PORT" "$ipaddr" "$CONTAINER_INDEX" diff --git a/plugins/scheduler-docker-local/internal-functions b/plugins/scheduler-docker-local/internal-functions index 614c20d32..bc1c29d30 100644 --- a/plugins/scheduler-docker-local/internal-functions +++ b/plugins/scheduler-docker-local/internal-functions @@ -224,6 +224,11 @@ fn-scheduler-docker-local-retire-images() { continue fi + if fn-scheduler-docker-local-image-is-app-latest "$RETIRE_APP" "$STATE"; then + dokku_log_warn "Image ${IMAGE_ID} is the current image for ${RETIRE_APP}, skipping rm" + continue + fi + dokku_log_verbose_quiet "Attempting to retire $RETIRE_APP image $IMAGE_ID" if RM_OUTPUT="$("$DOCKER_BIN" image remove "$IMAGE_ID" 2>&1)"; then DEAD_IMAGES+=("$IMAGE_ID") @@ -262,6 +267,15 @@ fn-scheduler-docker-local-retire-images() { sort -o "$DEAD_IMAGE_FILE" -r "$DEAD_IMAGE_FILE" } +fn-scheduler-docker-local-image-is-app-latest() { + declare desc="returns 0 if IMAGE_ID is the image currently tagged as the app's latest image" + declare APP="$1" IMAGE_ID="$2" + local LATEST_IMAGE_ID + + LATEST_IMAGE_ID="$("$DOCKER_BIN" image inspect --format "{{ .Id }}" "$(get_app_image_repo "$APP"):latest" 2>/dev/null || true)" + [[ -n "$LATEST_IMAGE_ID" ]] && [[ "$LATEST_IMAGE_ID" == "$IMAGE_ID" ]] +} + 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" diff --git a/tests/apps/python/app-failing-check.json b/tests/apps/python/app-failing-check.json new file mode 100644 index 000000000..bea74107a --- /dev/null +++ b/tests/apps/python/app-failing-check.json @@ -0,0 +1,15 @@ +{ + "healthchecks": { + "web": [ + { + "attempts": 1, + "content": "this-content-does-not-exist", + "name": "failing-check", + "path": "/", + "timeout": 2, + "type": "startup", + "wait": 1 + } + ] + } +} diff --git a/tests/unit/builder-dockerfile.bats b/tests/unit/builder-dockerfile.bats index 499f36494..9f34a5bdd 100644 --- a/tests/unit/builder-dockerfile.bats +++ b/tests/unit/builder-dockerfile.bats @@ -370,6 +370,55 @@ EOF assert_output_contains "CACHED" -1 } +@test "(builder-dockerfile) failed build restores previous image" { + local CUSTOM_TMP=$(mktemp -d "/tmp/${DOKKU_DOMAIN}.XXXXX") + trap 'popd &>/dev/null || true; rm -rf "$CUSTOM_TMP"' INT TERM + + CUSTOM_TMP="$CUSTOM_TMP" run deploy_app python "dokku@$DOKKU_DOMAIN:$TEST_APP" convert_to_dockerfile + echo "output: $output" + echo "status: $status" + assert_success + + local previous_image + previous_image="$(docker container inspect "$TEST_APP.web.1" --format '{{.Image}}')" + [[ -n "$previous_image" ]] + + run failing_build_callback "$TEST_APP" "$CUSTOM_TMP" + echo "output: $output" + echo "status: $status" + assert_success + + run git -C "$CUSTOM_TMP" add Dockerfile + echo "output: $output" + echo "status: $status" + assert_success + + run git -C "$CUSTOM_TMP" commit -m 'Break the build' + echo "output: $output" + echo "status: $status" + assert_success + + run git -C "$CUSTOM_TMP" push target master:master + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "App build failed" + assert_output_contains "Removing invalid image tag" 0 + + run /bin/bash -c "docker image inspect dokku/$TEST_APP:latest --format '{{.Id}}'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "$previous_image" + + run /bin/bash -c "dokku ps:restart $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + + rm -rf "$CUSTOM_TMP" +} + cron_run_wrapper() { local APP="$1" local APP_REPO_DIR="$2" @@ -379,3 +428,9 @@ cron_run_wrapper() { convert_to_dockerfile "$APP" "$APP_REPO_DIR" mv -f "$APP_REPO_DIR/app-cron.json" "$APP_REPO_DIR/app.json" } + +failing_build_callback() { + local APP="$1" + local APP_REPO_DIR="$2" + echo "RUN exit 1" >>"$APP_REPO_DIR/Dockerfile" +} diff --git a/tests/unit/scheduler-docker-local.bats b/tests/unit/scheduler-docker-local.bats index 610239dac..a87d833e7 100644 --- a/tests/unit/scheduler-docker-local.bats +++ b/tests/unit/scheduler-docker-local.bats @@ -615,6 +615,150 @@ teardown() { assert_output_not_exists } +@test "(scheduler-docker-local) failed deploy restores previous image" { + local CUSTOM_TMP=$(mktemp -d "/tmp/${DOKKU_DOMAIN}.XXXXX") + trap 'popd &>/dev/null || true; rm -rf "$CUSTOM_TMP"' INT TERM + + run create_app + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku checks:set $TEST_APP wait-to-retire 1" + echo "output: $output" + echo "status: $status" + assert_success + + CUSTOM_TMP="$CUSTOM_TMP" run deploy_app python "dokku@$DOKKU_DOMAIN:$TEST_APP" dockerfile_callback + echo "output: $output" + echo "status: $status" + assert_success + + local previous_image + previous_image="$(docker container inspect "$TEST_APP.web.1" --format '{{.Image}}')" + [[ -n "$previous_image" ]] + + run failing_check_callback "$TEST_APP" "$CUSTOM_TMP" + echo "output: $output" + echo "status: $status" + assert_success + + run git -C "$CUSTOM_TMP" add app.json + echo "output: $output" + echo "status: $status" + assert_success + + run git -C "$CUSTOM_TMP" commit -m 'Add a failing healthcheck' + echo "output: $output" + echo "status: $status" + assert_success + + run git -C "$CUSTOM_TMP" push target master:master + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "Retagging old image $previous_image as dokku/$TEST_APP:latest" + + run /bin/bash -c "docker image inspect dokku/$TEST_APP:latest --format '{{.Id}}'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "$previous_image" + + sleep 3 + + run /bin/bash -c "dokku ps:retire" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "docker image inspect dokku/$TEST_APP:latest --format '{{.Id}}'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "$previous_image" + + run /bin/bash -c "dokku ps:restart $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "docker container inspect $TEST_APP.web.1 --format '{{.Image}}'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "$previous_image" + + rm -rf "$CUSTOM_TMP" +} + +@test "(scheduler-docker-local) retire keeps image tagged as the app latest" { + local CUSTOM_TMP=$(mktemp -d "/tmp/${DOKKU_DOMAIN}.XXXXX") + trap 'popd &>/dev/null || true; rm -rf "$CUSTOM_TMP"' INT TERM + + run create_app + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku checks:set $TEST_APP wait-to-retire 1" + echo "output: $output" + echo "status: $status" + assert_success + + rmdir "$CUSTOM_TMP" + cp -r "${BATS_TEST_DIRNAME}/../../tests/apps/python" "$CUSTOM_TMP" + dockerfile_callback "$TEST_APP" "$CUSTOM_TMP" + failing_check_callback "$TEST_APP" "$CUSTOM_TMP" + + run git -C "$CUSTOM_TMP" init -b master + echo "output: $output" + echo "status: $status" + assert_success + + run git -C "$CUSTOM_TMP" config user.email "robot@example.com" + assert_success + run git -C "$CUSTOM_TMP" config user.name "Test Robot" + assert_success + run git -C "$CUSTOM_TMP" remote add target "dokku@$DOKKU_DOMAIN:$TEST_APP" + assert_success + + run git -C "$CUSTOM_TMP" add . + echo "output: $output" + echo "status: $status" + assert_success + + run git -C "$CUSTOM_TMP" commit -m 'initial commit' + echo "output: $output" + echo "status: $status" + assert_success + + run git -C "$CUSTOM_TMP" push target master:master + echo "output: $output" + echo "status: $status" + assert_failure + + local failed_image + failed_image="$(docker image inspect "dokku/$TEST_APP:latest" --format '{{.Id}}')" + [[ -n "$failed_image" ]] + + sleep 3 + + run /bin/bash -c "dokku ps:retire" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "is the current image for $TEST_APP, skipping rm" + + run /bin/bash -c "docker image inspect dokku/$TEST_APP:latest --format '{{.Id}}'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "$failed_image" + + rm -rf "$CUSTOM_TMP" +} + quoted_procfile_callback() { local APP="$1" local APP_REPO_DIR="$2" @@ -648,3 +792,15 @@ start_retire_timer() { systemctl start dokku-retire.timer fi } + +dockerfile_callback() { + local APP="$1" + local APP_REPO_DIR="$2" + mv "$APP_REPO_DIR/dockerfile.Dockerfile" "$APP_REPO_DIR/Dockerfile" +} + +failing_check_callback() { + local APP="$1" + local APP_REPO_DIR="$2" + mv "$APP_REPO_DIR/app-failing-check.json" "$APP_REPO_DIR/app.json" +}