diff --git a/cli/commands/utility/update.py b/cli/commands/utility/update.py index f2b54ef2..491696ae 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,60 @@ 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' + + +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()` @@ -1082,7 +1145,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,15 +1159,24 @@ 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) 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: @@ -1131,6 +1203,17 @@ 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 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' @@ -1153,7 +1236,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 or deploy_stale + ) if will_change: click.echo('Run without --check to apply.') ctx.exit(1) @@ -1256,10 +1342,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 +1656,24 @@ 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, + ) + _box_v = '' # assigned for real below whenever _gate stays 'skip' + if _gate == 'skip': import re as _re # Prefer the box's source-declared `__version__` over the CLI @@ -1594,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 @@ -1624,6 +1730,25 @@ def _render_boxcfg(ok): click.echo() ctx.exit(0) + 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(_fall_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 +1938,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, OSError): + 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, OSError): + 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..f494e1b9 --- /dev/null +++ b/cli/tests/test_update_gate.py @@ -0,0 +1,164 @@ +# 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 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, +) + +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' + + +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'] == ''