search

eclipse / kuksa.val

kuksa.val
Apache License 2.0
89 stars 52 forks source link

Systemd notify support #743

Closed SebastianSchildt closed 4 months ago

SebastianSchildt commented 4 months ago

Experimental support for systemd notify

From the discussion in chat

image

This is not perfect yet, so lets discuss here what is needed for "good enough" level

  1. It uses https://crates.io/crates/sd-notify crate. The way that crate is written I assume we do not even need to hide it behind a feature flag: On a non-systemd (maybe even non Linux) it does no harm, as it determines systemd availability by just checking existence of a file. On a a systemd system, even when starting manually on the commandline, a notify doesn't hurt, systemd will just ignore it (I assume that is probably how all notify-enabled dameons work)

  2. When to signal ready state: I put the code in the last reachable statement in main before the GRPC socket is up. So strictly speaking there is still a race conditions. As at this point all parsing an dinitlaization is done, I assume for practical reasosn it would solve your issue @g-scott-murray However anyone knows a "better" place to put this code, i.e. any callback/event when GRPC server is up (strictyl speaking VISS also might not be ready yet, if started in a thread before). I did not find anything obvious. Do you have an idea @argerus , what might work preferably without putting too much infrastructure in?

argerus commented 4 months ago

Looks good to me, but I haven't tested it. Would probably still make sense to put it behind a feature flag in order to not add a hard dependency (think compiling for QNX etc).

A possible improvement could be to bind the socket before notifying systemd, e.g.

    let socket_addr = addr.into();
    let listener = TcpListener::bind(socket_addr).await?;

    // Notify systemd here <---

    serve_with_incoming_shutdown(listener, ...)
SebastianSchildt commented 4 months ago

I moved the notify, so it is fired after bindng the socket.

I did not create an "explicit" feature flag, instead I used an existing flag, and now only build this when building for a Linux OS. I verified that the code is not included when building for mac OS.

if running on a Linux without systemd, this has no negative effect (the code detects whether systemd is in use or not). As the create is very small and includes zero dependencies, I figured not much would be gained by hiding it behind an explicit user-configurable flag.

-> Works for me, ready to review/merge fmpov