From 9207f96751574a9b1946fbc862a9c3ab22fd3f35 Mon Sep 17 00:00:00 2001 From: owine Date: Sat, 10 Oct 2026 01:11:09 -0500 Subject: [PATCH] fix(deploy): move compose_p into a tested lib; handle stale working_dir labels QA review of #101. compose_projects is now scripts/deployment/lib/compose-project.sh, checked out at job.workflow_sha and copied to $RUNNER_TEMP by the deploy, health-check and rollback jobs (replaces three inline "keep in sync" copies). - Stale working_dir: containers reused from an older checkout/manual `up` keep their original working_dir label, so a stack whose containers all came from another dir also named is now resolved by its normalised project name (with a ::warning::) instead of returning nothing. Previously a critical stack in that state failed the health check ("no containers") and triggered a rollback every deploy. - Foreign same-named project: still left alone, but now with a ::warning:: instead of a silent no-op down. - The fallback no longer passes the raw stack name to `docker compose -p` (rejected for uppercase/dots); the label lookup uses the normalised name. - health-check gains a compose-workflow checkout; rollback's checkout is now unconditional (its continue-on-error moves to the network step only). - New scripts/testing/test-compose-project.sh (17 cases, fake docker on PATH). --- .github/workflows/deploy.yml | 176 +++++----------------- CLAUDE.md | 8 +- scripts/deployment/lib/compose-project.sh | 76 ++++++++++ scripts/testing/test-compose-project.sh | 146 ++++++++++++++++++ 4 files changed, 265 insertions(+), 141 deletions(-) create mode 100644 scripts/deployment/lib/compose-project.sh create mode 100755 scripts/testing/test-compose-project.sh diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 0fd6a06..509d050 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -342,51 +342,21 @@ jobs: echo "skipped=false" >> "$GITHUB_OUTPUT" fi - # Shared by every compose ps/logs/down below that runs without `op run`. - # Keep in sync with the copies in the deploy, health-check and rollback jobs. + # Scripts pinned to this workflow's commit, as in the prepare job. The + # target checkout below cleans the workspace, so the helper is copied out + # to $RUNNER_TEMP first (the shared-networks checkout re-fetches later). + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + if: steps.skip-gate.outputs.skipped == 'false' + with: + repository: owine/compose-workflow + ref: ${{ job.workflow_sha }} + path: .compose-workflow + + # compose_p for every compose ps/logs/down below that runs without `op run` + # (scripts/deployment/lib/compose-project.sh). - name: Install compose project helper if: steps.skip-gate.outputs.skipped == 'false' - run: | - set -euo pipefail - cat > "$RUNNER_TEMP/compose-project.sh" <<'SCRIPT' - # usage: compose_projects -> the stack's Compose project name(s), one per line - # Read from the containers' working_dir label, so a top-level `name:` or - # a dir name Compose normalises (dots, uppercase, leading _/-) still - # resolves. Every distinct match is returned (e.g. leftovers from a renamed - # project). With no containers in the dir it falls back to the stack name, - # unless a project of that name runs from another dir (e.g. dockge's own - # /opt/dockge): then it returns nothing, so down is a no-op and the health - # check sees "no containers" rather than acting on a foreign project. - compose_projects() { - local dir="${LIVE_REPO_PATH%/}/$1" p others - p=$(docker ps -a --filter "label=com.docker.compose.project.working_dir=$dir" \ - --format '{{.Label "com.docker.compose.project"}}' 2>/dev/null \ - | sort -u) || p="" - if [[ -z "$p" ]]; then - # Captured first: piping docker into `grep -q` can SIGPIPE under pipefail. - others=$(docker ps -a --filter "label=com.docker.compose.project=$1" \ - --format '{{.Label "com.docker.compose.project.working_dir"}}' 2>/dev/null) || others="" - if [[ -n "$others" ]] && grep -qvxF -- "$dir" <<<"$others"; then - return 0 - fi - p="$1" - fi - printf '%s\n' "$p" - } - # usage: compose_p - # Runs compose against each of the stack's projects by name from / (no - # compose file there), so the stack's compose file is never parsed: these - # calls run without `op run`, and a file whose syntax needs ${VARS} would - # fail to parse. Non-zero if the command failed for any project. - compose_p() { - local stack="$1" proj rc=0; shift - while IFS= read -r proj; do - [[ -n "$proj" ]] || continue - (cd / && docker compose -p "$proj" "$@" "$RUNNER_TEMP/compose-project.sh" <<'SCRIPT' - # usage: compose_projects -> the stack's Compose project name(s), one per line - # Read from the containers' working_dir label, so a top-level `name:` or - # a dir name Compose normalises (dots, uppercase, leading _/-) still - # resolves. Every distinct match is returned (e.g. leftovers from a renamed - # project). With no containers in the dir it falls back to the stack name, - # unless a project of that name runs from another dir (e.g. dockge's own - # /opt/dockge): then it returns nothing, so down is a no-op and the health - # check sees "no containers" rather than acting on a foreign project. - compose_projects() { - local dir="${LIVE_REPO_PATH%/}/$1" p others - p=$(docker ps -a --filter "label=com.docker.compose.project.working_dir=$dir" \ - --format '{{.Label "com.docker.compose.project"}}' 2>/dev/null \ - | sort -u) || p="" - if [[ -z "$p" ]]; then - # Captured first: piping docker into `grep -q` can SIGPIPE under pipefail. - others=$(docker ps -a --filter "label=com.docker.compose.project=$1" \ - --format '{{.Label "com.docker.compose.project.working_dir"}}' 2>/dev/null) || others="" - if [[ -n "$others" ]] && grep -qvxF -- "$dir" <<<"$others"; then - return 0 - fi - p="$1" - fi - printf '%s\n' "$p" - } - # usage: compose_p - # Runs compose against each of the stack's projects by name from / (no - # compose file there), so the stack's compose file is never parsed: these - # calls run without `op run`, and a file whose syntax needs ${VARS} would - # fail to parse. Non-zero if the command failed for any project. - compose_p() { - local stack="$1" proj rc=0; shift - while IFS= read -r proj; do - [[ -n "$proj" ]] || continue - (cd / && docker compose -p "$proj" "$@" ` exits 0 with no output for an unknown project; an empty - # critical stack must fail, not pass vacuously. + # compose_p prints nothing for a stack with no project (and `ps -p` is + # empty for one with no containers); an empty critical stack must fail, + # not pass vacuously. if [[ "$(jq 'length' <<<"$services")" -eq 0 ]]; then failed+=("$stack") echo "::error::Critical stack $stack has no containers" @@ -1072,14 +1010,16 @@ jobs: # fails on a missing external network. Scripts pinned to this workflow's # commit, as in the other jobs. # - # continue-on-error on both: a failure here must not skip the plan and - # restore steps below. Stacks that need a missing network then fail - # their own `up` (with the usual recover-manually warning); every other - # stack still rolls back. The deploy job, by contrast, fails fast here, - # before any stack is touched, which itself triggers this rollback. + # The checkout is unconditional and required: the compose project helper + # comes from it, and every teardown below sources that helper. + # + # continue-on-error on the network step: a failure there must not skip + # the plan and restore steps below. Stacks that need a missing network + # then fail their own `up` (with the usual recover-manually warning); + # every other stack still rolls back. The deploy job, by contrast, fails + # fast there, before any stack is touched, which itself triggers this + # rollback. - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - if: env.SHARED_NETWORKS != '' - continue-on-error: true with: repository: owine/compose-workflow ref: ${{ job.workflow_sha }} @@ -1093,50 +1033,10 @@ jobs: read -ra nets <<< "$SHARED_NETWORKS" ./.compose-workflow/scripts/deployment/ensure-shared-networks.sh "${nets[@]}" - # Shared by every compose ps/logs/down below that runs without `op run`. - # Keep in sync with the copies in the deploy, health-check and rollback jobs. + # compose_p for every compose ps/logs/down below that runs without `op run` + # (scripts/deployment/lib/compose-project.sh). - name: Install compose project helper - run: | - set -euo pipefail - cat > "$RUNNER_TEMP/compose-project.sh" <<'SCRIPT' - # usage: compose_projects -> the stack's Compose project name(s), one per line - # Read from the containers' working_dir label, so a top-level `name:` or - # a dir name Compose normalises (dots, uppercase, leading _/-) still - # resolves. Every distinct match is returned (e.g. leftovers from a renamed - # project). With no containers in the dir it falls back to the stack name, - # unless a project of that name runs from another dir (e.g. dockge's own - # /opt/dockge): then it returns nothing, so down is a no-op and the health - # check sees "no containers" rather than acting on a foreign project. - compose_projects() { - local dir="${LIVE_REPO_PATH%/}/$1" p others - p=$(docker ps -a --filter "label=com.docker.compose.project.working_dir=$dir" \ - --format '{{.Label "com.docker.compose.project"}}' 2>/dev/null \ - | sort -u) || p="" - if [[ -z "$p" ]]; then - # Captured first: piping docker into `grep -q` can SIGPIPE under pipefail. - others=$(docker ps -a --filter "label=com.docker.compose.project=$1" \ - --format '{{.Label "com.docker.compose.project.working_dir"}}' 2>/dev/null) || others="" - if [[ -n "$others" ]] && grep -qvxF -- "$dir" <<<"$others"; then - return 0 - fi - p="$1" - fi - printf '%s\n' "$p" - } - # usage: compose_p - # Runs compose against each of the stack's projects by name from / (no - # compose file there), so the stack's compose file is never parsed: these - # calls run without `op run`, and a file whose syntax needs ${VARS} would - # fail to parse. Non-zero if the command failed for any project. - compose_p() { - local stack="$1" proj rc=0; shift - while IFS= read -r proj; do - [[ -n "$proj" ]] || continue - (cd / && docker compose -p "$proj" "$@" ` from `/` for each, so the stack's compose file — whose syntax may need `${VARS}` — is never parsed. Keep the three copies (deploy, health-check, rollback) in sync +- **No-env compose calls** — `ps`/`logs`/`down` run without `op run`, so they go through `compose_p` from `scripts/deployment/lib/compose-project.sh` (each of `deploy`, `health-check`, `rollback` checks out compose-workflow at `job.workflow_sha` and copies it to `$RUNNER_TEMP/compose-project.sh`). It resolves the stack's Compose project(s) from the containers' `com.docker.compose.project.working_dir` label (all distinct matches) and runs `docker compose -p ` from `/` for each, so the stack's compose file — whose syntax may need `${VARS}` — is never parsed. With no containers under the stack dir it falls back to the normalised stack name only if every container of that project came from a dir also named `` (a stale label from an older checkout; `::warning::`); a same-named project from any other dir is left alone (`::warning::`). Covered by `scripts/testing/test-compose-project.sh` - **Failure diagnostics** — on stack failure or health failure, dumps `docker compose ps -a`, `docker inspect` of `.State.Health.Log` (probe history with exit codes + stdout), and `docker compose logs --tail N` scoped to the failing service - **Automatic rollback** — `git reset --hard ` + redeploy if deploy or health-check failed - **Discord notifications** — pipeline-status icon line, removed-stacks list, commit link, user mention on failure @@ -321,13 +321,15 @@ Follow `docs/superpowers/runbooks/adding-a-new-host.md` (the generic step-by-ste │ │ └── lint-summary.sh │ ├── deployment/ │ │ ├── lib/ -│ │ │ └── common.sh # logging + validation +│ │ │ ├── common.sh # logging + validation +│ │ │ └── compose-project.sh # compose_p: no-env ps/logs/down by project label │ │ ├── detect-stack-changes.sh │ │ ├── detect-critical-stacks.sh │ │ ├── ensure-shared-networks.sh │ │ └── build-pr-comment.sh │ └── testing/ │ ├── test-detect-critical-stacks.sh +│ ├── test-compose-project.sh │ ├── test-workflow.sh │ ├── validate-compose.sh │ └── README.md diff --git a/scripts/deployment/lib/compose-project.sh b/scripts/deployment/lib/compose-project.sh new file mode 100644 index 0000000..52e020b --- /dev/null +++ b/scripts/deployment/lib/compose-project.sh @@ -0,0 +1,76 @@ +#!/usr/bin/env bash +# Compose project resolution for the no-env compose calls in deploy.yml. +# +# `ps`/`logs`/`down` run without `op run`, so they must not parse the stack's +# compose file (its syntax may need ${VARS}). Instead the project is resolved +# from the containers' labels and addressed by name from /. +# +# deploy.yml copies this file to $RUNNER_TEMP/compose-project.sh in each job +# that needs it; source it from there. Requires LIVE_REPO_PATH. + +# Compose's own project-name normalisation (compose-go NormalizeProjectName): +# lowercase, keep [a-z0-9_-], strip leading _ and -. +compose_normalize_name() { + local s + s=$(tr '[:upper:]' '[:lower:]' <<<"$1" | tr -cd 'a-z0-9_-') + s="${s#"${s%%[!_-]*}"}" + printf '%s' "$s" +} + +# usage: compose_projects -> the stack's Compose project name(s), one per line +# +# 1. Every distinct project whose containers have working_dir=/. +# Covers a top-level `name:` and dir names Compose normalises, and returns +# leftovers from a renamed project too. +# 2. Otherwise, containers of the stack's default project name that were created +# from another dir. If every such dir is also named (an older checkout +# or manual `up` of the same stack; reusing a container never refreshes its +# working_dir label) that project is returned, with a warning. If any dir has +# a different name it is a foreign project: nothing is returned, with a +# warning, because `-p down` would act on it. +# 3. No containers at all: nothing is returned (down is a no-op, ps is empty). +# +# Diagnostics go to stderr (stdout is the result); GitHub parses ::warning:: +# from either stream. +compose_projects() { + local stack="$1" dir="${LIVE_REPO_PATH%/}/$1" p name others d stale=true + p=$(docker ps -a --filter "label=com.docker.compose.project.working_dir=$dir" \ + --format '{{.Label "com.docker.compose.project"}}' 2>/dev/null \ + | sort -u) || p="" + if [[ -n "$p" ]]; then + printf '%s\n' "$p" + return 0 + fi + + name=$(compose_normalize_name "$stack") + [[ -n "$name" ]] || return 0 + others=$(docker ps -a --filter "label=com.docker.compose.project=$name" \ + --format '{{.Label "com.docker.compose.project.working_dir"}}' 2>/dev/null \ + | sort -u) || others="" + [[ -n "$others" ]] || return 0 + + while IFS= read -r d; do + [[ "${d%/}" == */"$stack" ]] || stale=false + done <<<"$others" + if [[ "$stale" == true ]]; then + echo "::warning::$stack: containers were created from $(paste -sd, - <<<"$others"), not $dir; using project '$name'. Recreate them from $dir to refresh the label." >&2 + printf '%s\n' "$name" + else + echo "::warning::$stack: no containers under $dir, and project '$name' belongs to $(paste -sd, - <<<"$others"); leaving it alone." >&2 + fi +} + +# usage: compose_p +# Runs compose against each of the stack's projects by name from / (no compose +# file there), so the stack's compose file is never parsed. stdin is /dev/null +# so compose can't eat the loop input. Non-zero if the command failed for any +# project; zero (and no output) if the stack has no project. +compose_p() { + local stack="$1" proj rc=0 + shift + while IFS= read -r proj; do + [[ -n "$proj" ]] || continue + (cd / && docker compose -p "$proj" "$@" | +export FAKE_CONTAINERS="$TMPROOT/containers" +export FAKE_COMPOSE_LOG="$TMPROOT/compose.log" +mkdir -p "$TMPROOT/bin" +cat > "$TMPROOT/bin/docker" <<'EOF' +#!/usr/bin/env bash +set -euo pipefail +if [[ "$1" == "compose" ]]; then + shift + echo "$PWD $*" >> "$FAKE_COMPOSE_LOG" + exit "${FAKE_COMPOSE_RC:-0}" +fi +[[ "$1 $2" == "ps -a" ]] || { echo "fake docker: unexpected $*" >&2; exit 2; } +filter="" format="" +shift 2 +while [[ $# -gt 0 ]]; do + case "$1" in + --filter) filter="${2#label=}"; shift 2 ;; + --format) format="$2"; shift 2 ;; + *) echo "fake docker: unexpected arg $1" >&2; exit 2 ;; + esac +done +key="${filter%%=*}" val="${filter#*=}" +while IFS='|' read -r proj wd; do + [[ -n "$proj" ]] || continue + case "$key" in + com.docker.compose.project) [[ "$proj" == "$val" ]] || continue ;; + com.docker.compose.project.working_dir) [[ "$wd" == "$val" ]] || continue ;; + esac + case "$format" in + *'"com.docker.compose.project"'*) echo "$proj" ;; + *'"com.docker.compose.project.working_dir"'*) echo "$wd" ;; + esac +done < "$FAKE_CONTAINERS" +EOF +chmod +x "$TMPROOT/bin/docker" +export PATH="$TMPROOT/bin:$PATH" +export LIVE_REPO_PATH="/opt/compose/" # trailing slash on purpose + +# shellcheck source=SCRIPTDIR/../deployment/lib/compose-project.sh +source "$LIB" + +PASS=0 +FAIL=0 +FAILURES=() + +check() { + local name="$1" expected="$2" actual="$3" + if [[ "$actual" == "$expected" ]]; then + PASS=$((PASS + 1)) + echo " ✅ $name" + else + FAIL=$((FAIL + 1)) + FAILURES+=("$name") + echo " ❌ $name" + echo " expected: $(printf '%q' "$expected")" + echo " actual: $(printf '%q' "$actual")" + fi +} + +containers() { printf '%s\n' "$@" > "$FAKE_CONTAINERS"; } + +# expect_projects +expect_projects() { + local name="$1" stack="$2" expected="$3" warn="$4" out err got_warn=none + err="$TMPROOT/err" + out=$(compose_projects "$stack" 2>"$err") + grep -q '::warning::.*created from' "$err" && got_warn=stale + grep -q '::warning::.*leaving it alone' "$err" && got_warn=foreign + check "$name" "$expected|$warn" "$out|$got_warn" +} + +echo "compose_normalize_name" +check "lowercases and drops dots" "myapp" "$(compose_normalize_name My.App)" +check "strips leading _ and -" "app_1-x" "$(compose_normalize_name _-App_1-x)" +check "all-invalid -> empty" "" "$(compose_normalize_name ...)" + +echo "compose_projects" +containers "traefik|/opt/compose/traefik" "traefik|/opt/compose/traefik" +expect_projects "project from working_dir" traefik "traefik" none + +containers "customname|/opt/compose/app" +expect_projects "top-level name: resolved" app "customname" none + +containers "myapp|/opt/compose/My.App" +expect_projects "normalised dir name resolved" My.App "myapp" none + +containers "app|/opt/compose/app" "app-old|/opt/compose/app" +expect_projects "every project in the dir" app "app"$'\n'"app-old" none + +containers "other|/opt/compose/other" +expect_projects "no containers -> nothing, no warning" app "" none + +containers "traefik|/home/admin/compose/traefik" "traefik|/home/admin/compose/traefik/" +expect_projects "stale working_dir of same stack -> used, warned" traefik "traefik" stale + +containers "myapp|/home/admin/compose/My.App" +expect_projects "stale + normalised name -> used, warned" My.App "myapp" stale + +containers "app|/srv/elsewhere" +expect_projects "same-named project elsewhere -> skipped, warned" app "" foreign + +containers "app|/home/admin/compose/app" "app|/srv/elsewhere" +expect_projects "mixed stale + foreign dirs -> skipped" app "" foreign + +containers "app|/opt/compose/app" "app|/srv/elsewhere" +expect_projects "dir match wins; others ignored" app "app" none + +containers "x|/opt/compose/x" +expect_projects "all-invalid stack name -> nothing" ... "" none + +echo "compose_p" +containers "app|/opt/compose/app" "app-old|/opt/compose/app" +: > "$FAKE_COMPOSE_LOG" +rc=0; compose_p app down --remove-orphans 2>/dev/null || rc=$? +check "runs per project from /" "/ -p app down --remove-orphans"$'\n'"/ -p app-old down --remove-orphans|0" \ + "$(cat "$FAKE_COMPOSE_LOG")|$rc" + +containers "other|/opt/compose/other" +: > "$FAKE_COMPOSE_LOG" +rc=0; compose_p app down 2>/dev/null || rc=$? +check "no project -> no call, rc 0" "|0" "$(cat "$FAKE_COMPOSE_LOG")|$rc" + +containers "app|/opt/compose/app" +rc=0; FAKE_COMPOSE_RC=3 compose_p app down 2>/dev/null || rc=$? +check "propagates compose failure" "3" "$rc" + +echo "" +echo "Passed: $PASS Failed: $FAIL" +if [[ $FAIL -gt 0 ]]; then + printf ' - %s\n' "${FAILURES[@]}" + exit 1 +fi