Skip to content

Commit

Permalink
Merge pull request #6561 from grondo/issue#6560
Browse files Browse the repository at this point in the history
broker: ensure `parent-uri` and `parent-kvs-namespace` are only set for jobs
  • Loading branch information
mergify[bot] authored Jan 21, 2025
2 parents 482028e + fb6f976 commit d32d543
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 6 deletions.
31 changes: 26 additions & 5 deletions src/broker/broker.c
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ static int handle_event (broker_ctx_t *ctx, const flux_msg_t *msg);

static void init_attrs (attr_t *attrs, pid_t pid, struct flux_msg_cred *cred);

static void init_attrs_post_boot (attr_t *attrs);

static void init_attrs_starttime (attr_t *attrs, double starttime);

static int init_local_uri_attr (struct overlay *ov, attr_t *attrs);
Expand Down Expand Up @@ -365,6 +367,8 @@ int main (int argc, char *argv[])
}
}

init_attrs_post_boot (ctx.attrs);

ctx.rank = overlay_get_rank (ctx.overlay);
ctx.size = overlay_get_size (ctx.overlay);

Expand Down Expand Up @@ -601,15 +605,27 @@ static void init_attrs_starttime (attr_t *attrs, double starttime)
log_err_exit ("error setting broker.starttime attribute");
}

static void init_attrs (attr_t *attrs, pid_t pid, struct flux_msg_cred *cred)
/* Initialize attributes after bootstrap since these attributes may depend
* on whether this instance is a job or not.
*/
static void init_attrs_post_boot (attr_t *attrs)
{
const char *val;
bool instance_is_job;

/* Use the jobid attribute instead of FLUX_JOB_ID in the current
* environment to determine if this instance was run as a job. This
* is because the jobid attribute is only set by PMI, whereas
* FLUX_JOB_ID could leak from the calling environment, e.g.
* `flux run flux start --test-size=2`.
*/
instance_is_job = attr_get (attrs, "jobid", NULL, NULL) == 0;

/* Set the parent-uri attribute IFF this instance was run as a job
* in the enclosing instance. "parent" in this context reflects
* a hierarchy of resource allocation.
*/
if (getenv ("FLUX_JOB_ID"))
if (instance_is_job)
val = getenv ("FLUX_URI");
else
val = NULL;
Expand All @@ -622,11 +638,16 @@ static void init_attrs (attr_t *attrs, pid_t pid, struct flux_msg_cred *cred)
*/
unsetenv ("FLUX_PROXY_REMOTE");

val = getenv ("FLUX_KVS_NAMESPACE");
if (attr_add (attrs, "parent-kvs-namespace", val, ATTR_IMMUTABLE) < 0)
log_err_exit ("setattr parent-kvs-namespace");
if (instance_is_job) {
val = getenv ("FLUX_KVS_NAMESPACE");
if (attr_add (attrs, "parent-kvs-namespace", val, ATTR_IMMUTABLE) < 0)
log_err_exit ("setattr parent-kvs-namespace");
}
unsetenv ("FLUX_KVS_NAMESPACE");
}

static void init_attrs (attr_t *attrs, pid_t pid, struct flux_msg_cred *cred)
{
init_attrs_broker_pid (attrs, pid);
init_attrs_rc_paths (attrs);
init_attrs_shell_paths (attrs);
Expand Down
12 changes: 11 additions & 1 deletion t/t0001-basic.t
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,17 @@ test_expect_success 'broker broker.pid attribute is readable' '
test -n "$BROKERPID" &&
test "$BROKERPID" -eq "$BROKERPID"
'

test_expect_success 'broker sets parent-uri attribute only for jobs' '
flux start flux run flux start flux getattr parent-uri &&
test_must_fail \
flux start flux run flux start -s1 flux getattr parent-uri
'
test_expect_success 'broker sets parent-kvs-namespace attribute only for jobs' '
flux start flux run flux start flux getattr parent-kvs-namespace &&
test_must_fail \
flux start flux run \
flux start -s1 flux getattr parent-kvs-namespace
'
test_expect_success 'local-uri override works' '
sockdir=$(mktemp -d) &&
newsock=local://$sockdir/meep &&
Expand Down

0 comments on commit d32d543

Please sign in to comment.