diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 971fcd9a..3c9caaa2 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -44,6 +44,8 @@ jobs: mv druid /usr/local/bin/druid - name: Validate all scrolls run: ./scripts/validate_all_scrolls.sh + - name: Test bounded push parallelism + run: bash ./scripts/tests/push-parallel.test.sh - name: Login to registry if: github.event.pull_request.head.repo.full_name == github.repository run: druid login --host ${{ secrets.SCROLL_REGISTRY_HOST }} --user '${{ secrets.SCROLL_REGISTRY_USER }}' --password ${{ secrets.SCROLL_REGISTRY_PASSWORD }} diff --git a/AGENTS.md b/AGENTS.md index 4e53858e..436d208a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -11,6 +11,10 @@ Context for humans and coding agents working in this repository. ## Release workflow - `.github/workflows/release.yml` installs `druid` from [highcard-dev/druid-cli](https://github.com/highcard-dev/druid-cli), validates scrolls, logs into the registry, pushes **categories**, then pushes individual scrolls. +- `scripts/push.sh` defaults to serial publication with `SCROLL_PUSH_JOBS=1`. + Local callers may use a bounded higher value, but every category push must + finish before artifact pushes begin and any background failure must fail the + script after all started jobs have been reaped. - When adding a new scroll family, you usually need both a **Push Categories** line (for that family’s `.meta`) and **Pushing new scrolls** lines for each version directory. - All production scroll/artifact changes must go through CI/CD. Do not manually push production scrolls or mutate production registry state unless explicitly authorized for an emergency; follow up with a repo change so CI is source of truth again. @@ -35,6 +39,8 @@ druid push category [] ## Validation - `./scripts/validate_all_scrolls.sh` runs `druid scroll validate --strict` on every directory that contains a `scroll.yaml`. +- `./scripts/tests/push-parallel.test.sh` verifies the serial default, bounded + concurrency, category barrier, full catalog invocation, and failure cleanup. ## Source of truth for CLI behavior diff --git a/scripts/push.sh b/scripts/push.sh index 2970c528..8973a0b5 100755 --- a/scripts/push.sh +++ b/scripts/push.sh @@ -16,6 +16,12 @@ SCROLL_REGISTRY_PASSWORD="${SCROLL_REGISTRY_PASSWORD:-}" DRUID_CLI_VERSION="${DRUID_CLI_VERSION:-v0.1.249}" SCROLL_PUSH_CATEGORIES="${SCROLL_PUSH_CATEGORIES:-1}" SCROLL_PUSH_ARTIFACTS="${SCROLL_PUSH_ARTIFACTS:-1}" +SCROLL_PUSH_JOBS="${SCROLL_PUSH_JOBS:-1}" + +if [[ ! "$SCROLL_PUSH_JOBS" =~ ^[1-9][0-9]*$ ]]; then + echo "SCROLL_PUSH_JOBS must be a positive integer (got: $SCROLL_PUSH_JOBS)" >&2 + exit 2 +fi # Used by PR CI to publish the same catalog as preview tags, for example: # SCROLL_REGISTRY_NAMESPACE=druid-team-experimental SCROLL_TAG_SUFFIX=-pr123. @@ -30,6 +36,28 @@ runtime_prefix="${registry_host}/${runtime_namespace}" runtime_image="${DRUID_SCROLL_RUNTIME_IMAGE:-${runtime_prefix}/druid:${DRUID_CLI_VERSION}}" steamcmd_image="${DRUID_SCROLL_STEAMCMD_IMAGE:-${runtime_image}-steamcmd}" +push_pids=() +push_phase_failed=0 + +wait_for_oldest_push() { + local pid="${push_pids[0]}" + + if ! wait "$pid"; then + push_phase_failed=1 + fi + push_pids=("${push_pids[@]:1}") +} + +wait_for_push_phase() { + while ((${#push_pids[@]} > 0)); do + wait_for_oldest_push + done + + local failed="$push_phase_failed" + push_phase_failed=0 + return "$failed" +} + login_if_configured() { if [[ "$SCROLL_PUSH_DRY_RUN" = "1" ]]; then return 0 @@ -68,7 +96,17 @@ run() { if [[ "$SCROLL_PUSH_DRY_RUN" = "1" ]]; then return 0 fi - eval "$command" + + if ((SCROLL_PUSH_JOBS == 1)); then + eval "$command" + return + fi + + eval "$command" & + push_pids+=("$!") + if ((${#push_pids[@]} >= SCROLL_PUSH_JOBS)); then + wait_for_oldest_push + fi } push_release_categories() { @@ -193,8 +231,16 @@ login_if_configured if [[ "$SCROLL_PUSH_CATEGORIES" = "1" ]]; then push_release_categories + if ! wait_for_push_phase; then + echo "One or more category pushes failed." >&2 + exit 1 + fi fi if [[ "$SCROLL_PUSH_ARTIFACTS" = "1" ]]; then push_release_artifacts + if ! wait_for_push_phase; then + echo "One or more artifact pushes failed." >&2 + exit 1 + fi fi diff --git a/scripts/tests/fixtures/fake-druid.sh b/scripts/tests/fixtures/fake-druid.sh new file mode 100644 index 00000000..434b6bee --- /dev/null +++ b/scripts/tests/fixtures/fake-druid.sh @@ -0,0 +1,54 @@ +#!/usr/bin/env bash + +set -u + +state_dir="${FAKE_DRUID_STATE_DIR:?FAKE_DRUID_STATE_DIR is required}" +sleep_seconds="${FAKE_DRUID_SLEEP:-0.02}" +lock_dir="$state_dir/.lock" + +acquire_lock() { + while ! mkdir "$lock_dir" 2>/dev/null; do + sleep 0.005 + done +} + +release_lock() { + rmdir "$lock_dir" +} + +kind="artifact" +ref="${2:-}" +if [[ "${1:-}" = "push" && "${2:-}" = "category" ]]; then + kind="category" + ref="${3:-}" +fi + +acquire_lock +active=0 +maximum=0 +[[ -f "$state_dir/active" ]] && read -r active < "$state_dir/active" +[[ -f "$state_dir/max" ]] && read -r maximum < "$state_dir/max" +active=$((active + 1)) +if ((active > maximum)); then + maximum="$active" +fi +printf '%s\n' "$active" > "$state_dir/active" +printf '%s\n' "$maximum" > "$state_dir/max" +printf 'start %s %s\n' "$kind" "$*" >> "$state_dir/events.log" +release_lock + +sleep "$sleep_seconds" + +status=0 +if [[ -n "${FAKE_DRUID_FAIL_REF:-}" && "$ref" = "$FAKE_DRUID_FAIL_REF" ]]; then + status=23 +fi + +acquire_lock +read -r active < "$state_dir/active" +active=$((active - 1)) +printf '%s\n' "$active" > "$state_dir/active" +printf 'end %s %s status=%s\n' "$kind" "$*" "$status" >> "$state_dir/events.log" +release_lock + +exit "$status" diff --git a/scripts/tests/push-parallel.test.sh b/scripts/tests/push-parallel.test.sh new file mode 100644 index 00000000..cf856e91 --- /dev/null +++ b/scripts/tests/push-parallel.test.sh @@ -0,0 +1,120 @@ +#!/usr/bin/env bash + +set -euo pipefail + +repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +push_script="$repo_root/scripts/push.sh" +fake_druid_source="$repo_root/scripts/tests/fixtures/fake-druid.sh" +tmp_dir="$(mktemp -d)" +trap 'rm -rf "$tmp_dir"' EXIT +fake_druid="$tmp_dir/fake-druid" +cp "$fake_druid_source" "$fake_druid" +chmod +x "$fake_druid" + +fail() { + echo "FAIL: $*" >&2 + exit 1 +} + +assert_equals() { + local expected="$1" + local actual="$2" + local message="$3" + [[ "$actual" = "$expected" ]] || fail "$message (expected $expected, got $actual)" +} + +reset_state() { + rm -rf "$tmp_dir/state" + mkdir -p "$tmp_dir/state" +} + +category_dry_run="$( + SCROLL_PUSH_DRY_RUN=1 \ + SCROLL_PUSH_CATEGORIES=1 \ + SCROLL_PUSH_ARTIFACTS=0 \ + bash "$push_script" +)" +artifact_dry_run="$( + SCROLL_PUSH_DRY_RUN=1 \ + SCROLL_PUSH_CATEGORIES=0 \ + SCROLL_PUSH_ARTIFACTS=1 \ + bash "$push_script" +)" +expected_categories="$(printf '%s\n' "$category_dry_run" | grep -Ec '^"?druid"? push category ')" +expected_artifacts="$(printf '%s\n' "$artifact_dry_run" | grep -Ec '^"?druid"? push ')" + +serial_dry_run="$(SCROLL_PUSH_DRY_RUN=1 SCROLL_PUSH_JOBS=1 bash "$push_script")" +parallel_dry_run="$(SCROLL_PUSH_DRY_RUN=1 SCROLL_PUSH_JOBS=4 bash "$push_script")" +assert_equals "$serial_dry_run" "$parallel_dry_run" \ + "dry-run catalog output must stay deterministic across job counts" + +reset_state +if SCROLL_PUSH_JOBS=0 \ + DRUID_BIN="$fake_druid" \ + FAKE_DRUID_STATE_DIR="$tmp_dir/state" \ + bash "$push_script" >"$tmp_dir/invalid.out" 2>"$tmp_dir/invalid.err"; then + fail "SCROLL_PUSH_JOBS=0 must be rejected" +fi +grep -q 'SCROLL_PUSH_JOBS must be a positive integer' "$tmp_dir/invalid.err" || \ + fail "invalid job count must report a useful validation error" +[[ ! -f "$tmp_dir/state/events.log" ]] || \ + fail "invalid job count must be rejected before invoking druid" + +reset_state +( + unset SCROLL_PUSH_JOBS + DRUID_BIN="$fake_druid" \ + FAKE_DRUID_STATE_DIR="$tmp_dir/state" \ + FAKE_DRUID_SLEEP=0.01 \ + SCROLL_PUSH_CATEGORIES=1 \ + SCROLL_PUSH_ARTIFACTS=0 \ + bash "$push_script" >"$tmp_dir/default.out" +) +assert_equals "1" "$(<"$tmp_dir/state/max")" \ + "the default must preserve serial push behavior" + +reset_state +DRUID_BIN="$fake_druid" \ +FAKE_DRUID_STATE_DIR="$tmp_dir/state" \ +FAKE_DRUID_SLEEP=0.02 \ +SCROLL_PUSH_JOBS=3 \ + bash "$push_script" >"$tmp_dir/parallel.out" + +maximum="$(<"$tmp_dir/state/max")" +((maximum >= 2)) || fail "SCROLL_PUSH_JOBS=3 must actually overlap push jobs" +((maximum <= 3)) || fail "parallel pushes exceeded the configured job bound (max=$maximum)" + +actual_categories="$(grep -c '^start category ' "$tmp_dir/state/events.log")" +actual_artifacts="$(grep -c '^start artifact ' "$tmp_dir/state/events.log")" +assert_equals "$expected_categories" "$actual_categories" \ + "parallel mode must invoke every category push exactly once" +assert_equals "$expected_artifacts" "$actual_artifacts" \ + "parallel mode must invoke every artifact push exactly once" + +last_category_end="$(grep -n '^end category ' "$tmp_dir/state/events.log" | tail -n 1 | cut -d: -f1)" +first_artifact_start="$(grep -m 1 -n '^start artifact ' "$tmp_dir/state/events.log" | cut -d: -f1)" +((last_category_end < first_artifact_start)) || \ + fail "artifact pushes started before all category pushes completed" + +reset_state +if DRUID_BIN="$fake_druid" \ + FAKE_DRUID_STATE_DIR="$tmp_dir/state" \ + FAKE_DRUID_SLEEP=0.01 \ + FAKE_DRUID_FAIL_REF='artifacts.druid.gg/druid-team/scroll-minecraft-spigot:1.17' \ + SCROLL_PUSH_JOBS=4 \ + SCROLL_PUSH_CATEGORIES=0 \ + SCROLL_PUSH_ARTIFACTS=1 \ + bash "$push_script" >"$tmp_dir/failure.out" 2>"$tmp_dir/failure.err"; then + fail "a failed parallel push must make push.sh fail" +fi + +failure_starts="$(grep -c '^start artifact ' "$tmp_dir/state/events.log")" +failure_ends="$(grep -c '^end artifact ' "$tmp_dir/state/events.log")" +assert_equals "$expected_artifacts" "$failure_starts" \ + "a job failure must not abandon the remaining queued artifact pushes" +assert_equals "$failure_starts" "$failure_ends" \ + "push.sh must wait for every started job before returning a failure" +assert_equals "0" "$(<"$tmp_dir/state/active")" \ + "push.sh must not leave background push jobs running after failure" + +echo "push parallelism tests passed ($expected_categories categories, $expected_artifacts artifacts)"