imp exec: call sd_notify()#201
Conversation
|
We'll have to make libsystemd optional or add it to all the fluxrm/testenv images. It is optional in flux-core and only installed in the el8 and el9 images at this point. |
|
Bummer. The code proposed in flux-framework/flux-core#6666 unconditionally uses Type=notify to run the unit. I don't know how we would test whether or not a given build of flux-security includes systemd support. If we get it wrong, systemd kills the unit during startup because it would never get notified of STARTED=1. Oh heh, there is a standalone version in the older copy of the sd_notify(3) man page. Maybe we could just add that to flux-security and call it good |
Problem: it's a pain to add a dependency on libsystemd-dev just to call sd_notify(). Add a standalone implementation of sd_notify() from the sd_notify(3) man page, modified slightly for Flux.
|
Here's with the standalone implementation. |
garlick
force-pushed
the
sd_notify
branch
2 times, most recently
from
9a953dc to
eaaa498
Compare
garlick
changed the title
grondo
left a comment
grondo
left a comment
There was a problem hiding this comment.
LGTM! One question inline. Feel free to ignore.
src/imp/exec/exec.c
Outdated
| sd_notify (0, "READY=1"); | ||
| sd_notify (0, "STATUS=IMP is monitoring child and forwarding signals"); |
There was a problem hiding this comment.
Debatable, but should the IMP log a warning if these calls fail?
There was a problem hiding this comment.
Yes that sounds like a good idea.
Problem: the IMP should support running with type=notify when launched by sdexec as the main process of a systemd unit. Call sd_notify(3) at various points to support this. The calls should be no-ops if running another way. READY=1 After shell process has been spawned. This transitions the unit to running state. STOPPING=1 After shell process has terminated. This transitions the unit to deactivating state. STATUS= Provide human readable status at those two points, including the exit status of the shell in the latter.
|
Thanks! I pushed that change (after sanity testing) and I'll set MWP. |
|
Oh that change made your review "stale" @grondo. Would you mind revisiting this? Thx. |
2 participants
Problem: sdexec gets no notification if the shell exits but the IMP cannot exit because the cgroup is not empty.
When run with Type=notify, this casues the unit to enter deactivating state, with visibility to sdexec.
It should be harmless when the IMP is not run with Type=notify.