Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nixos/home-assistant: change systemd network target #120309

Closed
wants to merge 1 commit into from
Closed

nixos/home-assistant: change systemd network target #120309

wants to merge 1 commit into from

Conversation

Janonard
Copy link

@Janonard Janonard commented Apr 23, 2021

Motivation for this change

Many Home Assistant integrations pull in information from websites at early stages and therefore need a fully set-up network when they start. If Home Assistant is starts directly after network.target, a connection to the internet might not be established, but it (probably) is after network-online.target. More info: https://www.freedesktop.org/wiki/Software/systemd/NetworkTarget/

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Many Home Assistant integrations pull in information from websites
at early stages and therefore need a fully set-up network config.
@SuperSandro2000
Copy link
Member

Many Home Assistant integrations pull in information from websites at early stages and therefore need a fully set-up network when they start

Do they refetch that information after network is available?

Using network-online is not great as it delays the startup unnecessary.

@mweinelt
Copy link
Member

Many Home Assistant integrations pull in information from websites at early stages and therefore need a fully set-up network when they start. If Home Assistant is starts directly after network.target, a connection to the internet might not be established, but it (probably) is after network-online.target. More info: freedesktop.org/wiki/Software/systemd/NetworkTarget

Home-assistant components are usually pretty resilient. What concrete issues do you have that are going to be fixed by this change?

@Mic92
Copy link
Member

Mic92 commented Apr 24, 2021

If this is a special case rather than the norm it is also fine to do:

systemd.services.home-assistant.after = [ ""network-online.target" ];

in your own NixOS configuration.

But it is better to fix the underlying issue because having no network is an expected network condition and hence connectivity cannot be relied on for a system not too fail

@Janonard
Copy link
Author

One integration that had some problems is met.no. If it can't fetch the weather data at startup, there are some error messages saying that the configuration is incorrect and no weather data is available for a couple of minutes. This might lead to some problems if automation is depending on the weather, and I think met.no is even part of the default configuration of Home Assistant. This is certainly not a special case.

However, I've also figured that there might be a scenario where one doesn't want their Home Assistant server to have any connection to any network whatsoever. In this case, starting Home Assistant after network-online.target would probably mean that Home Assistant won't start at all.

Adding the target manually is an option and also what I'm doing right now, but it is rather hacky to NixOS standards; Newcomers will probably miss this. So, may I suggest adding an option to wait for network-online instead of network?

@fabaff
Copy link
Member

fabaff commented Apr 24, 2021

Home Assistant's PlatformNotReady should retrigger the integration setup if there is a network issue, e.g., no uplink. Integrations that do polling (e.g., most weather integration or others that are getting their data from third-party webservices) might have outdated data for a while, depending on their update interval (The default interval is 30 s but integration are allowed to override this. Thus, in a worst case scenario for several hours).

From my point of the startup should be as fast as possible as you want to be able to control your devices. But use cases are different 😉

@lukegb
Copy link
Contributor

lukegb commented May 1, 2021

It sounds like the conclusion is that we don't want to move forwards with the change in this form.

So, may I suggest adding an option to wait for network-online instead of network?

This doesn't sound like the worst idea in the world, but that's up to the HA module maintainers.

@mweinelt
Copy link
Member

mweinelt commented May 1, 2021

On my raspberry pi 4 the difference between network.target and network-online.target equates to 52 seconds, even if the DHCP lease is configured 7 seconds after network.target. With the value that home-assistant puts on having things local I'm strongly against defaulting to After=network-online.target.

Personally, I think this is what overwrites are for. Maybe add yours and your use case to the wiki?
Either way your automation should be able to gracefully act around network outages, wherever and whenever they might occur.

Adding the target manually is an option and also what I'm doing right now, but it is rather hacky to NixOS standards

I disagree, this is pretty common when your setup becomes opinionated.

So, may I suggest adding an option to wait for network-online instead of network?

Not really a fan, given that is so easy to override.

@Janonard
Copy link
Author

Janonard commented May 5, 2021

Given the arguments, I'll withdraw this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants