From 9cee47e377583ca554965fef118de57ed90f7a65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89rico=20Andrei?= Date: Thu, 3 Sep 2026 18:52:17 -0300 Subject: [PATCH 1/4] Add shell test harness and CI Introduce a bats suite for the two shell scripts, run in CI alongside shellcheck. Neither existed before, so every open shell bug in the tracker sits in code with no test and no lint covering it. The runner image is built FROM the action image on purpose: the scripts need GNU xargs from findutils, and against the busybox xargs in a plain alpine image the env_file tests fail for reasons unrelated to the code. Tests drive a fake docker CLI from fixtures, so no Swarm is needed. A test documenting an open bug calls skip with a link to the issue and asserts the desired behaviour, so fixing the bug means deleting the skip line rather than writing a new test. Issues 13, 19 and 21 each have one. scripts/stack-wait.sh is now maintained as a fork rather than tracking upstream, and its header says so. All shellcheck findings are resolved; where word splitting is intentional the line carries an explanatory disable instead of a change. One behaviour change: an unknown flag is now rejected instead of being silently ignored. Also adds a dependabot config covering both the workflows and the Dockerfile base image, and bumps actions/checkout to v7. Refs #13, #19, #21 --- .dockerignore | 2 + .github/dependabot.yml | 29 +++ .github/workflows/ci.yml | 35 ++++ .github/workflows/release.yml | 2 +- Dockerfile.test | 14 ++ Makefile | 21 ++ README.md | 23 +++ scripts/docker-entrypoint.sh | 10 + scripts/stack-wait.sh | 29 ++- tests/entrypoint.bats | 186 ++++++++++++++++++ tests/fixtures/converged.services | 3 + tests/fixtures/paused.services | 2 + tests/fixtures/replicated-job.services | 5 + tests/fixtures/replicating-then-done.services | 7 + tests/fixtures/rollback.services | 2 + tests/fixtures/stuck.services | 2 + tests/fixtures/zero-replicas.services | 6 + tests/helpers/bin/docker | 169 ++++++++++++++++ tests/stack-wait.bats | 136 +++++++++++++ tests/test_helper.bash | 27 +++ 20 files changed, 701 insertions(+), 9 deletions(-) create mode 100644 .github/dependabot.yml create mode 100644 .github/workflows/ci.yml create mode 100644 Dockerfile.test create mode 100644 tests/entrypoint.bats create mode 100644 tests/fixtures/converged.services create mode 100644 tests/fixtures/paused.services create mode 100644 tests/fixtures/replicated-job.services create mode 100644 tests/fixtures/replicating-then-done.services create mode 100644 tests/fixtures/rollback.services create mode 100644 tests/fixtures/stuck.services create mode 100644 tests/fixtures/zero-replicas.services create mode 100755 tests/helpers/bin/docker create mode 100644 tests/stack-wait.bats create mode 100644 tests/test_helper.bash diff --git a/.dockerignore b/.dockerignore index 8ddf561..339cbcc 100644 --- a/.dockerignore +++ b/.dockerignore @@ -5,3 +5,5 @@ Dockerfile docs Makefile README.md +Dockerfile.test +tests diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..1280eae --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,29 @@ +version: 2 +updates: + + - package-ecosystem: "github-actions" + directory: "/" + schedule: + # Check for updates to GitHub Actions every week + interval: "weekly" + # No `cooldown` here: it is not supported for the github-actions + # ecosystem, so the weekly interval is the only pacing available. + groups: + # One pull request for all action bumps, so CI validates them together. + github-actions: + patterns: + - "*" + # Must already exist in the repository, otherwise it is silently ignored. + labels: + - "dependencies" + + - package-ecosystem: "docker" + # Keeps the pinned base image in Dockerfile current. + directory: "/" + schedule: + interval: "weekly" + cooldown: + # Supported for the docker ecosystem: ignore releases younger than this. + default-days: 7 + labels: + - "dependencies" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..c71cbcb --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,35 @@ +name: CI + +on: + push: + branches: + - 'main' + pull_request: + workflow_dispatch: + +jobs: + + lint: + name: Shellcheck + runs-on: ubuntu-latest + steps: + + - name: Checkout + uses: actions/checkout@v7 + + - name: Lint shell scripts + run: make lint + + test: + name: Bats + runs-on: ubuntu-latest + steps: + + - name: Checkout + uses: actions/checkout@v7 + + # Builds the action image, then the test runner on top of it, so the + # suite runs against the same environment the action ships. This also + # means a broken Dockerfile fails here instead of at release time. + - name: Run test suite + run: make test diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 8b7f03d..4fd7c37 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -17,7 +17,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v5 + uses: actions/checkout@v7 - name: Docker meta id: meta diff --git a/Dockerfile.test b/Dockerfile.test new file mode 100644 index 0000000..8be982f --- /dev/null +++ b/Dockerfile.test @@ -0,0 +1,14 @@ +# Test runner image. +# +# It is built FROM the action image on purpose: the scripts depend on what that +# image provides (bash, and GNU xargs from findutils rather than the busybox +# one), so the suite has to run in the same environment the action ships. +ARG BASE_IMAGE=ghcr.io/kitconcept/docker-stack-deploy:test +FROM ${BASE_IMAGE} + +RUN apk add --no-cache bats + +WORKDIR /code + +ENTRYPOINT [ "bats" ] +CMD [ "tests/" ] diff --git a/Makefile b/Makefile index a673ddf..fc29c43 100644 --- a/Makefile +++ b/Makefile @@ -11,6 +11,14 @@ MAKEFLAGS+=--no-builtin-rules BASE_NAME=docker-stack-deploy IMAGE_NAME=ghcr.io/kitconcept/$(BASE_NAME) +# Images used by the test suite, never pushed. +TEST_IMAGE=$(IMAGE_NAME):test +TEST_RUNNER_IMAGE=$(IMAGE_NAME):test-runner +SHELLCHECK_IMAGE=koalaman/shellcheck:stable + +# Everything shellcheck should look at. +SHELL_SOURCES=scripts/docker-entrypoint.sh scripts/stack-wait.sh tests/helpers/bin/docker + # We like colors # From: https://coderwall.com/p/izxssa/colored-makefile-for-golang-projects RED=`tput setaf 1` @@ -36,6 +44,19 @@ build-image: # Build Docker Image @echo "Building $(IMAGE_NAME)" docker build . -t $(IMAGE_NAME) +.PHONY: lint +lint: # Lint the shell scripts with shellcheck + docker run --rm -v "$(PWD)":/code -w /code $(SHELLCHECK_IMAGE) $(SHELL_SOURCES) + +.PHONY: build-test-image +build-test-image: # Build the bats test runner image + docker build . -t $(TEST_IMAGE) + docker build . -f Dockerfile.test --build-arg BASE_IMAGE=$(TEST_IMAGE) -t $(TEST_RUNNER_IMAGE) + +.PHONY: test +test: build-test-image # Run the bats test suite + docker run --rm -v "$(PWD)":/code -w /code $(TEST_RUNNER_IMAGE) tests/ + create-tag: # Create a new tag using git @test -n "$(VERSION)" if git show-ref --tags v$(VERSION) --quiet; \ diff --git a/README.md b/README.md index f2c5fbb..14a7ba7 100644 --- a/README.md +++ b/README.md @@ -201,6 +201,29 @@ Please **DO NOT** commit to version branches directly. Even for the smallest and **ALWAYS** open a pull request and ask somebody else to merge your code. **NEVER** merge it yourself. +### Development + +Both commands need Docker, and nothing else — there is no local toolchain to install. + +```shell +make lint # shellcheck the shell scripts +make test # run the bats suite +``` + +`make test` builds the action image and then builds the test runner on top of +it, so the suite runs in the same environment the action ships. That matters: +the scripts rely on GNU `xargs` from `findutils`, and against the busybox +`xargs` in a plain Alpine image the tests fail for reasons that have nothing to +do with the code. + +Tests live in `tests/` and use a fake `docker` CLI (`tests/helpers/bin/docker`) +that replays a scenario from `tests/fixtures/*.services`, so no Swarm is +needed. Each fixture describes one poll of the wait loop per section. + +Tests that document a currently open bug call `skip` with a link to the issue. +They are written to assert the *desired* behaviour, so fixing the bug means +deleting the `skip` line rather than writing a new test. + ## Credits diff --git a/scripts/docker-entrypoint.sh b/scripts/docker-entrypoint.sh index a01df33..6a2d0ea 100755 --- a/scripts/docker-entrypoint.sh +++ b/scripts/docker-entrypoint.sh @@ -76,10 +76,20 @@ check_deploy() { scale_after() { if [[ -n "$SCALE_AFTER" ]]; then echo "Scaling services: $SCALE_AFTER" + # Unquoted on purpose: SCALE_AFTER may hold several "service=n" pairs, + # and `docker service scale` expects them as separate arguments. + # shellcheck disable=SC2086 docker service scale $SCALE_AFTER fi } +# Everything above is a function definition; everything below is the deploy +# flow. Sourcing this script (as the test suite does) stops here, so the +# functions can be exercised individually without running a deploy. +if [ "${BASH_SOURCE[0]}" != "${0}" ]; then + return 0 +fi + [ -z ${DEBUG+x} ] && export DEBUG="0" # ADDITIONAL ENV VARIABLES diff --git a/scripts/stack-wait.sh b/scripts/stack-wait.sh index b913728..774ce13 100755 --- a/scripts/stack-wait.sh +++ b/scripts/stack-wait.sh @@ -1,8 +1,12 @@ #!/bin/sh -# By: Brandon Mitchell +# Originally by: Brandon Mitchell # License: MIT -# Source repo: https://github.com/sudo-bmitch/docker-stack-wait +# Upstream repo: https://github.com/sudo-bmitch/docker-stack-wait +# +# Forked into this repository and maintained here. Changes are not +# automatically taken from upstream; see tests/stack-wait.bats for the +# behaviour this fork is expected to preserve. set -e trap "{ exit 1; }" TERM INT @@ -14,7 +18,7 @@ opt_t=3600 start_epoc=$(date +%s) usage() { - echo "$(basename $0) [opts] stack_name" + echo "$(basename "$0") [opts] stack_name" echo " -f filter: only wait for services matching filter, may be passed multiple" echo " times, see docker stack services for the filter syntax" echo " -h: this help message" @@ -32,7 +36,7 @@ check_timeout() { # next sleep completes if [ "$opt_t" -gt 0 ]; then cur_epoc=$(date +%s) - cutoff_epoc=$(expr ${start_epoc} + $opt_t - $opt_s) + cutoff_epoc=$((start_epoc + opt_t - opt_s)) if [ "$cur_epoc" -gt "$cutoff_epoc" ]; then echo "Error: Timeout exceeded" print_service_logs @@ -46,8 +50,12 @@ get_service_ids() { for name in $opt_n; do service_list="${service_list:+${service_list} }${stack_name}_${name}" done + # Unquoted on purpose: service_list is a space-separated list of names. + # shellcheck disable=SC2086 docker service inspect --format '{{.ID}}' ${service_list} else + # Unquoted on purpose: opt_f accumulates repeated "-f filter" pairs. + # shellcheck disable=SC2086 docker stack services ${opt_f} -q "${stack_name}" fi } @@ -57,8 +65,12 @@ service_state() { # strip any invalid chars from service name for caching state service_safe=$(echo "$service" | sed 's/[^A-Za-z0-9_]/_/g') state=$2 + # Unquoted on purpose in both evals: service_safe is the sanitised service + # name spliced into a variable name, not a value. + # shellcheck disable=SC2086 if eval [ \"\$cache_${service_safe}\" != \"\$state\" ]; then echo "Service $service state: $state" + # shellcheck disable=SC2086 eval cache_${service_safe}=\"\$state\" fi } @@ -66,7 +78,7 @@ print_service_logs() { if [ "$opt_p" != "0" ]; then service_ids=$(get_service_ids) for service_id in ${service_ids}; do - docker service logs --tail $opt_p "$service_id" + docker service logs --tail "$opt_p" "$service_id" done fi } @@ -80,11 +92,12 @@ while getopts 'f:hn:p:rs:t:' opt; do r) opt_r=1;; s) opt_s="$OPTARG";; t) opt_t="$OPTARG";; + *) usage;; esac done -shift $(expr $OPTIND - 1) +shift $((OPTIND - 1)) -if [ $# -ne 1 -o "$opt_h" = "1" -o "$opt_s" -le "0" ]; then +if [ $# -ne 1 ] || [ "$opt_h" = "1" ] || [ "$opt_s" -le "0" ]; then usage fi @@ -145,7 +158,7 @@ while [ "$stack_done" != "1" ]; do if [ "$service_done" = "2" ]; then # error condition stack_done=2 - elif [ "$service_done" = "0" -a "$stack_done" = "1" ]; then + elif [ "$service_done" = "0" ] && [ "$stack_done" = "1" ]; then # only go to an updating state if not in an error state stack_done=0 fi diff --git a/tests/entrypoint.bats b/tests/entrypoint.bats new file mode 100644 index 0000000..05e3172 --- /dev/null +++ b/tests/entrypoint.bats @@ -0,0 +1,186 @@ +#!/usr/bin/env bats +# +# Behaviour of scripts/docker-entrypoint.sh. +# +# The script guards its deploy flow behind a BASH_SOURCE check, so `source` +# here defines the functions without connecting to anything. The required-input +# checks are exercised end-to-end instead: they all run before the first SSH +# call, so the script exits on its own before touching the network. + +load test_helper + +setup() { + ENTRYPOINT="$(script_path docker-entrypoint.sh)" + setup_stub converged +} + +# Run the entrypoint with a controlled environment. Every VAR=value argument is +# passed through; nothing else from the test environment leaks in except PATH. +run_entrypoint() { + run env -i \ + PATH="${PATH}" \ + HOME="${BATS_TEST_TMPDIR}" \ + DSD_STUB_DIR="${DSD_STUB_DIR}" \ + DSD_STUB_SCENARIO="${DSD_STUB_SCENARIO}" \ + "$@" \ + bash "${ENTRYPOINT}" +} + +# --- sourcing ----------------------------------------------------------------- + +@test "sourcing defines the functions without running a deploy" { + # shellcheck source=/dev/null + source "${ENTRYPOINT}" + run type -t configure_env_file + [ "$status" -eq 0 ] + [ "$output" = "function" ] + # Nothing should have been asked of docker. + [ -z "$(stub_calls)" ] +} + +# --- required inputs ---------------------------------------------------------- + +@test "remote_host is required" { + run_entrypoint + [ "$status" -eq 1 ] + [[ "$output" == *"Input remote_host is required!"* ]] +} + +@test "remote_user is required" { + run_entrypoint REMOTE_HOST=swarm.example.com + [ "$status" -eq 1 ] + [[ "$output" == *"Input remote_user is required!"* ]] +} + +@test "remote_private_key is required" { + run_entrypoint REMOTE_HOST=swarm.example.com REMOTE_USER=deploy + [ "$status" -eq 1 ] + [[ "$output" == *"Input private_key is required!"* ]] +} + +@test "stack_file is required" { + run_entrypoint REMOTE_HOST=swarm.example.com REMOTE_USER=deploy \ + REMOTE_PRIVATE_KEY=key + [ "$status" -eq 1 ] + [[ "$output" == *"Input stack_file is required!"* ]] +} + +@test "stack_file must exist" { + run_entrypoint REMOTE_HOST=swarm.example.com REMOTE_USER=deploy \ + REMOTE_PRIVATE_KEY=key STACK_FILE=/nope/missing.yml + [ "$status" -eq 1 ] + [[ "$output" == *"/nope/missing.yml does not exist."* ]] +} + +@test "stack_name is required" { + touch "${BATS_TEST_TMPDIR}/stack.yml" + run_entrypoint REMOTE_HOST=swarm.example.com REMOTE_USER=deploy \ + REMOTE_PRIVATE_KEY=key STACK_FILE="${BATS_TEST_TMPDIR}/stack.yml" + [ "$status" -eq 1 ] + [[ "$output" == *"Input stack_name is required!"* ]] +} + +# --- container registry login ------------------------------------------------- + +@test "skips login when no credentials are given (issue #4)" { + run_entrypoint + [[ "$output" == *"Container Registry: No authentication provided"* ]] + [[ "$(stub_calls)" != *"login"* ]] +} + +@test "logs in when credentials are given" { + run_entrypoint USERNAME=someone PASSWORD=secret REGISTRY=ghcr.io + [[ "$output" == *"Container Registry: Logged in ghcr.io as someone"* ]] + [[ "$(stub_calls)" == *"login ghcr.io -u someone --password-stdin"* ]] +} + +@test "aborts when login fails" { + run_entrypoint USERNAME=someone PASSWORD=secret REGISTRY=ghcr.io \ + DSD_STUB_LOGIN_FAILS=1 + [ "$status" -eq 1 ] + [[ "$output" == *"Login to ghcr.io as someone failed"* ]] +} + +@test "a preset DEBUG value does not abort the script" { + # `[ -z ${DEBUG+x} ] && export DEBUG="0"` runs under `set -e`; make sure a + # caller-supplied DEBUG does not make that line terminate the run. The + # action always passes DEBUG, so this path is the normal one. + run_entrypoint DEBUG=1 + [ "$status" -eq 1 ] + [[ "$output" == *"Verbose logging"* ]] + [[ "$output" == *"Input remote_host is required!"* ]] +} + +# --- env_file parsing --------------------------------------------------------- + +# Source the script and run configure_env_file against a given ENV_FILE body, +# printing the resulting value of one variable. +export_from_env_file() { + local body="$1" var="$2" + run env -i PATH="${PATH}" HOME="${BATS_TEST_TMPDIR}" \ + ENV_FILE="${body}" WANT="${var}" \ + bash -c " + source '${ENTRYPOINT}' + ENV_FILE_PATH=\"\${HOME}/.env\" + configure_env_file >/dev/null 2>&1 + printf '%s' \"\${!WANT}\" + " +} + +@test "env_file exports a simple value" { + export_from_env_file 'DB_USER=plone' DB_USER + [ "$status" -eq 0 ] + [ "$output" = "plone" ] +} + +@test "env_file exports several values" { + export_from_env_file $'DB_USER=plone\nDB_NAME=site' DB_NAME + [ "$status" -eq 0 ] + [ "$output" = "site" ] +} + +@test "env_file ignores comments and blank lines" { + export_from_env_file $'# a comment\n\nDB_USER=plone' DB_USER + [ "$status" -eq 0 ] + [ "$output" = "plone" ] +} + +@test "env_file keeps an = inside the value" { + export_from_env_file 'DSN=key=value' DSN + [ "$status" -eq 0 ] + [ "$output" = "key=value" ] +} + +@test "issue #21: env_file exports a value containing spaces" { + skip "https://github.com/kitconcept/docker-stack-deploy/issues/21 -- the unquoted \$(...) word-splits the value before export runs" + export_from_env_file 'SOLR_JAVA_MEM=-Xms1536m -Xmx1536m' SOLR_JAVA_MEM + [ "$status" -eq 0 ] + [ "$output" = "-Xms1536m -Xmx1536m" ] +} + +@test "issue #21: quotes in an env_file value are kept verbatim" { + skip "https://github.com/kitconcept/docker-stack-deploy/issues/21 -- pending the parser rewrite; we match 'docker --env-file' and do not strip quotes" + export_from_env_file 'GREETING="hello world"' GREETING + [ "$status" -eq 0 ] + [ "$output" = '"hello world"' ] +} + +# --- scale_after -------------------------------------------------------------- + +@test "scale_after does nothing when unset" { + # shellcheck source=/dev/null + source "${ENTRYPOINT}" + SCALE_AFTER="" + run scale_after + [ "$status" -eq 0 ] + [ -z "$output" ] +} + +@test "scale_after passes each service=n pair as its own argument" { + # shellcheck source=/dev/null + source "${ENTRYPOINT}" + SCALE_AFTER="demo_dbpack=1 demo_reindex=1" + run scale_after + [ "$status" -eq 0 ] + [[ "$(stub_calls)" == *"service scale demo_dbpack=1 demo_reindex=1"* ]] +} diff --git a/tests/fixtures/converged.services b/tests/fixtures/converged.services new file mode 100644 index 0000000..df778e8 --- /dev/null +++ b/tests/fixtures/converged.services @@ -0,0 +1,3 @@ +# Every service already settled: the wait loop should exit on its first poll. +svc_backend|demo_backend|deployed|1/1 +svc_frontend|demo_frontend|completed|2/2 diff --git a/tests/fixtures/paused.services b/tests/fixtures/paused.services new file mode 100644 index 0000000..d20b2e3 --- /dev/null +++ b/tests/fixtures/paused.services @@ -0,0 +1,2 @@ +# A paused update can never converge on its own. +svc_backend|demo_backend|paused|1/2 diff --git a/tests/fixtures/replicated-job.services b/tests/fixtures/replicated-job.services new file mode 100644 index 0000000..4e7f99c --- /dev/null +++ b/tests/fixtures/replicated-job.services @@ -0,0 +1,5 @@ +# Issue #13: a `mode: replicated-job` service that ran once and finished. +# `docker service ls` reports the completion count in a suffix that +# stack-wait.sh currently discards, leaving current(0) != target(1) forever. +svc_migrate|demo_migrate|deployed|0/1 (1/1 completed) +svc_backend|demo_backend|deployed|1/1 diff --git a/tests/fixtures/replicating-then-done.services b/tests/fixtures/replicating-then-done.services new file mode 100644 index 0000000..56122e0 --- /dev/null +++ b/tests/fixtures/replicating-then-done.services @@ -0,0 +1,7 @@ +# Round 1: frontend is still scaling up. +svc_backend|demo_backend|deployed|1/1 +svc_frontend|demo_frontend|updating|1/3 +--- +# Round 2: it converged. +svc_backend|demo_backend|deployed|1/1 +svc_frontend|demo_frontend|completed|3/3 diff --git a/tests/fixtures/rollback.services b/tests/fixtures/rollback.services new file mode 100644 index 0000000..d4fb6d9 --- /dev/null +++ b/tests/fixtures/rollback.services @@ -0,0 +1,2 @@ +# The update failed and swarm rolled it back. +svc_backend|demo_backend|rollback_completed|1/1 diff --git a/tests/fixtures/stuck.services b/tests/fixtures/stuck.services new file mode 100644 index 0000000..b7892c7 --- /dev/null +++ b/tests/fixtures/stuck.services @@ -0,0 +1,2 @@ +# Never converges: exercises the -t timeout path. +svc_backend|demo_backend|updating|1/3 diff --git a/tests/fixtures/zero-replicas.services b/tests/fixtures/zero-replicas.services new file mode 100644 index 0000000..4787425 --- /dev/null +++ b/tests/fixtures/zero-replicas.services @@ -0,0 +1,6 @@ +# Issue #19: a service the stack file declares with `replicas: 0`, which an +# external scheduler (swarm-cronjob) has since scaled to a target of 1. +# Indistinguishable from issue #13 at the replicas field alone -- there is no +# completion suffix here to disambiguate a finished run from a failed start. +svc_dbpack|demo_dbpack|deployed|0/1 +svc_backend|demo_backend|deployed|1/1 diff --git a/tests/helpers/bin/docker b/tests/helpers/bin/docker new file mode 100755 index 0000000..943eab6 --- /dev/null +++ b/tests/helpers/bin/docker @@ -0,0 +1,169 @@ +#!/usr/bin/env bash +# +# Fake `docker` CLI used by the test suite. +# +# It answers only the handful of invocations that scripts/ actually make, and +# reads the answers from a scenario fixture instead of a real Swarm. +# +# Scenario format (tests/fixtures/*.services) -- one "round" of polling per +# section, sections separated by a line containing exactly `---`: +# +# service_id|service_name|update_state|replicas +# --- +# service_id|service_name|update_state|replicas +# +# Blank lines and `#` comments are ignored. `docker stack services` advances to +# the next round on every call, which is exactly once per poll of the wait +# loop; the last section repeats forever so a scenario can settle or hang. +# +# Every invocation is appended to $DSD_STUB_DIR/calls.log so tests can assert +# on what the script under test actually asked docker to do. + +set -euo pipefail + +: "${DSD_STUB_DIR:?DSD_STUB_DIR must be set by the test}" +: "${DSD_STUB_SCENARIO:=}" + +mkdir -p "${DSD_STUB_DIR}" +printf '%s\n' "$*" >> "${DSD_STUB_DIR}/calls.log" + +round_file="${DSD_STUB_DIR}/round" + +section_count() { + awk 'BEGIN { s = 1 } /^---$/ { s++ } END { print s }' "${DSD_STUB_SCENARIO}" +} + +# Emit the data rows of section $1, ignoring blanks and comments. +section_rows() { + awk -v want="$1" ' + BEGIN { s = 1 } + /^---$/ { s++; next } + s == want && NF && $0 !~ /^[[:space:]]*#/ { print } + ' "${DSD_STUB_SCENARIO}" +} + +current_round() { + if [ -f "${round_file}" ]; then cat "${round_file}"; else echo 1; fi +} + +# Advance the round, clamped to the last section so it repeats forever. +advance_round() { + local now total + now=$(current_round) + total=$(section_count) + if [ "${now}" -lt "${total}" ]; then + echo $((now + 1)) > "${round_file}" + else + echo "${now}" > "${round_file}" + fi +} + +# Look up field $2 (1-indexed, pipe-separated) of the row whose id is $1. +lookup() { + section_rows "$(current_round)" | awk -F'|' -v id="$1" -v f="$2" '$1 == id { print $f; exit }' +} + +case "${1:-}" in + --version) + echo "Docker version 29.1.3, build stub" + ;; + + login) + # Consume the password on stdin so the caller's pipe does not break. + cat > /dev/null + if [ "${DSD_STUB_LOGIN_FAILS:-0}" = "1" ]; then + echo "stub: login failed" >&2 + exit 1 + fi + echo "Login Succeeded" + ;; + + stack) + case "${2:-}" in + services) + # `docker stack services [-f filter] -q ` -- one call per poll. + # Step to the next round *before* serving it, so the service inspect + # calls that follow in the same poll observe the same round. + if [ -f "${round_file}" ]; then + advance_round + else + echo 1 > "${round_file}" + fi + ids=$(section_rows "$(current_round)" | awk -F'|' '{ print $1 }') + if [ -n "${ids}" ]; then + printf '%s\n' "${ids}" + fi + ;; + deploy) + echo "stub: deployed" + ;; + ps) + # Used by the failure diagnostics (issue #19). + printf 'ID NAME IMAGE NODE DESIRED STATE CURRENT STATE\n' + section_rows "$(current_round)" | awk -F'|' '{ print $1 " " $2 " img node Running Running" }' + ;; + *) + echo "stub: unhandled 'docker stack ${2:-}'" >&2 + exit 64 + ;; + esac + ;; + + service) + case "${2:-}" in + inspect) + # Either --format '{{.ID}}' , or -f/--format . + fmt="" + args=() + shift 2 + while [ $# -gt 0 ]; do + case "$1" in + -f|--format) fmt="$2"; shift 2 ;; + *) args+=("$1"); shift ;; + esac + done + for target in "${args[@]}"; do + case "${fmt}" in + *'.ID'*) + # Resolving explicit service names back to ids (the -n option). + section_rows "$(current_round)" | awk -F'|' -v n="${target}" '$2 == n { print $1; exit }' + ;; + *'.Spec.Name'*) + lookup "${target}" 2 + ;; + *'.UpdateStatus'*) + lookup "${target}" 3 + ;; + *) + echo "stub: unhandled service inspect format '${fmt}'" >&2 + exit 64 + ;; + esac + done + ;; + ls) + # `docker service ls --format '{{.Replicas}}' --filter id=` + id="" + for arg in "$@"; do + case "${arg}" in id=*) id="${arg#id=}" ;; esac + done + lookup "${id}" 4 + ;; + logs) + echo "stub: log line" + ;; + scale) + echo "stub: scaled $*" + ;; + *) + echo "stub: unhandled 'docker service ${2:-}'" >&2 + exit 64 + ;; + esac + ;; + + *) + echo "stub: unhandled 'docker ${*}'" >&2 + exit 64 + ;; +esac diff --git a/tests/stack-wait.bats b/tests/stack-wait.bats new file mode 100644 index 0000000..28f8a82 --- /dev/null +++ b/tests/stack-wait.bats @@ -0,0 +1,136 @@ +#!/usr/bin/env bats +# +# Behaviour of scripts/stack-wait.sh, exercised against the fake docker CLI in +# tests/helpers/bin. The polling interval is kept at 1s (-s 1) so the suite +# stays fast; the script's default is 5s. + +load test_helper + +setup() { + STACK_WAIT="$(script_path stack-wait.sh)" +} + +@test "succeeds immediately when every service has already converged" { + setup_stub converged + run "${STACK_WAIT}" -s 1 -t 10 demo + [ "$status" -eq 0 ] + [[ "$output" == *"demo_backend state: deployed"* ]] + [[ "$output" == *"demo_frontend state: completed"* ]] +} + +@test "waits for a replicating service and then succeeds" { + setup_stub replicating-then-done + run "${STACK_WAIT}" -s 1 -t 20 demo + [ "$status" -eq 0 ] + # The transient state must be reported, then the settled one. + [[ "$output" == *"demo_frontend state: replicating 1/3"* ]] + [[ "$output" == *"demo_frontend state: completed"* ]] +} + +@test "reports each state change only once" { + setup_stub replicating-then-done + run "${STACK_WAIT}" -s 1 -t 20 demo + [ "$status" -eq 0 ] + count=$(printf '%s\n' "$output" | grep -c "demo_backend state: deployed") + [ "$count" -eq 1 ] +} + +@test "fails immediately when an update is paused" { + setup_stub paused + run "${STACK_WAIT}" -s 1 -t 10 demo + [ "$status" -eq 1 ] + [[ "$output" == *"This deployment will not complete"* ]] +} + +@test "treats a rollback as failure by default" { + setup_stub rollback + run "${STACK_WAIT}" -s 1 -t 10 demo + [ "$status" -eq 1 ] + [[ "$output" == *"This deployment will not complete"* ]] +} + +@test "treats a rollback as success with -r" { + setup_stub rollback + run "${STACK_WAIT}" -r -s 1 -t 10 demo + [ "$status" -eq 0 ] + [[ "$output" == *"demo_backend state: rollback_completed"* ]] +} + +@test "times out when a service never converges" { + setup_stub stuck + run "${STACK_WAIT}" -s 1 -t 3 demo + [ "$status" -eq 1 ] + [[ "$output" == *"Timeout exceeded"* ]] +} + +@test "-n waits only for the named services" { + setup_stub converged + run "${STACK_WAIT}" -n backend -s 1 -t 10 demo + [ "$status" -eq 0 ] + [[ "$output" == *"demo_backend state: deployed"* ]] + # frontend was not selected, so it must never be polled + [[ "$output" != *"demo_frontend"* ]] +} + +@test "-p prints service logs on success" { + setup_stub converged + run "${STACK_WAIT}" -p 5 -s 1 -t 10 demo + [ "$status" -eq 0 ] + [[ "$output" == *"stub: log line"* ]] + [[ "$(stub_calls)" == *"service logs --tail 5"* ]] +} + +@test "-p prints service logs on timeout" { + setup_stub stuck + run "${STACK_WAIT}" -p 5 -s 1 -t 3 demo + [ "$status" -eq 1 ] + [[ "$output" == *"Timeout exceeded"* ]] + [[ "$output" == *"stub: log line"* ]] +} + +@test "requires exactly one stack name" { + setup_stub converged + run "${STACK_WAIT}" -s 1 -t 10 + [ "$status" -eq 1 ] + [[ "$output" == *"[opts] stack_name"* ]] +} + +@test "rejects an unknown flag instead of ignoring it" { + setup_stub converged + run "${STACK_WAIT}" -Z -s 1 -t 10 demo + [ "$status" -eq 1 ] + [[ "$output" == *"[opts] stack_name"* ]] +} + +@test "-h prints usage and exits zero" { + setup_stub converged + run "${STACK_WAIT}" -h demo + [ "$status" -eq 0 ] + [[ "$output" == *"[opts] stack_name"* ]] +} + +# --- Known bugs, to be un-skipped by the fixes ------------------------------- + +@test "issue #13: a completed replicated-job counts as converged" { + skip "https://github.com/kitconcept/docker-stack-deploy/issues/13 -- the '(1/1 completed)' suffix is discarded, so current(0) != target(1) forever" + setup_stub replicated-job + run "${STACK_WAIT}" -s 1 -t 5 demo + [ "$status" -eq 0 ] + [[ "$output" != *"Timeout exceeded"* ]] +} + +@test "issue #19: a service scaled to zero does not block the deploy" { + skip "https://github.com/kitconcept/docker-stack-deploy/issues/19 -- target is 1 because an external scheduler scaled it, so the wait never settles" + setup_stub zero-replicas + run "${STACK_WAIT}" -s 1 -t 5 demo + [ "$status" -eq 0 ] + [[ "$output" != *"Timeout exceeded"* ]] +} + +@test "issue #19: the final stack state is logged on timeout" { + skip "https://github.com/kitconcept/docker-stack-deploy/issues/19 -- 'docker stack ps' diagnostics are not implemented yet" + setup_stub stuck + run "${STACK_WAIT}" -s 1 -t 3 demo + [ "$status" -eq 1 ] + [[ "$(stub_calls)" == *"stack ps"* ]] +} diff --git a/tests/test_helper.bash b/tests/test_helper.bash new file mode 100644 index 0000000..3d02a4c --- /dev/null +++ b/tests/test_helper.bash @@ -0,0 +1,27 @@ +#!/usr/bin/env bash +# +# Shared setup for the bats suite. + +# Put the fake docker CLI ahead of anything real on PATH and point it at a +# scenario fixture. Call as: setup_stub +setup_stub() { + export DSD_STUB_DIR="${BATS_TEST_TMPDIR}/stub" + export DSD_STUB_SCENARIO="${BATS_TEST_DIRNAME}/fixtures/${1}.services" + mkdir -p "${DSD_STUB_DIR}" + export PATH="${BATS_TEST_DIRNAME}/helpers/bin:${PATH}" + + if [ ! -f "${DSD_STUB_SCENARIO}" ]; then + echo "no such fixture: ${DSD_STUB_SCENARIO}" >&2 + return 1 + fi +} + +# Everything the script under test asked docker to do, one invocation per line. +stub_calls() { + cat "${DSD_STUB_DIR}/calls.log" 2>/dev/null || true +} + +# Absolute path to a script under scripts/. +script_path() { + echo "${BATS_TEST_DIRNAME}/../scripts/${1}" +} From f5aa7031dfb4f29afcfb99dae10b2483671cdab8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89rico=20Andrei?= Date: Thu, 3 Sep 2026 18:54:41 -0300 Subject: [PATCH 2/4] Read env_file line by line so values may contain spaces The whole file was passed through an unquoted command substitution, so the shell word-split it before export ran. Any value with a space in it was truncated to its first token and the remainder was handed to export as a separate argument, which then failed with a message naming the second token: export: `-Xmx1536m': not a valid identifier No quoting style in the env file could work around it, because the split happened after substitution. JVM options were the obvious casualty, but connection strings and user agents hit it too. Read one line at a time instead and pass export a single quoted argument. Values are taken verbatim, matching `docker --env-file`, so quotes in the file are part of the value rather than delimiters around it. The SC2046 suppression that was hiding this is gone. A line that is not NAME=VALUE is now rejected with a message naming the offending line, rather than reaching export and producing one about identifiers. This is also the error someone gets after passing a path where the option wants the variables themselves. Note that ENV_FILE carries no trailing newline, so the loop needs the -n test to avoid dropping the last line. Closes #21 --- scripts/docker-entrypoint.sh | 25 ++++++++++++++++--- tests/entrypoint.bats | 48 +++++++++++++++++++++++++++++++++--- 2 files changed, 65 insertions(+), 8 deletions(-) diff --git a/scripts/docker-entrypoint.sh b/scripts/docker-entrypoint.sh index 6a2d0ea..12b42e7 100755 --- a/scripts/docker-entrypoint.sh +++ b/scripts/docker-entrypoint.sh @@ -34,14 +34,31 @@ configure_ssh_key() { configure_env_file() { printf '%s' "$ENV_FILE" > "${ENV_FILE_PATH}" - env_file_len=$(grep -v '^#' ${ENV_FILE_PATH}|grep -v '^$' -c) - if [[ $env_file_len -gt 0 ]]; then + env_file_len=$(grep -cv -e '^#' -e '^$' "${ENV_FILE_PATH}" || true) + if [[ ${env_file_len} -gt 0 ]]; then echo "Environment Variables: Additional values" if [ "${DEBUG}" != "0" ]; then echo "Environment vars before: $(env|wc -l)" fi - # shellcheck disable=SC2046 - export $(grep -v '^#' ${ENV_FILE_PATH} | grep -v '^$' | xargs -d '\n') + # Read one line at a time and hand export a single quoted argument, so a + # value containing spaces survives. Passing the whole file through an + # unquoted $(...) word-split it before export ever ran. + # + # Values are taken verbatim, matching `docker --env-file`: quotes in the + # file are part of the value, not delimiters around it. + # + # ENV_FILE has no trailing newline, so the `-n` test is what keeps the + # final line from being dropped by read's non-zero exit. + while IFS= read -r line || [ -n "${line}" ]; do + case "${line}" in + ''|\#*) continue ;; + esac + if [[ ! "${line}" =~ ^[A-Za-z_][A-Za-z0-9_]*= ]]; then + echo "Environment Variables: '${line}' is not in NAME=VALUE format" + exit 1 + fi + export "${line?}" + done < "${ENV_FILE_PATH}" if [ "${DEBUG}" != "0" ]; then echo "Environment vars after: $(env|wc -l)" fi diff --git a/tests/entrypoint.bats b/tests/entrypoint.bats index 05e3172..cce23e6 100644 --- a/tests/entrypoint.bats +++ b/tests/entrypoint.bats @@ -151,20 +151,60 @@ export_from_env_file() { [ "$output" = "key=value" ] } -@test "issue #21: env_file exports a value containing spaces" { - skip "https://github.com/kitconcept/docker-stack-deploy/issues/21 -- the unquoted \$(...) word-splits the value before export runs" +@test "env_file exports a value containing spaces (issue #21)" { export_from_env_file 'SOLR_JAVA_MEM=-Xms1536m -Xmx1536m' SOLR_JAVA_MEM [ "$status" -eq 0 ] [ "$output" = "-Xms1536m -Xmx1536m" ] } -@test "issue #21: quotes in an env_file value are kept verbatim" { - skip "https://github.com/kitconcept/docker-stack-deploy/issues/21 -- pending the parser rewrite; we match 'docker --env-file' and do not strip quotes" +@test "env_file keeps quotes in a value verbatim (issue #21)" { export_from_env_file 'GREETING="hello world"' GREETING [ "$status" -eq 0 ] [ "$output" = '"hello world"' ] } +@test "env_file preserves several space-bearing values in one file (issue #21)" { + export_from_env_file $'JAVA_OPTS=-Xms1g -Xmx2g\nDB_USER=plone' JAVA_OPTS + [ "$status" -eq 0 ] + [ "$output" = "-Xms1g -Xmx2g" ] +} + +@test "env_file still exports later keys after a space-bearing value" { + export_from_env_file $'JAVA_OPTS=-Xms1g -Xmx2g\nDB_USER=plone' DB_USER + [ "$status" -eq 0 ] + [ "$output" = "plone" ] +} + +@test "env_file preserves a trailing space in a value" { + export_from_env_file 'GREETING=hello ' GREETING + [ "$status" -eq 0 ] + [ "$output" = "hello " ] +} + +@test "env_file rejects a line that is not NAME=VALUE" { + # Issue #3 reported passing a path (".env") and getting an opaque + # "not a valid identifier" error from export. Fail with a clear message. + run env -i PATH="${PATH}" HOME="${BATS_TEST_TMPDIR}" ENV_FILE=".env" \ + bash -c " + source '${ENTRYPOINT}' + ENV_FILE_PATH=\"\${HOME}/.env\" + configure_env_file + " + [ "$status" -eq 1 ] + [[ "$output" == *"'.env' is not in NAME=VALUE format"* ]] +} + +@test "env_file rejects a name with a hyphen" { + run env -i PATH="${PATH}" HOME="${BATS_TEST_TMPDIR}" ENV_FILE="MY-VAR=1" \ + bash -c " + source '${ENTRYPOINT}' + ENV_FILE_PATH=\"\${HOME}/.env\" + configure_env_file + " + [ "$status" -eq 1 ] + [[ "$output" == *"is not in NAME=VALUE format"* ]] +} + # --- scale_after -------------------------------------------------------------- @test "scale_after does nothing when unset" { From c84be84115effd5dbc02a0f45a93e345047fd658 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89rico=20Andrei?= Date: Thu, 3 Sep 2026 18:59:23 -0300 Subject: [PATCH 3/4] Stop waiting for services that are not meant to be running Both open timeout reports come from the same place: the wait loop treats "fewer replicas running than wanted" as "still converging", which is wrong for any service that is not supposed to be running. A `mode: replicated-job` service that has finished reports its replicas as "0/1 (1/1 completed)". The completion count was cut away before anything looked at it, leaving current(0) != target(1) forever. Read the whole field and use the count: a job is settled once every requested run has completed, and reported as job_running until then. A service the stack file declares with `replicas: 0` is a different case, and indistinguishable from the one above by the replicas field alone -- there is no completion count to disambiguate a finished run from a service that failed to start. So consult the spec instead of the observed target. The two disagree exactly when this bites: stack deploy returns before the new spec is applied, and an external scheduler may have scaled the service since. The spec is re-read every poll, so a stale target corrects itself rather than being trusted once. A service whose spec really does ask for replicas still times out; that is covered by a test, because the shortcut above would be easy to widen into swallowing genuine failures. On either failure the task list is now dumped via docker stack ps. The per-service log only reports changes, so working out which service stalled previously meant reading back through the whole log. Not included: passing --detach=false to docker stack deploy, suggested on the second issue. It would make deploy block until convergence, which replaces this script's configurable timeout with docker's own waiting and could hang on precisely the services above. Worth its own discussion rather than folding in here. Closes #13 Closes #19 --- scripts/stack-wait.sh | 49 +++++++++++++++++++++++++-- tests/fixtures/failed-start.services | 4 +++ tests/fixtures/running-job.services | 6 ++++ tests/fixtures/zero-replicas.services | 8 +++-- tests/helpers/bin/docker | 11 ++++++ tests/stack-wait.bats | 37 ++++++++++++++++---- 6 files changed, 102 insertions(+), 13 deletions(-) create mode 100644 tests/fixtures/failed-start.services create mode 100644 tests/fixtures/running-job.services diff --git a/scripts/stack-wait.sh b/scripts/stack-wait.sh index 774ce13..3928b89 100755 --- a/scripts/stack-wait.sh +++ b/scripts/stack-wait.sh @@ -39,6 +39,7 @@ check_timeout() { cutoff_epoc=$((start_epoc + opt_t - opt_s)) if [ "$cur_epoc" -gt "$cutoff_epoc" ]; then echo "Error: Timeout exceeded" + print_stack_state print_service_logs exit 1 fi @@ -74,6 +75,14 @@ service_state() { eval cache_${service_safe}=\"\$state\" fi } +print_stack_state() { + # On failure the per-service state log only shows what changed, so the state + # each service was left in has to be reconstructed by reading back through + # it. Dump the task list instead: it names the service that stalled and + # carries the error column. + echo "Stack state at failure:" + docker stack ps --no-trunc "${stack_name}" || true +} print_service_logs() { if [ "$opt_p" != "0" ]; then service_ids=$(get_service_ids) @@ -130,10 +139,41 @@ while [ "$stack_done" != "1" ]; do # identify/report current state if [ "$service_done" != "2" ]; then - replicas=$(docker service ls --format '{{.Replicas}}' --filter "id=$service_id" | cut -d' ' -f1) + # The whole field, which is "1/1" for a plain service but carries a + # completion count for a job: "0/1 (1/1 completed)". + replicas_full=$(docker service ls --format '{{.Replicas}}' --filter "id=$service_id") + replicas=$(echo "$replicas_full" | cut -d' ' -f1) current=$(echo "$replicas" | cut -d/ -f1) target=$(echo "$replicas" | cut -d/ -f2) - if [ "$current" != "$target" ]; then + + # Runs finished and runs wanted, for a `mode: replicated-job` service. + # Empty for every other mode, which is what selects the branch below. + job_done=$(echo "$replicas_full" | sed -n 's/.*(\([0-9]*\)\/[0-9]* completed).*/\1/p') + job_total=$(echo "$replicas_full" | sed -n 's/.*([0-9]*\/\([0-9]*\) completed).*/\1/p') + + # Replicas the stack file asks for, which is not always what + # `docker service ls` reports as the target yet: `docker stack deploy` + # returns before the new spec has been applied, and an external + # scheduler may have scaled the service in the meantime. Re-read every + # poll so a stale target corrects itself. + spec_replicas=$(docker service inspect \ + --format '{{if .Spec.Mode.Replicated}}{{.Spec.Mode.Replicated.Replicas}}{{end}}' \ + "$service_id" 2>/dev/null || echo "") + + if [ -n "$job_total" ]; then + # A job is finished when every requested run has completed. Its + # running count goes back to 0 and must not be read as "not started". + if [ "$job_done" = "$job_total" ]; then + state="job_completed" + else + service_done=0 + state="job_running $job_done/$job_total" + fi + elif [ "$spec_replicas" = "0" ]; then + # Deliberately stopped by the stack file, so there is nothing to wait + # for even if the target has not caught up yet. + state="scaled_to_zero" + elif [ "$current" != "$target" ]; then # actively replicating service service_done=0 state="replicating $replicas" @@ -142,9 +182,11 @@ while [ "$stack_done" != "1" ]; do service_state "$service" "$state" # check for states that indicate an update is done + # Keep this list in sync with the states assigned above: a state that is + # settled but missing here falls into the catch-all and waits forever. if [ "$service_done" = "1" ]; then case "$state" in - deployed|completed|rollback_completed) + deployed|completed|rollback_completed|job_completed|scaled_to_zero) service_done=1 ;; *) @@ -165,6 +207,7 @@ while [ "$stack_done" != "1" ]; do done if [ "$stack_done" = "2" ]; then echo "Error: This deployment will not complete" + print_stack_state print_service_logs exit 1 fi diff --git a/tests/fixtures/failed-start.services b/tests/fixtures/failed-start.services new file mode 100644 index 0000000..8462d64 --- /dev/null +++ b/tests/fixtures/failed-start.services @@ -0,0 +1,4 @@ +# Looks identical to the zero-replica case in the replicas field, but the +# stack file really does want 1 replica (fifth field), so this is a service +# that failed to start and must still be reported as a failure. +svc_backend|demo_backend|deployed|0/1|1 diff --git a/tests/fixtures/running-job.services b/tests/fixtures/running-job.services new file mode 100644 index 0000000..eb59b91 --- /dev/null +++ b/tests/fixtures/running-job.services @@ -0,0 +1,6 @@ +# A replicated-job still on its first run, then finished. +svc_migrate|demo_migrate|deployed|1/1 (0/1 completed) +svc_backend|demo_backend|deployed|1/1 +--- +svc_migrate|demo_migrate|deployed|0/1 (1/1 completed) +svc_backend|demo_backend|deployed|1/1 diff --git a/tests/fixtures/zero-replicas.services b/tests/fixtures/zero-replicas.services index 4787425..759bdcc 100644 --- a/tests/fixtures/zero-replicas.services +++ b/tests/fixtures/zero-replicas.services @@ -1,6 +1,8 @@ # Issue #19: a service the stack file declares with `replicas: 0`, which an # external scheduler (swarm-cronjob) has since scaled to a target of 1. -# Indistinguishable from issue #13 at the replicas field alone -- there is no -# completion suffix here to disambiguate a finished run from a failed start. -svc_dbpack|demo_dbpack|deployed|0/1 +# +# Fifth field is the spec replicas, i.e. what the stack file asks for. It +# diverges from the target in the replicas field here on purpose: that +# divergence is the bug, and reading the spec is what resolves it. +svc_dbpack|demo_dbpack|deployed|0/1|0 svc_backend|demo_backend|deployed|1/1 diff --git a/tests/helpers/bin/docker b/tests/helpers/bin/docker index 943eab6..61adad3 100755 --- a/tests/helpers/bin/docker +++ b/tests/helpers/bin/docker @@ -131,6 +131,17 @@ case "${1:-}" in *'.Spec.Name'*) lookup "${target}" 2 ;; + *'.Spec.Mode.Replicated'*) + # Replicas the stack file asks for. Fixtures may state it in a + # fifth field when it differs from what `service ls` reports -- + # that difference is what issue #19 is about. Otherwise it is + # just the target from the replicas field. + spec=$(lookup "${target}" 5) + if [ -z "${spec}" ]; then + spec=$(lookup "${target}" 4 | cut -d' ' -f1 | cut -d/ -f2) + fi + printf '%s\n' "${spec}" + ;; *'.UpdateStatus'*) lookup "${target}" 3 ;; diff --git a/tests/stack-wait.bats b/tests/stack-wait.bats index 28f8a82..906a3b7 100644 --- a/tests/stack-wait.bats +++ b/tests/stack-wait.bats @@ -109,28 +109,51 @@ setup() { [[ "$output" == *"[opts] stack_name"* ]] } -# --- Known bugs, to be un-skipped by the fixes ------------------------------- +# --- Services that are not meant to be running (issues #13, #19) ------------------------------- -@test "issue #13: a completed replicated-job counts as converged" { - skip "https://github.com/kitconcept/docker-stack-deploy/issues/13 -- the '(1/1 completed)' suffix is discarded, so current(0) != target(1) forever" +@test "a completed replicated-job counts as converged (issue #13)" { setup_stub replicated-job run "${STACK_WAIT}" -s 1 -t 5 demo [ "$status" -eq 0 ] [[ "$output" != *"Timeout exceeded"* ]] } -@test "issue #19: a service scaled to zero does not block the deploy" { - skip "https://github.com/kitconcept/docker-stack-deploy/issues/19 -- target is 1 because an external scheduler scaled it, so the wait never settles" +@test "a service the stack file scales to zero does not block (issue #19)" { setup_stub zero-replicas run "${STACK_WAIT}" -s 1 -t 5 demo [ "$status" -eq 0 ] [[ "$output" != *"Timeout exceeded"* ]] } -@test "issue #19: the final stack state is logged on timeout" { - skip "https://github.com/kitconcept/docker-stack-deploy/issues/19 -- 'docker stack ps' diagnostics are not implemented yet" +@test "the final stack state is logged on timeout (issue #19)" { setup_stub stuck run "${STACK_WAIT}" -s 1 -t 3 demo [ "$status" -eq 1 ] [[ "$(stub_calls)" == *"stack ps"* ]] } + +@test "a still-running replicated-job is waited for (issue #13)" { + setup_stub running-job + run "${STACK_WAIT}" -s 1 -t 20 demo + [ "$status" -eq 0 ] + # Reported as running while incomplete, then settled once it finishes. + [[ "$output" == *"demo_migrate state: job_running 0/1"* ]] + [[ "$output" == *"demo_migrate state: job_completed"* ]] +} + +@test "a genuinely failed service still times out (issue #19 regression guard)" { + # Same 0/1 as the zero-replica case, but the stack file wants 1 replica. + # This must NOT be swallowed by the scaled_to_zero shortcut. + setup_stub failed-start + run "${STACK_WAIT}" -s 1 -t 3 demo + [ "$status" -eq 1 ] + [[ "$output" == *"Timeout exceeded"* ]] +} + +@test "the final stack state is logged when a deployment cannot complete" { + setup_stub paused + run "${STACK_WAIT}" -s 1 -t 10 demo + [ "$status" -eq 1 ] + [[ "$output" == *"Stack state at failure:"* ]] + [[ "$(stub_calls)" == *"stack ps"* ]] +} From e25c923366169cc4deee34985977282b8451d1f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89rico=20Andrei?= Date: Thu, 3 Sep 2026 21:08:25 -0300 Subject: [PATCH 4/4] Manage the change log with towncrier Adds the towncrier config, an empty CHANGELOG.md, the fragment template and a workflow that fails a pull request carrying no news fragment. News fragments for the three fixes on this branch come with it. Two things had to follow from it. The changelog workflow exempts a pull request labelled "skip changelog", and that label did not exist in the repository, so the escape hatch worked for nobody; it exists now. Dependabot cannot write a fragment for itself, so both of its update blocks carry that label, otherwise every dependency bump would arrive with a failing check. The generated CHANGELOG.md pointed contributors at Plone's contributing guide. It points at this repository's README instead, which now documents the fragment types, the naming convention and the label. Also excludes the changelog files from the docker build context, the way README.md and Makefile already are. --- .dockerignore | 3 +++ .github/dependabot.yml | 4 +++ .github/workflows/changelog.yml | 48 +++++++++++++++++++++++++++++++++ CHANGELOG.md | 9 +++++++ Makefile | 8 ++++++ README.md | 22 +++++++++++++++ news/+test-harness.internal | 1 + news/+towncrier.internal | 1 + news/.changelog_template.jinja | 15 +++++++++++ news/13.bugfix | 1 + news/19.bugfix | 1 + news/21.bugfix | 1 + towncrier.toml | 33 +++++++++++++++++++++++ 13 files changed, 147 insertions(+) create mode 100644 .github/workflows/changelog.yml create mode 100644 CHANGELOG.md create mode 100644 news/+test-harness.internal create mode 100644 news/+towncrier.internal create mode 100644 news/.changelog_template.jinja create mode 100644 news/13.bugfix create mode 100644 news/19.bugfix create mode 100644 news/21.bugfix create mode 100644 towncrier.toml diff --git a/.dockerignore b/.dockerignore index 339cbcc..ddeb75f 100644 --- a/.dockerignore +++ b/.dockerignore @@ -7,3 +7,6 @@ Makefile README.md Dockerfile.test tests +CHANGELOG.md +news +towncrier.toml diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 1280eae..3d914b2 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -14,8 +14,11 @@ updates: patterns: - "*" # Must already exist in the repository, otherwise it is silently ignored. + # "skip changelog" is what exempts these pull requests from the changelog + # workflow, which dependabot has no way of satisfying itself. labels: - "dependencies" + - "skip changelog" - package-ecosystem: "docker" # Keeps the pinned base image in Dockerfile current. @@ -27,3 +30,4 @@ updates: default-days: 7 labels: - "dependencies" + - "skip changelog" diff --git a/.github/workflows/changelog.yml b/.github/workflows/changelog.yml new file mode 100644 index 0000000..78b94a4 --- /dev/null +++ b/.github/workflows/changelog.yml @@ -0,0 +1,48 @@ +name: "Changelog" +on: + pull_request: + types: [assigned, opened, synchronize, reopened, labeled, unlabeled] + branches: + - main + +env: + base-branch: main + +permissions: + contents: read + pull-requests: read + +jobs: + + checks: + runs-on: ubuntu-latest + if: ${{ !contains(github.event.pull_request.labels.*.name, 'skip changelog') }} + steps: + - uses: actions/checkout@v7 + with: + # Fetch all history + fetch-depth: '0' + + - name: Setup uv + uses: plone/meta/.github/actions/setup_uv@2.x + with: + python-version: "3.14" + working-directory: '.' + + - name: "Fetch base branch" + run: | + # Reference: https://github.com/actions/checkout/#fetch-all-branches. + git fetch --no-tags origin ${{ env.base-branch }} + + - name: "Repository: Check" + id: repository-changelog + run: | + uvx towncrier check --compare-with origin/${{ env.base-branch }} --config ./towncrier.toml --dir ./ + + - name: "Report check" + if: ${{ always() }} + run: | + echo '# Workflow Report' >> $GITHUB_STEP_SUMMARY + echo '| Job ID | Conclusion |' >> $GITHUB_STEP_SUMMARY + echo '| --- | --- |' >> $GITHUB_STEP_SUMMARY + echo '| repository |${{ steps.repository-changelog.conclusion }} |' >> $GITHUB_STEP_SUMMARY diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..d8e1283 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,9 @@ +# Change log + + + + diff --git a/Makefile b/Makefile index fc29c43..175518d 100644 --- a/Makefile +++ b/Makefile @@ -57,6 +57,13 @@ build-test-image: # Build the bats test runner image test: build-test-image # Run the bats test suite docker run --rm -v "$(PWD)":/code -w /code $(TEST_RUNNER_IMAGE) tests/ +# Changelog +.PHONY: draft-changelog +draft-changelog: # Display the draft of the changelog + @uvx towncrier build --draft --version unreleased --config towncrier.toml + +# Release +.PHONY: create-tag create-tag: # Create a new tag using git @test -n "$(VERSION)" if git show-ref --tags v$(VERSION) --quiet; \ @@ -65,6 +72,7 @@ create-tag: # Create a new tag using git else \ echo "Creating new tag $(VERSION)"; \ sed -i 's/$(BASE_NAME):latest/$(BASE_NAME):$(VERSION)/' action.yml; \ + uvx towncrier build --yes --version $(VERSION) --config towncrier.toml; \ git commit -am "Prepare release $(VERSION)"; \ git tag -a v$(VERSION) -m "Release $(VERSION)"; \ git push && git push --tags; \ diff --git a/README.md b/README.md index 14a7ba7..d25714a 100644 --- a/README.md +++ b/README.md @@ -224,6 +224,28 @@ Tests that document a currently open bug call `skip` with a link to the issue. They are written to assert the *desired* behaviour, so fixing the bug means deleting the `skip` line rather than writing a new test. +### Change log + +`CHANGELOG.md` is generated by [towncrier](https://towncrier.readthedocs.io/) — +do not edit it directly. Add a file to `news/` instead, named for the issue it +addresses, and CI will check that every pull request has one. + +``` +news/21.bugfix # an issue number, when there is an issue +news/+test-harness.internal # a short slug, when there is not +``` + +Types are `breaking`, `feature`, `bugfix`, `internal` and `documentation`. Write +one sentence in the past tense, describing the change from the user's side, and +sign it with your GitHub handle: + +``` +Fixed `env_file` values containing spaces being truncated. @ericof +``` + +Preview the result with `uvx towncrier build --draft --version `. A pull +request that genuinely needs no entry can carry the `skip changelog` label. + ## Credits diff --git a/news/+test-harness.internal b/news/+test-harness.internal new file mode 100644 index 0000000..e78d1a3 --- /dev/null +++ b/news/+test-harness.internal @@ -0,0 +1 @@ +Added a bats test suite for the shell scripts, run in CI together with shellcheck. @ericof diff --git a/news/+towncrier.internal b/news/+towncrier.internal new file mode 100644 index 0000000..12c75aa --- /dev/null +++ b/news/+towncrier.internal @@ -0,0 +1 @@ +Added towncrier to manage the change log, with a CI check that every pull request carries a news fragment. @ericof diff --git a/news/.changelog_template.jinja b/news/.changelog_template.jinja new file mode 100644 index 0000000..b35bff3 --- /dev/null +++ b/news/.changelog_template.jinja @@ -0,0 +1,15 @@ +{% if sections[""] %} +{% for category, val in definitions.items() if category in sections[""] %} + +### {{ definitions[category]['name'] }} + +{% for text, values in sections[""][category].items() %} +- {{ text }} {{ values|join(', ') }} +{% endfor %} + +{% endfor %} +{% else %} +No significant changes. + + +{% endif %} \ No newline at end of file diff --git a/news/13.bugfix b/news/13.bugfix new file mode 100644 index 0000000..58ced2d --- /dev/null +++ b/news/13.bugfix @@ -0,0 +1 @@ +Fixed the deploy waiting until it timed out on a `mode: replicated-job` service that had already run to completion. @ericof diff --git a/news/19.bugfix b/news/19.bugfix new file mode 100644 index 0000000..f0b7862 --- /dev/null +++ b/news/19.bugfix @@ -0,0 +1 @@ +Fixed the deploy waiting until it timed out on services the stack file scales to zero, and added the `docker stack ps` task list to the output when a deploy fails. @ericof diff --git a/news/21.bugfix b/news/21.bugfix new file mode 100644 index 0000000..45a3339 --- /dev/null +++ b/news/21.bugfix @@ -0,0 +1 @@ +Fixed `env_file` values containing spaces being truncated to their first word. Each line is now read whole and the value taken verbatim, matching `docker --env-file`. @ericof diff --git a/towncrier.toml b/towncrier.toml new file mode 100644 index 0000000..5d8d498 --- /dev/null +++ b/towncrier.toml @@ -0,0 +1,33 @@ +[tool.towncrier] +filename = "CHANGELOG.md" +directory = "news/" +title_format = "## {version} ({project_date})" +underlines = ["", "", ""] +template = "./news/.changelog_template.jinja" +start_string = "\n" +issue_format = "[#{issue}](https://github.com/kitconcept/docker-stack-deploy/issues/{issue})" + +[[tool.towncrier.type]] +directory = "breaking" +name = "Breaking" +showcontent = true + +[[tool.towncrier.type]] +directory = "feature" +name = "Feature" +showcontent = true + +[[tool.towncrier.type]] +directory = "bugfix" +name = "Bugfix" +showcontent = true + +[[tool.towncrier.type]] +directory = "internal" +name = "Internal" +showcontent = true + +[[tool.towncrier.type]] +directory = "documentation" +name = "Documentation" +showcontent = true \ No newline at end of file