systemd-style readiness notification not compatible with sd_notify?

[aanderse]

the intention of the NOTIFY_SOCKET environment variable is to point to a communication socket the service can use to send a message back to the service manager - both systemd and finit implement this ✔

sd_notify doesn't work if NOTIFY_SOCKET is a string representation of an integer... i think in our case it should be a path

https://www.freedesktop.org/software/systemd/man/latest/sd_notify.html#Notes

These functions send a single datagram with the state string as payload to the socket referenced in the $NOTIFY_SOCKET environment variable. If the first character of $NOTIFY_SOCKET is "/" or "@", the string is understood as an AF_UNIX or Linux abstract namespace socket (respectively), and in both cases the datagram is accompanied by the process credentials of the sending service, using SCM_CREDENTIALS.

details here, here, and here, if needed...

i believe finit should adjust at least the following code, but i'm not exactly clear on the details: https://github.com/troglobit/finit/blob/d318aa00c34ed713a72912e547c46531f4943a70/src/service.c#L732-L733

any advice is greatly appreciated 🙇‍♂

[troglobit]

Well this was quite a bit of a surprise. I remember when I was researching this topic I was pretty sure the s6 and systemd mechanisms were similar, but I seem to have completely misunderstood it. Thank you for taking the time to report this!

The fix is to split up the logic for handling s6 and systemd notify services. For s6 I believe we're still OK, but for systemd we need a bigger change. I'll have a look at it during the next couple of days.

In a way this is a good opportunity, because I've considered adding support for optionally installing a <systemd/sd-daemon.h> compat header (and library) for systems that do not install libsystemd when systemd itself is not installed.

[aanderse]

I'll have a look at it during the next couple of days.

fantastic! thank you very much!

In a way this is a good opportunity, because I've considered adding support for optionally installing a <systemd/sd-daemon.h> compat header (and library) for systems that do not install libsystemd when systemd itself is not installed.

that is super cool! if you end up doing that i will definitely make use of it 👍

[troglobit]

Update: very slow progress on this unfortunately.

[troglobit]

Finally some progress. Unfortunately it took a while to find the right (re)design for this to cover all the bases. The branch will receive some further changes before being merged, so not yet quite ready for prime time.

[aanderse]

awesome! thank you very much for this 🙇‍♂

[troglobit]

@aanderse I've now refactored the initial sd_notify() support into a libsystemd replacement library, and done my best testing it. Would you mind having a look at it before merging to master? The code is on branch refactor-systemd-readiness and here's the current level of the API: libsystemd/sd-daemon.h.

[aanderse]

i took a quick look tonight but ran into a few roadblocks... i will try to commit some more time to this tomorrow night

can you please share what daemon(s) you tested with?

[troglobit]

Sorry, that's just it, I've not. The current implementation is just based on the open code from sd_notify(3), extended with a few other ones that were easy to build on and around it. The only daemon I've used is test/src/serv.c which runs in a few of the tests. I was hoping you might know a few suitable candidates.

[aanderse]

ah, in that case please leave this to me 💪 i will come back with a full report

[troglobit]

I had a chat with a colleague and the closest we come atm. is podman, but it also needs the journalling API, which needs coordination with the sysklogd project as well, so it's a stretch goal at this point.

[aanderse]

i was able to test a couple real services using notify:systemd with <!> (Type=notify in systemd) :+1:

i couldn't quickly find any software to test notify: systemd without <!> (Type=notify-reload in systemd) which reloads on SIGHUP... but then i found out systemd provides a nice simple c program which i was able to test successfully :+1:

so i can confirm this branch definitely works!

---

as far as libsystemd goes... i had less success:

• transmission-daemon uses cmake+pkg-config and didn't link properly when simply replaced
• mariadb had the same issue when simply replaced
• php-fpm has a version check against libsystemd and fails to build, though simply patching out the version check seems to work fine from some initial testing

i might not have time to get to the root of these linking issues soon but i think the fact that it works with at least one piece of software i tried means that this is good enough to ship and subsequent PRs can improve the situation

thank you very much for your work on this @troglobit! i really appreciate it

[troglobit]

Awesome, thank you for taking the time to verify! <3

---

Hmm, yeah I suspected as much. Depending on what they check for this replacement library may or may not work at all, and even though faking the version might work for some cases it's a bit sinister and could lead to other issues.

But you're right, this is probably good enough for now and we can document the state we leave it in for future explorers.

Cheers! 🙇‍♂

[aanderse]

awesome! thanks again @troglobit 🚀