From 3932b25e108de5e85d754c98dd2d7af0f1dd4a60 Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Mon, 10 Aug 2026 13:21:39 -0700 Subject: [PATCH 01/10] job-manager: fix comment block alignment Problem: The multi-line comments on the 'requires' and 'parent' fields of struct queue are indented one space past the opening slash, so the comments do not properly align. Align the continuation stars with the opening comment. Assisted-by: Claude:Opus-4.8 --- src/modules/job-manager/queues.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/modules/job-manager/queues.c b/src/modules/job-manager/queues.c index 85e08b9ecc48..deba24c3ae9d 100644 --- a/src/modules/job-manager/queues.c +++ b/src/modules/job-manager/queues.c @@ -45,16 +45,16 @@ struct queue { bool is_started_sticky; /* tracks is_started unless --nocheckpoint */ char *stop_reason; /* reason if stopped (optionally set) */ json_t *requires; /* required properties array (own; a - * virtual queue's is always NULL - see - * queue_root() for the effective value) - */ + * virtual queue's is always NULL - see + * queue_root() for the effective value) + */ struct queue *parent; /* resolved parent queue (RFC 33 virtual - * queues), or NULL if not virtual. Not - * owned; borrowed from the same queues - * table. Inheritance is one level - * (validated elsewhere) so this is - * never itself virtual. - */ + * queues), or NULL if not virtual. Not + * owned; borrowed from the same queues + * table. Inheritance is one level + * (validated elsewhere) so this is + * never itself virtual. + */ struct queues *queues; /* back-pointer for notify */ }; From c522f6401f1bdf9dd36411325ebe4328d5f75d8f Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Sat, 8 Aug 2026 13:43:50 -0700 Subject: [PATCH 02/10] job-manager: order queues by config declaration order Problem: The queue-list RPC returns queue names in arbitrary hash order, so tools cannot present queues reproducibly, and there is no ordered structure to build a richer response on. Maintain a zlistx of named queues alongside the hash, re-sequenced to config declaration order at the end of each reconfigure, and encode the name list from it. The list holds borrowed pointers with per-queue handles for O(1) removal, and is structured so future runtime queues can be appended after the static queues. Assisted-by: Claude:Opus-4.8 --- src/modules/job-manager/queues.c | 86 +++++++++++++++++++++++++++++--- 1 file changed, 78 insertions(+), 8 deletions(-) diff --git a/src/modules/job-manager/queues.c b/src/modules/job-manager/queues.c index deba24c3ae9d..aff0ae2748ca 100644 --- a/src/modules/job-manager/queues.c +++ b/src/modules/job-manager/queues.c @@ -55,12 +55,20 @@ struct queue { * (validated elsewhere) so this is * never itself virtual. */ + void *order_handle; /* handle into queues->order for O(1) + * removal; NULL for the anon queue + */ struct queues *queues; /* back-pointer for notify */ }; struct queues { struct queue *anon; /* live when named == NULL */ zhashx_t *named; /* non-NULL selects named mode */ + zlistx_t *order; /* named queues in config declaration + * order. Holds borrowed struct queue + * pointers (the named hash owns them; + * NULL destructor). + */ bool restoring; /* suppress notify during queues_restore */ queues_change_f notify_cb; void *notify_arg; @@ -82,6 +90,8 @@ static void queue_free (struct queue *q) { if (q) { int saved_errno = errno; + if (q->queues && q->order_handle) + zlistx_delete (q->queues->order, q->order_handle); json_decref (q->requires); free (q->name); free (q->disable_reason); @@ -100,6 +110,18 @@ static void queue_destructor (void **item) } } +/* zlistx destructor for the order list. The list borrows queue pointers + * so do not free the queue, just reset the order_handle to NULL to + * protect against use-after-free. + */ +static void order_destructor (void **item) +{ + if (item && *item) { + struct queue *q = *item; + q->order_handle = NULL; + } +} + /* Extract the "parent" key from a queue's own config entry 'entry' (may * be NULL). On success, '*namep' is set to the borrowed name string, * or NULL if 'entry' has no "parent" key. Returns -1 if "parent" is @@ -278,13 +300,14 @@ struct queues *queues_create (void) { struct queues *queues; - if (!(queues = calloc (1, sizeof (*queues)))) - return NULL; /* Start with anonymous queue: enabled + started */ - if (!(queues->anon = queue_alloc (queues, NULL, NULL))) { - free (queues); + if (!(queues = calloc (1, sizeof (*queues))) + || !(queues->order = zlistx_new ()) + || !(queues->anon = queue_alloc (queues, NULL, NULL))) { + queues_destroy (queues); return NULL; } + zlistx_set_destructor (queues->order, order_destructor); return queues; } @@ -296,6 +319,7 @@ void queues_destroy (struct queues *queues) zhashx_destroy (&queues->named); else queue_free (queues->anon); + zlistx_destroy (&queues->order); free (queues); errno = saved_errno; } @@ -429,6 +453,7 @@ static int remove_all_named_queues (struct queues *queues) qname = zlistx_next (names); } zlistx_destroy (&names); + /* queue_free() removes each queue from the order list as it goes. */ zhashx_destroy (&queues->named); return 0; } @@ -484,10 +509,13 @@ static struct queue *queue_add_internal (struct queues *queues, * mode the new hash, before mutating any collection state. The anon * queue is not torn down until the new queue is successfully in place, * so an OOM failure here leaves the prior state (and its observers) - * intact. + * intact. Append new queue to order list. */ - if (!(q = queue_alloc (queues, name, config))) + if (!(q = queue_alloc (queues, name, config)) + || !(q->order_handle = zlistx_add_end (queues->order, q))) { + errno = ENOMEM; goto error; + } if (!queues->named) { if (!(named = zhashx_new ()) || zhashx_insert (named, name, q) < 0) { @@ -731,6 +759,35 @@ static int queues_config_validate (json_t *config, flux_error_t *error) return 0; } +/* Re-establish the canonical order of the named queues to match config + * declaration order after a configure. The purge clears every + * order_handle (via order_destructor), so a queue left un-re-added on OOM + * is safe. + * + * Returns -1 with errno set on failure. + */ +static int queues_reorder (struct queues *queues, json_t *config) +{ + const char *name; + json_t *value; + + zlistx_purge (queues->order); + + if (!queues->named) + return 0; + + json_object_foreach (config, name, value) { + struct queue *q; + if ((q = zhashx_lookup (queues->named, name))) { + if (!(q->order_handle = zlistx_add_end (queues->order, q))) { + errno = ENOMEM; + return -1; + } + } + } + return 0; +} + int queues_configure (struct queues *queues, json_t *config, flux_error_t *error) @@ -820,6 +877,15 @@ int queues_configure (struct queues *queues, return -1; } } + + /* Re-sequence the order list to match config declaration order. + * Every named queue is present in 'config' at this point, so each + * queue's order_handle is refreshed (no stale handles remain). + */ + if (queues_reorder (queues, config) < 0) { + errprintf (error, "failed to order queues: %s", strerror (errno)); + return -1; + } } else { /* No named queues configured: transition to anon mode @@ -917,7 +983,11 @@ json_t *queues_list_encode (struct queues *queues) if (!(a = json_array ())) goto error; if (queues->named) { - struct queue *q = zhashx_first (queues->named); + /* Iterate the order list, not the hash, so names are returned in + * canonical (config declaration) order rather than arbitrary hash + * order. + */ + struct queue *q = zlistx_first (queues->order); while (q) { json_t *o; if (!(o = json_string (q->name)) @@ -925,7 +995,7 @@ json_t *queues_list_encode (struct queues *queues) /* jansson decrefs the new object on failure */ goto error; } - q = zhashx_next (queues->named); + q = zlistx_next (queues->order); } } return a; From 37295c46c5ae49aaf746fca32b532bd828081956 Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Sat, 8 Aug 2026 13:45:30 -0700 Subject: [PATCH 03/10] testsuite: cover queue ordering across config reloads Problem: The new config-declaration ordering of the queue name list is not exercised by the unit tests, so a regression in the reorder logic would go unnoticed. Add test_list_order() covering initial declaration order and reloads that reorder, add, remove, combine all three, drop to anon mode, and re-enter named mode. Assisted-by: Claude:Opus-4.8 --- src/modules/job-manager/test/queues.c | 192 ++++++++++++++++++++++++++ 1 file changed, 192 insertions(+) diff --git a/src/modules/job-manager/test/queues.c b/src/modules/job-manager/test/queues.c index 7c50e19bbba6..a14759512268 100644 --- a/src/modules/job-manager/test/queues.c +++ b/src/modules/job-manager/test/queues.c @@ -1606,6 +1606,195 @@ static void test_list_encode (void) queues_destroy (qs); } +/* Return true if queues_list_encode() yields exactly the names in + * 'expect' (a NULL-terminated array) in that order. + */ +static bool encoded_order_is (struct queues *qs, const char *expect[]) +{ + json_t *a; + bool match = true; + size_t i; + json_t *v; + + if (!(a = queues_list_encode (qs))) + return false; + json_array_foreach (a, i, v) { + const char *s = json_string_value (v); + if (!expect[i] || !s || !streq (s, expect[i])) { + match = false; + break; + } + } + /* also require the array length to match the expected count */ + if (match) { + size_t n = 0; + while (expect[n]) + n++; + if (json_array_size (a) != n) + match = false; + } + json_decref (a); + return match; +} + +/* Configure 'qs' from a NULL-terminated array of queue names, each with + * an empty config table, preserving declaration order. + */ +static void configure_names (struct queues *qs, const char *names[]) +{ + json_t *config; + flux_error_t error; + + if (!(config = json_object ())) + BAIL_OUT ("json_object failed"); + for (int i = 0; names[i]; i++) { + if (json_object_set_new (config, names[i], json_object ()) < 0) + BAIL_OUT ("json_object_set_new failed"); + } + if (queues_configure (qs, config, &error) < 0) + BAIL_OUT ("queues_configure failed: %s", error.text); + json_decref (config); +} + +/* Exercise ordering of the encoded name list across config reloads: + * initial declaration order, pure reorder, add, remove, combined + * add+remove+reorder, and the transition back to anon mode. + */ +static void test_list_order (void) +{ + struct queues *qs; + + qs = queues_create (); + if (!qs) + BAIL_OUT ("queues_create failed"); + + /* initial load: declaration order is preserved */ + configure_names (qs, (const char *[]){"a", "b", "c", NULL}); + ok (encoded_order_is (qs, (const char *[]){"a", "b", "c", NULL}), + "initial load lists queues in declaration order"); + + /* pure reorder (same set, different order) is reflected */ + configure_names (qs, (const char *[]){"c", "a", "b", NULL}); + ok (encoded_order_is (qs, (const char *[]){"c", "a", "b", NULL}), + "reorder reload re-sequences the encoded name list"); + + /* add a queue: appears in its config position */ + configure_names (qs, (const char *[]){"c", "a", "z", "b", NULL}); + ok (encoded_order_is (qs, (const char *[]){"c", "a", "z", "b", NULL}), + "added queue appears in config declaration order"); + + /* remove a queue: survivors keep config order */ + configure_names (qs, (const char *[]){"c", "z", "b", NULL}); + ok (encoded_order_is (qs, (const char *[]){"c", "z", "b", NULL}), + "removed queue drops out, survivors keep config order"); + + /* combined add + remove + reorder in one reload */ + configure_names (qs, (const char *[]){"b", "q", "c", NULL}); + ok (encoded_order_is (qs, (const char *[]){"b", "q", "c", NULL}), + "combined add/remove/reorder reload matches config order"); + + /* transition to anon mode: empty encoded list */ + { + flux_error_t error; + if (queues_configure (qs, NULL, &error) < 0) + BAIL_OUT ("queues_configure to anon failed"); + } + ok (encoded_order_is (qs, (const char *[]){NULL}), + "transition to anon mode yields empty encoded list"); + + /* back to named mode: order list rebuilt cleanly from config */ + configure_names (qs, (const char *[]){"x", "y", NULL}); + ok (encoded_order_is (qs, (const char *[]){"x", "y", NULL}), + "re-entering named mode rebuilds order from config"); + + queues_destroy (qs); +} + +/* The queues_add()/queues_remove() primitives maintain the encoded order: + * a standalone add appends after existing queues, and a remove drops its + * entry while the rest keep their order. (These are not yet driven by any + * external caller, but the order list must stay correct for when they + * are.) + */ +static void test_add_remove_order (void) +{ + struct queues *qs; + flux_error_t error; + + qs = queues_create (); + if (!qs) + BAIL_OUT ("queues_create failed"); + + /* first add switches from anon to named mode */ + if (!queues_add (qs, "a", NULL, &error)) + BAIL_OUT ("queues_add a failed: %s", error.text); + if (!queues_add (qs, "b", NULL, &error)) + BAIL_OUT ("queues_add b failed: %s", error.text); + if (!queues_add (qs, "c", NULL, &error)) + BAIL_OUT ("queues_add c failed: %s", error.text); + ok (encoded_order_is (qs, (const char *[]){"a", "b", "c", NULL}), + "queues_add appends in add order"); + + /* remove from the middle: survivors keep their order */ + if (queues_remove (qs, "b", &error) < 0) + BAIL_OUT ("queues_remove b failed: %s", error.text); + ok (encoded_order_is (qs, (const char *[]){"a", "c", NULL}), + "queues_remove drops its entry, survivors keep order"); + + /* add after a remove appends at the end */ + if (!queues_add (qs, "d", NULL, &error)) + BAIL_OUT ("queues_add d failed: %s", error.text); + ok (encoded_order_is (qs, (const char *[]){"a", "c", "d", NULL}), + "queues_add after a remove appends at the end"); + + /* remove the first queue */ + if (queues_remove (qs, "a", &error) < 0) + BAIL_OUT ("queues_remove a failed: %s", error.text); + ok (encoded_order_is (qs, (const char *[]){"c", "d", NULL}), + "removing the first queue leaves the rest in order"); + + queues_destroy (qs); +} + +/* A failed queues_configure() must leave the previous configuration + * fully intact - including the encoded order, which the reorder step + * only reaches after all validation has passed. + */ +static void test_configure_failure_preserves_order (void) +{ + struct queues *qs; + flux_error_t error; + json_t *bad; + + qs = queues_create (); + if (!qs) + BAIL_OUT ("queues_create failed"); + + configure_names (qs, (const char *[]){"a", "b", "c", NULL}); + ok (encoded_order_is (qs, (const char *[]){"a", "b", "c", NULL}), + "initial order established"); + + /* A new config that reorders and adds an invalid (non-object) entry + * must be rejected whole, leaving the old order untouched. + */ + bad = json_pack ("{s:{} s:{} s:i s:{}}", + "c", + "a", + "bad", 5, + "b"); + if (!bad) + BAIL_OUT ("json_pack failed"); + errno = 0; + ok (queues_configure (qs, bad, &error) < 0 && errno == EINVAL, + "configure with an invalid entry fails with EINVAL"); + json_decref (bad); + + ok (encoded_order_is (qs, (const char *[]){"a", "b", "c", NULL}), + "failed configure leaves the previous order intact"); + + queues_destroy (qs); +} + /* ---------- virtual queue (RFC 33) tests -------------------------------- */ static void test_vqueue_configure (void) @@ -2324,6 +2513,9 @@ int main (int argc, char *argv[]) test_list_names (); test_status_encode (); test_list_encode (); + test_list_order (); + test_add_remove_order (); + test_configure_failure_preserves_order (); test_vqueue_configure (); test_vqueue_reparent_on_reload (); From d0a89100b170c0d75b228e0263c9f12f1704f503 Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Sat, 8 Aug 2026 13:47:43 -0700 Subject: [PATCH 04/10] job-manager: add effective queue config to queue-list response Problem: Components that need a queue's effective configuration re-read the [queues] table and re-derive inherited fields client-side, so the job-manager is not the authority on queue configuration. Add a "conf" object to the queue-list response carrying each queue's effective configuration ({name, requires, parent}) in declaration order, so consumers can stop re-deriving it. A virtual queue's requires is inherited from its parent. The conf object is an object, not a bare array, leaving room for future global fields. Assisted-by: Claude:Opus-4.8 --- src/modules/job-manager/queue.c | 10 ++-- src/modules/job-manager/queues.c | 91 ++++++++++++++++++++++++++++++++ src/modules/job-manager/queues.h | 11 ++++ 3 files changed, 107 insertions(+), 5 deletions(-) diff --git a/src/modules/job-manager/queue.c b/src/modules/job-manager/queue.c index a1db4e49a5da..d74bd83dc52f 100644 --- a/src/modules/job-manager/queue.c +++ b/src/modules/job-manager/queue.c @@ -144,20 +144,20 @@ static void queue_list_cb (flux_t *h, void *arg) { struct queue_ctx *qctx = arg; - json_t *a = NULL; + json_t *resp = NULL; if (flux_request_decode (msg, NULL, NULL) < 0) goto error; - if (!(a = queues_list_encode (qctx->queues))) + if (!(resp = queues_list_response (qctx->queues))) goto error; - if (flux_respond_pack (h, msg, "{s:O}", "queues", a) < 0) + if (flux_respond_pack (h, msg, "O", resp) < 0) flux_log_error (h, "error responding to job-manager.queue-list"); - json_decref (a); + json_decref (resp); return; error: if (flux_respond_error (h, msg, errno, NULL) < 0) flux_log_error (h, "error responding to job-manager.queue-list"); - json_decref (a); + json_decref (resp); } static void queue_status_cb (flux_t *h, diff --git a/src/modules/job-manager/queues.c b/src/modules/job-manager/queues.c index aff0ae2748ca..fafa1cb54635 100644 --- a/src/modules/job-manager/queues.c +++ b/src/modules/job-manager/queues.c @@ -1005,6 +1005,97 @@ json_t *queues_list_encode (struct queues *queues) return NULL; } +/* Encode one queue's effective configuration for the conf object: + * {"name":s, "requires"?:[...], "parent"?:s}. 'requires' is the + * effective required-properties array (a virtual queue inherits its + * parent's - see queue_requires()), omitted when the queue has none. + * 'parent' is present only for a virtual queue (RFC 33). + */ +static json_t *queue_conf_encode (struct queue *q) +{ + json_t *o; + json_t *requires; + + if (!(o = json_pack ("{s:s}", "name", q->name))) + goto nomem; + if ((requires = queue_requires (q))) { + if (json_object_set (o, "requires", requires) < 0) + goto nomem; + } + if (queue_is_virtual (q)) { + if (set_string (o, "parent", queue_name (queue_parent (q))) < 0) + goto error; + } + return o; +nomem: + errno = ENOMEM; +error: + ERRNO_SAFE_WRAP (json_decref, o); + return NULL; +} + +/* Encode the effective queue configuration object returned by the + * queue-list RPC: {"queues":[{name,requires?,parent?}, ...]} in canonical + * (config declaration) order. The anonymous queue is omitted (empty + * array in anon mode). Future global fields (default queue, merged + * policy) attach here as siblings of "queues". + */ +static json_t *queues_conf_encode (struct queues *queues) +{ + json_t *conf; + json_t *a; + + if (!(a = json_array ())) + goto nomem; + if (queues->named) { + struct queue *q = zlistx_first (queues->order); + while (q) { + json_t *o; + if (!(o = queue_conf_encode (q)) + || json_array_append_new (a, o) < 0) { + /* jansson decrefs `o` on failure */ + goto error; + } + q = zlistx_next (queues->order); + } + } + if (!(conf = json_pack ("{s:o}", "queues", a))) { + goto nomem; + } + return conf; +nomem: + errno = ENOMEM; +error: + ERRNO_SAFE_WRAP (json_decref, a); + return NULL; +} + +/* Assemble the full queue-list RPC response: + * {"queues":[names...], "conf":{"queues":[{name,requires?,parent?}...]}} + * The "queues" name array is retained for backwards compatibility; "conf" + * carries the effective per-queue configuration. Returns a new reference + * the caller must decref. + */ +json_t *queues_list_response (struct queues *queues) +{ + json_t *names = NULL; + json_t *conf = NULL; + json_t *resp; + + if (!(names = queues_list_encode (queues)) + || !(conf = queues_conf_encode (queues))) + goto error; + if (!(resp = json_pack ("{s:o s:o}", "queues", names, "conf", conf))) + goto nomem; + return resp; +nomem: + errno = ENOMEM; +error: + ERRNO_SAFE_WRAP (json_decref, names); + ERRNO_SAFE_WRAP (json_decref, conf); + return NULL; +} + static int save_one (json_t *a, struct queue *q) { json_t *entry; diff --git a/src/modules/job-manager/queues.h b/src/modules/job-manager/queues.h index 2bdb39fec823..de08dddbff42 100644 --- a/src/modules/job-manager/queues.h +++ b/src/modules/job-manager/queues.h @@ -124,6 +124,17 @@ int queues_restore (struct queues *queues, int version, json_t *o); json_t *queue_status_encode (struct queue *q, bool sched_ready); json_t *queues_list_encode (struct queues *queues); +/* Assemble the full job-manager.queue-list RPC response: + * {"queues":[names...], + * "conf":{"queues":[{"name":s,"requires"?:[...],"parent"?:s}, ...]}} + * Both parts are in canonical (config declaration) order. The "queues" + * name array is retained for backwards compatibility; "conf" carries the + * effective per-queue configuration (a virtual queue's 'requires' is + * inherited from its parent). Returns a new reference the caller must + * destroy, or NULL with errno set on error. + */ +json_t *queues_list_response (struct queues *queues); + /* Per-queue accessors * If q == NULL, assume anonymous queue. */ From 144758e6a3dadd0334b3a3b4c1ac17bfce8736c9 Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Sat, 8 Aug 2026 13:51:24 -0700 Subject: [PATCH 05/10] testsuite: cover the queue-list conf object Problem: The new conf object in the queue-list response, including its ordering and virtual-queue requires inheritance, has no test coverage. Add a queues_list_response() unit test asserting the conf shape, declaration order, and effective requires/parent, and extend the t0034-queuelist Python test to check the conf object over the wire, including a reconfigure that re-sequences queues. Assisted-by: Claude:Opus-4.8 --- src/modules/job-manager/test/queues.c | 94 +++++++++++++++++++++++++++ t/python/t0034-queuelist.py | 53 ++++++++++++++- 2 files changed, 146 insertions(+), 1 deletion(-) diff --git a/src/modules/job-manager/test/queues.c b/src/modules/job-manager/test/queues.c index a14759512268..234165bbddb8 100644 --- a/src/modules/job-manager/test/queues.c +++ b/src/modules/job-manager/test/queues.c @@ -1795,6 +1795,99 @@ static void test_configure_failure_preserves_order (void) queues_destroy (qs); } +/* The queue-list response carries both the "queues" name array and a + * "conf" object with each queue's effective config, both in canonical + * order. Effective 'requires'/'parent' are checked on a virtual queue. + */ +static void test_list_response (void) +{ + struct queues *qs; + flux_error_t error; + json_t *config; + json_t *resp; + json_t *names; + json_t *cq; + const char *n0, *n1, *n2; + json_t *req0; + const char *parent2 = NULL; + + qs = queues_create (); + if (!qs) + BAIL_OUT ("queues_create failed"); + + /* anon mode: names empty and conf.queues empty */ + resp = queues_list_response (qs); + ok (resp != NULL + && json_unpack (resp, + "{s:o s:{s:o}}", + "queues", &names, + "conf", + "queues", &cq) == 0 + && json_array_size (names) == 0 + && json_array_size (cq) == 0, + "response in anon mode: empty queues and empty conf.queues"); + json_decref (resp); + + /* batch (real, requires=batch), then expedite (virtual, parent + * batch, no own requires). Declaration order: debug, batch, expedite. + */ + config = json_pack ("{s:{s:[s]} s:{s:[s]} s:{s:s}}", + "debug", + "requires", "debug", + "batch", + "requires", "batch", + "expedite", + "parent", "batch"); + if (!config) + BAIL_OUT ("json_pack failed"); + if (queues_configure (qs, config, &error) < 0) + BAIL_OUT ("queues_configure failed: %s", error.text); + json_decref (config); + + resp = queues_list_response (qs); + ok (resp != NULL, "queues_list_response works in named mode"); + ok (resp + && json_unpack (resp, + "{s:o s:{s:o}}", + "queues", &names, + "conf", + "queues", &cq) == 0, + "response has queues array and conf.queues array"); + + /* conf.queues is in declaration order */ + ok (cq && json_array_size (cq) == 3 + && json_unpack (json_array_get (cq, 0), "{s:s}", "name", &n0) == 0 + && json_unpack (json_array_get (cq, 1), "{s:s}", "name", &n1) == 0 + && json_unpack (json_array_get (cq, 2), "{s:s}", "name", &n2) == 0 + && streq (n0, "debug") + && streq (n1, "batch") + && streq (n2, "expedite"), + "conf.queues is in config declaration order"); + + /* real queue carries its own requires, no parent key */ + ok (json_unpack (json_array_get (cq, 1), + "{s:s s:o !}", + "name", &n1, + "requires", &req0) == 0 + && json_array_size (req0) == 1 + && streq (json_string_value (json_array_get (req0, 0)), "batch"), + "real queue conf carries own requires and no parent key"); + + /* virtual queue inherits parent's requires and reports parent */ + ok (json_unpack (json_array_get (cq, 2), + "{s:s s:o s:s !}", + "name", &n2, + "requires", &req0, + "parent", &parent2) == 0 + && streq (parent2, "batch") + && json_array_size (req0) == 1 + && streq (json_string_value (json_array_get (req0, 0)), "batch"), + "virtual queue conf inherits parent requires and reports parent"); + + json_decref (resp); + queues_destroy (qs); +} + /* ---------- virtual queue (RFC 33) tests -------------------------------- */ static void test_vqueue_configure (void) @@ -2516,6 +2609,7 @@ int main (int argc, char *argv[]) test_list_order (); test_add_remove_order (); test_configure_failure_preserves_order (); + test_list_response (); test_vqueue_configure (); test_vqueue_reparent_on_reload (); diff --git a/t/python/t0034-queuelist.py b/t/python/t0034-queuelist.py index e3c05eb89b85..52a443ebf51a 100755 --- a/t/python/t0034-queuelist.py +++ b/t/python/t0034-queuelist.py @@ -102,7 +102,58 @@ def test_002_named(self): self.assertEqual(qlist.debug.limits.duration, 3600.0) self.assertEqual(qlist.debug.limits.timelimit, 3600.0) - def test_003_vqueue_stale_parent(self): + def test_003_queue_list_conf(self): + # Exercise the queue-list RPC response directly: the conf object + # must carry each queue's effective config in config declaration + # order, with a virtual queue inheriting its parent's requires. + testconf = """ + [queues.debug] + requires = ["debug"] + + [queues.batch] + requires = ["batch"] + + [queues.expedite] + parent = "batch" + """ + self.fh.rpc("config.load", tomllib.loads(testconf)).get() + + resp = self.fh.rpc("job-manager.queue-list").get() + # top-level names array is in declaration order: + self.assertEqual(resp["queues"], ["debug", "batch", "expedite"]) + conf = resp["conf"]["queues"] + self.assertEqual([q["name"] for q in conf], ["debug", "batch", "expedite"]) + + by_name = {q["name"]: q for q in conf} + # real queue: own requires, no parent key + self.assertEqual(by_name["batch"]["requires"], ["batch"]) + self.assertNotIn("parent", by_name["batch"]) + # virtual queue: inherits parent's requires and reports parent + self.assertEqual(by_name["expedite"]["requires"], ["batch"]) + self.assertEqual(by_name["expedite"]["parent"], "batch") + + # A reconfigure that reorders queues is reflected in the response. + # N.B. the config module coalesces a config.load whose JSON is + # json_equal() to the current config (order-insensitive), so a + # *pure* reorder does not fire a reconfigure. Bundle the reorder + # with a real change (drop 'debug') so a reconfigure actually + # occurs, then assert the surviving queues are re-sequenced. + reordered = """ + [queues.expedite] + parent = "batch" + + [queues.batch] + requires = ["batch"] + """ + self.fh.rpc("config.load", tomllib.loads(reordered)).get() + resp = self.fh.rpc("job-manager.queue-list").get() + self.assertEqual(resp["queues"], ["expedite", "batch"]) + self.assertEqual( + [q["name"] for q in resp["conf"]["queues"]], + ["expedite", "batch"], + ) + + def test_004_vqueue_stale_parent(self): # A QueueInfo parent (sourced from the queue-status RPC) missing # from the config (a separate RPC, so a config reload can race # the two) must raise, not fall back to the full instance From a085153364416488c8d71a1ae1a7830422306b87 Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Sat, 8 Aug 2026 14:16:36 -0700 Subject: [PATCH 06/10] job-manager: cache the queue-list response Problem: The queue-list response will be fetched by many components but changes rarely, so rebuilding it on every request is wasted work. Cache the assembled response in the queues object, rebuilding it lazily and invalidating it from notify() on any mutation. queues_list_response() now returns a borrowed reference owned by the cache. Since notify() is the single choke point for every mutation, no change can bypass invalidation. The existing test_list_response() decref'd the return value, which is now owned by the cache, so those decrefs are removed here to keep the test correct. Assisted-by: Claude:Opus-4.8 --- src/modules/job-manager/queue.c | 7 +++--- src/modules/job-manager/queues.c | 34 +++++++++++++++++++++++---- src/modules/job-manager/queues.h | 8 +++++-- src/modules/job-manager/test/queues.c | 6 ++--- 4 files changed, 42 insertions(+), 13 deletions(-) diff --git a/src/modules/job-manager/queue.c b/src/modules/job-manager/queue.c index d74bd83dc52f..9947ffc69224 100644 --- a/src/modules/job-manager/queue.c +++ b/src/modules/job-manager/queue.c @@ -144,20 +144,21 @@ static void queue_list_cb (flux_t *h, void *arg) { struct queue_ctx *qctx = arg; - json_t *resp = NULL; + json_t *resp; if (flux_request_decode (msg, NULL, NULL) < 0) goto error; + /* resp is a borrowed reference owned by the queues cache; "O" + * increfs it for the response, so it is not decref'd here. + */ if (!(resp = queues_list_response (qctx->queues))) goto error; if (flux_respond_pack (h, msg, "O", resp) < 0) flux_log_error (h, "error responding to job-manager.queue-list"); - json_decref (resp); return; error: if (flux_respond_error (h, msg, errno, NULL) < 0) flux_log_error (h, "error responding to job-manager.queue-list"); - json_decref (resp); } static void queue_status_cb (flux_t *h, diff --git a/src/modules/job-manager/queues.c b/src/modules/job-manager/queues.c index fafa1cb54635..e48cd5ca079f 100644 --- a/src/modules/job-manager/queues.c +++ b/src/modules/job-manager/queues.c @@ -69,19 +69,38 @@ struct queues { * pointers (the named hash owns them; * NULL destructor). */ + json_t *list_cache; /* cached queue-list response, built + * lazily by queues_list_response() and + * invalidated by notify() on any + * mutation. NULL when not yet built. + */ bool restoring; /* suppress notify during queues_restore */ queues_change_f notify_cb; void *notify_arg; }; +/* Invalidate any cached queue-list response. + */ +static void cache_invalidate (struct queues *queues) +{ + json_decref (queues->list_cache); + queues->list_cache = NULL; +} + /* Internal: fire change notification if callback is registered. * Suppressed during queues_restore: restore reconstructs state that * was already notified when it originally changed. + * + * The response cache is invalidated unconditionally (even during restore, + * which is not a notify consumer) since any mutation may change it. The + * cache depends only on config-derived fields, so enable/start/stop + * events over-invalidate harmlessly. */ static void notify (struct queues *queues, struct queue *q, const char *event) { + cache_invalidate (queues); if (queues->notify_cb && !queues->restoring) queues->notify_cb (queues, q, event, queues->notify_arg); } @@ -320,6 +339,7 @@ void queues_destroy (struct queues *queues) else queue_free (queues->anon); zlistx_destroy (&queues->order); + json_decref (queues->list_cache); free (queues); errno = saved_errno; } @@ -1070,13 +1090,10 @@ static json_t *queues_conf_encode (struct queues *queues) return NULL; } -/* Assemble the full queue-list RPC response: +/* Build the full queue-list RPC response (a new reference): * {"queues":[names...], "conf":{"queues":[{name,requires?,parent?}...]}} - * The "queues" name array is retained for backwards compatibility; "conf" - * carries the effective per-queue configuration. Returns a new reference - * the caller must decref. */ -json_t *queues_list_response (struct queues *queues) +static json_t *list_response_build (struct queues *queues) { json_t *names = NULL; json_t *conf = NULL; @@ -1096,6 +1113,13 @@ json_t *queues_list_response (struct queues *queues) return NULL; } +json_t *queues_list_response (struct queues *queues) +{ + if (!queues->list_cache) + queues->list_cache = list_response_build (queues); + return queues->list_cache; +} + static int save_one (json_t *a, struct queue *q) { json_t *entry; diff --git a/src/modules/job-manager/queues.h b/src/modules/job-manager/queues.h index de08dddbff42..61e77f02f4d5 100644 --- a/src/modules/job-manager/queues.h +++ b/src/modules/job-manager/queues.h @@ -130,8 +130,12 @@ json_t *queues_list_encode (struct queues *queues); * Both parts are in canonical (config declaration) order. The "queues" * name array is retained for backwards compatibility; "conf" carries the * effective per-queue configuration (a virtual queue's 'requires' is - * inherited from its parent). Returns a new reference the caller must - * destroy, or NULL with errno set on error. + * inherited from its parent). + * + * The response is cached and rebuilt lazily; any queue mutation + * invalidates the cache. Returns a BORROWED reference owned by the queues + * object (do not destroy it; incref if it must outlive the next + * mutation), or NULL with errno set on error. */ json_t *queues_list_response (struct queues *queues); diff --git a/src/modules/job-manager/test/queues.c b/src/modules/job-manager/test/queues.c index 234165bbddb8..5eafcb610ca7 100644 --- a/src/modules/job-manager/test/queues.c +++ b/src/modules/job-manager/test/queues.c @@ -1815,7 +1815,9 @@ static void test_list_response (void) if (!qs) BAIL_OUT ("queues_create failed"); - /* anon mode: names empty and conf.queues empty */ + /* anon mode: names empty and conf.queues empty. The returned + * reference is borrowed (owned by the cache), so it is not decref'd. + */ resp = queues_list_response (qs); ok (resp != NULL && json_unpack (resp, @@ -1826,7 +1828,6 @@ static void test_list_response (void) && json_array_size (names) == 0 && json_array_size (cq) == 0, "response in anon mode: empty queues and empty conf.queues"); - json_decref (resp); /* batch (real, requires=batch), then expedite (virtual, parent * batch, no own requires). Declaration order: debug, batch, expedite. @@ -1884,7 +1885,6 @@ static void test_list_response (void) && streq (json_string_value (json_array_get (req0, 0)), "batch"), "virtual queue conf inherits parent requires and reports parent"); - json_decref (resp); queues_destroy (qs); } From 8aaddb962ffd0744dba0a01d67e606b66f774b9c Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Sat, 8 Aug 2026 14:17:01 -0700 Subject: [PATCH 07/10] testsuite: cover queue-list response caching Problem: The response caching added to queues_list_response() has no test asserting the cache is reused when unchanged and dropped on a mutation. Assert that a second call with no intervening mutation returns the same object, and that a queue mutation causes a fresh object to be built. Assisted-by: Claude:Opus-4.8 --- src/modules/job-manager/test/queues.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/modules/job-manager/test/queues.c b/src/modules/job-manager/test/queues.c index 5eafcb610ca7..74bbda4d3414 100644 --- a/src/modules/job-manager/test/queues.c +++ b/src/modules/job-manager/test/queues.c @@ -1885,6 +1885,22 @@ static void test_list_response (void) && streq (json_string_value (json_array_get (req0, 0)), "batch"), "virtual queue conf inherits parent requires and reports parent"); + /* cache: a second call with no mutation returns the same object */ + ok (queues_list_response (qs) == resp, + "queues_list_response returns cached object when unchanged"); + + /* cache: a mutation invalidates, so a fresh object is built. Hold a + * reference across the mutation: the borrowed 'resp' is owned by the + * cache, which decrefs (frees) it on invalidation, and comparing the + * rebuilt response against freed memory is undefined - the allocator + * may hand the same address back. + */ + json_incref (resp); + queue_stop (queues_lookup (qs, "batch", NULL), NULL, false); + ok (queues_list_response (qs) != resp, + "a queue mutation invalidates the cached response"); + json_decref (resp); + queues_destroy (qs); } From 3fa0c8f57f84e93f9b97df3005b2cf6fbc5a20dd Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Tue, 11 Aug 2026 12:50:34 -0700 Subject: [PATCH 08/10] python: add queue_conf_from_config() to flux.queue Problem: Clients that read the [queues] config to derive a queue's effective requires re-implement RFC 33 virtual-queue parent inheritance, and there is no shared way to produce the job-manager.queue-list "conf" object from a raw config. Add queue_conf_from_config(), which builds the "conf" object from a raw broker config in declaration order, resolving a virtual queue's requires from its parent. It backs the fallback path for older job-managers that predate the "conf" object and the hidden --config-file/--from-stdin test options. Assisted-by: Claude:Opus-4.8 --- src/bindings/python/flux/queue.py | 45 +++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/src/bindings/python/flux/queue.py b/src/bindings/python/flux/queue.py index d4c2425d0ad1..bf822fc85f08 100644 --- a/src/bindings/python/flux/queue.py +++ b/src/bindings/python/flux/queue.py @@ -176,6 +176,51 @@ def __getattr__(self, attr): return getattr(self._resource_list, attr) +def queue_conf_from_config(config): + """Build the job-manager.queue-list "conf" object from a raw broker config. + + This helper creates a job-manager.queue-list response ``conf`` object + directly from broker config. For use in testing and as a fallback when + the job-manager is from an older version of Flux that does not provide + the queue config directly. + + Returns ``{"queues": [{"name": str, "requires"?: list, "parent"?: str}, + ...]}`` in config declaration order. A virtual queue (RFC 33) has no + "requires" of its own, so its effective "requires" is resolved from its + parent. + + Args: + config (dict): a broker config, e.g. from the ``config.get`` RPC. + + Raises: + ValueError: a virtual queue names a parent that is not configured. + """ + queues = config.get("queues", {}) + entries = [] + # dict iteration preserves insertion order (Python 3.7+), and both + # tomllib and json.load preserve config file order, so this matches the + # job-manager's config declaration order. + for name, entry in queues.items(): + conf = {"name": name} + parent = entry.get("parent") + if parent is not None: + # Virtual queue: inherit the parent's requires. An unresolvable + # parent is fatal (fail closed) - falling through would report + # the vqueue as covering the full instance resource set. + if parent not in queues: + raise ValueError( + f"queue '{name}': parent queue '{parent}' is not configured" + ) + conf["parent"] = parent + requires = queues[parent].get("requires") + else: + requires = entry.get("requires") + if requires is not None: + conf["requires"] = requires + entries.append(conf) + return {"queues": entries} + + class QueueInfo: """ Information for a single queue. From f90ac285104924e04dd9ff1c586cd62f234ec125 Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Tue, 11 Aug 2026 13:11:27 -0700 Subject: [PATCH 09/10] flux-resource: get queue config from the queue-list RPC Problem: flux-resource reads the [queues] config directly and re-derives each queue's effective requires, duplicating the RFC 33 virtual-queue parent inheritance that the job-manager already performs, so it is not the authority on queue configuration. Take the effective queue config from the job-manager.queue-list "conf" object instead. An older job-manager without "conf" and the hidden --config-file/--from-stdin test options fall back to deriving it locally via queue_conf_from_config(). This removes the client-side queue_effective_entry() resolver. Assisted-by: Claude:Opus-4.8 --- src/cmd/flux-resource.py | 144 +++++++++++++++++++++------------------ 1 file changed, 79 insertions(+), 65 deletions(-) diff --git a/src/cmd/flux-resource.py b/src/cmd/flux-resource.py index bf0967708d03..ecc7cb89e6af 100755 --- a/src/cmd/flux-resource.py +++ b/src/cmd/flux-resource.py @@ -24,6 +24,7 @@ from flux.eventlog import EventLogFormatter from flux.hostlist import Hostlist from flux.idset import IDset +from flux.queue import queue_conf_from_config from flux.resource import ( ResourceJournalConsumer, ResourceSet, @@ -196,27 +197,35 @@ def undrain(args): RPC(flux.Flux(), "resource.undrain", payload, nodeid=0).get() -def queue_effective_entry(queues, name): +def queue_conf(args, handle, queue_list_rpc): """ - Return the effective [queues] config entry for queue ``name``: its own - entry, or its parent's entry if it is an RFC 33 virtual queue. - - A virtual queue has no "requires" of its own, so resolving to the - parent's entry is what maps it to the parent's resource slice rather - than wrongly matching every node as an unconstrained queue would. An - unresolvable parent raises ValueError (defense in depth: config - validation rejects it in a live instance, but --config-file and - --from-stdin input is not validated). + Return the effective queue configuration as a name->entry dict, where + each entry has its effective "requires" (an RFC 33 virtual queue's is + resolved from its parent) and, for a virtual queue, its "parent". This + is the job-manager's authoritative view. + + The primary source is the job-manager.queue-list RPC "conf" object. An + older job-manager omits "conf", so fall back to deriving it from the + [queues] config via queue_conf_from_config(). The hidden --config-file/ + --from-stdin test options are also derived from the raw config. + + 'queue_list_rpc' is a job-manager.queue-list future the caller has + already sent (so it can overlap with other RPCs). It is required on the + online path and must be non-None whenever ``handle`` is set and + ``--config-file`` is not. It is ignored (and may be None) otherwise. """ - entry = queues[name] - if "parent" in entry: - parent = entry["parent"] - if parent not in queues: - raise ValueError( - f"queue '{name}': parent queue '{parent}' is not configured" - ) - entry = queues[parent] - return entry + if args.config_file: + with open(args.config_file) as fp: + conf = queue_conf_from_config(json.load(fp)) + elif handle is None: + return {} + else: + resp = queue_list_rpc.get() + # Get conf from response or derive from config (fallback) + conf = resp.get("conf") or queue_conf_from_config( + handle.rpc("config.get").get() + ) + return {entry["name"]: entry for entry in conf["queues"]} class QueueResources: @@ -224,17 +233,14 @@ class QueueResources: Convenience class to map queues to resource sets """ - def __init__(self, resource_set, config): + def __init__(self, resource_set, queues): self._queues = {} - if "queues" not in config: - return - for queue in config["queues"]: - entry = queue_effective_entry(config["queues"], queue) + for name, entry in queues.items(): if "requires" in entry: result = resource_set.copy_constraint({"properties": entry["requires"]}) else: result = resource_set.copy() - self._queues[queue] = result + self._queues[name] = result def queue(self, queue): if queue not in self._queues: @@ -242,15 +248,16 @@ def queue(self, queue): return self._queues[queue] -def ranks_by_queue(resource_set, config, queues): +def ranks_by_queue(resource_set, queue_config, queues): """ Return all ranks associated with a list of queues Args: resource_set: The resource set to query - config: a Flux config object including queue configuration, if any. + queue_config: effective queue config as a name->entry dict (see + queue_conf()) queues: one or more queues specified as a comma-separated string """ - queue_resources = QueueResources(resource_set, config) + queue_resources = QueueResources(resource_set, queue_config) ranks = IDset() for queue in queues: ranks.add(queue_resources.queue(queue).ranks) @@ -452,6 +459,7 @@ def status(args): fmt = FluxResourceConfig("status").load().get_format_string(args.format) handle = None + queue_list_rpc = None # Get payload from stdin or from resource.status RPC: if args.from_stdin: @@ -459,18 +467,21 @@ def status(args): rstatus = ResourceStatus(json.loads(input_str) if input_str else None) else: handle = flux.Flux() + # Send the queue-list RPC (only needed for --queue) before draining + # resource.status, so the two overlap: + if args.queue: + queue_list_rpc = handle.rpc("job-manager.queue-list") rstatus = resource_status(handle).get() if args.queue: - if args.config_file: - with open(args.config_file) as fp: - config = json.load(fp) - else: - config = {} - if handle is not None: - config = handle.rpc("config.get").get() try: - rstatus.filter(ranks_by_queue(rstatus.rset, config, args.queue)) + rstatus.filter( + ranks_by_queue( + rstatus.rset, + queue_conf(args, handle, queue_list_rpc), + args.queue, + ) + ) except ValueError as exc: raise ValueError(f"--queue: {exc}") from None @@ -534,12 +545,12 @@ def __init__( # lgtm[py/missing-call-to-init] self, arg=None, version=1, - flux_config=None, + queue_config=None, queue=None, queue_filter=None, hidden_queues=None, ): - self.flux_config = flux_config + self.queue_config = queue_config or {} self._queue = queue self._queue_filter = queue_filter self._hidden_queues = hidden_queues or set() @@ -558,9 +569,8 @@ def propertiesx(self): properties = json.loads(self.get_properties()) # Strip all configured queue names from properties, so that # properties used only for queue membership are not displayed. - if self.flux_config and "queues" in self.flux_config: - for q in self.flux_config["queues"]: - properties.pop(q, None) + for q in self.queue_config: + properties.pop(q, None) return ",".join(properties.keys()) @property @@ -573,11 +583,11 @@ def queue(self): # If self._queue is not set, then build list of queues from # set properties and queue configuration: queues = "" - if self.flux_config and "queues" in self.flux_config: + if self.queue_config: if not self.ranks: return "" properties = json.loads(self.get_properties()) - for key, value in self.flux_config["queues"].items(): + for key, entry in self.queue_config.items(): if self._queue_filter: if key not in self._queue_filter: continue @@ -587,9 +597,8 @@ def queue(self): # was explicitly requested via -q (i.e. is in the filter), # otherwise it would match every node in the default QUEUE # column via its resolved parent's requires: - if "parent" in value and not self._queue_filter: + if "parent" in entry and not self._queue_filter: continue - entry = queue_effective_entry(self.flux_config["queues"], key) if "requires" not in entry or set(entry["requires"]).issubset( set(properties) ): @@ -626,7 +635,7 @@ def constraint_combinations(rset): def resources_uniq_lines( - resources, states, formatter, config, queues=None, hidden_queues=None + resources, states, formatter, queue_config, queues=None, hidden_queues=None ): """ Generate a set of resource sets that would produce unique lines given @@ -670,10 +679,10 @@ def resources_uniq_lines( # If no queues are configured then one "anonymous" queue is simulated # with [None]. if not queues: - if config and "queues" in config: + if queue_config: queues = [ q - for q, entry in config["queues"].items() + for q, entry in queue_config.items() if q not in hidden_queues and "parent" not in entry ] if not queues: @@ -691,7 +700,7 @@ def resources_uniq_lines( # state would be suppressed. # for queue in queues: - rset = ResourceSetExtra(flux_config=config, queue=queue) + rset = ResourceSetExtra(queue_config=queue_config, queue=queue) rset.state = state key = fmt.format(rset) if key not in lines: @@ -706,7 +715,7 @@ def resources_uniq_lines( rset.state = state rset = ResourceSetExtra( rset, - flux_config=config, + queue_config=queue_config, queue_filter=queue_filter, hidden_queues=hidden_queues, ) @@ -725,7 +734,7 @@ def get_resource_list(args): Common function for list_handler() and emit_R() """ valid_states = ["up", "down", "allocated", "free", "all"] - config = None + handle = None args.states = args.states.split(",") for state in args.states: @@ -733,23 +742,28 @@ def get_resource_list(args): LOGGER.error("Invalid resource state %s specified", state) sys.exit(1) + queue_list_rpc = None if args.from_stdin: resources = SchedResourceList(json.load(sys.stdin)) - if args.config_file: - with open(args.config_file) as fp: - config = json.load(fp) else: handle = flux.Flux() - rpcs = [resource_list(handle), handle.rpc("config.get")] - resources = rpcs[0].get() - try: - config = rpcs[1].get() - except Exception as e: - LOGGER.warning("Could not get flux config: " + str(e)) + # Send both RPCs before draining either, so they overlap: + queue_list_rpc = handle.rpc("job-manager.queue-list") + resources = resource_list(handle).get() + + # Tolerate a failure to reach the job-manager/broker so listing still + # works without queue annotation. A bad queue config (e.g. an + # unresolvable virtual queue parent) raises ValueError, not OSError, and + # so still propagates. + try: + queue_config = queue_conf(args, handle, queue_list_rpc) + except OSError as e: + LOGGER.warning("Could not get queue configuration: " + str(e)) + queue_config = {} if args.queue: try: - resources.filter(ranks_by_queue(resources.all, config, args.queue)) + resources.filter(ranks_by_queue(resources.all, queue_config, args.queue)) except ValueError as exc: raise ValueError(f"--queue: {exc}") from None @@ -759,7 +773,7 @@ def get_resource_list(args): except (ValueError, TypeError) as exc: raise ValueError(f"--include: {exc}") from None - return resources, config + return resources, queue_config def sort_output(args, items): @@ -783,7 +797,7 @@ def list_handler(args): "nodelist": "NODELIST", "rlist": "LIST", } - resources, config = get_resource_list(args) + resources, queue_config = get_resource_list(args) list_config = FluxResourceConfig("list").load() fmt = list_config.get_format_string(args.format) @@ -794,7 +808,7 @@ def list_handler(args): resources, args.states, formatter, - config, + queue_config, queues=args.queue, hidden_queues=hidden_queues, ) @@ -815,7 +829,7 @@ def info(args): def emit_R(args): """Emit R in JSON on stdout for requested set of resources""" - resources, config = get_resource_list(args) + resources, _ = get_resource_list(args) rset = ResourceSet() rset.starttime = resources["all"].starttime From f76c9be5cf73735f13aaebb46e36fb4f3e5e29f1 Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Tue, 11 Aug 2026 13:15:10 -0700 Subject: [PATCH 10/10] testsuite: cover queue_conf_from_config() Problem: The new queue_conf_from_config() helper and its use as the authoritative queue config source for flux-resource have no test coverage, and nothing guards the helper against drifting from the job-manager's "conf" encoding. Add t0049-queue-conf.py unit tests for the helper (declaration order, effective requires, virtual-queue inheritance, and fail-closed on an unconfigured parent), and add a t0034 case asserting the helper reproduces the live queue-list "conf" object exactly. Assisted-by: Claude:Opus-4.8 --- t/Makefile.am | 1 + t/python/t0034-queuelist.py | 23 +++++++++- t/python/t0049-queue-conf.py | 84 ++++++++++++++++++++++++++++++++++++ t/t2350-resource-list.t | 13 ++++++ 4 files changed, 120 insertions(+), 1 deletion(-) create mode 100755 t/python/t0049-queue-conf.py diff --git a/t/Makefile.am b/t/Makefile.am index 984123394632..123b6fde5af6 100644 --- a/t/Makefile.am +++ b/t/Makefile.am @@ -366,6 +366,7 @@ TESTSCRIPTS = \ python/t0046-job-watcher.py \ python/t0047-flux-argument-parser.py \ python/t0048-frobnicator-vqueue.py \ + python/t0049-queue-conf.py \ python/t0100-modprobe.py \ python/t1000-service-add-remove.py \ python/t6010-fake-resources.py diff --git a/t/python/t0034-queuelist.py b/t/python/t0034-queuelist.py index 52a443ebf51a..48f452775ad5 100755 --- a/t/python/t0034-queuelist.py +++ b/t/python/t0034-queuelist.py @@ -18,7 +18,7 @@ from flux.utils import tomli as tomllib import flux -from flux.queue import QueueInfo, QueueList +from flux.queue import QueueInfo, QueueList, queue_conf_from_config from flux.resource import resource_list from subflux import rerun_under_flux @@ -153,6 +153,27 @@ def test_003_queue_list_conf(self): ["expedite", "batch"], ) + def test_0035_conf_matches_helper(self): + # The queue_conf_from_config() helper (used as the fallback for + # older brokers and by the hidden --config-file/--from-stdin + # options) must reproduce the live job-manager 'conf' object + # exactly, so the two implementations do not drift. + testconf = """ + [queues.debug] + requires = ["debug"] + + [queues.batch] + requires = ["batch"] + + [queues.expedite] + parent = "batch" + """ + config = tomllib.loads(testconf) + self.fh.rpc("config.load", config).get() + + resp = self.fh.rpc("job-manager.queue-list").get() + self.assertEqual(resp["conf"], queue_conf_from_config(config)) + def test_004_vqueue_stale_parent(self): # A QueueInfo parent (sourced from the queue-status RPC) missing # from the config (a separate RPC, so a config reload can race diff --git a/t/python/t0049-queue-conf.py b/t/python/t0049-queue-conf.py new file mode 100755 index 000000000000..3ae3a3376eca --- /dev/null +++ b/t/python/t0049-queue-conf.py @@ -0,0 +1,84 @@ +#!/usr/bin/env python3 + +############################################################### +# Copyright 2026 Lawrence Livermore National Security, LLC +# (c.f. AUTHORS, NOTICE.LLNS, COPYING) +# +# This file is part of the Flux resource manager framework. +# For details, see https://github.com/flux-framework. +# +# SPDX-License-Identifier: LGPL-3.0 +############################################################### + +import unittest + +import subflux # noqa: F401 - To set up PYTHONPATH +from flux.queue import queue_conf_from_config +from pycotap import TAPTestRunner + + +class TestQueueConfFromConfig(unittest.TestCase): + """queue_conf_from_config() builds the queue-list 'conf' object.""" + + def test_empty_config(self): + self.assertEqual(queue_conf_from_config({}), {"queues": []}) + self.assertEqual(queue_conf_from_config({"queues": {}}), {"queues": []}) + + def test_declaration_order_preserved(self): + config = { + "queues": { + "debug": {"requires": ["debug"]}, + "batch": {"requires": ["batch"]}, + "expedite": {"parent": "batch"}, + } + } + conf = queue_conf_from_config(config) + self.assertEqual( + [q["name"] for q in conf["queues"]], ["debug", "batch", "expedite"] + ) + + def test_real_queue_own_requires(self): + conf = queue_conf_from_config({"queues": {"batch": {"requires": ["batch"]}}}) + entry = conf["queues"][0] + self.assertEqual(entry, {"name": "batch", "requires": ["batch"]}) + self.assertNotIn("parent", entry) + + def test_queue_without_requires_omits_key(self): + conf = queue_conf_from_config({"queues": {"plain": {}}}) + self.assertEqual(conf["queues"], [{"name": "plain"}]) + + def test_vqueue_inherits_parent_requires(self): + config = { + "queues": { + "batch": {"requires": ["batch"]}, + "expedite": {"parent": "batch"}, + } + } + by_name = {q["name"]: q for q in queue_conf_from_config(config)["queues"]} + # The vqueue reports its parent and inherits the parent's requires. + self.assertEqual(by_name["expedite"]["parent"], "batch") + self.assertEqual(by_name["expedite"]["requires"], ["batch"]) + + def test_vqueue_parent_without_requires_omits_key(self): + config = { + "queues": { + "batch": {}, + "expedite": {"parent": "batch"}, + } + } + by_name = {q["name"]: q for q in queue_conf_from_config(config)["queues"]} + self.assertNotIn("requires", by_name["expedite"]) + self.assertEqual(by_name["expedite"]["parent"], "batch") + + def test_missing_parent_fails_closed(self): + # An unresolvable parent must raise (fail closed) rather than fall + # through to reporting the vqueue as unconstrained. + with self.assertRaises(ValueError) as ctx: + queue_conf_from_config({"queues": {"expedite": {"parent": "nosuchqueue"}}}) + self.assertIn( + "parent queue 'nosuchqueue' is not configured", str(ctx.exception) + ) + + +if __name__ == "__main__": + unittest.main(testRunner=TAPTestRunner()) diff --git a/t/t2350-resource-list.t b/t/t2350-resource-list.t index ea345def6973..4ae1a13a906c 100755 --- a/t/t2350-resource-list.t +++ b/t/t2350-resource-list.t @@ -518,6 +518,19 @@ test_expect_success 'flux resource list -q strips all queue names from propertie test_debug "cat listpropx_multi_q.out" && grep "free 2 $" listpropx_multi_q.out ' +# Exercise the online status -q path (queue config from the queue-list RPC, +# not --from-stdin/--config-file): batch is configured on 2 of the 4 nodes. +test_expect_success 'flux resource status -q filters to the requested queue' ' + flux resource status -q batch -s all -o "{state} {nnodes}" \ + >statusqueue_q.out && + test_debug "cat statusqueue_q.out" && + test $(awk "{ n += \$2 } END { print n }" statusqueue_q.out) -eq 2 +' +test_expect_success 'flux resource status -q rejects an invalid queue' ' + test_must_fail flux resource status -q notaqueue 2>statusqueue_bad.err && + test_debug "cat statusqueue_bad.err" && + grep -i "no such queue\|not a valid queue\|notaqueue" statusqueue_bad.err +' test_expect_success 'configure queues and resource with extra property' ' flux R encode -r 0-3 -p batch:0-1 -p debug:2-3 -p foo:0-3\ | tr -d "\n" \