diff --git a/scripts/check-installed-state.sh b/scripts/check-installed-state.sh index 8bde0581..9331f63c 100755 --- a/scripts/check-installed-state.sh +++ b/scripts/check-installed-state.sh @@ -37,7 +37,7 @@ config_ref=${values[2]} if [[ "$config_repository" =~ ^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$ ]]; then [[ -x "$remote_reconciler" ]] || { echo "ERROR: remote reconciler is unavailable: $remote_reconciler" >&2; exit 2; } - CI_FLEET_REMOTE_STATE_FILE="$state_file" exec "$remote_reconciler" --check-only --desired-ref "$config_ref" + CI_FLEET_REMOTE_STATE_FILE="$state_file" exec "$remote_reconciler" --check-only --installed-ref fi exec "$installer" --check \ diff --git a/scripts/remote-reconcile.sh b/scripts/remote-reconcile.sh index e615d185..a4b5bb16 100755 --- a/scripts/remote-reconcile.sh +++ b/scripts/remote-reconcile.sh @@ -6,7 +6,7 @@ # validates the configuration, checks for drift, and reconciles if needed. # # Usage: -# remote-reconcile.sh [--check-only] [--desired-ref SHA] [--no-op] +# remote-reconcile.sh [--check-only] [--installed-ref] [--no-op] set -Eeuo pipefail script_dir=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) @@ -26,19 +26,21 @@ trap cleanup_temp EXIT mode=reconcile # reconcile or check-only no_op=false -desired_ref= +installed_ref=false max_attempts=${CI_FLEET_RECONCILE_MAX_ATTEMPTS:-3} +# ponytail: one-hour queue cap; align with any future installer transaction timeout. +lock_wait_seconds=3600 usage() { cat >&2 <<'EOF' usage: - remote-reconcile.sh [--check-only] [--desired-ref SHA] [--no-op] + remote-reconcile.sh [--check-only] [--installed-ref] [--no-op] Fetches the desired-state repository at the current default-branch HEAD, validates it, and reconciles the controller if a newer commit is available. --check-only Validate and report without reconciling. - --desired-ref Check this exact commit instead of remote HEAD (check-only only). + --installed-ref Check the locked installation's commit (check-only only). --no-op Log what would be done without side effects. EOF } @@ -46,15 +48,14 @@ EOF while (($#)); do case "$1" in --check-only) mode=check-only ;; - --desired-ref) shift; (($#)) || { echo 'ERROR: --desired-ref requires a commit' >&2; exit 2; }; desired_ref=$1 ;; + --installed-ref) installed_ref=true ;; --no-op) no_op=true ;; -h|--help) usage; exit 0 ;; *) echo "ERROR: unknown argument: $1" >&2; usage; exit 2 ;; esac shift done -[[ -z "$desired_ref" || "$mode" == check-only ]] || { echo 'ERROR: --desired-ref requires --check-only' >&2; exit 2; } -[[ -z "$desired_ref" || "$desired_ref" =~ ^[0-9a-f]{40}$ ]] || { echo 'ERROR: --desired-ref must be a full lowercase commit SHA' >&2; exit 2; } +[[ "$installed_ref" == false || "$mode" == check-only ]] || { echo 'ERROR: --installed-ref requires --check-only' >&2; exit 2; } note() { printf 'RECONCILE %s\n' "$*"; } die() { @@ -322,10 +323,13 @@ require_commands lock_file=${CI_FLEET_INSTALLER_LOCK:-/run/ci-fleet-installer.lock} install -d -m 0755 "$(dirname "$lock_file")" exec 9>"$lock_file" -flock -n 9 || die "another reconcile or installer is already running" +flock -w "$lock_wait_seconds" 9 || die "timed out waiting for another reconcile or installer" # Load installed state load_installed_state || die "no installed state found at $state_file" +[[ "$installed_config_ref" =~ ^[0-9a-f]{40}$ ]] || die "installed config ref is not a full lowercase commit SHA" +fetch_ref=HEAD +[[ "$installed_ref" == false ]] || fetch_ref=$installed_config_ref note "INSTALLED controller=${installed_controller} config_repo=${installed_config_repo} config_ref=${installed_config_ref}" # Prefer host.env for token generation; fall back to rendered_env @@ -354,7 +358,7 @@ while ((attempt < max_attempts)); do # Fetch remote config note "FETCHING_CONFIG repo=${installed_config_repo}" - desired_commit=$(fetch_remote_config "$installed_config_repo" "$token" "${desired_ref:-HEAD}") || { + desired_commit=$(fetch_remote_config "$installed_config_repo" "$token" "$fetch_ref") || { note "FETCH_FAILED attempt=${attempt}" ((attempt < max_attempts)) && { sleep 5; continue; } die "fetch exhausted after ${max_attempts} attempts" diff --git a/scripts/test-install-worker-controller.sh b/scripts/test-install-worker-controller.sh index 4e34c07f..6cfc4af6 100755 --- a/scripts/test-install-worker-controller.sh +++ b/scripts/test-install-worker-controller.sh @@ -331,7 +331,7 @@ with open(path, "w") as output: PY expect_success env CI_FLEET_INSTALL_STATE_FILE="$install_state" CI_FLEET_INSTALLER="$installer" CI_FLEET_REMOTE_RECONCILER="$remote_reconciler" REMOTE_RECONCILER_LOG="$remote_reconciler_log" "$repo_root/scripts/check-installed-state.sh" mapfile -t remote_call <"$remote_reconciler_log" -[[ ${remote_call[0]} == "--check-only --desired-ref $ref_one" && ${remote_call[1]} == "$install_state" ]] || fail 'remote drift check did not delegate the exact installed state to authenticated reconciliation' +[[ ${remote_call[0]} == "--check-only --installed-ref" && ${remote_call[1]} == "$install_state" ]] || fail 'remote drift check did not delegate the exact installed state to authenticated reconciliation' expect_success "$installer" --install "${base_args[@]}" --ref "$ref_one" >/dev/null export FAKE_DISABLED_TIMER=ci-fleet-cleanup.timer diff --git a/scripts/test_remote_reconcile.py b/scripts/test_remote_reconcile.py index 2c4851c3..c2308297 100644 --- a/scripts/test_remote_reconcile.py +++ b/scripts/test_remote_reconcile.py @@ -197,20 +197,24 @@ def test_help_exits_0(self): self.assertEqual(result.returncode, 0) self.assertIn("usage", result.stdout + result.stderr) - def test_desired_ref_is_full_sha_and_check_only(self): - invalid = subprocess.run( - [str(RECONCILE_SCRIPT), "--check-only", "--desired-ref", "main"], - capture_output=True, text=True, env=self.env, - ) + def test_installed_ref_is_check_only(self): mutating = subprocess.run( - [str(RECONCILE_SCRIPT), "--desired-ref", "a" * 40], + [str(RECONCILE_SCRIPT), "--installed-ref"], capture_output=True, text=True, env=self.env, ) - self.assertEqual(invalid.returncode, 2) - self.assertIn("full lowercase commit SHA", invalid.stderr) self.assertEqual(mutating.returncode, 2) self.assertIn("requires --check-only", mutating.stderr) + def test_installed_ref_rejects_symbolic_state_before_fetch(self): + self._write_state("RandomDevelopment/rd-delivery-config", "HEAD", "rd-ci-fleet-01") + result = subprocess.run( + [str(RECONCILE_SCRIPT), "--check-only", "--installed-ref"], + capture_output=True, text=True, env=self.env, + ) + self.assertEqual(result.returncode, 2) + self.assertIn("full lowercase commit SHA", result.stderr) + self.assertNotIn("GENERATING_TOKEN", result.stdout) + def test_reconcile_state_saved_on_failure(self): """State file is saved even when reconciliation fails.""" result = subprocess.run( @@ -263,6 +267,9 @@ def test_remote_installer_calls_keep_identity_and_lock(self): reconcile = RECONCILE_SCRIPT.read_text() installer = INSTALLER.read_text() self.assertNotIn("release_lock", reconcile) + self.assertIn('flock -w "$lock_wait_seconds" 9', reconcile) + self.assertLess(reconcile.index('flock -w "$lock_wait_seconds" 9'), reconcile.index("fetch_ref=HEAD")) + self.assertIn('fetch_ref=$installed_config_ref', reconcile) self.assertEqual(reconcile.count("CI_FLEET_INSTALLER_LOCK_FD=9"), 3) self.assertEqual(reconcile.count("--config-identity"), 3) self.assertIn("--config-identity)", installer)