From 01302bca3801d15ef0314190d43882362be61ea9 Mon Sep 17 00:00:00 2001 From: Griffen Fargo <3642037+gfargo@users.noreply.github.com> Date: Thu, 17 Sep 2026 18:16:44 -0400 Subject: [PATCH] feat(deploy): add REQUIRED_HOST_PATHS guard to abort before teardown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit deploy_stack's standard mode always runs `compose down` before `compose up` (see _deploy_guard_project_collision's neighboring comment). For a stack whose compose file hard-requires a host path via `devices:` (a udev symlink to a USB serial device, most commonly), `up` can legitimately fail if that path isn't present — leaving the stack fully torn down with nothing to roll back to, since down already succeeded. Mirrors docker_require_images' "check before you tear down" shape: declare REQUIRED_HOST_PATHS (space-separated) in services.conf, and deploy aborts before registry auth / image pulls / the down step if any listed path is missing, leaving the previous, working containers untouched. Opt-in only (unset is a no-op) — every existing stack is unaffected. Motivated by watch's octoprint-ender3 stack losing its container for weeks after a 2026-08-24 deploy hit exactly this gap while the printer was disconnected. Note: the guard's abort site uses `{ fail ...; return 1; }`, not a bare `|| fail`, matching _deploy_guard_project_collision — fail()'s `exit 1` doesn't reliably unwind through deploy_stack's callers on its own under some call shapes (e.g. cmd_deploy.sh's `deploy_stack ... || _deploy_rc=$?` dispatch); the explicit return is load-bearing, confirmed by writing an integration test with the bare form first and watching it fail (guard's error printed, deploy proceeded anyway). --- lib/deploy.sh | 60 +++++++ templates/services.conf.template | 19 ++ tests/test_deploy_required_host_paths.bats | 194 +++++++++++++++++++++ 3 files changed, 273 insertions(+) create mode 100644 tests/test_deploy_required_host_paths.bats diff --git a/lib/deploy.sh b/lib/deploy.sh index 8659fe2..09521fd 100644 --- a/lib/deploy.sh +++ b/lib/deploy.sh @@ -134,6 +134,52 @@ _deploy_guard_project_collision() { return 1 } +# _deploy_guard_required_host_paths +# +# Aborts BEFORE touching a running stack if any path listed in the +# REQUIRED_HOST_PATHS services.conf variable (space-separated) doesn't exist +# on the host. Mirrors docker_require_images' "check before you tear down" +# shape for a different failure mode: a stack whose compose file hard- +# requires a `devices:` mapping (a udev symlink to a USB serial device, most +# commonly) that might not be present will have its `down` step succeed and +# its `up` step fail — `down` always runs first (see deploy_stack), so a +# failure at `up` leaves the stack fully removed with nothing to roll back +# to, and nothing short of a separate self-heal mechanism ever recreates it. +# +# Bit us for real on watch's octoprint-ender3 stack (2026-08-24): a +# `strut release` ran while the printer was disconnected, `compose down` +# succeeded, `compose up` failed on the missing `devices:` path, and the +# container was gone for weeks until a udev-triggered `docker compose up -d` +# happened to fire on printer reconnect. Checking first means the deploy +# aborts instead, with the previous, working container left untouched. +# +# Opt-in only (REQUIRED_HOST_PATHS unset is a no-op): every existing stack +# is unaffected unless it declares this. +_deploy_guard_required_host_paths() { + local paths="${REQUIRED_HOST_PATHS:-}" + [ -z "$paths" ] && return 0 + + local -a missing=() + local p + for p in $paths; do + [ -e "$p" ] || missing+=("$p") + done + + [ "${#missing[@]}" -eq 0 ] && return 0 + + error "REQUIRED_HOST_PATHS missing — this stack's compose file depends on a host path that isn't present:" + local m + for m in "${missing[@]}"; do + echo " - $m" >&2 + done + echo "" >&2 + echo " Deploying now would stop the current containers (if any) and then" >&2 + echo " fail to recreate them, leaving the stack fully down. Reconnect/mount" >&2 + echo " the missing path(s) first, or remove REQUIRED_HOST_PATHS from" >&2 + echo " services.conf if this dependency is now intentionally optional." >&2 + return 1 +} + # _deploy_resolve_data_dirs # # Resolves which data directories deploy_stack should create for the stack @@ -402,6 +448,10 @@ deploy_stack() { _deploy_guard_project_collision "$compose_file" "${dry_project_name:-$stack}" \ || { fail "[DRY-RUN] Would abort here — see above."; return 1; } + # Also read-only — see _deploy_guard_required_host_paths. + _deploy_guard_required_host_paths \ + || { fail "[DRY-RUN] Would abort here — see above."; return 1; } + run_cmd "Stop existing containers" $compose_cmd down --remove-orphans run_cmd "Start services" $compose_cmd up -d --remove-orphans local proxy="${REVERSE_PROXY:-nginx}" @@ -419,6 +469,16 @@ deploy_stack() { load_services_conf "$stack_dir" local build_mode="${BUILD_MODE:-registry}" + # Abort here, before registry auth, image pulls, or the down/up below — + # see _deploy_guard_required_host_paths. Cheapest possible check (no + # docker calls), so it runs first. `{ fail ...; return 1; }`, not a bare + # `|| fail`, matching _deploy_guard_project_collision's call site below — + # fail's `exit 1` doesn't reliably unwind through this call's callers + # (cmd_deploy.sh's `deploy_stack ... || _deploy_rc=$?` dispatch) on its + # own; the explicit `return 1` is the part that actually does. + _deploy_guard_required_host_paths \ + || { fail "Aborting deploy: the running stack was left untouched (see above)."; return 1; } + # Registry login (dispatches based on REGISTRY_TYPE from config) # Skipped when BUILD_MODE=local or BUILD_MODE=none (no registry needed) # A multi-arch PLATFORMS build (even under BUILD_MODE=local) can't --load diff --git a/templates/services.conf.template b/templates/services.conf.template index 43a3f89..cce87bf 100644 --- a/templates/services.conf.template +++ b/templates/services.conf.template @@ -73,3 +73,22 @@ # # by the container or managed # # externally (e.g. a bind mount # # the container itself populates) +# +# ── Required Host Paths ──────────────────────────── +# `deploy`/`release` always stops the current containers before starting new +# ones (`compose down` then `compose up`) — if `up` then fails, the stack is +# left fully DOWN with nothing to roll back to. That's normally fine (Docker +# images/ports basically never go missing mid-deploy), but a compose file +# with a `devices:` mapping (a udev symlink to a USB serial device, a bind +# mount to removable media, etc.) CAN legitimately fail here if that host +# path isn't present at deploy time. List those paths here and `deploy` will +# abort BEFORE stopping anything if any is missing — the previous, working +# containers are left completely untouched instead of torn down for nothing. +# +# Space-separated, checked with a plain existence test (`[ -e path ]`): +# +# REQUIRED_HOST_PATHS="/dev/3dprinter" +# REQUIRED_HOST_PATHS="/dev/3dprinter /dev/webcam0" +# +# Unset (default) — no check, existing behavior for every stack that doesn't +# declare this. diff --git a/tests/test_deploy_required_host_paths.bats b/tests/test_deploy_required_host_paths.bats new file mode 100644 index 0000000..084e2b3 --- /dev/null +++ b/tests/test_deploy_required_host_paths.bats @@ -0,0 +1,194 @@ +#!/usr/bin/env bats +# ================================================== +# tests/test_deploy_required_host_paths.bats — REQUIRED_HOST_PATHS guard +# ================================================== +# Covers _deploy_guard_required_host_paths: aborts a deploy BEFORE the +# down/up teardown if a services.conf-declared host path (a udev device +# symlink, most commonly) is missing, instead of tearing down a working +# stack and then failing to bring it back — the exact shape that removed +# watch's octoprint-ender3 container on 2026-08-24 (life-automation-infra +# 2026-09-17-octoprint-boot-restore-gap devlog). + +setup() { + source "$(dirname "$BATS_TEST_FILENAME")/test_helper/common.bash" + load_utils +} + +teardown() { + common_teardown +} + +_load_deploy() { + source "$CLI_ROOT/lib/config.sh" + source "$CLI_ROOT/lib/docker.sh" + source "$CLI_ROOT/lib/deploy.sh" + error() { echo "ERROR: $*"; } +} + +@test "_deploy_guard_required_host_paths: unset REQUIRED_HOST_PATHS is a no-op" { + _load_deploy + unset REQUIRED_HOST_PATHS + + run _deploy_guard_required_host_paths + [ "$status" -eq 0 ] +} + +@test "_deploy_guard_required_host_paths: empty REQUIRED_HOST_PATHS is a no-op" { + _load_deploy + REQUIRED_HOST_PATHS="" + + run _deploy_guard_required_host_paths + [ "$status" -eq 0 ] +} + +@test "_deploy_guard_required_host_paths: all declared paths present — passes" { + _load_deploy + local dev1="$TEST_TMP/3dprinter" + local dev2="$TEST_TMP/webcam0" + touch "$dev1" "$dev2" + REQUIRED_HOST_PATHS="$dev1 $dev2" + + run _deploy_guard_required_host_paths + [ "$status" -eq 0 ] +} + +@test "_deploy_guard_required_host_paths: a missing path aborts with diagnostics" { + _load_deploy + local missing="$TEST_TMP/3dprinter" + REQUIRED_HOST_PATHS="$missing" + + run _deploy_guard_required_host_paths + [ "$status" -eq 1 ] + [[ "$output" == *"$missing"* ]] + [[ "$output" == *"REQUIRED_HOST_PATHS"* ]] +} + +@test "_deploy_guard_required_host_paths: lists every missing path, not just the first" { + _load_deploy + local present="$TEST_TMP/present" + local missing1="$TEST_TMP/missing1" + local missing2="$TEST_TMP/missing2" + touch "$present" + REQUIRED_HOST_PATHS="$present $missing1 $missing2" + + run _deploy_guard_required_host_paths + [ "$status" -eq 1 ] + [[ "$output" == *"$missing1"* ]] + [[ "$output" == *"$missing2"* ]] + [[ "$output" != *"$present"* ]] +} + +@test "_deploy_guard_required_host_paths: accepts a directory, not just a file" { + _load_deploy + local dir="$TEST_TMP/some-dir" + mkdir -p "$dir" + REQUIRED_HOST_PATHS="$dir" + + run _deploy_guard_required_host_paths + [ "$status" -eq 0 ] +} + +# ── Full deploy_stack integration ────────────────────────────────────────── +# Same harness as test_deploy_up_failure.bats: proves the guard actually +# blocks a real deploy_stack call before `compose down` ever runs, not just +# that the standalone function returns non-zero in isolation. + +setup_deploy_stack_harness() { + load_docker + + source "$CLI_ROOT/lib/config.sh" + source "$CLI_ROOT/lib/deploy.sh" + + registry_login() { echo "REGISTRY_LOGIN_CALLED"; } + docker_pull_stack() { echo "DOCKER_PULL_CALLED: $*"; } + docker_require_images() { return 0; } + rollback_save_snapshot() { :; } + export_volume_paths() { :; } + fire_hook() { return 0; } + fire_hook_or_warn() { :; } + fire_first_run_hook() { :; } + maybe_apply_db_schema() { :; } + _bg_wait_healthy() { return 0; } + notify_event() { echo "notify_event $*" >> "$TEST_TMP/notify_calls"; } + print_banner() { :; } + require_cmd() { :; } + is_running_on_vps() { return 0; } + cmd_validate() { return 0; } + diff_warn_env_divergence() { return 0; } + lock_acquire_local() { echo "test-nonce"; return 0; } + lock_release_local() { return 0; } + lock_is_stale_local() { return 1; } + lock_force_break_local() { return 0; } + export -f registry_login docker_pull_stack docker_require_images \ + rollback_save_snapshot export_volume_paths fire_hook \ + fire_hook_or_warn fire_first_run_hook maybe_apply_db_schema \ + _bg_wait_healthy \ + notify_event print_banner require_cmd is_running_on_vps \ + cmd_validate diff_warn_env_divergence lock_acquire_local \ + lock_release_local lock_is_stale_local lock_force_break_local + + # `docker compose down` must never be observed by this suite — if it is, + # the guard didn't fire before teardown. + docker() { + if [ "$1" = "compose" ]; then + case " $* " in + *" down "*) echo "COMPOSE_DOWN_CALLED" >> "$TEST_TMP/docker_calls"; return 0 ;; + *) return 0 ;; + esac + fi + return 0 + } + export -f docker + + mkdir -p "$TEST_TMP/stacks/octoprint-ender3" + cat > "$TEST_TMP/stacks/octoprint-ender3/docker-compose.yml" <<'EOF' +services: + octoprint-ender3: + image: octoprint/octoprint:latest + devices: + - /dev/3dprinter:/dev/ttyACM0 +EOF + cat > "$TEST_TMP/.prod.env" <<'EOF' +VPS_HOST=10.0.0.1 +EOF + + export LIB="$CLI_ROOT/lib" + export CLI_ROOT="$TEST_TMP" + export DRY_RUN="false" + export PRE_DEPLOY_VALIDATE="false" + export SKIP_VALIDATION="false" + : > "$TEST_TMP/notify_calls" + : > "$TEST_TMP/docker_calls" +} + +@test "deploy_stack: missing REQUIRED_HOST_PATHS aborts before compose down ever runs" { + setup_deploy_stack_harness + cat > "$TEST_TMP/stacks/octoprint-ender3/services.conf" < "$TEST_TMP/stacks/octoprint-ender3/services.conf" <