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.

Sign up for GitHub

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

broker: ensure parent-uri and parent-kvs-namespace are only set for jobs #6561

Merged
merged 4 commits into from
Jan 21, 2025
Merged

broker: ensure parent-uri and parent-kvs-namespace are only set for jobs #6561

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c64c950
broker: move initialization of some attrs to after bootstrap
grondo Jan 17, 2025
8455bf1
broker: only set parent-uri if instance is a job
grondo Jan 17, 2025
5cae75f
broker: only set parent-kvs-namespace if instance is a job
grondo Jan 17, 2025
fb6f976
testsuite: ensure parent-* attributes are set only for jobs
grondo Jan 17, 2025
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: 26 additions & 5 deletions src/broker/broker.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@

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 @@
}
}

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 @@
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 @@
*/
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");

Check warning on line 644 in src/broker/broker.c

View check run for this annotation

Codecov / codecov/patch

src/broker/broker.c#L644 

Added line #L644 was not covered by tests
}
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
View file
Open in desktop
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
Loading