diff --git a/infra/bin/fly-loop-reset b/infra/bin/fly-loop-reset index 00d2d9c..95ae93a 100755 --- a/infra/bin/fly-loop-reset +++ b/infra/bin/fly-loop-reset @@ -94,12 +94,21 @@ migration_accepted() { return 1 } +# The root copy must be the flysim that runs: a manual rollback of /opt/fly/current without a +# deploy would otherwise approve archives the running build refuses. +LIVE_FLYSIM=/opt/fly/current/flysim +[ "$(id -u)" -eq 0 ] || LIVE_FLYSIM="${FLY_LOOP_RESET_TEST_LIVE_FLYSIM:-$FLYSIM}" +same_build() { + [ -f "$LIVE_FLYSIM" ] \ + && [ "$(sha256sum < "$FLYSIM" | cut -d' ' -f1)" = "$(sha256sum < "$LIVE_FLYSIM" | cut -d' ' -f1)" ] +} + build_compat="" -if [ -x "$FLYSIM" ] && [ "$(stat -c %u "$FLYSIM")" = "$(id -u)" ]; then +if [ -x "$FLYSIM" ] && [ "$(stat -c %u "$FLYSIM")" = "$(id -u)" ] && same_build; then build_compat="$(clean_env timeout 90 "$FLYSIM" --print-compatibility 2>/dev/null | tail -n1 || true)" fi if [ -z "$build_compat" ]; then - log "no compatibility string from $FLYSIM (missing, not owned by $(id -un), or failed); no rung is restorable" + log "no compatibility string from $FLYSIM (missing, not owned by $(id -un), not the running build, or failed); no rung is restorable" [ "$LIST" -eq 1 ] && exit 0 exit 3 fi @@ -141,11 +150,18 @@ trap 'exit 143' TERM INT HUP log "pausing ${WATCHDOG_TIMER} and stopping ${SERVICE} to reset to rung ${RANK}" systemctl stop "$WATCHDOG_TIMER" +# A oneshot probe mid-run is "activating", which `is-active` does not count; wait on ActiveState. +watchdog_idle() { + case "$(systemctl show -p ActiveState --value "$WATCHDOG_SERVICE")" in + inactive|failed) return 0 ;; + *) return 1 ;; + esac +} for _ in $(seq 1 60); do - systemctl is-active --quiet "$WATCHDOG_SERVICE" || break + watchdog_idle && break sleep 1 done -if systemctl is-active --quiet "$WATCHDOG_SERVICE"; then +if ! watchdog_idle; then log "${WATCHDOG_SERVICE} still running after 60 s; not resetting" exit 4 fi diff --git a/infra/docs/loop-recovery.md b/infra/docs/loop-recovery.md index 953353a..f06d878 100644 --- a/infra/docs/loop-recovery.md +++ b/infra/docs/loop-recovery.md @@ -15,7 +15,8 @@ a ladder one step per trap that outlives the previous step: - **Outlives** means the reports that confirm it again are at least 20 minutes after the step, so their 10-brain-minute window lies after it. The helper waits that long after every step. - **Budget:** at most two milestone resets per 24 hours. With the budget spent the step is a - restart, at most every three hours, until a reset is free again. + restart, at most every three hours, until a reset is free again. The same three-hour spacing applies + when a reset level finds no restorable rung and restarts instead. - **Starting over:** the ladder returns to level 0 when the fly reaches a new best rung, or after six hours with no suspected report. @@ -29,9 +30,12 @@ release env file so a deploy does not drop it. The step runs `/opt/fly/bin/fly-loop-reset ` through sudo (the one line in `config/fly-sudoers`). As root it: -- runs only the root-owned `/opt/fly/sbin/flysim` that `05-deploy.sh` installs from the release - tarball after checking it against the tarball's MANIFEST, never the fly-owned release tree; - without that copy no rung is restorable; +- runs only the root-owned `/opt/fly/sbin/flysim`, and only while it is byte-identical to the + flysim `/opt/fly/current` points to, that `05-deploy.sh` installs from the release + tarball after checking it against the tarball's MANIFEST (never the fly-owned release tree). + Without that copy (an infra-only deploy, or a manual rollback) no rung is restorable and the + ladder only restarts; after a release deploy, check as `fly` that + `sudo -n /opt/fly/bin/fly-loop-reset --list` names the current rung; - reads `fly.env` as `KEY=VALUE` data, never sources it, and ignores the caller's environment; - stops `fly-watchdog.timer` and waits for a running probe to finish, so the watchdog cannot start flysim on a half-rewritten store; stops flysim; runs `fly-reset-to-milestone`; and on every @@ -41,7 +45,9 @@ The reset copies both stores to `/srv/fly/state.reset-` first, as in the ru clears milestone archives above the rung. After each step the helper waits up to four minutes for `/status` to report `running` (and the target rank for a reset). Otherwise the step is recorded as failed and the ladder still climbs. The step is recorded before it runs, so a helper killed -mid-step has still climbed and spent the reset. +mid-step has still climbed and spent the reset. Do not stop `fly-loop-recover.service` during a +step: that kills the reset too, and the wrapper's exit trap starts flysim on whatever state is +left. ## On stream diff --git a/infra/tests/test_loop_recover.py b/infra/tests/test_loop_recover.py index 405135c..c3b15ff 100644 --- a/infra/tests/test_loop_recover.py +++ b/infra/tests/test_loop_recover.py @@ -267,7 +267,12 @@ class WrapperTests(unittest.TestCase): self.calls = root / "calls.log" (root / "state").mkdir() (root / "bin").mkdir() - self.script(root / "bin/systemctl", f'echo "systemctl $*" >> {self.calls}; [ "$1" != is-active ] || exit 3') + # The watchdog probe is mid-run ("activating") for the first two looks. + self.script(root / "bin/systemctl", f'''echo "systemctl $*" >> {self.calls} +if [ "$1" = show ]; then + n=$(grep -c "^systemctl show" {self.calls}) + if [ "$n" -le 2 ]; then echo activating; else echo inactive; fi +fi''') self.script(root / "flysim", f'[ "$1" = --print-compatibility ] && echo "{BUILD}"') self.script(root / "reset", f'echo "reset $* FLY_BIN=$FLY_BIN" >> {self.calls}') self.env_file = root / "fly.env" @@ -288,6 +293,7 @@ class WrapperTests(unittest.TestCase): env = {"PATH": f"{self.root}/bin:/usr/bin:/bin", "FLY_LOOP_RESET_TEST_STATE_DIR": str(self.root / "state"), "FLY_LOOP_RESET_TEST_ENV_FILE": str(self.env_file), "FLY_LOOP_RESET_TEST_FLYSIM": str(self.root / "flysim"), "FLY_LOOP_RESET_TEST_RESET_BIN": str(self.root / "reset")} + env.update(getattr(self, "extra_env", {})) return recover.subprocess.run([str(WRAPPER), *args], env=env, capture_output=True, text=True, timeout=60) def log(self): @@ -303,7 +309,9 @@ class WrapperTests(unittest.TestCase): done = self.run_wrapper("12") self.assertEqual(done.returncode, 0, done.stderr) self.assertEqual([line.split()[:3] for line in self.log().splitlines() if not line.startswith("systemctl is-active")], [ - ["systemctl", "stop", "fly-watchdog.timer"], ["systemctl", "stop", "flysim.service"], + ["systemctl", "stop", "fly-watchdog.timer"], + ["systemctl", "show", "-p"], ["systemctl", "show", "-p"], ["systemctl", "show", "-p"], ["systemctl", "show", "-p"], + ["systemctl", "stop", "flysim.service"], ["reset", "12", f"FLY_BIN={self.root}/flysim"], ["systemctl", "start", "flysim.service"], ["systemctl", "start", "fly-watchdog.timer"]]) @@ -324,6 +332,20 @@ class WrapperTests(unittest.TestCase): self.assertEqual(self.run_wrapper("12").returncode, 3) self.assertEqual(self.log(), "") + def test_a_root_copy_that_is_not_the_running_build_restores_nothing(self): + other = self.root / "live-flysim" + self.script(other, "echo other build") + self.extra_env = {"FLY_LOOP_RESET_TEST_LIVE_FLYSIM": str(other)} + self.assertEqual(self.run_wrapper("--list").stdout, "") + self.assertEqual(self.run_wrapper("12").returncode, 3) + + def test_a_watchdog_probe_that_never_finishes_blocks_the_reset(self): + self.script(self.root / "bin/systemctl", f'echo "systemctl $*" >> {self.calls}; [ "$1" != show ] || echo activating') + self.script(self.root / "bin/sleep", "exit 0") + self.assertEqual(self.run_wrapper("12").returncode, 4) + self.assertNotIn("systemctl stop flysim.service", self.log()) + self.assertIn("systemctl start fly-watchdog.timer", self.log()) + def test_bad_arguments_are_refused(self): for args in ((), ("--check", "12"), ("12", "13"), ("../12",), ("123",), ("-1",)): self.assertEqual(self.run_wrapper(*args).returncode, 2, args)