diff --git a/src/controller/include/cluster.h b/src/controller/include/cluster.h index 2090d9f9..977a6d8c 100644 --- a/src/controller/include/cluster.h +++ b/src/controller/include/cluster.h @@ -286,7 +286,8 @@ extern int on_job_postdone(cluster_t*, endpoint_t*, uint64_t); extern int on_job_ckpt(cluster_t*, endpoint_t*, xce_ckptinfo_t*); extern int notify(cluster_t*, endpoint_t*, xce_notify_data_t*); extern int check_worker_(xce_worker_t*); -extern int update_resval_list(cluster_t*, worker_t*, xce_resval_list_t*); +extern int update_resval_list(cluster_t*, worker_t*, xce_resval_list_t*, + xce_resval_list_t*); extern int do_jobcontrol_op(cluster_t*, endpoint_t*, xce_request_t*); extern int job_ctrl(cluster_t*, xce_jobctrl_t*); extern int update_job_rusages(cluster_t*, endpoint_t*, xce_request_t*); @@ -299,7 +300,8 @@ extern int get_ep_role(endpoint_t*); extern worker_t* get_ep_worker(cluster_t*, endpoint_t*); extern int check_worker_(xce_worker_t*); -extern int update_resval_list(cluster_t*, worker_t*, xce_resval_list_t*); +extern int update_resval_list(cluster_t*, worker_t*, xce_resval_list_t*, + xce_resval_list_t*); extern int do_jobcontrol_op(cluster_t*, endpoint_t*, xce_request_t*); extern int job_ctrl(cluster_t*, xce_jobctrl_t*); extern int do_queuecontrol_op(cluster_t*, endpoint_t*, xce_request_t*); diff --git a/src/controller/opupd.c b/src/controller/opupd.c index 77119e1b..6b46b03c 100644 --- a/src/controller/opupd.c +++ b/src/controller/opupd.c @@ -1,6 +1,7 @@ #include "config.h" #include +#include #include #include @@ -31,14 +32,30 @@ static int do_update_resvals(cluster_t* c, endpoint_t* ep, } else { xce_update_data_t* updatemsg = request->body.updatemsg; xce_resval_list_t* resvals = updatemsg->data.resvals; + xce_resval_list_t applied = { .count = 0, .vals = NULL }; + + /* bug-1583: the AFTER_EVENT listeners (scheduler push, + * apiserver store) must only hear about the values this + * controller actually accepted -- not the raw client list, + * which may name resources it rejected. `applied` borrows + * pointers into `resvals`; nothing changes ownership. */ + if ((resvals != NULL) && (resvals->count > 0)) { + applied.vals = (xce_resval_t**)calloc( + (size_t)resvals->count, sizeof(xce_resval_t*)); + } xce_run_hooks(&(c->hooksys), XCE_CONTROLLER_HOOK_RESOURCE, HOOK_BEFORE_EVENT, HOOK_DETAIL_RESOURCE_UPDATE, c, resvals); - rc = update_resval_list(c, w, resvals); - xce_run_hooks(&(c->hooksys), XCE_CONTROLLER_HOOK_RESOURCE, - HOOK_AFTER_EVENT, HOOK_DETAIL_RESOURCE_UPDATE, c, - resvals); + rc = update_resval_list(c, w, resvals, + (applied.vals != NULL) ? &applied + : NULL); + if (applied.count > 0) { + xce_run_hooks(&(c->hooksys), XCE_CONTROLLER_HOOK_RESOURCE, + HOOK_AFTER_EVENT, + HOOK_DETAIL_RESOURCE_UPDATE, c, &applied); + } + free(applied.vals); } } } diff --git a/src/controller/testsuite/test_worker_join.c b/src/controller/testsuite/test_worker_join.c index bce1b169..ec83767a 100644 --- a/src/controller/testsuite/test_worker_join.c +++ b/src/controller/testsuite/test_worker_join.c @@ -383,6 +383,47 @@ static void test_update_resvals_logs_actual_rc_not_stale_errno(void) printf("test_update_resvals_logs_actual_rc_not_stale_errno: PASS\n"); } +/* bug-1583: update_resval_list() must report which items it actually + * applied, so do_update_resvals() can hand ONLY those to the AFTER_EVENT + * resource hook (scheduler + apiserver). It used to pass the raw client + * list, so the apiserver stored resource names the controller rejected. */ +static void test_update_resval_list_reports_only_applied_items(void) +{ + setup_test_cluster(); + + hostattrs_t attrs; + memset(&attrs, 0, sizeof(attrs)); + attrs.hosttype = makeString("LINUX"); + attrs.hostmodel = makeString("x86"); + worker_t* w = make_test_worker("host-resval-applied", &attrs); + TEST_ASSERT_NOT_NULL(w); + + xce_val_t good_val, bad_val; + memset(&good_val, 0, sizeof(good_val)); + good_val.which = Value_u_fvalue; + good_val.u.fvalue = 4.0f; + bad_val = good_val; + + xce_resval_t good = { .name = "slots", .val = &good_val }; + xce_resval_t bad = { .name = "totally-fake-resource-xyz", + .val = &bad_val }; + xce_resval_t* items[2] = { &bad, &good }; + xce_resval_list_t in = { .count = 2, .vals = items }; + + xce_resval_t* out_items[2] = { NULL, NULL }; + xce_resval_list_t applied = { .count = -1, .vals = out_items }; + + TEST_ASSERT_EQUAL_INT(0, update_resval_list(&g_tc.c, w, &in, &applied)); + + TEST_ASSERT_EQUAL_INT_MESSAGE(1, applied.count, + "bug-1583: only the resource the controller accepted may be " + "reported as applied"); + TEST_ASSERT_TRUE(applied.vals[0] == &good); + TEST_ASSERT_EQUAL_FLOAT(4.0f, get_worker_res_fval(w, "slots")); + + teardown_test_cluster(); +} + /* ── Task 1.1: Tests for apply_join_state ──────────────────────────── */ /* @@ -1638,6 +1679,8 @@ void test_worker_join(void) RUN_TEST(test_update_resvals_logs_actual_rc_not_stale_errno); + RUN_TEST(test_update_resval_list_reports_only_applied_items); + /* Task 1.1: apply_join_state (shared status/resource update) */ RUN_TEST(test_apply_join_state_clears_unavail); RUN_TEST(test_apply_join_state_applies_hosttype_override); diff --git a/src/controller/worker.c b/src/controller/worker.c index 5ecc682a..8d2617b4 100644 --- a/src/controller/worker.c +++ b/src/controller/worker.c @@ -205,15 +205,24 @@ int update_resvals(cluster_t* c, worker_t* w, int nres, xce_resval_t* vals) return 0; } -int update_resval_list(cluster_t* c, worker_t* w, xce_resval_list_t* l) +/* bug-1583: `applied` (optional; its vals[] must hold l->count entries) gets + * borrowed pointers to just the items that were accepted and applied, so the + * caller can tell downstream consumers only about those. */ +int update_resval_list(cluster_t* c, worker_t* w, xce_resval_list_t* l, + xce_resval_list_t* applied) { int rc = 0; int i; + if (applied != NULL) + applied->count = 0; + for (i = 0; i < l->count; i++) { rc = update_resval(c, w, l->vals[i]); if (rc < 0) { log_update_resval_failure(l->vals[i]->name, rc); + } else if (applied != NULL) { + applied->vals[applied->count++] = l->vals[i]; } }