Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sdexec: set KillMode=process SendSIGKILL=no for multi-user jobs #6402

Merged
merged 4 commits into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions src/common/libsdexec/start.c
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,26 @@ static int prop_add_bool (json_t *prop, const char *name, int val)
return 0;
}

// per systemd.syntax(7), boolean values are: 1|yes|true|on, 0|no|false|off
static bool is_true (const char *s)
{
if (streq (s, "1")
|| !strcasecmp (s, "yes")
|| !strcasecmp (s, "true")
|| !strcasecmp (s, "on"))
return true;
return false;
}
static bool is_false (const char *s)
{
if (streq (s, "0")
|| !strcasecmp (s, "no")
|| !strcasecmp (s, "false")
|| !strcasecmp (s, "off"))
return true;
return false;
}

static int prop_add_u32 (json_t *prop, const char *name, uint32_t val)
{
json_int_t i = val;
Expand Down Expand Up @@ -254,6 +274,17 @@ static int prop_add (json_t *prop, const char *name, const char *val)
}
free (bitmap);
}
else if (streq (name, "SendSIGKILL")) {
bool value;
if (is_false (val))
value = false;
else if (is_true (val))
value = true;
else
return -1;
if (prop_add_bool (prop, name, value) < 0)
return -1;
}
else {
if (prop_add_string (prop, name, val) < 0)
return -1;
Expand Down
14 changes: 14 additions & 0 deletions src/modules/job-exec/exec.c
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,20 @@ static int exec_init (struct jobinfo *job)
flux_log_error (job->h, "exec_init: flux_cmd_setenvf");
goto err;
}
/* The systemd user instance running as user flux is not privileged
* to signal guest processes, therefore only signal the IMP and
* never use SIGKILL. See flux-framework/flux-core#6399
*/
if (streq (service, "sdexec")) {
if (flux_cmd_setopt (cmd, "SDEXEC_PROP_KillMode", "process") < 0
|| flux_cmd_setopt (cmd,
"SDEXEC_PROP_SendSIGKILL",
"off") < 0) {
flux_log_error (job->h,
"Unable to set multiuser sdexec options");
return -1;
}
}
if (flux_cmd_argv_append (cmd, config_get_imp_path ()) < 0
|| flux_cmd_argv_append (cmd, "exec") < 0) {
flux_log_error (job->h, "exec_init: flux_cmd_argv_append");
Expand Down
2 changes: 0 additions & 2 deletions src/modules/sdexec/sdexec.c
Original file line number Diff line number Diff line change
Expand Up @@ -824,8 +824,6 @@ static void kill_continuation (flux_future_t *f, void *arg)
* only the pids of units started with sdexec.exec since the sdexec module
* was loaded. Since this sends an sdbus RPC, the response is handled in
* kill_continuation() when the sdbus response is received.
* N.B. in a typical system instance, job-exec would remotely execute
* flux-imp kill and this would not be used.
*/
static void kill_cb (flux_t *h,
flux_msg_handler_t *mh,
Expand Down
27 changes: 27 additions & 0 deletions t/t2409-sdexec.t
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,33 @@ test_expect_success 'sdexec can set unit Description property' '
t2409-desc.service >desc.out &&
test_cmp desc.exp desc.out
'
test_expect_success 'sdexec can set unit SendSIGKILL property to no' '
cat >sigkill.exp <<-EOT &&
SendSIGKILL=no
EOT
$sdexec -r 0 \
--setopt=SDEXEC_NAME="t2409-sigkill.service" \
--setopt=SDEXEC_PROP_SendSIGKILL=no \
$systemctl --user show --property SendSIGKILL \
t2409-sigkill.service >sigkill.out &&
test_cmp sigkill.exp sigkill.out
'
test_expect_success 'setting SendSIGKILL to an invalid value fails' '
test_must_fail $sdexec -r 0 --setopt=SDEXEC_PROP_SendSIGKILL=zzz \
$true 2>sigkill_badval.err &&
grep "error setting property" sigkill_badval.err
'
test_expect_success 'sdexec can set unit KillMode property to process' '
cat >killmode.exp <<-EOT &&
KillMode=process
EOT
$sdexec -r 0 \
--setopt=SDEXEC_NAME="t2409-killmode.service" \
--setopt=SDEXEC_PROP_KillMode=process \
$systemctl --user show --property KillMode \
t2409-killmode.service >killmode.out &&
test_cmp killmode.exp killmode.out
'
# Check that we can set resource control attributes on our transient units,
# but expect resource control testing to occur elsewhere.
# See also:
Expand Down
Loading