From 8095364cab0e7d8a624396bfea1a898842266042 Mon Sep 17 00:00:00 2001 From: DanielRMErskine Date: Tue, 21 Jul 2026 11:08:05 -0700 Subject: [PATCH 1/3] Update: never report success on a box whose container is not running A box update that fails during the Docker build exits after the containers have already been stopped and removed, leaving the box with no services. On the next run the early-exit gate read only source state (git in sync, no flatten work, build-inputs hash matching), so it printed a green "already at version X" and exited 0 on a box that was actually dead. Found via a field report of an update that failed during Docker image export; the retry then claimed the box was up to date and the user had to restart it by hand over SSH. Fixes, all in cli/commands/utility/update.py: - The single-SSH probe now reports whether the lager container is running (tri-state: running / not running / unknown when docker itself does not answer). The early-exit gate requires it: source-in-sync but container-down falls through to the rebuild path with an explanatory message instead of early-exiting. Unknown fails open, matching the BuildKit preflight. - `lager update --check` reports the stopped container ("Container: NOT RUNNING (will rebuild and restart)") and exits 1 so scripted checks see that the box needs work. - A failed build stores the sentinel value FAILED as the build-inputs hash. The sentinel never matches a recomputed hash, so the retry always invalidates the cached image and rebuilds. This closes the case where recovery (below) brings the box back up on the old image and source state alone would again read as "up to date". - After a failed build, the update restarts the box on its previous image (unless the image was wiped by --force / a hash change) instead of leaving it with no services, and reports plainly that the update did NOT apply. - New hint for BuildKit build-cache corruption ("failed to prepare extraction snapshot ... parent snapshot does not exist"): clear with `docker builder prune -af` and re-run. Unit tests cover the probe parsing, the gate verdict, and the sentinel predicate. Hardware-validated on a real box: a no-op update still early-exits in ~2s; with the containers removed, update rebuilds and restarts instead of claiming success; a failed build leaves the sentinel, restarts the previous version, and the following update heals with a clean rebuild. --- cli/commands/utility/update.py | 169 +++++++++++++++++++++++++++++---- cli/tests/test_update_gate.py | 94 ++++++++++++++++++ 2 files changed, 242 insertions(+), 21 deletions(-) create mode 100644 cli/tests/test_update_gate.py diff --git a/cli/commands/utility/update.py b/cli/commands/utility/update.py index f2b54ef2..7de79433 100644 --- a/cli/commands/utility/update.py +++ b/cli/commands/utility/update.py @@ -229,6 +229,15 @@ def _probe_shell_script(): else echo "LAGER_PROBE_BOXCFG_SUDOERS_OK=0" fi +# Container liveness: 1 = the `lager` container is running, 0 = docker +# answered and it is not, empty = docker itself didn't answer (unknown; +# callers fail open). `docker ps` lists running containers only, and its +# name filter is a substring match, so exact-match the name on our side. +if _dps=$(docker ps --filter name=lager --format '{{.Names}}' 2>/dev/null); then + echo "LAGER_PROBE_LAGER_RUNNING=$(printf '%s\n' "$_dps" | grep -cx lager)" +else + echo "LAGER_PROBE_LAGER_RUNNING=" +fi echo "LAGER_PROBE_ETC_VERSION=$(cat /etc/lager/version 2>/dev/null)" ''' return script.replace('__BUILD_HASH_CMD__', _build_hash_shell_cmd()) @@ -249,6 +258,44 @@ def _parse_probe_output(stdout): return facts +def _build_hash_mismatch(new_hash, stored_hash): + """True when the docker-build inputs changed relative to the last + successful build. + + Both values must be non-empty: an empty new hash means the probe couldn't + measure the inputs, an empty stored hash means no successful build has + recorded one — in either case auto-invalidation is skipped rather than + guessed. A failed build stores the sentinel value 'FAILED' (never a real + sha256), which forces a mismatch — and therefore a clean rebuild — on the + next run. + """ + return bool(new_hash) and bool(stored_hash) and new_hash != stored_hash + + +def _rebuild_gate_verdict(facts, *, git_sync_confirmed, needs_pull, + needs_flatten, hash_mismatch, force): + """Decide whether the update can stop before the container rebuild. + + Returns: + 'rebuild' — source state demands the full stop/build/start path. + 'container-down' — source is in sync but the lager container is not + running (e.g. a previous update failed after the + containers were removed): rebuild so the box comes + back up, instead of falsely reporting success. + 'skip' — in sync and running; safe to early-exit. + + The probe's LAGER_RUNNING fact is tri-state: '1' running, '0' definitely + not running, ''/absent unknown (the docker probe itself failed). Only a + definite '0' blocks the skip — unknown fails open, same philosophy as the + BuildKit preflight. + """ + if not git_sync_confirmed or needs_pull or needs_flatten or hash_mismatch or force: + return 'rebuild' + if facts.get('LAGER_RUNNING', '') == '0': + return 'container-down' + return 'skip' + + # Progress-bar denominator. 15 steps always run; 3 are conditional (flatten, # cached-image wipe, J-Link install). We use the max so the denominator never # jumps mid-flight — light paths simply finish below 18/18 and `finish()` @@ -1082,7 +1129,7 @@ def _parse_fetch_result(result): # any mutation (no git checkout, no udev install, no docker stop). # Exit codes: # 0 — already in sync, nothing to do - # 1 — would update (code, deps, or both) + # 1 — would update (code, deps, or a stopped container to bring back up) # 2 — could not determine state (network error, etc.) if check: if progress: @@ -1096,10 +1143,8 @@ def _parse_fetch_result(result): # reflect the tree that would be built — no extra round-trip needed. _check_new_hash = facts.get('BUILD_HASH_NEW', '') _check_stored_hash = facts.get('BUILD_HASH_STORED', '') - deps_will_change = ( - bool(_check_new_hash) and bool(_check_stored_hash) - and _check_new_hash != _check_stored_hash - ) + deps_will_change = _build_hash_mismatch(_check_new_hash, _check_stored_hash) + container_down = facts.get('LAGER_RUNNING', '') == '0' if not git_sync_confirmed: click.secho('Could not determine update state (git rev-list failed).', fg='red', err=True) @@ -1131,6 +1176,11 @@ def _parse_fetch_result(result): if force: container_status = 'will restart (forced clean rebuild)' est = '~6 min (fresh build)' + elif container_down: + # Code may be fully in sync, but there is nothing serving it — a + # previous update likely failed after removing the containers. + container_status = 'NOT RUNNING (will rebuild and restart)' + est = '~6 min (fresh build possible)' if deps_will_change else '~90s (cached build)' elif commits_behind == 0 and commits_ahead == 0 and not deps_will_change: container_status = 'no restart needed' est = '~5s' @@ -1153,7 +1203,10 @@ def _parse_fetch_result(result): click.echo(f' Estimated: {est}') click.echo() - will_change = force or commits_behind != 0 or commits_ahead != 0 or deps_will_change + will_change = ( + force or commits_behind != 0 or commits_ahead != 0 + or deps_will_change or container_down + ) if will_change: click.echo('Run without --check to apply.') ctx.exit(1) @@ -1256,10 +1309,7 @@ def _parse_fetch_result(result): new_build_hash = _read_build_hash(run_ssh_command_with_output) else: new_build_hash = facts.get('BUILD_HASH_NEW', '') - hash_mismatch = ( - bool(new_build_hash) and bool(stored_build_hash) - and new_build_hash != stored_build_hash - ) + hash_mismatch = _build_hash_mismatch(new_build_hash, stored_build_hash) # `--force` requests a clean rebuild: wipe the cached image and the # cargo/npm volumes so a prior failed/partial run can't leave a stale layer # or half-installed toolchain behind on the retry. @@ -1573,17 +1623,23 @@ def _render_boxcfg(ok): # but code is in sync); previously this branch ignored the hash and the # rebuild silently never happened. # - # `--force` deliberately skips this early-exit: a box whose previous update - # failed can read as "in sync" here (git fetched fine, hash matches) even - # though its container never came up, so the user needs a way to push the - # rebuild through regardless. - if ( - git_sync_confirmed - and not needs_pull - and not needs_flatten - and not hash_mismatch - and not force - ): + # Source state alone is not enough to declare success: an update that + # failed after Step 8 (containers stopped and REMOVED) leaves a box that + # reads as "in sync" here — git pulled fine, the stored hash predates the + # failure — but has nothing running at all. Early-exiting then reports + # success on a dead box, so the verdict also requires the lager container + # to be running: 'container-down' falls through to the rebuild path below + # and brings the box back up. `--force` still skips the early-exit + # unconditionally. + _gate = _rebuild_gate_verdict( + facts, + git_sync_confirmed=git_sync_confirmed, + needs_pull=needs_pull, + needs_flatten=needs_flatten, + hash_mismatch=hash_mismatch, + force=force, + ) + if _gate == 'skip': import re as _re # Prefer the box's source-declared `__version__` over the CLI @@ -1624,6 +1680,18 @@ def _render_boxcfg(ok): click.echo() ctx.exit(0) + if _gate == 'container-down': + _down_msg = ( + 'Code is in sync but the lager container is not running ' + '(a previous update may have failed mid-rebuild) — rebuilding ' + 'and restarting.' + ) + if progress: + progress.pause() + click.echo(_down_msg) + if progress: + progress.resume() + # BuildKit preflight — run BEFORE the lock and the container stop below so a # box whose Docker can't build the image fails fast WITHOUT being taken # offline. box.Dockerfile uses a `# syntax=` directive and `RUN @@ -1813,12 +1881,71 @@ def _docker_supports_buildkit(): "`sudo apt-get install -y docker-buildx`), then re-run `lager update`.", fg='yellow', err=True, ) + elif "failed to prepare extraction snapshot" in full_output or ( + "parent snapshot" in full_output and "does not exist" in full_output + ): + click.secho( + "Hint: the box's Docker build cache is corrupted (a cached snapshot " + "references a parent that no longer exists). Clear it with: " + f"ssh {ssh_host} 'docker builder prune -af' and re-run `lager update`. " + "The next build runs cold and can take 10-15 minutes.", + fg='yellow', err=True, + ) elif "No space left on device" in full_output: click.secho("Hint: Disk space is full on the box. Run: ssh lagerdata@[BOX_NAME] 'docker system prune -af'", fg='yellow', err=True) elif "network" in full_output.lower() and ("timeout" in full_output.lower() or "error" in full_output.lower()): click.secho("Hint: Network issue during build. Check box internet connectivity.", fg='yellow', err=True) elif "permission denied" in full_output.lower(): click.secho("Hint: Permission issue. Check Docker daemon is running and user has access.", fg='yellow', err=True) + + # Poison the stored build-inputs hash so the next run can't read this + # box as up to date: the sentinel never equals a recomputed sha256, so + # hash_mismatch fires on the retry, blocking the "already at version" + # early-exit and forcing a clean image wipe + rebuild. Deleting the + # file would NOT do this — _build_hash_mismatch requires both values + # non-empty. Best-effort: if the write fails, the liveness gate still + # catches the dead-container case. + try: + store_build_hash('FAILED') + except subprocess.SubprocessError: + pass + + # Step 8 already stopped and removed the containers, so exiting here + # would strand the box with no services — the state that previously + # made the next run report "already at version" on a dead box. Unless + # this run wiped the image (hash change / --force), the previous image + # is still tagged `lager` and its baked-in code matches what was + # running before the pull, so restart it: the box stays usable on its + # prior version while the operator deals with the build failure. + restarted = False + if not must_wipe_image: + click.echo() + click.secho('Restarting the box on its previous version...', fg='yellow', err=True) + try: + recover = run_ssh_command_with_output( + 'cd ~/box && chmod +x start_box.sh && LAGER_SKIP_BUILD=1 ./start_box.sh', + timeout_secs=600, + ) + # Exit 3 = container up but post-start installs failed; the + # box is serving, which is all this recovery promises. + restarted = recover.returncode in (0, 3) + except subprocess.SubprocessError: + restarted = False + if restarted: + restarted = wait_for_box_ready(resolved_box, timeout_s=60) + if restarted: + click.secho( + 'The update FAILED and was not applied. The box has been ' + 'restarted on its previous version.', + fg='yellow', err=True, + ) + else: + click.secho( + 'The update FAILED and the box was left with its services ' + 'stopped. Bring it back up manually with:', + fg='red', err=True, + ) + click.echo(f' ssh {ssh_host} "cd ~/box && ./start_box.sh"', err=True) ctx.exit(1) if verbose: click.secho(' Build complete', fg='green') diff --git a/cli/tests/test_update_gate.py b/cli/tests/test_update_gate.py new file mode 100644 index 00000000..d3a9d4ce --- /dev/null +++ b/cli/tests/test_update_gate.py @@ -0,0 +1,94 @@ +# Copyright 2026 Lager Data +# SPDX-License-Identifier: Apache-2.0 + +""" +Tests for the update flow's rebuild gate: probe parsing, the build-hash +mismatch predicate, and the early-exit verdict (including container liveness). +""" +import pytest + +from cli.commands.utility.update import ( + _build_hash_mismatch, + _parse_probe_output, + _probe_shell_script, + _rebuild_gate_verdict, +) + +IN_SYNC = dict( + git_sync_confirmed=True, + needs_pull=False, + needs_flatten=False, + hash_mismatch=False, + force=False, +) + +SHA_A = 'a' * 64 +SHA_B = 'b' * 64 + + +class TestParseProbeOutput: + @pytest.mark.parametrize('raw', ['1', '0', '']) + def test_lager_running_values_pass_through(self, raw): + facts = _parse_probe_output(f'LAGER_PROBE_LAGER_RUNNING={raw}\n') + assert facts['LAGER_RUNNING'] == raw + + def test_absent_key_stays_absent(self): + facts = _parse_probe_output('LAGER_PROBE_ETC_VERSION=1.2.3\n') + assert 'LAGER_RUNNING' not in facts + + def test_noise_lines_ignored(self): + stdout = 'Welcome to the box\nLAGER_PROBE_LAGER_RUNNING=1\nsudo lecture\n' + assert _parse_probe_output(stdout) == {'LAGER_RUNNING': '1'} + + def test_probe_script_emits_liveness_fact(self): + assert 'LAGER_PROBE_LAGER_RUNNING=' in _probe_shell_script() + + +class TestBuildHashMismatch: + def test_changed_inputs_mismatch(self): + assert _build_hash_mismatch(SHA_A, SHA_B) + + def test_matching_inputs_no_mismatch(self): + assert not _build_hash_mismatch(SHA_A, SHA_A) + + def test_failed_sentinel_forces_mismatch(self): + # A failed build stores 'FAILED'; any real recomputed sha must mismatch + # so the retry rebuilds instead of early-exiting. + assert _build_hash_mismatch(SHA_A, 'FAILED') + + def test_absent_stored_hash_skips_auto_invalidation(self): + assert not _build_hash_mismatch(SHA_A, '') + + def test_unmeasurable_new_hash_skips_auto_invalidation(self): + assert not _build_hash_mismatch('', SHA_A) + + +class TestRebuildGateVerdict: + def test_in_sync_and_running_skips(self): + assert _rebuild_gate_verdict({'LAGER_RUNNING': '1'}, **IN_SYNC) == 'skip' + + def test_container_down_blocks_skip(self): + # The reported failure: a prior update removed the containers and died + # mid-build; source reads as in-sync but nothing is running. + assert _rebuild_gate_verdict({'LAGER_RUNNING': '0'}, **IN_SYNC) == 'container-down' + + @pytest.mark.parametrize('facts', [{}, {'LAGER_RUNNING': ''}]) + def test_unknown_liveness_fails_open(self, facts): + assert _rebuild_gate_verdict(facts, **IN_SYNC) == 'skip' + + @pytest.mark.parametrize('override', [ + dict(git_sync_confirmed=False), + dict(needs_pull=True), + dict(needs_flatten=True), + dict(hash_mismatch=True), + dict(force=True), + ]) + def test_source_divergence_rebuilds_regardless_of_liveness(self, override): + args = {**IN_SYNC, **override} + for facts in ({'LAGER_RUNNING': '1'}, {'LAGER_RUNNING': '0'}, {}): + assert _rebuild_gate_verdict(facts, **args) == 'rebuild' + + def test_failed_sentinel_feeds_through_to_rebuild(self): + mismatch = _build_hash_mismatch(SHA_A, 'FAILED') + args = {**IN_SYNC, 'hash_mismatch': mismatch} + assert _rebuild_gate_verdict({'LAGER_RUNNING': '1'}, **args) == 'rebuild' From 413efd42916ff96945d81cbd862eb6ae888f91fb Mon Sep 17 00:00:00 2001 From: DanielRMErskine Date: Tue, 21 Jul 2026 15:18:43 -0700 Subject: [PATCH 2/3] Update: also catch a container running code older than the tree The liveness gate closes the dead-box case, but a prior update can pull the new code and then exit BEFORE the container stop (a BuildKit preflight failure, or an interrupt in the pull->stop window). The box then reads as in sync AND running while the container still serves the previous build, and the early-exit would stamp the new version onto it. /etc/lager/version records the last version a successful update actually deployed, so the skip path now also requires it to match the tree's version (the semver target, or the tree's declared __version__). A mismatch falls through to the rebuild path with an explanatory message; unknown values on either side fail open so legacy boxes with a missing or empty version file do not rebuild forever. `lager update --check` reports the same state as "running a STALE build (will rebuild and restart)" and exits 1. Also adds shell-level tests for the probe's liveness fact (stubbed docker: running / exact-name-only matching / no rows / docker failing) and unit tests for the staleness predicate. Hardware-validated end to end on a real box: a staged stale version record is flagged by --check, the update rebuilds and restarts instead of early-exiting, and the record reconciles; the no-op fast path stays ~2s. The build-failure recovery messaging for the wiped-image case (--force / hash change) was also exercised on hardware: honest failure text, manual recovery command, exit 1. --- cli/commands/utility/update.py | 73 ++++++++++++++++++++++++++++++---- cli/tests/test_update_gate.py | 70 ++++++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+), 8 deletions(-) diff --git a/cli/commands/utility/update.py b/cli/commands/utility/update.py index 7de79433..46d632fb 100644 --- a/cli/commands/utility/update.py +++ b/cli/commands/utility/update.py @@ -296,6 +296,22 @@ def _rebuild_gate_verdict(facts, *, git_sync_confirmed, needs_pull, return 'skip' +def _deployed_version_stale(tree_version, etc_version_raw): + """True when the box tree's version differs from the last version a + successful update actually deployed (/etc/lager/version, `box|cli` + format — written only after a successful rebuild+restart). + + Catches the tree-ahead-of-deploy state: an update that pulled and then + exited before the container stop (BuildKit preflight failure, an + interrupt in the pull->stop window) leaves the container serving the + previous build while git reads as in sync and the container as running. + Unknown on either side fails open (returns False) so legacy boxes with a + missing/empty version file don't rebuild forever. + """ + deployed = (etc_version_raw or '').split('|', 1)[0].strip() + return bool(tree_version) and bool(deployed) and deployed != tree_version + + # Progress-bar denominator. 15 steps always run; 3 are conditional (flatten, # cached-image wipe, J-Link install). We use the max so the denominator never # jumps mid-flight — light paths simply finish below 18/18 and `finish()` @@ -1150,6 +1166,17 @@ def _parse_fetch_result(result): click.secho('Could not determine update state (git rev-list failed).', fg='red', err=True) ctx.exit(2) + # Only worth a look when everything else reads clean — any other + # divergence already forces the rebuild that would fix it. (Local + # import: a later `import re` in this function makes `re` local to + # the whole function, so the module-level binding is shadowed here.) + deploy_stale = False + if not container_down and commits_behind == 0 and commits_ahead == 0: + import re + _vp = re.match(r'^v?(\d+\.\d+\.\d+)$', target_version) + _tree_v = _vp.group(1) if _vp else _read_box_source_version(run_ssh_command_with_output) + deploy_stale = _deployed_version_stale(_tree_v, current_version_raw) + if commits_behind == 0 and commits_ahead == 0: code_status = 'in sync' elif is_rollback: @@ -1181,6 +1208,12 @@ def _parse_fetch_result(result): # previous update likely failed after removing the containers. container_status = 'NOT RUNNING (will rebuild and restart)' est = '~6 min (fresh build possible)' if deps_will_change else '~90s (cached build)' + elif deploy_stale: + # In sync and running, but the container was deployed from an + # older version — a prior update pulled and stopped before the + # rebuild. + container_status = 'running a STALE build (will rebuild and restart)' + est = '~90s (cached build)' elif commits_behind == 0 and commits_ahead == 0 and not deps_will_change: container_status = 'no restart needed' est = '~5s' @@ -1205,7 +1238,7 @@ def _parse_fetch_result(result): will_change = ( force or commits_behind != 0 or commits_ahead != 0 - or deps_will_change or container_down + or deps_will_change or container_down or deploy_stale ) if will_change: click.echo('Run without --check to apply.') @@ -1639,6 +1672,7 @@ def _render_boxcfg(ok): hash_mismatch=hash_mismatch, force=force, ) + _box_v = '' # assigned for real below whenever _gate stays 'skip' if _gate == 'skip': import re as _re @@ -1650,10 +1684,26 @@ def _render_boxcfg(ok): _vp = _re.match(r'^v?(\d+\.\d+\.\d+)$', target_version) if _vp: _box_v = _vp.group(1) + _tree_v = _box_v else: _src_v = _read_box_source_version(run_ssh_command_with_output) _box_v = _src_v if _src_v else cli_version + # For the staleness check, only trust an actually-read source + # version — the cli_version fallback is a display default, and + # comparing it against the box would manufacture mismatches. + _tree_v = _src_v + + # Liveness alone can't tell whether the RUNNING container was built + # from the code now in the tree: an update that pulled and then + # exited before the container stop (BuildKit preflight failure, an + # interrupt in the pull->stop window) leaves the old build serving + # while git reads in sync. The deployed-version record catches that; + # fall through and rebuild instead of stamping the new version onto + # a box still running the old one. + if _deployed_version_stale(_tree_v, facts.get('ETC_VERSION', '')): + _gate = 'stale-deploy' + if _gate == 'skip': # Reconcile /etc/lager/version on the box with the local cache. # Previously this branch only updated the local ~/.lager file, so if # the box's /etc/lager/version was stale (e.g. an earlier update @@ -1680,15 +1730,22 @@ def _render_boxcfg(ok): click.echo() ctx.exit(0) - if _gate == 'container-down': - _down_msg = ( - 'Code is in sync but the lager container is not running ' - '(a previous update may have failed mid-rebuild) — rebuilding ' - 'and restarting.' - ) + if _gate in ('container-down', 'stale-deploy'): + if _gate == 'container-down': + _fall_msg = ( + 'Code is in sync but the lager container is not running ' + '(a previous update may have failed mid-rebuild) — rebuilding ' + 'and restarting.' + ) + else: + _fall_msg = ( + 'Code is in sync but the running container was deployed from ' + 'a previous version (a prior update may have stopped before ' + 'the rebuild) — rebuilding and restarting.' + ) if progress: progress.pause() - click.echo(_down_msg) + click.echo(_fall_msg) if progress: progress.resume() diff --git a/cli/tests/test_update_gate.py b/cli/tests/test_update_gate.py index d3a9d4ce..f494e1b9 100644 --- a/cli/tests/test_update_gate.py +++ b/cli/tests/test_update_gate.py @@ -5,10 +5,15 @@ Tests for the update flow's rebuild gate: probe parsing, the build-hash mismatch predicate, and the early-exit verdict (including container liveness). """ +import os +import stat +import subprocess + import pytest from cli.commands.utility.update import ( _build_hash_mismatch, + _deployed_version_stale, _parse_probe_output, _probe_shell_script, _rebuild_gate_verdict, @@ -92,3 +97,68 @@ def test_failed_sentinel_feeds_through_to_rebuild(self): mismatch = _build_hash_mismatch(SHA_A, 'FAILED') args = {**IN_SYNC, 'hash_mismatch': mismatch} assert _rebuild_gate_verdict({'LAGER_RUNNING': '1'}, **args) == 'rebuild' + + +class TestDeployedVersionStale: + def test_matching_versions_not_stale(self): + assert not _deployed_version_stale('0.32.1', '0.32.1|0.32.1') + + def test_tree_ahead_of_deploy_is_stale(self): + # The tree-ahead-of-deploy state: a prior update pulled the new code + # but exited before the rebuild, so the container still serves the + # version last recorded by a successful update. + assert _deployed_version_stale('0.32.1', '0.32.0|0.32.1') + + def test_legacy_value_without_cli_part(self): + assert not _deployed_version_stale('0.31.0', '0.31.0') + assert _deployed_version_stale('0.32.0', '0.31.0') + + @pytest.mark.parametrize('etc_raw', ['', None, ' ', '|0.32.1']) + def test_unknown_deployed_fails_open(self, etc_raw): + assert not _deployed_version_stale('0.32.1', etc_raw) + + def test_unknown_tree_version_fails_open(self): + assert not _deployed_version_stale('', '0.32.0|0.32.0') + + +class TestProbeLivenessSnippet: + """Run the real probe script against a stubbed `docker` to pin the + liveness fact's tri-state contract at the shell level. + + The script is designed to exit 0 and emit a value (possibly empty) for + every fact regardless of what is installed on the host, so executing it + verbatim also guards against a syntax error sneaking into the heredoc. + """ + + def _probe_facts(self, tmp_path, docker_body): + shim_dir = tmp_path / 'bin' + shim_dir.mkdir() + shim = shim_dir / 'docker' + shim.write_text(f'#!/bin/sh\n{docker_body}\n') + shim.chmod(shim.stat().st_mode | stat.S_IXUSR) + env = dict(os.environ, PATH=f'{shim_dir}:{os.environ["PATH"]}') + result = subprocess.run( + ['sh'], input=_probe_shell_script(), text=True, + capture_output=True, env=env, timeout=30, + ) + assert result.returncode == 0, result.stderr + return _parse_probe_output(result.stdout) + + def test_running_container_reports_1(self, tmp_path): + facts = self._probe_facts(tmp_path, 'printf "lager\\nstout\\n"') + assert facts['LAGER_RUNNING'] == '1' + + def test_substring_named_container_does_not_count(self, tmp_path): + # `docker ps --filter name=lager` matches substrings, so a container + # named e.g. `lagertest` comes back from the filter; only an exact + # `lager` row may count as the box container. + facts = self._probe_facts(tmp_path, 'printf "lagertest\\n"') + assert facts['LAGER_RUNNING'] == '0' + + def test_no_rows_reports_0(self, tmp_path): + facts = self._probe_facts(tmp_path, ':') + assert facts['LAGER_RUNNING'] == '0' + + def test_docker_failure_reports_unknown(self, tmp_path): + facts = self._probe_facts(tmp_path, 'exit 1') + assert facts['LAGER_RUNNING'] == '' From f4e0baf917082c967ed9c43d81e2114c79f53cde Mon Sep 17 00:00:00 2001 From: DanielRMErskine Date: Wed, 22 Jul 2026 14:21:18 -0700 Subject: [PATCH 3/3] Also catch OSError in the failure path's best-effort operations run_ssh_command_with_output wraps subprocess.run, which raises OSError (FileNotFoundError, PermissionError, fork failures) rather than SubprocessError when the ssh process cannot be spawned. An escaping OSError in the build-failure branch would replace the honest failure message and the recovery restart with a traceback. Raised in review. --- cli/commands/utility/update.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/commands/utility/update.py b/cli/commands/utility/update.py index 46d632fb..491696ae 100644 --- a/cli/commands/utility/update.py +++ b/cli/commands/utility/update.py @@ -1964,7 +1964,7 @@ def _docker_supports_buildkit(): # catches the dead-container case. try: store_build_hash('FAILED') - except subprocess.SubprocessError: + except (subprocess.SubprocessError, OSError): pass # Step 8 already stopped and removed the containers, so exiting here @@ -1986,7 +1986,7 @@ def _docker_supports_buildkit(): # Exit 3 = container up but post-start installs failed; the # box is serving, which is all this recovery promises. restarted = recover.returncode in (0, 3) - except subprocess.SubprocessError: + except (subprocess.SubprocessError, OSError): restarted = False if restarted: restarted = wait_for_box_ready(resolved_box, timeout_s=60)