K2GO-393 fix(maps): recover a half-done install (runrole mode, completion gate, stall kill) - #557
Merged
Merged
Conversation
…ot error MapsRunroleCommand hardcoded 'runrole --reinstall maps'. --reinstall requires an existing maps_ line in iiab_state.yml to delete; after a failed install that line is gone, so a retry errors out (exit 1). This is the retry bug for a half-done maps install. Choose the mode at runtime by the marker, mirroring runrole's own grep '^maps_' $IIAB_STATE_FILE gate: marker present (first selection or a re-selection over a completed/base-seeded install) -> --reinstall; marker absent (a prior --reinstall run failed after deleting it) -> plain runrole, which re-enters install.yml and lets creates: re-fetch only the missing file. The app's decision now always matches runrole's. Unit test asserts both branches and the runtime gate.
…/K2GO-393-maps-recover-half-done
… the intent Maps is a proot install. An app death mid-install left the local_vars intent (maps_install: True) claiming maps was installed, with no download done and no recovery offered. Read maps "installed" from the iiab_state completion marker (maps_installed: True, written only when the role finishes) instead. - InstalledModules.isCompleted: pure rule for the completion marker. - InstalledModulesReader: one readRootfsYaml behind both files (local_vars intent and iiab_state completion); installedKeys reads maps from the marker; isCompletionGated names the proot-module set (today maps), one source shared with the hub. - ModuleHubFragment.confirmByProbe skips completion-gated keys, so a live probe cannot re-mask a half-done maps whose partial content still answers. - LocalVarsYamlParser also keeps _installed keys (one parser, both files). Verified on device (OnePlus): with the marker absent and the intent still true, both the module detail and the hub show maps Not installed / Install now, so the half-done build is recoverable; with the marker present, both show Installed.
… Retry (A4) The module stall watch was surface-only: on an unstable network the in-proot download can hang forever (the aria2c/meta4 fetch blocks with no timeout) and the install spun with no way out. Add a longer hard threshold that kills the runrole. The kill reuses the existing failure path: a non-zero exit makes installNextModule's onProcessExit revert the module and offer Retry, exactly as a self-failed module -- no new recovery plumbing. A generation token drops a kill queued for a module that has since ended. The soft "stalled" hint is unchanged. Note: Ansible does not stream a shell task's stdout, so during a long download the log goes quiet; the write-dir growth is the live movement signal. Safe for maps -- aria2c writes a 60s-summary log into library/downloads/maps.
…te the kill limit Device test showed the kill drives recovery through onError (the kill closes the runrole output stream mid-read), not onProcessExit as the comment claimed; both reach the same revert + Retry path. Also note that killProcess orphans proot's in-container child rather than reaping it (shared with doCancel) -- recovery still works; reaping the subtree is a follow-up.
…flags Deep recon of roles/maps/tasks/download_large_file.yml: the orphaned in-container child is low impact -- aria2c self-exits (default max-tries=5, timeout=60) and the meta4 requests.get just idles a socket; neither firehoses disk. So the orphan does not warrant a bespoke reaper. Reap it (reusing EnvironmentProcess's /proc sweep, the established idiom) only if a retry-conflict is ever observed in the field.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Maps is a proot install. Three defects left a half-done install stuck with no way
out; this branch fixes recovery for all three.
What changed:
a finished/base-seeded install, plain
runrole mapsto recover a half-done one. Abare --reinstall errored when a prior failed run had already deleted the marker --
the retry bug this removes.
the local_vars intent (maps_install) (A3). An app death mid-install left the intent
claiming maps was installed with nothing downloaded; the completion marker is written
only when the role finishes, so a half-done install reads not-installed and stays
recoverable. isCompletionGated shares this decision with the hub's probe, so a live
probe cannot re-mask a half-done maps.
no-movement backstop beside the existing soft "stalled" hint. The kill drives the
existing per-module failure path (revert + Retry) -- no new plumbing.
Why: on an unstable network the in-proot download can hang forever, and a half-done
install could not be retried (the retry errored, or the app showed maps falsely
installed).
Verification (device: OnePlus 7T, arm64):
file, PLAY RECAP failed=0).
detail and the hub show maps Not installed / Install now even while content still
serves; with the marker present, both show Installed.
("[Stall] ... killing the runrole"), which drove the failure path to the "Couldn't
build maps ... retry" card. The kill reaches recovery via onError (it closes the
output stream), which routes to Retry the same as onProcessExit.
Notes:
Left as-is deliberately, grounded in recon: per roles/maps/download_large_file.yml the
orphan is transient (aria2c self-exits: max-tries=5, timeout=60) or idle (the meta4
requests.get), and neither firehoses disk. A device test also ruled out a clean fix:
proot ignores SIGTERM (neither dies nor runs --kill-on-exit), so SIGKILL is the only
kill and the sole reap path is a /proc sweep. That sweep (reuse EnvironmentProcess) is
only warranted if a retry-conflict is ever observed in the field.
ModuleHubFragment, MapsRunroleCommand, InstallService. Assets (help.db,
module_sizes.csv, rootfs_sizes.csv) deliberately excluded.