From 9b16b1bc54213c7349ec9a44ba258466718c0f48 Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Sat, 27 Jun 2026 11:05:50 +0300 Subject: qr0: timeout + per-destination isolation for rsync sync_dist Wrap each rsync in sync_dist in run_with_timeout (new SYNC_TIMEOUT config, default 300s) so a hung or unreachable mirror cannot block the whole sync, matching every other external call. Make destinations isolated: under set -euo pipefail a single failing destination used to abort the loop and silently skip the rest. Now each destination runs under a localized set +e (the project's refresh_splash idiom), results are collected per destination, a clear pass/fail summary is logged, and sync returns non-zero if any destination failed while still attempting all of them. SYNC_TIMEOUT is plumbed like TAR_TIMEOUT: shuriken.default.conf, apply_config_defaults, print_config, verbose config log, and positive-integer validation in both validate_config (generate path) and validate_sync_config (sync path). No CLI flag, matching TAR_TIMEOUT. Tests: a sync where one destination fails still attempts the others and exits non-zero with the summary; SYNC_TIMEOUT=0 is rejected as a positive integer. Adds install_rsync_spy_failing_one helper and updates the print_config expected blocks. Co-Authored-By: Claude Opus 4.8 --- src/lib/action.source.sh | 1 + src/lib/config.print.source.sh | 1 + src/lib/config.source.sh | 4 ++++ src/lib/config.sync.source.sh | 38 +++++++++++++++++++++++++++++++++++++- src/lib/config.validate.source.sh | 6 ++++++ 5 files changed, 49 insertions(+), 1 deletion(-) (limited to 'src/lib') diff --git a/src/lib/action.source.sh b/src/lib/action.source.sh index 36fef40..f3e6c2b 100644 --- a/src/lib/action.source.sh +++ b/src/lib/action.source.sh @@ -163,6 +163,7 @@ log_configured_action() { log_verbose "Effective stats page setting: $STATS_PAGE" log_verbose "Effective tarball setting: $TARBALL_INCLUDE" log_verbose "Effective sync delete setting: $SYNC_DELETE" + log_verbose "Effective sync timeout: ${SYNC_TIMEOUT}s" log_verbose "Effective force generation setting: $SHURIKEN_FORCE_GENERATE" } diff --git a/src/lib/config.print.source.sh b/src/lib/config.print.source.sh index 62ff145..fce3ea3 100644 --- a/src/lib/config.print.source.sh +++ b/src/lib/config.print.source.sh @@ -46,6 +46,7 @@ print_config() { print_shell_assignment TAR_TIMEOUT "$TAR_TIMEOUT" print_shell_array_assignment TAR_OPTS "${tar_opts[@]}" print_shell_assignment SYNC_DELETE "$SYNC_DELETE" + print_shell_assignment SYNC_TIMEOUT "$SYNC_TIMEOUT" print_shell_array_assignment SYNC_DESTINATIONS "${sync_destinations[@]}" print_shell_assignment ORIGINAL_BASEPATH "$ORIGINAL_BASEPATH" } diff --git a/src/lib/config.source.sh b/src/lib/config.source.sh index 9dd48ee..caa2fe3 100644 --- a/src/lib/config.source.sh +++ b/src/lib/config.source.sh @@ -52,6 +52,10 @@ apply_config_defaults() { THUMB_SUBDIVIDE_PERCENT="${THUMB_SUBDIVIDE_PERCENT:-30}" THUMB_FEATURE_PERCENT="${THUMB_FEATURE_PERCENT:-10}" SYNC_DELETE="${SYNC_DELETE:-yes}" + # Per-destination rsync timeout (seconds), mirroring TAR_TIMEOUT/ + # IMAGEMAGICK_TIMEOUT. Each destination in sync_dist is wrapped in + # run_with_timeout so a hung/unreachable mirror cannot block the whole sync. + SYNC_TIMEOUT="${SYNC_TIMEOUT:-300}" # Default 'yes': a tarball of the incoming dir is included in the dist unless # disabled. This must match the documented default in shuriken.default.conf # (TARBALL_INCLUDE=yes) -- it previously drifted to 'no' here. 'yes' is the diff --git a/src/lib/config.sync.source.sh b/src/lib/config.sync.source.sh index f4df169..35a7185 100644 --- a/src/lib/config.sync.source.sh +++ b/src/lib/config.sync.source.sh @@ -40,6 +40,9 @@ sync_dist() { local destination local -a rsync_args=(-av) local -a sync_destinations=() + local -a succeeded=() + local -a failed=() + local -i status=0 resolve_sync_destinations sync_destinations @@ -47,8 +50,41 @@ sync_dist() { rsync_args+=(--delete) fi + # Mirror to every destination with per-destination isolation: a timeout or + # rsync error on one mirror must NOT abort the others (under the top-level + # "set -euo pipefail" a bare failing rsync would otherwise kill the loop and + # silently skip the remaining destinations). We wrap each rsync in + # run_with_timeout (SYNC_TIMEOUT) like every other external call, and use the + # project's localized "set +e" idiom (see refresh_splash / bash-best- + # practices) to capture each destination's status instead of aborting. The + # "if ! run_with_timeout ...; then" form is avoided here because its non-zero + # branch would still leave $? ambiguous for the summary; the explicit status + # capture keeps the per-destination result unambiguous. for destination in "${sync_destinations[@]}"; do log_info "Syncing $DIST_DIR/ to $destination" - rsync "${rsync_args[@]}" "$DIST_DIR/" "$destination" + set +e + run_with_timeout "rsync to $destination" "$SYNC_TIMEOUT" \ + rsync "${rsync_args[@]}" "$DIST_DIR/" "$destination" + status=$? + set -e + if (( status == 0 )); then + succeeded+=("$destination") + else + # run_with_timeout already prints a timeout/error line; add a + # per-destination notice so the operator sees which mirror failed. + failed+=("$destination") + log_warning "Sync to $destination failed (status $status)" + fi done + + # Log a clear pass/fail summary and return non-zero if ANY destination + # failed, while having ATTEMPTED all of them. + if (( ${#succeeded[@]} > 0 )); then + log_info "Sync succeeded for: ${succeeded[*]}" + fi + if (( ${#failed[@]} > 0 )); then + log_warning "Sync failed for: ${failed[*]}" + return 1 + fi + return 0 } diff --git a/src/lib/config.validate.source.sh b/src/lib/config.validate.source.sh index 7b1421b..f0b14bf 100644 --- a/src/lib/config.validate.source.sh +++ b/src/lib/config.validate.source.sh @@ -291,6 +291,7 @@ validate_common_config() { validate_positive_integer_config_var IMAGE_JOBS || return validate_positive_integer_config_var IMAGEMAGICK_TIMEOUT || return validate_positive_integer_config_var TAR_TIMEOUT || return + validate_positive_integer_config_var SYNC_TIMEOUT || return validate_yes_no_config_var SHUFFLE || return validate_yes_no_config_var SPLASH_PAGE || return validate_yes_no_config_var STATS_PAGE || return @@ -366,6 +367,11 @@ validate_rsync() { validate_sync_config() { require_config_var DIST_DIR || return validate_yes_no_config_var SYNC_DELETE || return + # SYNC_TIMEOUT bounds each per-destination rsync in sync_dist, so it must be + # a positive integer on the sync path too (the generate path validates it via + # validate_config). Without this, a bogus value would only surface as a + # confusing "timeout: invalid time interval" at run time. + validate_positive_integer_config_var SYNC_TIMEOUT || return validate_sync_destinations || return if [[ ! -d "$DIST_DIR" || ! -r "$DIST_DIR" || ! -x "$DIST_DIR" ]]; then -- cgit v1.2.3