Skip to content

Refactor updater complexity hotspots - #286

Open
gmmcosta15 wants to merge 11 commits into
devfrom
refactor/updater-complexity-hotspots
Open

Refactor updater complexity hotspots#286
gmmcosta15 wants to merge 11 commits into
devfrom
refactor/updater-complexity-hotspots

Conversation

@gmmcosta15

@gmmcosta15 gmmcosta15 commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Description

Select the type:

  • Feature
  • Bug fix
  • Code refactor
  • Documentation

Extract-method pass over the updater module to drop every radon complexity hotspot below grade C, plus correctness fixes and one feature surfaced during review and on-device testing. Behavior-preserving except the fixes/feature noted below.

Refactor

All updater modules now grade C or below.

Bug fixes

  • _read_state/_read_inflight catch (OSError, ValueError) so a corrupt state/inflight file degrades to empty instead of crashing boot recovery.
  • _revert_inflight keeps the in-flight marker on a failed boot revert (retries next boot) instead of clearing it; drops permanently-un-revertible entries. +1 test.
  • Restore component.path narrowing lost during helper extraction (pyright).
  • _prepare_rollback_point / _persist_batch_rollback abort if the in-flight marker write fails, instead of proceeding to move the tree with no boot-revert net (power-fail brick gap).
  • Disk-space pre-flight emits the terminal on_component_done(False), not just on_error, so callback consumers do not hang on the low-disk path.
  • apt snapshot removes its temp file if chmod/replace fails (leak on a 2GB card).
  • _prune_broken_refs rejects refs containing ".." before the raw filesystem unlink (path-traversal hardening on the corrupt-repo recovery path).
  • recover CLI exits non-zero on failure, matching update/bless (scripts see the failure).
  • _filter_dead_branch_batch also validates the effective ref when a component has no configured branch, so a deleted current branch no longer aborts the whole atomic batch.

Feature

  • Spoolman provision/update is gated on a real HTTP readiness probe (GET /api/v1/health, loopback-only) instead of trusting Type=simple is-active. New optional health_url component field (validated to loopback http); _restart_one/_provision_restart_service verify health after the service is active. A bad Spoolman update now auto-reverts and a bad provision removes the clone, instead of both silently reporting success.

Motivation

service.py/executor.py had F/E/D-grade functions that were hard to test. Review and on-device testing also caught a corrupt-file crash, in-flight markers that violated crash-recovery retry semantics, a power-fail brick gap, and a weak Type=simple readiness signal that let broken Spoolman installs report success.

@gmmcosta15 gmmcosta15 added the enhancement New feature or request. label Jul 21, 2026
@gmmcosta15 gmmcosta15 self-assigned this Jul 21, 2026
@gmmcosta15
gmmcosta15 requested a review from HugoCLSC July 21, 2026 09:14
@gmmcosta15 gmmcosta15 changed the title refactor/updater complexity hotspots Updater complexity hotspots Jul 21, 2026
@gmmcosta15 gmmcosta15 changed the title Updater complexity hotspots Refactor updater complexity hotspots Jul 21, 2026
…back, recover CLI exit code, ref path-traversal + apt snapshot temp-leak
…h readiness probe instead of Type=simple is-active
…ts so a deleted current branch no longer aborts the whole update
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant