* Make Core.shutdown idempotent and safe to call concurrently
After #6887, Core.shutdown() now runs in the SIGTERM path during host
shutdown in addition to the existing host.control reboot/shutdown and
backup restore paths. Multiple concurrent callers were possible (e.g.
SIGTERM arriving while a reboot API call is mid-flight), so __main__.py
debounced the signal handler by stashing the in-flight task in a single-
element list and bailing out on the second SIGTERM.
Move the idempotency into Core.shutdown() itself, where it belongs:
- A second call while shutdown is in progress awaits the in-flight
shutdown via an asyncio.Event rather than re-running the sequence.
- Calls during STOPPING/CLOSE return early (Supervisor is already going
away; the work is moot).
- Calls during STARTING_STATES (INITIALIZE/STARTUP/SETUP) return early
too. There is nothing coherent to gracefully stop before startup
completes, and on the SIGTERM-during-startup path the caller cancels
startup_task first, so waiting for it to complete would deadlock.
- The sequence is wrapped in try/finally so the completion event is set
even when an inner step raises.
With that in place the closure workaround in __main__.py collapses to a
plain coresys.create_task(stop_supervisor()): repeat SIGTERMs spawn
extra tasks but each just observes the in-flight shutdown and waits.
Tests cover the four state branches and confirm the event is reset
between repeated shutdown cycles (backup restore re-enters RUNNING).
* Split Core.shutdown() into teardown_services + shutdown
PR feedback (@mdegat01) flagged the "supports repeated use" comment on
_shutdown_event.clear() as describing a use case that does not exist.
Investigating, the real source of confusion is that the old shutdown()
did two different things stitched together:
- Stop user-facing containers (add-ons + Home Assistant Core), which
backup restore uses while leaving the host alone.
- Run the full shutdown ceremony (state transition to SHUTDOWN, stop
plugins), which only the SIGTERM signal handler and the host
reboot/power-off API want.
That dispatch was implemented with an asymmetric state transition
("only set SHUTDOWN if state == RUNNING") and a plugin-shutdown gate
("only stop plugins if state in (STOPPING, SHUTDOWN)"). It worked but
made the intent of each branch hard to read, broke the reentrancy
guard on the restore path (state never reaches SHUTDOWN, so concurrent
callers fall through every early return), and forced the misleading
"repeated cycles" framing on the event handling.
Split into two methods with one job each:
- teardown_services(): stop add-ons + Home Assistant Core. Does not
change Core state and does not stop plugins. Backup restore calls
this directly so HA Core's watchdog stays registered (it only
disables on transitions into CLOSING_STATES) and plugins keep
running for the restore body to use.
- shutdown(): real shutdown ceremony. Unconditionally transitions to
SHUTDOWN, calls teardown_services(), then stops plugins. The
reentrancy guard (state == SHUTDOWN -> await event) now works
correctly because every caller transitions state on entry. One-shot
per process lifetime; no clear() needed.
Update backups/manager.py:867 to call teardown_services() instead.
remove_homeassistant_container moves to teardown_services() since
restore is the only caller that passes it.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Release shutdown waiters when set_state() is cancelled
PR feedback from Copilot: set_state() updates Core._state synchronously
(line 84 in core.py) before awaiting _write_run_state(). If the
shutdown task is cancelled while awaiting that write, in-memory state
is already SHUTDOWN but the function exits before entering the
try/finally that sets _shutdown_event. Any concurrent or later
shutdown() caller then sees state == SHUTDOWN and blocks forever on
_shutdown_event.wait().
Move the set_state(SHUTDOWN) call inside the try so finally always
runs and releases waiters. CancelledError still propagates to the
caller after finally as expected; we just no longer leak the lock.
Add a regression test that simulates cancellation inside
_write_run_state() and asserts both that state has moved to SHUTDOWN
and that _shutdown_event is set.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
#6765 renamed Supervisor.check_connectivity to
check_and_update_connectivity, but the mocked_setup_loads fixture in
tests/test_core.py still patched the old name. The patch.object call
raised AttributeError at fixture setup, erroring out the
test_setup_app_file_read_error_not_captured test before it could run.
Update the patch target to the new method name so Core.setup() sees an
AsyncMock for the connectivity probe again.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Rework Supervisor connectivity check with coalescing and force flag
Previously, a failed connectivity probe could strand Supervisor in a
"no connectivity" state indefinitely. After an Ethernet reconnect, a
probe kicked by NetworkManager's connectivity transition could race
with CoreDNS being restarted (due to DNS locals changing), time out on
DNS, and leave supervisor.connectivity = False. The retry that
_on_dns_container_running was meant to fire landed inside the 5 s
JobThrottle window from the just-failed probe and was silently dropped,
since JobThrottle.THROTTLE drops rather than waits.
The rework replaces the @Job(throttle=THROTTLE) decorator and the
public connectivity setter with a single authoritative state-updating
method:
- check_and_update_connectivity(force=False) is the only path that
runs the HTTP probe and updates the cached state. Concurrent callers
coalesce onto a single in-flight probe. A min-interval throttle
lives inside the method and reuses the cached result within window
instead of dropping calls.
- request_connectivity_check(force=False) is a fire-and-forget wrapper
for signal handlers (D-Bus, plugin callbacks) that must return
quickly without blocking signal dispatch on the HTTP round-trip.
- force=True bypasses the min-interval and, when a probe is in flight,
sets a trailing-rerun flag so the owning task runs one more probe
after the current one completes. Used for signals that carry fresh
state-change information (NM connectivity transition to FULL, DNS
container RUNNING, startup, post-NTP sync).
- _update_connectivity is the sole writer of the cached flag and
emits SUPERVISOR_CONNECTIVITY_CHANGE only on actual transitions.
Call sites migrate accordingly. The opportunistic
supervisor.connectivity = False writes in update_apparmor,
updater.fetch_data, os.manager, and addon_pwned error paths are
replaced with request_connectivity_check() calls so the probe remains
authoritative - an endpoint-specific failure no longer lies about the
overall connectivity state.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Propagate connectivity-probe cancellation and skip last-check on cancel
Awaiting an asyncio.Task does not propagate cancellation INTO the task,
so the previous owner-doesn't-shield comment was misleading: a cancelled
owner left the spawned probe running orphaned, and the next caller could
start a second probe alongside it. The owner now explicitly cancels and
awaits the probe on CancelledError before re-raising.
The last-check timestamp is also moved out of the finally block so a
cancelled probe does not leave a "fresh result just ran" cache behind
that would short-circuit the next non-forced caller.
A regression test exercises both: that owner cancellation clears the
in-flight reference and leaves the timestamp untouched, and that a
subsequent non-forced check therefore still actually probes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Clarify why post-NTP-sync forces a connectivity probe
The previous comment claimed the last-check timestamp may be unreliable
after a time jump, but _connectivity_last_check uses loop.time() which
is monotonic and unaffected by wall-clock corrections. The real reason
to force a fresh probe is TLS validation: certificates that appeared
expired or not-yet-valid before the system clock was corrected may now
verify, so a probe that just failed with an SSL error can succeed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add debug logs to Supervisor connectivity probe paths
The original stuck-offline bug was hard to spot in logs because the
silent throttle-drop and the cached state had no audit trail. With
debug-level logging at each decision point, a future investigation can
reconstruct from a single log file:
- who requested a check (force flag distinguishes signal-driven probes
from precondition / opportunistic-error-path requests)
- why a probe did not actually run (in-flight coalesce, cached within
min-interval, owner cancellation)
- when a forced rerun was queued and when it ran (the precise failure
mode that stranded the supervisor in the original incident)
- when the cached state actually flipped (with the previous value in
the message so transitions are visible)
All new lines are debug-level. The existing _do_connectivity_check
"failed" / "succeeded" lines are kept unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Skip system-checks fan-out in test_events_on_issue_changes
The test asserts that apply_suggestion fires an ISSUE_REMOVED event.
ISSUE_REMOVED is fired by dismiss_issue inside FixupBase.__call__, before
apply_suggestion calls healthcheck. The healthcheck call afterwards is
incidental to this test's intent, but it fans out into check_system()
which runs CheckDNSServer (A and AAAA) - real aiodns query_dns() probes
against the NetworkManager mock's stub nameserver 192.168.30.1 that each
hit the default ~10 s aiodns timeout. The file took ~21 s to run.
The slowness has been latent since #3818 (Aug 2022), which added the
apply_suggestion step at the end of test_events_on_issue_changes two
days after the DNS check landed in its current form (#3811). The default
24 h JobThrottle on CheckDNSServer.run_check tends to mask the cost in
full-suite runs once any earlier test has tripped the throttle, which is
likely why this slipped through.
Mock coresys.resolution.healthcheck for just this one apply_suggestion
call rather than introducing a file-wide DNS mock. The patch is local to
the slow call site and the test's assertion is unaffected. The file
drops from ~21 s to ~2.5 s.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Handle add-on filesystem errors gracefully and reduce Sentry noise
Add AddonFileReadError for add-on metadata read failures (long_description,
refresh_path_cache) caused by filesystem errors like EBADMSG (errno 74).
The new exception calls check_oserror() to mark the system unhealthy via
the resolution system, then raises a translatable API error so callers
get a proper error response instead of an unhandled OSError.
Fixes SUPERVISOR-BC6 (548K events from the API path) and
SUPERVISOR-BZJ (from the startup/load path).
In core.py setup(), skip reporting exceptions to Sentry when the error
has already been handled by the resolution system. This is detected by
checking if a new unhealthy reason was added during the task execution
(e.g. via check_oserror). In that case the user is already notified, so
we log at error level (no stack trace) instead of critical (which would
also send to Sentry via the LoggingIntegration) and skip the explicit
capture_exception call.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Skip Sentry capture for AppFileReadError in setup()
Replace the unhealthy-state comparison logic with an explicit
`except AppFileReadError` clause. The error is already reported to
the user via the resolution system (check_oserror adds an unhealthy
reason), so capturing it to Sentry just adds noise.
Log at error level without stack trace instead of critical to avoid
the LoggingIntegration picking it up.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add tests for AppFileReadError and setup() Sentry handling
Test that long_description and refresh_path_cache raise AppFileReadError
and mark the system unhealthy for EBADMSG errors, and raise without
marking unhealthy for other OSError types.
Also test Core.setup() to verify AppFileReadError is handled without
Sentry capture while other exceptions are captured as before.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Wait for systemd-timesyncd to stop before setting time
The previous 1-second sleep was not always enough for
systemd-timesyncd to fully stop, causing timedated to still reject
the set_time call with "Automatic time synchronization is enabled".
Instead, listen for the unit's ActiveState D-Bus property to become
inactive before proceeding, with a 10-second timeout.
Refs SUPERVISOR-92R
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Add wait_for_active_state helper to SystemdUnit
Centralize the repeated pattern of listening for D-Bus
PropertiesChanged signals to wait for a systemd unit's ActiveState
to reach a target state. Refactor core.py, host/firewall.py, and
mounts/mount.py to use the new helper.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Test time sync with active-to-inactive state transition
Exercise the actual wait_for_active_state signal-driven transition
in the time sync test: start the mock unit as "active" and drive it
to "inactive" via a PropertiesChanged signal, rather than starting
it as "inactive" which would make the wait a no-op.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Fix fallback time sync, create repair issue if time is out of sync
The "poor man's NTP" using the whois service didn't work because it attempted
to sync the time when the NTP service was enabled, which is rejected by the
timedated service. To fix this, Supervisor now first disables the
systemd-timesyncd service and creates a repair issue before adjusting the time.
The timesyncd service stays disabled until submitting the fixup. Theoretically,
if the time moves backwards from an invalid time in the future,
systemd-timesyncd could otherwise restore the wrong time from a timestamp if we
did that after the time was set.
Also, the sync is now performed if the time is more that 1 hour off and in both
directions (previously it only intervened if it was more than 3 days in the
past).
Fixes#6015, refs #6549
* Update test_adjust_system_datetime_if_time_behind
* Propagate timezone setting to host in OS 16.2 and newer
With home-assistant/operating-system#4224, timezone setting in OS can be
peristently set in HAOS as well. Propagate the timezone configured in
Supervisor config (which can be changed through general system settings
in HA Core) through the DBus API for setting the timezone.
* Persist timezone also when it's been obtained from Whoami
* Suppress pylint fixme error
* Recreate aiohttp ClientSession after DNS plug-in load
Create a temporary ClientSession early in case we need to load version
information from the internet. This doesn't use the final DNS setup
and hence might fail to load in certain situations since we don't have
the fallback mechanims in place yet. But if the DNS container image
is present, we'll continue the setup and load the DNS plug-in. We then
can recreate the ClientSession such that it uses the DNS plug-in.
This works around an issue with aiodns, which today doesn't reload
`resolv.conf` automatically when it changes. This lead to Supervisor
using the initial `resolv.conf` as created by Docker. It meant that
we did not use the DNS plug-in (and its fallback capabilities) in
Supervisor. Also it meant that changes to the DNS setup at runtime
did not propagate to the aiohttp ClientSession (as observed in #5332).
* Mock aiohttp.ClientSession for all tests
Currently in several places pytest actually uses the aiohttp
ClientSession and reaches out to the internet. This is not ideal
for unit tests and should be avoided.
This creates several new fixtures to aid this effort: The `websession`
fixture simply returns a mocked aiohttp.ClientSession, which can be
used whenever a function is tested which needs the global websession.
A separate new fixture to mock the connectivity check named
`supervisor_internet` since this is often used through the Job
decorator which require INTERNET_SYSTEM.
And the `mock_update_data` uses the already existing update json
test data from the fixture directory instead of loading the data
from the internet.
* Log ClientSession nameserver information
When recreating the aiohttp ClientSession, log information what
nameservers exactly are going to be used.
* Refuse ClientSession initialization when API is available
Previous attempts to reinitialize the ClientSession have shown
use of the ClientSession after it was closed due to API requets
being handled in parallel to the reinitialization (see #5851).
Make sure this is not possible by refusing to reinitialize the
ClientSession when the API is available.
* Fix pytests
Also sure we don't create aiohttp ClientSession objects unnecessarily.
* Apply suggestions from code review
Co-authored-by: Jan Čermák <sairon@users.noreply.github.com>
---------
Co-authored-by: Jan Čermák <sairon@users.noreply.github.com>
* Initialize Supervisor Core state in constructor
Make sure the Supervisor Core state is set to a value early on. This
makes sure that the state is always of type CoreState, and makes sure
that any use of the state can rely on it being an actual value from the
CoreState enum.
This fixes Sentry filter during early startup, where the state
previously was None. Because of that, the Sentry filter tried to
collect more Context, which lead to an exception and not reporting
errors.
* Fix pytest
It seems that with initializing the state early, the pytest actually
runs a system evaluation with:
Starting system evaluation with state initialize
Before it did that with:
Starting system evaluation with state None
It detects that the container runs as privileged, and declares the
system as unhealthy.
It is unclear to me why coresys.core.healthy was checked in this
context, it doesn't seem useful. Just remove the check, and validate
the state through the getter instead.
* Update supervisor/core.py
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* Make sure Supervisor container is privileged in pytest
With the Supervisor Core state being valid now, some evaluations
now actually run when loading the resolution center. This leads to
Supervisor getting declared unhealthy due to not running in a privileged
container under pytest.
Fake the host container to be privileged to make evaluations not
causing the system to be declared unhealthy under pytest.
* Avoid writing actual Supervisor run state file
With the Supervisor Core state being valid from the very start, we end
up writing a state everytime.
Instead of actually writing a state file, simply validate the the
necessary calls are being made. This is more conform to typical unit
tests and avoids writing a file for every test.
* Extend WebSocket client fixture and use it consistently
Extend the ha_ws_client WebSocket client fixture to set Supervisor Core
into run state and clear all pending messages.
Currently only some tests use the ha_ws_client WebSocket client fixture.
Use it consistently for all tests.
---------
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* Migrate to Ruff for lint and format
* Fix pylint issues
* DBus property sets into normal awaitable methods
* Fix tests relying on separate tasks in connect
* Fixes from feedback
* Bad message error marks system as unhealthy
* Finish adding test cases for changes
* Rename test file for uniqueness
* bad_message to oserror_bad_message
* Omit some checks and check for network mounts
* Fix fallback to non-SSL whoami call
In case of an exception "data" is not set leading to an error:
cannot access local variable 'data' where it is not associated with a value
Make sure to fallback to the non-SSL whoami call properly.
* Add pytests
* Ignore protected access in pytests
* Add test when system time is behind by more than 3 days
* Fix test_adjust_system_datetime_if_time_behind test and cleanup
* Add new time handling
* migrate date for python3.9
* add timedate
* add tests & simplify it
* better testing
* use ssl
* use hostname with new interface
* expose to API
* update data
* add base handler
* new timezone handling
* improve handling
* Improve handling
* Add tests
* Time adjustment function
* Fix logging
* tweak condition
* don't adjust synchronized time
* Guard
* ignore UTC
* small cleanup
* like that, we can leaf it
* add URL
* add comment
* Apply suggestions from code review
Co-authored-by: Joakim Sørensen <joasoe@gmail.com>
Co-authored-by: Joakim Sørensen <joasoe@gmail.com>
Jan ČermákandGitHub
Mike DegatanoandGitHub
