From c4b164a270a614c3859cba2667dcd20ec075adae Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sat, 24 Oct 2026 13:58:47 -0400 Subject: [PATCH] fix: release deploy locks on exit and honor apps:lock A deploy that exited before releasing its lock, such as after a failed build, left `.deploy.lock` behind, so `apps:locked` reported a lock nothing held and the build record stayed running. The lock is now released and the record finalized whenever the deploy exits. `apps:lock` never actually blocked deploys, as it only created the file deploys lock. It now creates its own lock that git pushes, `ps:rebuild`, `git:sync`, and the `git:from-*` commands refuse to deploy past, and `apps:locked` only reports a lock when the app was locked via `apps:lock` or a deploy is in progress. Locks created by `apps:lock` in older versions are migrated on install. --- docs/advanced-usage/builds.md | 4 +- docs/deployment/application-management.md | 8 +- docs/deployment/methods/git.md | 2 +- plugins/apps/functions.go | 39 ++++++- plugins/apps/subcommands.go | 13 ++- plugins/apps/triggers.go | 46 +++++++- plugins/builds/subcommands.go | 31 ++++-- plugins/common/functions | 39 ++++++- plugins/git/internal-functions | 2 +- tests/apps/python/fail.Dockerfile | 3 + tests/unit/apps_1.bats | 129 +++++++++++++++++++++- tests/unit/apps_2.bats | 32 ++++++ tests/unit/apps_3.bats | 83 ++++++++++++++ tests/unit/builds.bats | 69 ++++++++++++ tests/unit/deploy-lock.bats | 24 ++++ tests/unit/git_4.bats | 39 ------- tests/unit/test_helper.bash | 41 +++++++ 17 files changed, 535 insertions(+), 69 deletions(-) create mode 100644 tests/apps/python/fail.Dockerfile create mode 100644 tests/unit/apps_3.bats diff --git a/docs/advanced-usage/builds.md b/docs/advanced-usage/builds.md index c7a431876..12be61a8e 100644 --- a/docs/advanced-usage/builds.md +++ b/docs/advanced-usage/builds.md @@ -25,7 +25,9 @@ Every build is identified by a sortable base36 ULID-style id (`DOKKU_BUILD_ID`) Output is also tagged into syslog as `dokku-` so `journalctl -t dokku-` continues to work. The on-disk log file is the durable source of truth and is read for `builds:output` even when journald has rotated old entries away. -A run that builds nothing leaves no record behind. `git:sync` has to fetch before it can tell whether there is anything to deploy, so it opens a record up front and discards it again when it turns out there is not - a `git:sync` with no `--build` flag, or a `--build-if-changes` poll that finds the remote unchanged. Without that, an app polled on a timer would accumulate `running` records that are later reaped as spurious failures, pushing real deploys out of the retention window. +A deploy that exits early - for example when the build fails - still finalizes its record as `failed` with its exit code. + +A run that builds nothing leaves no record behind. This includes a deploy refused because the app was locked via `apps:lock`. `git:sync` has to fetch before it can tell whether there is anything to deploy, so it opens a record up front and discards it again when it turns out there is not - a `git:sync` with no `--build` flag, or a `--build-if-changes` poll that finds the remote unchanged. Without that, an app polled on a timer would accumulate `running` records that are later reaped as spurious failures, pushing real deploys out of the retention window. ### Record schema diff --git a/docs/deployment/application-management.md b/docs/deployment/application-management.md index 31812a570..057f31f25 100644 --- a/docs/deployment/application-management.md +++ b/docs/deployment/application-management.md @@ -238,6 +238,12 @@ dokku apps:lock node-js-app -----> Deploy lock created ``` +While an app is locked, deploys that build the app - `git push`, `ps:rebuild`, `git:sync`, `git:from-archive`, `git:from-image`, and `git:load-image` - fail without building or fetching any code, and leave no build record behind. A deploy that is already in progress when the lock is created is not interrupted. Commands that redeploy an existing image, such as `ps:restart`, `ps:start`, `config:set`, and `deploy`, are not blocked by the lock. + +A locked app cannot be renamed via `apps:rename` until it is unlocked, and an app created via `apps:clone` does not inherit the lock of the app it was cloned from. + +The deploy lock taken by a deploy is released when the deploy finishes, including when the build fails or the deploy process exits early. + ### Unlocking app deploys > [!IMPORTANT] @@ -263,7 +269,7 @@ dokku apps:unlock node-js-app > [!IMPORTANT] > New as of 0.13.0 -In some cases, you may wish to inspect the state of an app lock. To do so, you can issue an `apps:lock` command. This will exit non-zero if there is no app lock in place. +In some cases, you may wish to inspect the state of an app lock. To do so, you can issue an `apps:locked` command. This will exit non-zero if there is no app lock in place. An app is considered locked if it was locked via `apps:lock` or if a deploy is currently in progress. ```shell dokku apps:locked node-js-app diff --git a/docs/deployment/methods/git.md b/docs/deployment/methods/git.md index 961ae075b..6a9112b97 100644 --- a/docs/deployment/methods/git.md +++ b/docs/deployment/methods/git.md @@ -170,7 +170,7 @@ By default, when running `git:sync` with a git branch reference, Dokku will auto dokku git:sync --skip-deploy-branch node-js-app https://github.com/heroku/node-js-getting-started.git main ``` -The `git:sync` command holds the app's deploy lock while fetching, and will fail if a deploy is already in progress. When the fetched code changes the repository or a build is triggered, the `source-image` property set by [image deploys](/docs/deployment/methods/image.md) is cleared. As the synced code is kept even when a build fails, the property is not restored on build failure. +The `git:sync` command holds the app's deploy lock while fetching, and will fail if a deploy is already in progress or the app has been locked via `apps:lock`. When the fetched code changes the repository or a build is triggered, the `source-image` property set by [image deploys](/docs/deployment/methods/image.md) is cleared. As the synced code is kept even when a build fails, the property is not restored on build failure. ### Initializing from private repositories diff --git a/plugins/apps/functions.go b/plugins/apps/functions.go index 798ddd9ff..5d271d4f7 100644 --- a/plugins/apps/functions.go +++ b/plugins/apps/functions.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "os" + "syscall" "time" "github.com/dokku/dokku/plugins/common" @@ -19,11 +20,9 @@ func appNameTaken(appName string) bool { return common.DirectoryExists(common.AppRoot(appName)) } -// checks if an app is locked +// checks if an app is locked via apps:lock or by an in-progress deploy func appIsLocked(appName string) bool { - lockPath := getLockPath(appName) - _, err := os.Stat(lockPath) - return !os.IsNotExist(err) + return appIsManuallyLocked(appName) || deployLockHeld(appName) } // verifies app name and creates an app @@ -131,6 +130,38 @@ func getLockPath(appName string) string { return fmt.Sprintf("%v/.deploy.lock", common.GetAppDataDirectory("apps", appName)) } +// returns the path of the lock created by apps:lock +func getManualLockPath(appName string) string { + return fmt.Sprintf("%v/.manual-deploy.lock", common.GetAppDataDirectory("apps", appName)) +} + +// checks if an app has been locked via apps:lock +func appIsManuallyLocked(appName string) bool { + _, err := os.Stat(getManualLockPath(appName)) + return err == nil +} + +// checks if a deploy currently holds the flock on the deploy lock file +// +// a shared lock is attempted and immediately released. Deploys taking the +// lock exclusively wait briefly before failing, so this check does not make +// a concurrent deploy fail. +func deployLockHeld(appName string) bool { + lockFile, err := os.Open(getLockPath(appName)) + if err != nil { + return false + } + defer lockFile.Close() + + fd := int(lockFile.Fd()) + if err := syscall.Flock(fd, syscall.LOCK_SH|syscall.LOCK_NB); err != nil { + return errors.Is(err, syscall.EWOULDBLOCK) + } + + _ = syscall.Flock(fd, syscall.LOCK_UN) + return false +} + // creates an app if allowed func maybeCreateApp(appName string) error { if err := appExists(appName); err == nil { diff --git a/plugins/apps/subcommands.go b/plugins/apps/subcommands.go index 72a7946ac..93bdaf7b1 100644 --- a/plugins/apps/subcommands.go +++ b/plugins/apps/subcommands.go @@ -137,9 +137,7 @@ func CommandLock(appName string) error { return err } - // the file is not truncated as it may hold the build id of an in-progress deploy - lockPath := getLockPath(appName) - lockFile, err := os.OpenFile(lockPath, os.O_CREATE|os.O_WRONLY, 0644) + lockFile, err := os.OpenFile(getManualLockPath(appName), os.O_CREATE|os.O_WRONLY, 0644) if err != nil { return errors.New("Unable to create deploy lock") } @@ -187,6 +185,10 @@ func CommandRename(oldAppName string, newAppName string, skipDeploy bool) error return errors.New("Name is already taken") } + if appIsManuallyLocked(oldAppName) { + return errors.New("Unable to rename a locked app, run apps:unlock first") + } + common.LogInfo1Quiet(fmt.Sprintf("Renaming %s to %s", oldAppName, newAppName)) if err := createApp(newAppName); err != nil { return err @@ -264,13 +266,12 @@ func CommandUnlock(appName string) error { return err } - lockfilePath := getLockPath(appName) - if _, err := os.Stat(lockfilePath); !os.IsNotExist(err) { + if deployLockHeld(appName) { common.LogWarn("A deploy may be in progress.") common.LogWarn("Removing the app lock will not stop in progress deploys.") } - if err := os.Remove(lockfilePath); err != nil { + if err := removeLockFiles(getManualLockPath(appName), getLockPath(appName)); err != nil { return errors.New("Unable to remove deploy lock") } diff --git a/plugins/apps/triggers.go b/plugins/apps/triggers.go index 78526e8a8..6a579e593 100644 --- a/plugins/apps/triggers.go +++ b/plugins/apps/triggers.go @@ -83,6 +83,40 @@ func TriggerInstall() error { } } + for _, appName := range apps { + if err := migrateManualLock(appName); err != nil { + return err + } + } + + return nil +} + +// migrateManualLock converts a lock created by apps:lock before manual locks +// had their own file. Deploys write their build id into the deploy lock file, +// so an empty one that no deploy holds was created by apps:lock. +func migrateManualLock(appName string) error { + lockPath := getLockPath(appName) + fi, err := os.Stat(lockPath) + if err != nil || fi.Size() != 0 || deployLockHeld(appName) { + return nil + } + + if err := os.Rename(lockPath, getManualLockPath(appName)); err != nil { + return fmt.Errorf("Unable to migrate the deploy lock for %s: %w", appName, err) + } + + return nil +} + +// removeLockFiles removes the given lock files, ignoring ones that do not exist +func removeLockFiles(lockPaths ...string) error { + for _, lockPath := range lockPaths { + if err := os.Remove(lockPath); err != nil && !os.IsNotExist(err) { + return err + } + } + return nil } @@ -93,7 +127,11 @@ func TriggerPostAppCloneSetup(oldAppName string, newAppName string) error { return err } - return common.CloneAppData("apps", oldAppName, newAppName) + if err := common.CloneAppData("apps", oldAppName, newAppName); err != nil { + return err + } + + return removeLockFiles(getManualLockPath(newAppName), getLockPath(newAppName)) } // TriggerPostAppRename removes the old app data @@ -111,7 +149,11 @@ func TriggerPostAppRenameSetup(oldAppName string, newAppName string) error { return err } - return common.CloneAppData("apps", oldAppName, newAppName) + if err := common.CloneAppData("apps", oldAppName, newAppName); err != nil { + return err + } + + return removeLockFiles(getLockPath(newAppName)) } // TriggerPostCreate ensures apps have the correct data directory structure diff --git a/plugins/builds/subcommands.go b/plugins/builds/subcommands.go index 5260f4701..e9f321e11 100644 --- a/plugins/builds/subcommands.go +++ b/plugins/builds/subcommands.go @@ -319,15 +319,12 @@ func CommandCancel(appName string) error { if err := WriteBuild(b); err != nil { return err } - _ = os.Remove(lockPath) + removeLockForBuild(lockPath, buildID) return nil } - common.LogInfo1("Killing app deploy") - if err := killProcessGroup(b.PID); err != nil { - common.LogWarn(fmt.Sprintf("Failed to signal process group %d: %s", b.PID, err)) - } - + // the record is finalized before the signal is sent so that a deploy + // finalizing itself on exit leaves it as canceled current, err := ReadBuild(appName, buildID) if err == nil && current.Status == BuildStatusRunning { now := time.Now().UTC() @@ -340,10 +337,30 @@ func CommandCancel(appName string) error { } } - _ = os.Remove(lockPath) + common.LogInfo1("Killing app deploy") + if err := killProcessGroup(b.PID); err != nil { + common.LogWarn(fmt.Sprintf("Failed to signal process group %d: %s", b.PID, err)) + } + + removeLockForBuild(lockPath, buildID) return nil } +// removeLockForBuild removes the deploy lock file if it still holds the given +// build id, leaving a lock taken by a newer deploy in place +func removeLockForBuild(lockPath string, buildID string) { + body, err := os.ReadFile(lockPath) + if err != nil { + return + } + + if strings.TrimSpace(string(body)) != buildID { + return + } + + _ = os.Remove(lockPath) +} + func killProcessGroup(pid int) error { pgid, err := syscall.Getpgid(pid) if err != nil { diff --git a/plugins/common/functions b/plugins/common/functions index f27755a45..b63ab2073 100755 --- a/plugins/common/functions +++ b/plugins/common/functions @@ -986,14 +986,35 @@ app_deploy_lock_file() { echo "$DOKKU_LIB_ROOT/data/apps/$APP/.deploy.lock" } -acquire_app_deploy_lock() { - declare desc="acquire advisory lock for use in deploys" - local APP="$1" - local LOCK_TYPE="${2:-waiting}" +app_manual_lock_file() { + declare desc="outputs the path to the manual lock file created by apps:lock for an app" + declare APP="$1" + echo "$DOKKU_LIB_ROOT/data/apps/$APP/.manual-deploy.lock" +} + +acquire_app_advisory_lock() { + declare desc="acquire the advisory lock for an app's deploys, failing if the app was locked via apps:lock" + declare APP="$1" LOCK_TYPE="${2:-waiting}" local APP_DEPLOY_LOCK_FILE="$(app_deploy_lock_file "$APP")" local LOCK_WAITING_MSG="$APP currently has a deploy lock in place. Waiting..." local LOCK_FAILED_MSG="$APP currently has a deploy lock in place. Exiting..." + acquire_advisory_lock "$APP_DEPLOY_LOCK_FILE" "$LOCK_TYPE" "$LOCK_WAITING_MSG" "$LOCK_FAILED_MSG" + if [[ ! -f "$(app_manual_lock_file "$APP")" ]]; then + return 0 + fi + + release_advisory_lock "$APP_DEPLOY_LOCK_FILE" + dokku_discard_build_capture "$APP" + dokku_log_warn "$APP has been locked via apps:lock" + dokku_log_fail "Run 'apps:unlock' to allow deploys" +} + +acquire_app_deploy_lock() { + declare desc="acquire advisory lock for use in deploys, releasing it and finalizing the build record when the shell exits" + local APP="$1" + local LOCK_TYPE="${2:-waiting}" + if [[ -z "$DOKKU_BUILD_ID" ]]; then DOKKU_BUILD_ID="$(plugn trigger builds-generate-id 2>/dev/null)" if [[ -z "$DOKKU_BUILD_ID" ]]; then @@ -1002,7 +1023,8 @@ acquire_app_deploy_lock() { export DOKKU_BUILD_ID fi - acquire_advisory_lock "$APP_DEPLOY_LOCK_FILE" "$LOCK_TYPE" "$LOCK_WAITING_MSG" "$LOCK_FAILED_MSG" + acquire_app_advisory_lock "$APP" "$LOCK_TYPE" + trap "release_app_deploy_lock '$APP' \"\$?\"" EXIT } release_app_deploy_lock() { @@ -1040,7 +1062,9 @@ acquire_advisory_lock() { sleep 1 done else - if ! flock -n "$LOCK_FD" &>/dev/null; then + # the short wait lets an apps:locked check that briefly takes the lock + # finish instead of failing the deploy + if ! flock -w 1 "$LOCK_FD" &>/dev/null; then dokku_log_warn "$LOCK_FAILED_MSG" dokku_log_fail "Run 'apps:unlock' to release the existing deploy lock" fi @@ -1054,6 +1078,7 @@ acquire_advisory_lock() { eval "exec $LOCK_FD>&-" done echo "${DOKKU_BUILD_ID:-$DOKKU_PID}" >"$LOCK_FILE" + trap "release_advisory_lock '$LOCK_FILE'" EXIT } release_advisory_lock() { @@ -1061,6 +1086,8 @@ release_advisory_lock() { local LOCK_FILE="$1" local LOCK_FD="200" + trap - EXIT + # the file is removed before unlocking so a waiter that locks it afterwards # can tell that its lock is stale. It is only removed if it is still the file # this process locked, as apps:unlock may have handed the path to another deploy diff --git a/plugins/git/internal-functions b/plugins/git/internal-functions index 41f1f2b4e..5a4a95104 100644 --- a/plugins/git/internal-functions +++ b/plugins/git/internal-functions @@ -286,7 +286,7 @@ cmd-git-sync() { # acquire_app_deploy_lock is not used as releasing it would finalize the # build record before the build starts - acquire_advisory_lock "$APP_DEPLOY_LOCK_FILE" "exclusive" "$APP currently has a deploy lock in place. Waiting..." "$APP currently has a deploy lock in place. Exiting..." + acquire_app_advisory_lock "$APP" "exclusive" DOKKU_DEPLOY_BRANCH="$(fn-git-computed-deploy-branch "$APP")" CURRENT_REF="$(fn-git-cmd "$APP_ROOT" rev-parse "$DOKKU_DEPLOY_BRANCH" 2>/dev/null || true)" diff --git a/tests/apps/python/fail.Dockerfile b/tests/apps/python/fail.Dockerfile new file mode 100644 index 000000000..b5644761d --- /dev/null +++ b/tests/apps/python/fail.Dockerfile @@ -0,0 +1,3 @@ +FROM alpine:3.20 + +RUN false diff --git a/tests/unit/apps_1.bats b/tests/unit/apps_1.bats index 6264065bb..c5ce24cc3 100644 --- a/tests/unit/apps_1.bats +++ b/tests/unit/apps_1.bats @@ -9,6 +9,7 @@ setup() { } teardown() { + release_deploy_lock rm -rf "${PLUGIN_ENABLED_PATH:?}/$APP_FILTER_PLUGIN_NAME" "${PLUGIN_AVAILABLE_PATH:?}/$APP_FILTER_PLUGIN_NAME" dokku --force apps:destroy "$TEST_APP-source" &>/dev/null || true global_teardown @@ -402,7 +403,7 @@ teardown() { destroy_app } -@test "(apps) apps:lock does not truncate an existing lock file" { +@test "(apps) apps:lock does not modify the deploy lock file" { run create_app echo "output: $output" echo "status: $status" @@ -432,6 +433,132 @@ teardown() { destroy_app } +@test "(apps:locked) ignores a deploy lock file that no deploy holds" { + run create_app + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "echo stale-id >$DOKKU_LIB_ROOT/data/apps/$TEST_APP/.deploy.lock && chown dokku:dokku $DOKKU_LIB_ROOT/data/apps/$TEST_APP/.deploy.lock" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku apps:locked $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_failure + + run /bin/bash -c "dokku apps:report $TEST_APP --app-locked" + echo "output: $output" + echo "status: $status" + assert_output "false" + + destroy_app +} + +@test "(apps:locked) reports a deploy lock held by a running deploy" { + run create_app + echo "output: $output" + echo "status: $status" + assert_success + + hold_deploy_lock + + run /bin/bash -c "dokku apps:locked $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku apps:unlock $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "A deploy may be in progress." + + release_deploy_lock + destroy_app +} + +@test "(apps:unlock) succeeds for an app that is not locked" { + run create_app + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku apps:unlock $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_not_contains "A deploy may be in progress." + + destroy_app +} + +@test "(apps:rename) fails for a locked app" { + run create_app + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku apps:lock $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku apps:rename $TEST_APP great-test-name" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "Unable to rename a locked app" + + run /bin/bash -c "dokku apps:exists great-test-name" + echo "output: $output" + echo "status: $status" + assert_failure + + run /bin/bash -c "dokku apps:locked $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + + destroy_app +} + +@test "(apps) [install] converts a lock created by apps:lock in older versions" { + run create_app + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "sudo -u dokku touch $DOKKU_LIB_ROOT/data/apps/$TEST_APP/.deploy.lock" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "DOKKU_ROOT=$DOKKU_ROOT PLUGIN_PATH=$PLUGIN_PATH PLUGIN_AVAILABLE_PATH=$PLUGIN_AVAILABLE_PATH PLUGIN_ENABLED_PATH=$PLUGIN_ENABLED_PATH PLUGIN_CORE_AVAILABLE_PATH=$PLUGIN_CORE_AVAILABLE_PATH DOKKU_LIB_ROOT=$DOKKU_LIB_ROOT $PLUGIN_ENABLED_PATH/apps/install" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "[[ ! -e $DOKKU_LIB_ROOT/data/apps/$TEST_APP/.deploy.lock ]]" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "[[ -f $DOKKU_LIB_ROOT/data/apps/$TEST_APP/.manual-deploy.lock ]]" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku apps:locked $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + + destroy_app +} + @test "(apps:report) emits new stripped JSON keys alongside legacy" { create_app diff --git a/tests/unit/apps_2.bats b/tests/unit/apps_2.bats index 1ab9549da..fbc6dc733 100644 --- a/tests/unit/apps_2.bats +++ b/tests/unit/apps_2.bats @@ -33,6 +33,38 @@ teardown() { assert_success } +@test "(apps:clone) does not copy the lock of a locked app" { + run /bin/bash -c "dokku apps:create $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku apps:lock $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku apps:clone --skip-deploy $TEST_APP great-test-name" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku apps:locked great-test-name" + echo "output: $output" + echo "status: $status" + assert_failure + + run /bin/bash -c "dokku apps:locked $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku --force apps:destroy great-test-name" + echo "output: $output" + echo "status: $status" + assert_success +} + @test "(apps) apps:clone (no app)" { run /bin/bash -c "dokku apps:create $TEST_APP" echo "output: $output" diff --git a/tests/unit/apps_3.bats b/tests/unit/apps_3.bats new file mode 100644 index 000000000..31be60d0e --- /dev/null +++ b/tests/unit/apps_3.bats @@ -0,0 +1,83 @@ +#!/usr/bin/env bats + +load test_helper + +setup() { + global_setup + create_app +} + +teardown() { + destroy_app + global_teardown +} + +@test "(apps:lock) blocks ps:rebuild until apps:unlock" { + run deploy_app python dokku@$DOKKU_DOMAIN:$TEST_APP convert_to_dockerfile + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku builds:list $TEST_APP --format json | jq length" + echo "output: $output" + echo "status: $status" + assert_success + local BUILD_COUNT="$output" + + run /bin/bash -c "dokku apps:lock $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "$TEST_APP has been locked via apps:lock" -1 + assert_output_not_contains "Building $TEST_APP" + + run /bin/bash -c "dokku builds:list $TEST_APP --format json | jq length" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "$BUILD_COUNT" + + run /bin/bash -c "dokku apps:locked $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku apps:unlock $TEST_APP" + 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 +} + +@test "(apps:lock) blocks git:sync before fetching" { + run /bin/bash -c "dokku apps:lock $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku git:sync --build $TEST_APP https://github.com/dokku/smoke-test-app.git" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "$TEST_APP has been locked via apps:lock" + + run /bin/bash -c "sudo -u dokku git -C $DOKKU_ROOT/$TEST_APP rev-parse --verify -q master" + echo "output: $output" + echo "status: $status" + assert_failure + + run /bin/bash -c "dokku builds:list $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "No builds recorded" +} diff --git a/tests/unit/builds.bats b/tests/unit/builds.bats index 115916884..1c476ca81 100644 --- a/tests/unit/builds.bats +++ b/tests/unit/builds.bats @@ -101,6 +101,75 @@ EOF assert_output_contains "abandoned" } +@test "(builds) [lock] a failed build releases the deploy lock and finalizes the record as failed" { + run deploy_app python dokku@$DOKKU_DOMAIN:$TEST_APP convert_to_dockerfile + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku builder-dockerfile:set $TEST_APP dockerfile-path fail.Dockerfile" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "App build failed" -1 + + run /bin/bash -c "dokku apps:locked $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_failure + + run /bin/bash -c "[[ ! -e $DOKKU_LIB_ROOT/data/apps/$TEST_APP/.deploy.lock ]]" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku builds:report $TEST_APP --build-status" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "failed" + + run /bin/bash -c "dokku builds:report $TEST_APP --build-exit-code" + echo "output: $output" + echo "status: $status" + assert_success + assert_not_output "0" +} + +@test "(builds) [lock] a deploy that exits while holding the lock finalizes its record" { + run /bin/bash -c "sudo -u dokku env PLUGIN_PATH=$PLUGIN_PATH PLUGIN_AVAILABLE_PATH=$PLUGIN_AVAILABLE_PATH PLUGIN_ENABLED_PATH=$PLUGIN_ENABLED_PATH PLUGIN_CORE_AVAILABLE_PATH=$PLUGIN_CORE_AVAILABLE_PATH DOKKU_LIB_ROOT=$DOKKU_LIB_ROOT DOKKU_BUILD_ID=exit0001 bash -c 'source $PLUGIN_CORE_AVAILABLE_PATH/common/functions; plugn trigger builds-record-start $TEST_APP exit0001 \$\$ git-hook; acquire_app_deploy_lock $TEST_APP exclusive; exit 3'" + echo "output: $output" + echo "status: $status" + assert_equal 3 "$status" + + run /bin/bash -c "[[ ! -e $DOKKU_LIB_ROOT/data/apps/$TEST_APP/.deploy.lock ]]" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku apps:locked $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_failure + + run /bin/bash -c "dokku builds:info $TEST_APP exit0001 --format json | jq -r .status" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "failed" + + run /bin/bash -c "dokku builds:info $TEST_APP exit0001 --format json | jq -r .exit_code" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "3" +} + @test "(builds:list --kind=build) filters to build-kind records" { write_finished_record "$TEST_APP" "b1" "succeeded" "git-hook" 1001 "build" write_finished_record "$TEST_APP" "d1" "succeeded" "ps:restart" 1002 "deploy" diff --git a/tests/unit/deploy-lock.bats b/tests/unit/deploy-lock.bats index 4abd87ff3..6aa894298 100644 --- a/tests/unit/deploy-lock.bats +++ b/tests/unit/deploy-lock.bats @@ -90,6 +90,30 @@ wait_for() { assert_output_contains "lock held" } +@test "(deploy-lock) the lock file is removed when the holder exits without releasing it" { + run /bin/bash -c "$(lock_script "holder-a" "acquire_advisory_lock '$LOCK_FILE' exclusive 'waiting' 'lock held'; exit 1")" + echo "output: $output" + echo "status: $status" + assert_failure + + run /bin/bash -c "[[ ! -e '$LOCK_FILE' ]]" + echo "output: $output" + echo "status: $status" + assert_success +} + +@test "(deploy-lock) the lock file is removed when the holder is terminated" { + start_lock_holder "holder-a" + + kill -TERM "${LOCK_PIDS[0]}" + wait_for "! kill -0 ${LOCK_PIDS[0]} 2>/dev/null" + + run /bin/bash -c "[[ ! -e '$LOCK_FILE' ]]" + echo "output: $output" + echo "status: $status" + assert_success +} + @test "(deploy-lock) release only removes the lock file it holds" { start_lock_holder "holder-a" diff --git a/tests/unit/git_4.bats b/tests/unit/git_4.bats index 530e832cf..650f40454 100644 --- a/tests/unit/git_4.bats +++ b/tests/unit/git_4.bats @@ -49,45 +49,6 @@ deploy_second_procfile_image() { assert_output_contains "second.Procfile" } -hold_deploy_lock() { - DEPLOY_LOCK_FILE="$DOKKU_LIB_ROOT/data/apps/$TEST_APP/.deploy.lock" - DEPLOY_LOCK_HOLDER_PID_FILE="$(mktemp "/tmp/${DOKKU_DOMAIN}.XXXXX")" - chown dokku:dokku "$DEPLOY_LOCK_HOLDER_PID_FILE" - - # the holder writes a build id like a deploy would, then becomes the sleep - # process so that killing its pid releases the lock - sudo -u dokku bash -c "exec 200>>'$DEPLOY_LOCK_FILE'; flock -n 200 || exit 1; echo test-lock-holder >'$DEPLOY_LOCK_FILE'; echo \$\$ >'$DEPLOY_LOCK_HOLDER_PID_FILE'; exec sleep 120" >/dev/null 2>&1 3>&- & - - local i - for i in $(seq 1 20); do - if [[ -s "$DEPLOY_LOCK_HOLDER_PID_FILE" ]] && ! flock -n "$DEPLOY_LOCK_FILE" true; then - return 0 - fi - sleep 0.5 - done - - echo "unable to acquire the deploy lock for $TEST_APP" - return 1 -} - -release_deploy_lock() { - if [[ -z "$DEPLOY_LOCK_HOLDER_PID_FILE" ]]; then - return 0 - fi - - local holder_pid i - if [[ -s "$DEPLOY_LOCK_HOLDER_PID_FILE" ]]; then - holder_pid="$(cat "$DEPLOY_LOCK_HOLDER_PID_FILE")" - kill "$holder_pid" &>/dev/null || true - for i in $(seq 1 20); do - kill -0 "$holder_pid" &>/dev/null || break - sleep 0.5 - done - fi - rm -f "$DEPLOY_LOCK_FILE" "$DEPLOY_LOCK_HOLDER_PID_FILE" - DEPLOY_LOCK_HOLDER_PID_FILE="" -} - deploy_branch_rev() { sudo -u dokku git -C "/home/dokku/$TEST_APP" rev-parse master } diff --git a/tests/unit/test_helper.bash b/tests/unit/test_helper.bash index 44674a910..d49ca6961 100644 --- a/tests/unit/test_helper.bash +++ b/tests/unit/test_helper.bash @@ -251,6 +251,47 @@ run_plugin_script() { run /bin/bash -c "PLUGIN_PATH=$PLUGIN_PATH PLUGIN_AVAILABLE_PATH=$PLUGIN_AVAILABLE_PATH PLUGIN_ENABLED_PATH=$PLUGIN_ENABLED_PATH PLUGIN_CORE_AVAILABLE_PATH=$PLUGIN_CORE_AVAILABLE_PATH DOKKU_LIB_ROOT=$DOKKU_LIB_ROOT $PLUGIN_ENABLED_PATH/$PLUGIN_NAME/$SCRIPT_NAME $*" } +# Hold the deploy lock for $TEST_APP from a background process, as an in-progress deploy would. +# Call release_deploy_lock (typically from teardown) to release it. +hold_deploy_lock() { + DEPLOY_LOCK_FILE="$DOKKU_LIB_ROOT/data/apps/$TEST_APP/.deploy.lock" + DEPLOY_LOCK_HOLDER_PID_FILE="$(mktemp "/tmp/${DOKKU_DOMAIN}.XXXXX")" + chown dokku:dokku "$DEPLOY_LOCK_HOLDER_PID_FILE" + + # the holder writes a build id like a deploy would, then becomes the sleep + # process so that killing its pid releases the lock + sudo -u dokku bash -c "exec 200>>'$DEPLOY_LOCK_FILE'; flock -n 200 || exit 1; echo test-lock-holder >'$DEPLOY_LOCK_FILE'; echo \$\$ >'$DEPLOY_LOCK_HOLDER_PID_FILE'; exec sleep 120" >/dev/null 2>&1 3>&- & + + local i + for i in $(seq 1 20); do + if [[ -s "$DEPLOY_LOCK_HOLDER_PID_FILE" ]] && ! flock -n "$DEPLOY_LOCK_FILE" true; then + return 0 + fi + sleep 0.5 + done + + echo "unable to acquire the deploy lock for $TEST_APP" + return 1 +} + +release_deploy_lock() { + if [[ -z "$DEPLOY_LOCK_HOLDER_PID_FILE" ]]; then + return 0 + fi + + local holder_pid i + if [[ -s "$DEPLOY_LOCK_HOLDER_PID_FILE" ]]; then + holder_pid="$(cat "$DEPLOY_LOCK_HOLDER_PID_FILE")" + kill "$holder_pid" &>/dev/null || true + for i in $(seq 1 20); do + kill -0 "$holder_pid" &>/dev/null || break + sleep 0.5 + done + fi + rm -f "$DEPLOY_LOCK_FILE" "$DEPLOY_LOCK_HOLDER_PID_FILE" + DEPLOY_LOCK_HOLDER_PID_FILE="" +} + check_urls() { local PATTERN="$1" run /bin/bash -c "dokku urls $TEST_APP | grep -E \"${PATTERN}\""