zigbee2mqtt
zigbee2mqtt copied to clipboard
Notify systemd for start, stop, watchdog
As already suggested in #20413, this PR implements Type=notify. This makes zigbee2mqtt send a notification to systemd when it has finished starting up and when it starts shutting down. My use case for example was to add an ExecStartPost= line to the service file that changes the ACL of the web interface socket. For this I needed a reliable way to know when this socket has actually been created.
It is also possible to specify a watchdog timeout in the service file; in this case zigbee2mqtt will regularly notify systemd that it's main loop is still alive, and systemd will restart it if it fails to do so for too long.
One drawback is that requiring sd-notify adds a dependency on the libsystemd-dev deb package. So I've made the sd-notify dependency optional in an attempt to not break builds on other platforms. This might need some refinement or updates to the install instructions. The deb package might also need to be added to the Docker images.
Could you take a look at the failing test and also make a PR for the docs?
The "failing tests" were in fact just a failing coverage check because the CI tests run without optional dependencies. I now mock the module in the controller tests so the code is covered. I haven't added an actual test case since the application logic is rather trivial and testing end-to-end by running under systemd is the only way to be sure it really works. Working on the docs update now
Documentation PR added: Koenkk/zigbee2mqtt.io#2475
Hi, I'm glad to see the progress on this task!
I just wanted to share some thoughts about the dependency: It should be possible to avoid the dependency at all by using exec with the right parameters. In fact, there is an executable systemd-notify that accepts parameters for userid, pid and status.
The documentation is here
Note, the pid parameter is important because the exec will be launched in a new process and the pid to send is the pid of the running node.
There is more information here. I wonder why for the solution 2 the article don't mention the pid parameter (maybe the pid parameter was added afterwards to resolve the race condition)
Hi, I'm glad to see the progress on this task!
I just wanted to share some thoughts about the dependency: It should be possible to avoid the dependency at all by using
execwith the right parameters. In fact, there is an executablesystemd-notifythat accepts parameters foruserid,pidandstatus.The documentation is here
Note, the
pidparameter is important because theexecwill be launched in a new process and thepidto send is thepidof the runningnode.There is more information here. I wonder why for the solution 2 the article don't mention the
pidparameter (maybe thepidparameter was added afterwards to resolve the race condition)
Hi! I was aware that systemd-notify exists, but as the documentation says, it is more intended for (shell) scripts. In a service that is written in a proper programming language, it would be somewhat wasteful to start a process when all one really needs to do is write a message to a socket. Especially if it happens every few seconds when the watchdog is used.
Now node is a bit of an in-between case; it can write to unix sockets directly just fine, but they dropped support for datagram sockets -.- https://groups.google.com/g/nodejs/c/iCzhcuxGP1I so the sd-notify module that uses the libsystemd C library for that seems to be the next-best solution.
Removed the optional-require dependency and used the try/catch suggested on https://www.npmjs.com/package/optional-require instead. Unfortunately the let sdNotify; causes some implicit-any warnings and eslint complains about the empty catch, so the result became a bit ugly. If someone knows better, feel free to make suggestions...
Thanks!
Great work!
This doesn't work in the docker images published on docker.io and I think this is because the Dockerfile's npm ci specifies --no-optional. Would you accept a PR to remove that?
I've created a PR to update the documentation
https://github.com/Koenkk/zigbee2mqtt.io/pull/2558
In order for this to work, the user needs to install the sd-notify requirements
I was referring to the fact the Docker images built by this repo's Dockerfile don't currently include libsystemd so the sd_notify stuff doesn't work. Sure I could build my own Docker image but I'd rather just get it into this repo if that is OK. Would any committers accept a PR to remove --no-optional?
I should say that my use case here is that I'm trying to run this (and mosquitto which also doesn't have it enabled in their Docker images) in Podman via systemd and whenever I configured it wrong not having sd_notify resulted in less than ergonomic debugging.