Skip to content

Update: never report success on a box whose container is not running #137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
danielrmerskine merged 3 commits into from
Jul 22, 2026
Merged

Update: never report success on a box whose container is not running #137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8095364
Update: never report success on a box whose container is not running
danielrmerskine Jul 21, 2026
413efd4
Update: also catch a container running code older than the tree
danielrmerskine Jul 21, 2026
f4e0baf
Also catch OSError in the failure path's best-effort operations
danielrmerskine Jul 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
228 changes: 206 additions & 22 deletions cli/commands/utility/update.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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()`
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -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'
Expand All @@ -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)
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Comment on lines +1965 to +1968

@gemini-code-assist gemini-code-assist Bot Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The store_build_hash function internally calls run_ssh_command_with_output, which can raise OSError (e.g., if the ssh executable is missing or cannot be executed). Since OSError is not a subclass of subprocess.SubprocessError in Python, any OSError raised here will propagate and crash the CLI. Consider catching (subprocess.SubprocessError, OSError) to ensure this best-effort operation is truly safe.

Suggested change
try:
store_build_hash('FAILED')
except subprocess.SubprocessError:
pass
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
Comment on lines +1981 to +1990

@gemini-code-assist gemini-code-assist Bot Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similarly, run_ssh_command_with_output can raise OSError if the ssh command is unavailable or fails to execute. Since OSError is not caught by subprocess.SubprocessError, an unhandled exception could crash the update command during the recovery phase. Catching (subprocess.SubprocessError, OSError) ensures the recovery fallback is robust.

Suggested change
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
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')
Expand Down
Loading
Loading