* Fix cloning of add-on store repository
Since #5669, the add-on store reset no longer deletes the root
directory. However, if the root directory is not present, the current
code no longer invokes cloning, instead tries to load the git
repository directly.
With this change, the code clones whenever there is no .git directory,
which works for both cases.
* Fix pytest
* Move read_text to executor
* Fix issues found by coderabbit
* formated to formatted
* switch to async_capture_exception
* Find and replace got one too many
* Update patch mock to async_capture_exception
* Drop Sentry capture from format_message
The error handling got introduced in #2052, however, #2100 essentially
makes sure there will never be a byte object passed to this function.
And even if, the Sentry aiohttp plug-in will properly catch such an
exception.
---------
Co-authored-by: Stefan Agner <stefan@agner.ch>
Since #5696 we don't need to load the resolution center early. In fact,
with #5686 this is even problematic for pytests in devcontainer, since
the Supervisor Core state is valid and this causes AppArmor evaluations
to run (and fail).
Actually, #5696 removed the resolution center. #5686 brought it
accidentally back. This was seemingly a merge error.
By default, warnings are simply printed to stderr. This makes them
easy to miss in the log. Capture warnings and user Python logger to log
them with warning level.
Also, if the message is an instance of Exception (which it typically
is), report the warning to Sentry. This is e.g. useful for asyncio
RuntimeWarning warnings "coroutine was never awaited".
* 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>
* Load resolution evaluation, check and fixups early
Before #5652, these modules were loaded in the constructor, hence early
in `initialize_coresys()`. Moving them late actually exposed an issue
where NetworkManager connectivity setter couldn't get the
`connectivity_check` evaluation, leading to an exception early in
bootstrap.
Technically, it might be safe to load the resolution modules only in
`Core.connect()`, however then we'd have to load them separately for
pytest. Let's go conservative and load them the same place where they
got loaded before #5652.
* Load resolution modules in a single executor call
* Fix pytest
A newline is converted to a space as per YAML folding rules. The space
breaks markdown parsing of the link. Use a single line for the target
version link.
When developing/testing in a Supervised environment, the
systemd-journal-gatewayd socket is actually available. Mock the
socket Path file to make the test independent of the pytest
environment.
Overwrite the default body with useful version update information and
a link to the new release.
Also rename the title and use lower caps for local shell variables.
Enable the Sentry asyncio integration. This makes sure that exception
in non-awaited tasks get reported to sentry.
While at it, use partial instead of lambda for the filter function.
* Correctly handle aiohttp requests
The request header seems to be a dictionary in current Sentry SDK.
The previous code actually failed with an exception when trying to
unpack the header. However, it seems that Exceptions are not handled
or printed in this filter function, so those issues were simply
swallowed.
The new code has been tested to correctly sanitize and report issues
during aiohttp requests.
* Fix pytests
* Initialize machine information before Sentry
* Set user and machine for all reports
Now that we initialize machine earlier we can report user and machine
for all events, even before Supervisor is completely initialized.
Also use the new tag format which is a dictionary.
Note that it seems that with the current Sentry SDK version the
AioHttpIntegration no longer sets the URL as a tag. So sanitation is
no longer reuqired.
* Update pytests
The word "reboot" is usually used when a operating system is restarted.
The current log message could be interpreted that the Supervisor
detected an operating system reboot.
Use restart to make it clear that the Supervisor detected a restart of
itself.
Make sure that add-on store resets do not delete the root folder. This
is important so that successive reset attempts do not fail (the
directory passed to `remove_folder` must exist, otherwise find fails
with an non-zero exit code).
While at it, handle find errors properly and report errors as critical.
* Improve D-Bus timeout error handling
Typically D-Bus timeouts are related to systemd activation timing out
after 25s. The current dbus-fast timeout of 10s is well below that
so we never get the actual D-Bus error. This increases the dbus-fast
timeout to 30s, which will make sure we wait long enought to get the
actual D-Bus error from the broker.
Note that this should not slow down a typical system, since we tried
three times each waiting for 10s. With the new error handling typically
we'll end up waiting 25s and then receive the actual D-Bus error. There
is no point in waiting for multiple D-Bus/systemd caused timeouts.
* Create D-Bus TimedOut exception
Make sure that add-on store resets do not delete the root folder. This
is important so that successive reset attempts do not fail (the
directory passed to `remove_folder` must exist, otherwise find fails
with an non-zero exit code).
While at it, handle find errors properly and report errors as critical.
* Handle permission error on backup create
Make sure we handle (write) permission errors when creating a backup.
* Introduce BackupFileExistError and BackupPermissionError exceptions
* Make error messages a bit more uniform
* Drop use of exclusive mode
SecureTar does not handle exclusive mode nicely. Drop use of it for now.
With PR #5634 (which had the goal to remove I/O in event loop for backup
operations) the semantics of `remove_folder` changed slightly: Non-zero
exits of subprocesses were no longer handled, but lead to a
CalledProcessError.
Now to restore the semantics of `remove_folder` we should simply log an
error. However, this semantic change actually uncovered a potential
problem in deployed systems: There are 34 users on beta channel which
regularly seem to run `FixupStoreExecuteReset`, and with the semantic
change we see those errors in Sentry.
An obvious problem could be no storage. But in a quick test that would
not execute the repair in first place since the fixup has the job
condition `FREE_SPACE` set. So the problem is likely elsewhere.
With this change, we log the stderr of find, while still raising the
exception. With that we should get more context in Sentry to see what
could be the underlying error.
* Remove I/O in event loop for add-on backup and restore
Remove I/O in event loop for add-on backup and restore operations. On
backup, this moves the add-on shutdown before metadata is stored in the
backup, which slightly lenghens the time the add-on is actually stopped.
However, the biggest contributor here is likely adding the image
itself if it is a local backup. However, since that is the minority of
cases, I've opted for simplicity over optimizing for this case.
* Use partial to explicitly bind arguments
* Remove I/O in event loop for Home Assistant Core backup
The Home Assistant Core backup still contains some I/O in the event
loop. Move all I/O into the executor.
* Update supervisor/homeassistant/module.py
Co-authored-by: Mike Degatano <michael.degatano@gmail.com>
---------
Co-authored-by: Mike Degatano <michael.degatano@gmail.com>
* Avoid IO in event loop when removing backup
* Refactor backup size calculation
Currently size is lazy loaded when required via properties. This
however is blocking the async event loop.
Backup sizes don't change. Instead of lazy loading the size of a backup
simply determine it on loading/after creation.
* Fix tests for backup size change
* Avoid IO in event loop when loading backups
* Avoid IO in event loop when importing a backup
