mirror of
https://github.com/home-assistant/supervisor.git
synced 2026-08-20 13:18:21 +01:00
core-lifecycle-group-queue
14
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
   
|
f49b81840e |
Add app-based mapping options for app configs (#6992)
* Rename addon map options to app equivalents * Add tests for apps/addons default mount targets * Fixes from feedback * Fix tests and clean up validation logic a bit * Update supervisor/apps/validate.py * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Stefan Agner <stefan@agner.ch> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
84ad6b9446 |
Reject apps mapping a dynamic ingress port range port (#6989)
Dynamic ingress port selection (ingress_port: 0) picks a random port from the 62000-65500 range and hands it to the app to listen on for ingress. That port is reached over the internal Docker network only. If an app also maps a container port from that range to the host, the dynamically chosen ingress port could coincide with it. The ingress endpoint would then be reachable directly on the host, bypassing ingress authentication. Reject such configs during validation instead: an app using dynamic ingress port selection must not map a port from the dynamic ingress port range itself. The range bounds are extracted into INGRESS_DYNAMIC_PORT_MIN and INGRESS_DYNAMIC_PORT_MAX constants shared between the validator and the allocator. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Mike DegatanoandGitHub
|
00c48a88ea |
Improve port conflict detection and resolution across Core and apps (#6916)
* Finalize port conflict handling and test coverage * Fix port conflict regressions and align resolution tests * Simplify port conflict detection: remove app options validation and manager helper * Refactor approve_check and process_fixup to accept typed Issue/Suggestion objects * Remove core port conflict detection; keep app startup handling * Remove unused code * Auto-dismiss issue on app start and remove more dead code |
||
|
|
81e235376e |
Fix typos repo-wide and add codespell pre-commit hook (#6949)
* Fix typos in comments, docstrings and log messages Correct 39 spelling mistakes across comments, docstrings and log/error message strings throughout the package (e.g. "conection" -> "connection", "Incomming" -> "Incoming", "Rasie" -> "Raise"). All changes are confined to human-readable text; no identifiers, attributes or D-Bus contracts are touched, so there is no behavior change. Found with codespell. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix typos in tests and CI workflow Correct spelling mistakes in test comments, docstrings and data, plus one in the builder workflow, so the whole tree is clean for the codespell hook added next. The assertion in test_network_manager.py is updated to match the corrected "Unknown error while processing" log message in the source. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Add codespell pre-commit hook Wire up codespell so spelling mistakes in comments, docstrings and strings are caught automatically. The vendored frontend panel is excluded, and "hass" and "astroid" are added to the ignore list as known false positives (the Home Assistant abbreviation and the pylint dependency package). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Address review feedback Improve grammar in several of the touched comments and docstrings: use the plural "ignore conditions" for the list-returning property, add the missing auxiliary verb and fix agreement in the timezone-filter comment, fix "backups ... use" agreement, and reword "underlay" to "underlying" in the arch module docstring. Also drop the "*.json" skip from the codespell hook. It was carried over from another project but is unnecessary here (all tracked JSON is clean), and skipping it would needlessly leave translation and data JSON unchecked. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Reword onboarding comment "overflight" was a literal calque of the German "überflogen"; use the idiomatic "skimmed through" instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
dc6a77507b |
fix(docker): restore add-on device access after USB re-enumeration (#6877)
* fix(docker): register hw listener and match by-id paths for options-based devices Two bugs caused a crash loop when a USB device re-enumerates to a different minor number (e.g. ttyACM0→ttyACM1) after a HAOS reboot: 1. _hw_listener was only registered when addon.static_devices was non-empty. Addons that expose a device via the options schema (e.g. Z-Wave JS `device:` option) never had the listener registered, so add_devices_allowed was never called when the device reappeared at a new minor. 2. _hardware_events matched only device.path and device.sysfs against static_devices. When static_devices (or the new options path) contains a by-id symlink, the match always failed because by-id paths live in device.links. Fix: extend the listener registration condition to also cover addon.devices (options-based), and expand the path-matching set to include device.links so by-id paths resolve correctly. For options-based devices, compare the incoming Device against addon.devices (which re-evaluates options.json against the live hardware list, picking up the new minor number automatically). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(docker): use option_device_paths for cheap by-id hw event matching Refactor _hardware_events to avoid per-event full options validation (including pwnd hashing). Introduce AppOptions.extract_device_paths and AppModel.option_device_paths to extract raw device paths from options without resolving against live hardware. Use set-intersection against {device.path, device.sysfs, *device.links} so by-id symlinks match correctly after re-enumeration for both static and options-based devices. Update test to use real schema/options setup and simulate a minor-number change (ttyACM0→ttyACM1) with a stable by-id symlink. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test: improve hw listener test coverage and add policy check Address PR review feedback: - Add hardware policy check in _hardware_events to prevent bypassing access restrictions on hotplug events (follows same pattern as startup cgroup setup) - Fix test_app_options_device_hw_listener to properly simulate USB re-enumeration with different minor numbers (166:0 → 166:1) - Add test_app_options_device_policy_check to verify policy enforcement for options-based devices - Update TEST_HW_DEVICE with realistic major/minor attributes (166:0 for tty) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * test: mock HostFeature.OS_AGENT in hardware event tests The _hardware_events method has a @Job decorator with conditions=[JobCondition.OS_AGENT], which checks if HostFeature.OS_AGENT is in sys_host.features. Without mocking this, the job conditions fail and the hardware event handler is never invoked, causing add_devices_allowed to not be called and tests to fail. Add patch.object(type(coresys.host), "features", ...) to all four hardware event tests to ensure the OS_AGENT job condition is met. Fixes test failures: - test_app_new_device (all 6 parametrized cases) - test_app_new_device_no_haos - test_app_options_device_hw_listener - test_app_options_device_policy_check Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * test: fix TEST_DEV_PATH to match TEST_HW_DEVICE.path TEST_DEV_PATH was set to /dev/ttyAMA0 but TEST_HW_DEVICE.path is /dev/ttyACM0. This mismatch would cause the dev_path=TEST_DEV_PATH parametrized test cases to fail because the hardware event handler checks if the device path intersects with the app's allowed devices, and "/dev/ttyAMA0" != "/dev/ttyACM0". Update TEST_DEV_PATH from /dev/ttyAMA0 to /dev/ttyACM0 to match TEST_HW_DEVICE. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * test: mock option_device_paths in test_app_options_device_hw_listener The test sets up schema and options but option_device_paths property may not be working as expected in the test environment. Add an explicit mock for option_device_paths to ensure it returns the by-id path, guaranteeing that: 1. The hardware listener is registered (checks option_device_paths at registration) 2. The device path matching works correctly in _hardware_events This ensures the test properly validates that hardware events are processed for options-based devices after re-enumeration. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * test: make policy check test actually exercise the policy guard test_app_options_device_policy_check set the device option via persist["options"], but option_device_paths reads the merged options and does not pick that override up during the test, so it returned an empty set. The hardware event therefore failed the path-match guard and returned before reaching the allowed_for_access check. The assert_not_called() assertion then passed regardless of the policy outcome -- it would still pass if the policy guard were removed entirely. Mock option_device_paths to return the configured by-id path (mirroring test_app_options_device_hw_listener) so the event device matches and execution actually reaches the policy guard the test is meant to verify. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: add unit test for AppOptions.extract_device_paths The integration tests exercise extract_device_paths only through a mocked option_device_paths property, so the schema-walking logic introduced for the hardware-event matching had no direct coverage. Add a unit test that drives every schema shape the recursion handles -- flat, optional, filtered, list, nested dict and list of dicts -- and asserts that non-device options, unset keys and empty values are skipped, without requiring the devices to exist in hardware. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Stefan Agner <stefan@agner.ch> |
||
|
|
153108754c |
Don't offer a rebuild repair for a detached app (#6932)
This is a corner case: a locally-built app whose container image went missing for some external reason and which is also detached from its store, e.g. its local source folder was removed. There is no normal Supervisor command that produces this combination -- rebuild removes the image before rebuilding, but it is rejected for detached apps, so it cannot leave a detached app without an image. The image loss here came from outside Supervisor's normal flow. Such an app cannot be rebuilt: there is no source. App.load still surfaced a MISSING_IMAGE repair with an EXECUTE_REPAIR suggestion for it, and the resolution autofix loop then tried to rebuild it. That dead-ends in App.path_location, which raises for a detached app, crashing the autofix with an unhandled exception. Don't create the rebuild repair when the app is detached. The detached-app check already surfaces a DETACHED_ADDON_REMOVED issue with an EXECUTE_REMOVE suggestion for these apps (the built-in local repository is always loaded), so the user is offered removal instead of a repair that can never succeed. Also guard the repair fixup itself against a detached build app. With the App.load change this is only reachable via a race: the repair is created while the app is attached and the app detaches before the autofix runs (or between its retries). Skip gracefully there too rather than throwing, mirroring the is_detached guards already in AppManager.update/rebuild. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
7eaf69fcab |
Derive installed-app source location from the store (#6929)
* Derive installed-app source location from the store The installed app's source location was persisted as an absolute string in apps.json, captured at install time. The addons->apps directory migration renamed the source directories but left that stored string pointing at the old addons path, so locally-built apps failed to build with a misleading "dockerfile is missing" error on install/update/rebuild (#6917). The location is not really app state: it is the source directory discovered during the store scan and recomputed on every store reload. Derive it from the store data (App.path_location -> data_store) instead of persisting it, and drop ATTR_LOCATION from the system schema. REMOVE_EXTRA strips the stale value from existing apps.json files, so no migration is needed and already-migrated instances are fixed as well. location only exists for a store-backed app. The store is loaded before apps during setup (Core.setup ordering), so an installed, attached app can always resolve it; reaching path_location while detached is a programming error and now raises. Detached apps have no source, so their asset accessors (with_icon/logo/changelog/documentation, long_description) report absence rather than reading a path, and the path cache is no longer refreshed for them. Build, apparmor install and backup/restore never touch path_location on a detached app (build/update are blocked when detached; apparmor and the built image are captured from the host and restored from the backup), so those paths are unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Guard path_location on app_store instead of is_detached Address review feedback: path_location guarded on is_detached while the asset accessors guarded on app_store, two ways of expressing the same condition. Key path_location off app_store as well, matching install()/update() and the store API, so the check is consistent and is_detached is no longer needed here. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
a973d22e35 |
Derive App state from container state (#6890)
* Derive App state from container state
The App.state setter mixed two responsibilities: it both mutated a
private `_state` field and dispatched side effects (WebSocket events,
issue dismissal, startup_event signaling). On top of that, an installed
but never-started app stayed in AppState.UNKNOWN forever, because the
attach() image-only fallback never fires a container state-change event
and the AppState therefore kept its constructor default. Conceptually,
ContainerState.UNKNOWN ("container does not exist") and AppState.UNKNOWN
("nothing observed yet") happened to share a name but meant different
things, which made the distinction easy to lose.
Make App.state a pure derived property. The source of truth is the last
observed ContainerState (cached on the App), plus a sticky operation-
error flag for start/stop failures that the docker event stream cannot
reflect. When no container has been observed yet, the derivation falls
back to install signals: an attached instance (image present) is
STOPPED, otherwise UNKNOWN. As a side effect, an installed-but-never-
started app now correctly reports STOPPED instead of UNKNOWN.
container_state_changed updates the cached container state and routes
all side effects through a single _emit_state_change helper that diffs
old vs new derived state. The two start/stop failure paths route
through _set_operation_error. Uninstall resets the cached signals so
the derivation naturally returns UNKNOWN.
Tests use a new tests/common.force_app_state helper that pokes the
underlying signals directly; the production class no longer carries
test-only setters.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Fix App state drive to AppState.UNKNOWN
* Unify state mutation through _update_state
Previously, state-driving signal changes were spread across two helpers
(_set_operation_error, _emit_state_change(old_state)) and required each
caller to capture self.state before mutating a private field — leaking
implementation details to call sites and raising the "why am I emitting
the old state?" question pointed out in code review.
Replace both helpers with a single _update_state(*, container_state=,
operation_error=) entry point. Callers describe what changed via
keyword arguments (None leaves a signal untouched); the helper captures
the previous state, applies the updates, recomputes the derived state
and emits side effects if anything changed.
Diff against a tracked _last_state instead of a freshly derived
"current" state, so that an out-of-band mutation between updates does
not silently shift the comparison baseline. The concrete case is
App.uninstall: instance.remove() clears the docker meta mid-flow, which
would otherwise reshape the derivation (RUNNING with no healthcheck
becomes STARTED instead of STARTUP) and suppress the STARTUP transition
that resolves the start-wait task. As a side effect, the initial
UNKNOWN -> STOPPED transition on attach is also now reliably emitted.
Switch the uninstall path to ContainerState.UNKNOWN ("we know there is
no container") rather than the constructor sentinel None.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Cache app state instead of deriving on every read
Building on the previous commit, make App.state a plain read of a
cached _state field rather than re-deriving on every property access.
The derivation moves to _derive_state(), and _update_state() is the
sole place that recomputes and assigns _state, so the value consumers
read always matches what was last emitted to listeners.
This removes the _last_state bookkeeping introduced previously: with a
single cached value there is no longer a separate "derived now" vs
"last emitted" distinction to reconcile, and out-of-band mutations
(e.g. instance.remove() clearing _meta during uninstall) can no longer
silently shift what state returns between updates.
Call _update_state() at the end of load() so the cached state settles
once attach() has run. Image-only attaches do not fire a docker event,
so without this an installed app would stay in the constructor-default
UNKNOWN until first start; this also makes the initial transition on
attach observable to listeners.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Pass operation error to _derive_state instead of storing it
The two state-driving signals were not symmetric. _container_state is
genuinely persisted state ("the last thing docker told us") that
re-derivation legitimately reads across calls. _operation_error, on the
other hand, is a momentary "force ERROR for this transition" signal; the
persistence of an error condition already lives in the cached _state.
Storing it as an instance attribute implied a sticky cross-call behavior
that no call path actually exercised: every caller either set it
explicitly right before deriving (start/stop failures, container events)
or ran argless only at load time, where no failure has occurred.
Drop the _operation_error field and pass operation_error as a parameter
to _derive_state(), defaulting to False in _update_state(). A container
observation now supersedes a prior error implicitly via the default,
which lets the container-event and uninstall call sites drop their
explicit operation_error=False.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Settle load state synchronously from current_state
The argless _update_state() settle at the end of load() raced attach()'s
container-state event. attach() fires DOCKER_CONTAINER_STATE_CHANGE via
the bus, which schedules the container_state_changed listener as a task
rather than running it inline. In the deprecated-arch early-return path
there is no await between attach() and the settle, so the listener had
not run yet: _container_state was still None and the settle derived
STOPPED (instance attached) — emitting a transient UNKNOWN->STOPPED even
for a running container before the listener corrected it. The main path
only avoided this incidentally, by having awaits (check_image,
save_persist) in between for the listener to run.
Derive the load-time state synchronously from instance.current_state()
instead of relying on the asynchronously delivered event. current_state()
returns the real container state, or UNKNOWN when only an image is
present (which derives to STOPPED), so both paths settle correctly
without racing the event.
Add a regression test that loading a running container settles to
STARTED, and mock current_state() in the state-listener test which
relies on a clean UNKNOWN baseline.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
355396aeab |
Migrate addon/addons config paths and schema names to app/apps (#6865)
* Migrate config file and directory paths from addons to apps
- Rename addons.json -> apps.json (FILE_HASSIO_APPS constant)
- Rename addons/{core,data,local,git} -> apps/{core,data,local,git}
- Rename addon_configs -> app_configs
Backwards compatibility: on startup, Supervisor checks for legacy
paths and renames them if the new paths don't already exist.
- addons.json migration runs in AppManager.load_config (executor)
- Directory migrations run in bootstrap before initialize_system (executor)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Rename SCHEMA_ADDON(S)_* constants to SCHEMA_APP(S)_* in apps/validate.py
Update all references in supervisor and tests.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix remaining test references to legacy addons/* paths
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Opportunistic remove of addons dir since it should be empty post migration
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
||
|
|
f4962208b0 |
watchdog: include container exit code in restart log message (#6873)
* watchdog: include container exit code in restart log message Reported in #6868: a user saw a tight loop of "Watchdog found app phpMyAdmin is failed, restarting..." and assumed the watchdog itself was the problem, asking whether its window could be widened. The message gives no hint that the container is exiting on its own, nor what exit code it returned. #6848 already plumbed the container exit code through DockerContainerStateEvent and added a separate log line in container_state_changed when an app exits non-zero. Build on that by forwarding event.exit_code into _restart_after_problem for apps, Home Assistant Core, and plugins, and use it in the watchdog warning when the state is FAILED. The fallback message is kept for STOPPED and UNHEALTHY where an exit code is not meaningful. After this change the example above reads "Watchdog found app phpMyAdmin exited with code 1, restarting...", making it immediately clear that the container itself is dying and giving the user a code to grep for in the add-on logs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Drop unnecessary None check * tests: set exit_code on FAILED watchdog events The previous commit dropped the `exit_code is not None` guard from the watchdog `_restart_after_problem` log statements, which assumed the production invariant that `ContainerState.FAILED` always carries a non-None exit code (enforced by `docker/monitor.py` and `docker/interface.py`). Several tests, however, fired FAILED `DockerContainerStateEvent`s with no `exit_code`, causing the new unconditional `%d` formatter to raise `TypeError` at log time. Align the test fixtures with the production invariant by passing `exit_code=1` on FAILED events in the apps, Home Assistant Core, plugin base, and DNS plugin watchdog tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Stefan AgnerandGitHub
|
ed91b18c4b |
tests: enable flake8-pytest-style (PT) ruff rules (#6857)
* tests: enable flake8-pytest-style (PT) ruff rules Enable the `PT` ruff rule set and fix the resulting violations across the test suite: - PT006: pass parametrize argument names as tuples instead of a single comma-separated string. - PT022: switch fixtures that have no teardown from `yield` to `return` so the lack of cleanup is obvious at a glance. - PT011: add `match=` to broad `pytest.raises(ValueError)` blocks so the expected error is anchored to a specific message. - PT012: hoist setup (patches, branching) out of `pytest.raises()` blocks so only the call that is expected to raise remains inside. - PT013: replace `from pytest import X` with `import pytest` and access attributes via the module. - PT015: replace `try/except` + `assert False` patterns with `pytest.raises(...)`. - PT017: replace `assert` on exceptions inside `except` blocks with `pytest.raises(...) as exc_info` and assert on `exc_info.value`. No behavioral changes to the tests; the full suite still passes. * tests: address review feedback on PT ruff rule enablement - Fix fixture return-type annotations after switching `yield` to `return` in tests/conftest.py: drop the `Generator[...]`/`AsyncGenerator[...]` wrapper for `dns_manager_service`, `supervisor_internet`, `websession`, and `mock_update_data` so the annotation matches what the fixture actually returns. - Correct the return-type annotation of `fixture_ip6config_service` from `IP4ConfigService` to `IP6ConfigService`. - Fix recurring "excepiton" typo in tests/utils/test_exception_helper.py. * tests: verify backup cleanup on permission error After `test_new_backup_permission_error` raises `BackupPermissionError`, assert that no tarfile was left behind and `tmp_path` is empty. The previous version only checked that the exception was raised, which missed any regression where a partial tarfile would survive the failed create. * tests: rename DNS_GOOD_V6 to DNS_V6_UNSUPPORTED The constant was named "good" but its tests assert that the URLs are rejected by the DNS validator. The IPv6 URLs are well-formed but currently rejected because IPv6 doesn't work with the Docker network (see `dns_url` in supervisor/validate.py). Rename the constant and the related test to make the intent obvious. |
||
|
Stefan AgnerandGitHub
|
0bcedf5b98 |
Don't fail Supervisor setup when an app image is missing (#6816)
* Don't fail Supervisor setup when an app image is missing A missing builder image (docker:<version>-cli) during a build-required app load aborted Supervisor setup entirely, leaving the system stuck in setup state where every subsequent operation was blocked by the not-healthy guard. Triggered in practice when the host's Docker patch version had no matching `-cli` tag published on Docker Hub. Two issues compounded the failure: `images.pull` in `run_command` leaked a raw `aiodocker.DockerError` past the `@Job` decorator, which rewrapped it as `JobException` and bypassed the `suppress(DockerError, ...)` guard in `addon.load()`; and the load path treated all Docker errors the same whether the image was simply missing or the daemon itself was misbehaving. Wrap the pull error in `run_command` so it propagates as Supervisor's `DockerError` (a `HassioError`) and is preserved by the decorator. Distinguish 404s in `attach()` and `check_image()` by raising `DockerNotFound`/`DockerAPIError` instead of generic `DockerError`. In `addon.load()`, only the `DockerNotFound` path is treated as "image missing": for build-required apps we skip the inline build and surface a `MISSING_IMAGE` repair so the resolution autofix loop handles it off the critical path; for pull-based apps we still attempt install during load and create the repair on failure. Other `DockerError`s (daemon trouble or a failed internal install in `check_image`) are logged at CRITICAL — which the Sentry logging integration captures — and the addon is left detached rather than masked as a misleading missing-image repair. In the autofix path, swallow `DockerBuildError`, `DockerNoSpaceOnDevice`, `DockerRegistryAuthError`, and `DockerRegistryRateLimitExceeded` as `ResolutionFixupError` so they don't generate Sentry events on every retry. The repair stays available for manual retry once the underlying cause (registry tag published, disk freed, credentials fixed, rate limit expired) is resolved. * Clarify outer DockerError comment in App.load() The comment claimed "a future load will reattempt and surface a MISSING_IMAGE repair if appropriate", but App.load() is only called at Supervisor startup, on fresh install, and on backup restore — there is no automatic retry mechanism. Reword to match reality: the CRITICAL log captures the issue for diagnostics (Sentry), and the user can trigger a manual repair once the daemon is healthy. * Clarify comment about user interaction |
||
|
|
7ecfe42602 |
apps: log container exit code when app exits non-zero (#6848)
* apps: log container exit code when app exits non-zero Issue #6840 reports that stopping an app whose process exits 143 (SIGTERM default disposition) leaves the app in AppState.ERROR. ERROR is the right state for that — Docker itself treats any non-zero exit as a failure (e.g. `--restart on-failure`), and 143 specifically means the SIGTERM grace period was wasted because the app never installed a handler. But Supervisor previously logged nothing about it, leaving authors with no hint that their image is misbehaving. Plumb the exit code through DockerContainerStateEvent and log it from App.container_state_changed on transitions to FAILED: a warning for 143 nudging the author to trap SIGTERM and exit 0, and an error for any other non-zero code (crashes, SIGKILL after grace, app's own error exit). Refactor _container_state_from_model to return (state, exit_code) so the docker event monitor and DockerInterface.attach feed the same exit code through one code path instead of re-reading State.ExitCode in the caller. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * apps: address review feedback on exit-code logging - Replace bare 143 with EXIT_CODE_SIGTERM_DEFAULT (128 + signal.SIGTERM) in supervisor/docker/const.py so the reasoning is documented in code, not just in the log string. - Stop populating exit_code on STOPPED transitions. Previously the refactor made DockerInterface.attach emit exit_code=0 for cleanly stopped containers, while the monitor only emitted an exit code for abnormal exits. Align both paths so exit_code is only set on FAILED. - Add test_app_failed_logs_exit_code covering the three new branches (warning on 143, error on other non-zero, silent when None) and extend test_attach_existing_container to assert the event's exit_code field per state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docker/monitor: flatten exit_code branch to satisfy pylint The previous if/else inside the `die` branch pushed the function over pylint's too-many-nested-blocks threshold (6/5). Collapse it back into a pair of conditional expressions: container_state via ternary on the exit code, exit_code via `die_exit_code or None` so 0 stays None. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Update supervisor/apps/app.py Co-authored-by: Mike Degatano <michael.degatano@gmail.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Mike Degatano <michael.degatano@gmail.com> |
||
|
|
f8880a72be |
Rename addon/addons to app/apps in filenames and imports (#6837)
* Rename addon/addons to app/apps in filenames and imports Continues the addon→app terminology migration (#6786). Renames all source files, test files, fixture files, and directories that contained 'addon'/'addons' in their names, and updates all imports accordingly. Resolution check files in supervisor/resolution/checks/ that were renamed override the slug property to preserve the existing API contract (slugs are exposed via the resolution info API and used to run checks by name). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Rename add-on.json fixture --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |