imp run: wait for cgroup and call sd_notify() #203
Conversation
src/imp/run.c
Outdated
| @@ -281,6 +281,7 @@ int imp_run_privileged (struct imp_state *imp, | |||
| struct kv *kv_env; | |||
| const char *name; | |||
| const cf_t *cf_run; | |||
| const char *notify_socket = getenv ("NOTIFY_SOCKET"); | |||
There was a problem hiding this comment.
Could instead add this variable to run_env_var_allowed() for consistency with FLUX_JOB_ID and FLUX_JOB_USERID.
If that isn't desirable, a comment here might help remind a reader why this variable is special (though admittedly at this point it's obvious)
There was a problem hiding this comment.
Hmm, i tried that originally and it didn't work for some reason and got myself a little confused in the process. Let me try again and see if I can run down what's happening.
There was a problem hiding this comment.
Oh, the privileged imp process has its environment cleared, and the environment curated by the unprivileged imp process is passed through to execve(). But the imp itself, whose environment remains empty, will be making the sd_notify() calls so this does need to be handled differently.
I'll add a comment.
mergify
bot
dismissed
grondo’s stale review
The merge-base changed after approval.
Problem: imp run doesn't wait for its cgroup to empty out, even when running as the main process of a systemd unit in the flux systemd instance. Add a call to cgroup_wait_for_empty (). Fixes flux-framework#202
Problem: NOTIFY_SOCKET is filtered out of imp run's environment, but it is needed to call sd_notify(). Ensure it passes through.
Problem: imp run should support running with type=notify when launched by sdexec as the main process of a systemd unit. Make the appropriate sd_notify(3) calls in imp run, similar to what was done for imp exec.
garlick
force-pushed
the
imp_run_cgroup
branch
from
f34471b to
1cbd846
Compare
|
Also need reapproval on this one :-) |
This prepares to let
imp runrun as the main process in a user systemd instanceIt is built on top of #201