fix: hold the deploy lock across git-from-directory and harden the advisory lock

git-from-directory updated the deploy branch before taking the deploy
lock and rolled it back after releasing it. The lock is now held for the
whole update, build, and rollback, and the rollback only moves the branch
if it still points at the commit that was created.

A failed lock attempt and apps:lock both truncated the lock file,
dropping the build id of the in-progress deploy that builds:cancel and
builds:output read. Releasing the lock unlocked before removing the
file, so a waiter could end up holding a lock on a deleted file while
another deploy locked a new one. The file is now removed before
unlocking, and acquiring retries when the locked file is no longer the
one at the lock path.
This commit is contained in:
Jose Diaz-Gonzalez
2026-10-11 16:21:13 -04:00
parent 7955e8ee06
commit 11c4e908d5
8 changed files with 312 additions and 56 deletions

View File

@@ -137,8 +137,13 @@ 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)
if _, err := os.Create(lockPath); err != nil {
lockFile, err := os.OpenFile(lockPath, os.O_CREATE|os.O_WRONLY, 0644)
if err != nil {
return errors.New("Unable to create deploy lock")
}
if err := lockFile.Close(); err != nil {
return errors.New("Unable to create deploy lock")
}

View File

@@ -980,11 +980,17 @@ dokku_discard_build_capture() {
unset DOKKU_BUILD_ID DOKKU_BUILD_LOG_FILE DOKKU_BUILD_SOURCE
}
app_deploy_lock_file() {
declare desc="outputs the path to the deploy lock file for an app"
declare APP="$1"
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}"
local APP_DEPLOY_LOCK_FILE="$DOKKU_LIB_ROOT/data/apps/$APP/.deploy.lock"
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..."
@@ -1003,7 +1009,7 @@ release_app_deploy_lock() {
declare desc="release advisory lock used in deploys"
local APP="$1"
local EXIT_CODE="${2:-0}"
local APP_DEPLOY_LOCK_FILE="$DOKKU_LIB_ROOT/data/apps/$APP/.deploy.lock"
local APP_DEPLOY_LOCK_FILE="$(app_deploy_lock_file "$APP")"
if [[ -n "$DOKKU_BUILD_ID" ]]; then
plugn trigger builds-record-finalize "$APP" "$DOKKU_BUILD_ID" "$EXIT_CODE" || true
@@ -1018,24 +1024,35 @@ acquire_advisory_lock() {
local LOCK_FD="200"
local SHOW_MSG=true
eval "exec $LOCK_FD>$LOCK_FILE"
if [[ "$LOCK_TYPE" == "waiting" ]]; then
while [[ $(
flock -n "$LOCK_FD" &>/dev/null
echo $?
) -ne 0 ]]; do
if [[ "$SHOW_MSG" == "true" ]]; then
echo "$LOCK_WAITING_MSG"
SHOW_MSG=false
while true; do
# opened for appending so a failed attempt does not truncate the build id
# written by the current lock holder
eval "exec $LOCK_FD>>$LOCK_FILE"
if [[ "$LOCK_TYPE" == "waiting" ]]; then
while [[ $(
flock -n "$LOCK_FD" &>/dev/null
echo $?
) -ne 0 ]]; do
if [[ "$SHOW_MSG" == "true" ]]; then
echo "$LOCK_WAITING_MSG"
SHOW_MSG=false
fi
sleep 1
done
else
if ! flock -n "$LOCK_FD" &>/dev/null; then
dokku_log_warn "$LOCK_FAILED_MSG"
dokku_log_fail "Run 'apps:unlock' to release the existing deploy lock"
fi
sleep 1
done
else
if ! flock -n "$LOCK_FD" &>/dev/null; then
dokku_log_warn "$LOCK_FAILED_MSG"
dokku_log_fail "Run 'apps:unlock' to release the existing deploy lock"
fi
fi
# the previous holder removes the lock file on release, so the file locked
# above may no longer be the one at the lock path
if [[ "/proc/self/fd/$LOCK_FD" -ef "$LOCK_FILE" ]]; then
break
fi
eval "exec $LOCK_FD>&-"
done
echo "${DOKKU_BUILD_ID:-$DOKKU_PID}" >"$LOCK_FILE"
}
@@ -1044,7 +1061,13 @@ release_advisory_lock() {
local LOCK_FILE="$1"
local LOCK_FD="200"
flock -u "$LOCK_FD" && rm -f "$LOCK_FILE" &>/dev/null
# 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
if [[ "/proc/self/fd/$LOCK_FD" -ef "$LOCK_FILE" ]]; then
rm -f "$LOCK_FILE" &>/dev/null
fi
flock -u "$LOCK_FD"
}
suppress_output() {

View File

@@ -150,28 +150,36 @@ git_receive_app() {
if [[ ! -d "$DOKKU_ROOT/$APP/refs" ]]; then
true
else
local exit_code=0 has_source_image=false source_image previous_source_image
local exit_code=0
acquire_app_deploy_lock "$APP" "exclusive"
# DOKKU_GIT_SOURCE_IMAGE is only applied while the deploy lock is held. An
# empty value clears the property, while an unset variable leaves it as-is.
if [[ -n "${DOKKU_GIT_SOURCE_IMAGE+x}" ]]; then
has_source_image=true
source_image="$DOKKU_GIT_SOURCE_IMAGE"
unset DOKKU_GIT_SOURCE_IMAGE
previous_source_image="$(fn-git-source-image "$APP")"
fn-plugin-property-write "git" "$APP" "source-image" "$source_image"
fi
(git_build "$APP" "$REV") || exit_code="$?"
if [[ "$exit_code" -ne 0 ]] && [[ "$has_source_image" == "true" ]]; then
fn-plugin-property-write "git" "$APP" "source-image" "$previous_source_image"
fi
fn-git-build-with-lock-held "$APP" "$REV" || exit_code="$?"
release_app_deploy_lock "$APP" "$exit_code"
return "$exit_code"
fi
}
fn-git-build-with-lock-held() {
declare desc="builds an app at a revision, expecting the caller to hold the app deploy lock"
declare APP="$1" REV="$2"
local exit_code=0 has_source_image=false source_image previous_source_image
# DOKKU_GIT_SOURCE_IMAGE is only applied while the deploy lock is held. An
# empty value clears the property, while an unset variable leaves it as-is.
if [[ -n "${DOKKU_GIT_SOURCE_IMAGE+x}" ]]; then
has_source_image=true
source_image="$DOKKU_GIT_SOURCE_IMAGE"
unset DOKKU_GIT_SOURCE_IMAGE
previous_source_image="$(fn-git-source-image "$APP")"
fn-plugin-property-write "git" "$APP" "source-image" "$source_image"
fi
(git_build "$APP" "$REV") || exit_code="$?"
if [[ "$exit_code" -ne 0 ]] && [[ "$has_source_image" == "true" ]]; then
fn-plugin-property-write "git" "$APP" "source-image" "$previous_source_image"
fi
return "$exit_code"
}
cmd-git-upload-pack() {
declare desc="executes git-upload-pack"
declare cmd="git-upload-pack"

View File

@@ -5,20 +5,19 @@ source "$PLUGIN_AVAILABLE_PATH/git/internal-functions"
set -eo pipefail
[[ $DOKKU_TRACE ]] && set -x
trigger-git-git-from-directory() {
declare desc="updates a repository from a directory"
declare trigger="git-from-directory"
declare APP="$1" SOURCECODE_WORK_DIR="$2" USER_NAME="${3:-Dokku}" USER_EMAIL="${4:-automated@dokku.sh}"
fn-git-from-directory-with-lock-held() {
declare desc="updates a repository from a directory and builds it, expecting the caller to hold the app deploy lock"
declare APP="$1" SOURCECODE_WORK_DIR="$2" USER_NAME="$3" USER_EMAIL="$4"
local APP_ROOT="$DOKKU_ROOT/$APP"
local DOKKU_DEPLOY_BRANCH="$(fn-git-computed-deploy-branch "$APP")"
local REV
local REV PREVIOUS_REV
local TMP_WORK_DIR=$(mktemp -d "/tmp/dokku-${DOKKU_PID}-${FUNCNAME[0]}.XXXXXX")
local TMP_WORK_DIR_2=$(mktemp -d "/tmp/dokku-${DOKKU_PID}-${FUNCNAME[0]}.XXXXXX")
trap "rm -rf '$TMP_WORK_DIR' '$TMP_WORK_DIR_2' >/dev/null" RETURN INT TERM EXIT
local has_code=false
if fn-git-cmd "$APP_ROOT" rev-parse "$DOKKU_DEPLOY_BRANCH" &>/dev/null; then
if PREVIOUS_REV="$(fn-git-cmd "$APP_ROOT" rev-parse "$DOKKU_DEPLOY_BRANCH" 2>/dev/null)"; then
has_code=true
fi
@@ -60,19 +59,39 @@ trigger-git-git-from-directory() {
fi
rm -rf "$TMP_WORK_DIR" "$TMP_WORK_DIR_2" >/dev/null
# run in a subshell so a failure to acquire the deploy lock still reaches the rollback
if (git_receive_app "$APP" "$REV"); then
if fn-git-build-with-lock-held "$APP" "$REV"; then
return
else
local exit_code=$?
local DOKKU_DEPLOY_BRANCH="$(fn-git-computed-deploy-branch "$APP")"
# only roll back the deploy branch if it still points at the commit created above
if [[ "$has_code" == "false" ]]; then
fn-git-cmd "$APP_ROOT" update-ref -d HEAD
fn-git-cmd "$APP_ROOT" update-ref -d "refs/heads/$DOKKU_DEPLOY_BRANCH" "$REV" || dokku_log_warn "Unable to roll back the $DOKKU_DEPLOY_BRANCH branch"
else
fn-git-cmd "$APP_ROOT" update-ref "refs/heads/$DOKKU_DEPLOY_BRANCH" "$DOKKU_DEPLOY_BRANCH^"
fn-git-cmd "$APP_ROOT" update-ref "refs/heads/$DOKKU_DEPLOY_BRANCH" "$PREVIOUS_REV" "$REV" || dokku_log_warn "Unable to roll back the $DOKKU_DEPLOY_BRANCH branch"
fi
return "$exit_code"
fi
}
trigger-git-git-from-directory() {
declare desc="updates a repository from a directory"
declare trigger="git-from-directory"
declare APP="$1" SOURCECODE_WORK_DIR="$2" USER_NAME="${3:-Dokku}" USER_EMAIL="${4:-automated@dokku.sh}"
local exit_code=0
# the lock is held from before the deploy branch is updated until after it is
# rolled back, and the subshell ensures it is released on any failure
acquire_app_deploy_lock "$APP" "exclusive"
set +e
(
set -e
fn-git-from-directory-with-lock-held "$APP" "$SOURCECODE_WORK_DIR" "$USER_NAME" "$USER_EMAIL"
)
exit_code="$?"
set -e
release_app_deploy_lock "$APP" "$exit_code"
return "$exit_code"
}
trigger-git-git-from-directory "$@"

View File

@@ -279,7 +279,7 @@ cmd-git-sync() {
fi
local APP_ROOT="$DOKKU_ROOT/$APP"
local APP_DEPLOY_LOCK_FILE="$DOKKU_LIB_ROOT/data/apps/$APP/.deploy.lock"
local APP_DEPLOY_LOCK_FILE="$(app_deploy_lock_file "$APP")"
local fetch_exit_code=0
dokku_setup_build_capture "$APP" "git:sync"

View File

@@ -402,6 +402,36 @@ teardown() {
destroy_app
}
@test "(apps) apps:lock does not truncate an existing lock file" {
run create_app
echo "output: $output"
echo "status: $status"
assert_success
run /bin/bash -c "echo holder-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:lock $TEST_APP"
echo "output: $output"
echo "status: $status"
assert_success
run /bin/bash -c "cat $DOKKU_LIB_ROOT/data/apps/$TEST_APP/.deploy.lock"
echo "output: $output"
echo "status: $status"
assert_success
assert_output "holder-id"
run /bin/bash -c "dokku apps:unlock $TEST_APP"
echo "output: $output"
echo "status: $status"
assert_success
destroy_app
}
@test "(apps:report) emits new stripped JSON keys alongside legacy" {
create_app

109
tests/unit/deploy-lock.bats Normal file
View File

@@ -0,0 +1,109 @@
#!/usr/bin/env bats
load test_helper
setup() {
global_setup
LOCK_DIR="$(mktemp -d "/tmp/${DOKKU_DOMAIN}.XXXXX")"
LOCK_FILE="$LOCK_DIR/test.lock"
LOCK_PIDS=()
}
teardown() {
local pid
touch "$LOCK_DIR/release"
for pid in "${LOCK_PIDS[@]}"; do
kill "$pid" &>/dev/null || true
done
rm -rf "$LOCK_DIR"
global_teardown
}
lock_script() {
declare BUILD_ID="$1" SCRIPT="$2"
echo "source '$PLUGIN_CORE_AVAILABLE_PATH/common/functions'; export DOKKU_BUILD_ID='$BUILD_ID'; $SCRIPT"
}
start_lock_holder() {
declare BUILD_ID="$1"
/bin/bash -c "$(lock_script "$BUILD_ID" "acquire_advisory_lock '$LOCK_FILE' exclusive 'waiting' 'lock held'; while [[ ! -f '$LOCK_DIR/release' ]]; do sleep 0.2; done; release_advisory_lock '$LOCK_FILE'")" >/dev/null 2>&1 3>&- &
LOCK_PIDS+=("$!")
wait_for "[[ \"\$(cat '$LOCK_FILE' 2>/dev/null)\" == '$BUILD_ID' ]]"
}
wait_for() {
declare CONDITION="$1"
local i
for i in $(seq 1 50); do
if eval "$CONDITION"; then
return 0
fi
sleep 0.2
done
echo "timed out waiting for: $CONDITION"
return 1
}
@test "(deploy-lock) a failed acquire does not truncate the lock holder's build id" {
start_lock_holder "holder-a"
run /bin/bash -c "$(lock_script "other" "acquire_advisory_lock '$LOCK_FILE' exclusive 'waiting' 'lock held'")"
echo "output: $output"
echo "status: $status"
assert_failure
assert_output_contains "lock held"
run /bin/bash -c "cat '$LOCK_FILE'"
echo "output: $output"
echo "status: $status"
assert_success
assert_output "holder-a"
}
@test "(deploy-lock) a waiter does not keep a stale lock after the holder removes the lock file" {
start_lock_holder "holder-a"
/bin/bash -c "$(lock_script "holder-b" "acquire_advisory_lock '$LOCK_FILE' waiting 'waiting' 'lock held'; touch '$LOCK_DIR/b.acquired'; exec sleep 60")" >"$LOCK_DIR/b.log" 2>&1 3>&- &
local B_PID="$!"
LOCK_PIDS+=("$B_PID")
wait_for "grep -q waiting '$LOCK_DIR/b.log'"
touch "$LOCK_DIR/release"
wait_for "[[ -f '$LOCK_DIR/b.acquired' ]]"
run /bin/bash -c "[[ /proc/$B_PID/fd/200 -ef '$LOCK_FILE' ]]"
echo "output: $output"
echo "status: $status"
assert_success
run /bin/bash -c "cat '$LOCK_FILE'"
echo "output: $output"
echo "status: $status"
assert_success
assert_output "holder-b"
run /bin/bash -c "$(lock_script "holder-c" "acquire_advisory_lock '$LOCK_FILE' exclusive 'waiting' 'lock held'")"
echo "output: $output"
echo "status: $status"
assert_failure
assert_output_contains "lock held"
}
@test "(deploy-lock) release only removes the lock file it holds" {
start_lock_holder "holder-a"
run /bin/bash -c "rm -f '$LOCK_FILE' && echo 'replacement' >'$LOCK_FILE'"
echo "output: $output"
echo "status: $status"
assert_success
touch "$LOCK_DIR/release"
wait_for "! kill -0 ${LOCK_PIDS[0]} 2>/dev/null"
run /bin/bash -c "cat '$LOCK_FILE'"
echo "output: $output"
echo "status: $status"
assert_success
assert_output "replacement"
}

View File

@@ -51,12 +51,16 @@ deploy_second_procfile_image() {
hold_deploy_lock() {
DEPLOY_LOCK_FILE="$DOKKU_LIB_ROOT/data/apps/$TEST_APP/.deploy.lock"
sudo -u dokku flock -o "$DEPLOY_LOCK_FILE" sleep 120 >/dev/null 2>&1 3>&- &
DEPLOY_LOCK_HOLDER_PID="$!"
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 ! flock -n "$DEPLOY_LOCK_FILE" true; then
if [[ -s "$DEPLOY_LOCK_HOLDER_PID_FILE" ]] && ! flock -n "$DEPLOY_LOCK_FILE" true; then
return 0
fi
sleep 0.5
@@ -67,15 +71,25 @@ hold_deploy_lock() {
}
release_deploy_lock() {
if [[ -z "$DEPLOY_LOCK_HOLDER_PID" ]]; then
if [[ -z "$DEPLOY_LOCK_HOLDER_PID_FILE" ]]; then
return 0
fi
kill "$DEPLOY_LOCK_HOLDER_PID" &>/dev/null || true
wait "$DEPLOY_LOCK_HOLDER_PID" &>/dev/null || true
pkill -u dokku -f "flock -o $DEPLOY_LOCK_FILE" &>/dev/null || true
rm -f "$DEPLOY_LOCK_FILE"
DEPLOY_LOCK_HOLDER_PID=""
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
}
@test "(git) git:from-image [missing]" {
@@ -113,11 +127,21 @@ release_deploy_lock() {
assert_output_contains "Image exists on host, skipping pull"
assert_output_contains "No changes detected, skipping git commit"
run /bin/bash -c "dokku apps:locked $TEST_APP"
echo "output: $output"
echo "status: $status"
assert_failure
run /bin/bash -c "dokku git:from-image --force $TEST_APP linuxserver/foldingathome:7.6.21"
echo "output: $output"
echo "status: $status"
assert_output_contains "Force pulling image"
assert_output_contains "No changes detected, skipping git commit"
run /bin/bash -c "dokku apps:locked $TEST_APP"
echo "output: $output"
echo "status: $status"
assert_failure
}
@test "(git) git:from-image [normal-custom-branch]" {
@@ -210,6 +234,11 @@ release_deploy_lock() {
assert_success
assert_output ""
run /bin/bash -c "dokku apps:locked $TEST_APP"
echo "output: $output"
echo "status: $status"
assert_failure
run /bin/bash -c "dokku config:set --no-restart $TEST_APP FAIL_ON_STARTUP=false"
echo "output: $output"
echo "status: $status"
@@ -236,6 +265,11 @@ release_deploy_lock() {
echo "status: $status"
assert_failure
run /bin/bash -c "dokku apps:locked $TEST_APP"
echo "output: $output"
echo "status: $status"
assert_failure
run /bin/bash -c "dokku git:report $TEST_APP --git-source-image"
echo "output: $output"
echo "status: $status"
@@ -500,6 +534,10 @@ EOF
echo "status: $status"
assert_success
local DEPLOY_REV
DEPLOY_REV="$(deploy_branch_rev)"
echo "deploy branch rev: $DEPLOY_REV"
hold_deploy_lock
run /bin/bash -c "dokku git:from-image $TEST_APP dokku-test/$TEST_APP:default-procfile"
@@ -514,6 +552,18 @@ EOF
assert_success
assert_output "dokku-test/$TEST_APP:second-procfile"
run deploy_branch_rev
echo "output: $output"
echo "status: $status"
assert_success
assert_output "$DEPLOY_REV"
run /bin/bash -c "cat $DEPLOY_LOCK_FILE"
echo "output: $output"
echo "status: $status"
assert_success
assert_output "test-lock-holder"
run /bin/bash -c "dokku git:sync $TEST_APP https://github.com/dokku/smoke-test-app.git 2.0.0"
echo "output: $output"
echo "status: $status"
@@ -526,6 +576,18 @@ EOF
assert_success
assert_output "dokku-test/$TEST_APP:second-procfile"
run deploy_branch_rev
echo "output: $output"
echo "status: $status"
assert_success
assert_output "$DEPLOY_REV"
run /bin/bash -c "cat $DEPLOY_LOCK_FILE"
echo "output: $output"
echo "status: $status"
assert_success
assert_output "test-lock-holder"
release_deploy_lock
run /bin/bash -c "dokku git:from-image $TEST_APP dokku-test/$TEST_APP:default-procfile"