Merge pull request #6180 from grondo/issue#6166

flux-resource: fix missing queue in `flux resource list` output for states with no nodes

Author: mergify[bot]

src/cmd/flux-resource.py

@@ -27,7 +27,7 @@
     resource_status,
 )
 from flux.rpc import RPC
-from flux.util import Deduplicator, UtilConfig
+from flux.util import Deduplicator, FilterActionSetUpdate, UtilConfig
 
 
 class FluxResourceConfig(UtilConfig):
@@ -198,7 +198,7 @@ def ranks_by_queue(resource_set, config, queues):
     """
     queue_resources = QueueResources(resource_set, config)
     ranks = IDset()
-    for queue in queues.split(","):
+    for queue in queues:
         ranks.add(queue_resources.queue(queue).ranks)
     return ranks
 
@@ -492,8 +492,9 @@ def drain_list(args):
 
 
 class ResourceSetExtra(ResourceSet):
-    def __init__(self, arg=None, version=1, flux_config=None):
+    def __init__(self, arg=None, version=1, flux_config=None, queue=None):
         self.flux_config = flux_config
+        self._queue = queue
         if isinstance(arg, ResourceSet):
             self._rset = arg
             if arg.state:
@@ -517,6 +518,13 @@ def propertiesx(self):
 
     @property
     def queue(self):
+        #  Note: queue may be set manually in self._queue for an empty
+        #  ResourceSet, which cannot otherwise have an associated queue.
+        if self._queue is not None:
+            return self._queue
+
+        #  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 not self.ranks:
@@ -558,7 +566,7 @@ def constraint_combinations(rset):
     return [rset.copy_constraint(x) for x in constraint_combinations(rset)]
 
 
-def resources_uniq_lines(resources, states, formatter, config):
+def resources_uniq_lines(resources, states, formatter, config, queues=None):
     """
     Generate a set of resource sets that would produce unique lines given
     the ResourceSet formatter argument. Include only the provided states
@@ -583,6 +591,15 @@ def resources_uniq_lines(resources, states, formatter, config):
 
     fmt = flux.util.OutputFormat(uniq_fmt, headings=formatter.headings)
 
+    #  Get a list of configured queues if a specific list of queues
+    #  was not supplied by the caller. If no queues are configured then
+    #  one "anonymous" queue is simulated with [None]
+    if not queues:
+        if config and "queues" in config:
+            queues = config["queues"].keys()
+        else:
+            queues = [None]
+
     #  Create a mapping of resources sets that generate uniq "lines":
     lines = {}
     for state in states:
@@ -592,13 +609,14 @@ def resources_uniq_lines(resources, states, formatter, config):
             #   resource set for output purposes. O/w the output for this
             #   state would be suppressed.
             #
-            rset = ResourceSetExtra(flux_config=config)
-            rset.state = state
-            key = fmt.format(rset)
-            if key not in lines:
-                lines[key] = rset
-            else:
-                lines[key].add(rset)
+            for queue in queues:
+                rset = ResourceSetExtra(flux_config=config, queue=queue)
+                rset.state = state
+                key = fmt.format(rset)
+                if key not in lines:
+                    lines[key] = rset
+                else:
+                    lines[key].add(rset)
             continue
 
         for rset in split_by_property_combinations(resources[state]):
@@ -684,7 +702,9 @@ def list_handler(args):
     fmt = FluxResourceConfig("list").load().get_format_string(args.format)
     formatter = flux.util.OutputFormat(fmt, headings=headings)
 
-    lines = resources_uniq_lines(resources, args.states, formatter, config)
+    lines = resources_uniq_lines(
+        resources, args.states, formatter, config, queues=args.queue
+    )
     items = sort_output(args, lines.values())
     formatter.print_items(items, no_header=args.no_header)
 
@@ -759,6 +779,8 @@ def main():
     drain_parser.add_argument(
         "-q",
         "--queue",
+        action=FilterActionSetUpdate,
+        default=set(),
         metavar="QUEUE,...",
         help="Include only specified queues in output",
     )
@@ -824,6 +846,8 @@ def main():
     status_parser.add_argument(
         "-q",
         "--queue",
+        action=FilterActionSetUpdate,
+        default=set(),
         metavar="QUEUE,...",
         help="Include only specified queues in output",
     )
@@ -879,6 +903,8 @@ def main():
     list_parser.add_argument(
         "-q",
         "--queue",
+        action=FilterActionSetUpdate,
+        default=set(),
         metavar="QUEUE,...",
         help="Include only specified queues in output",
     )
@@ -911,6 +937,8 @@ def main():
     info_parser.add_argument(
         "-q",
         "--queue",
+        action=FilterActionSetUpdate,
+        default=set(),
         metavar="QUEUE,...",
         help="Include only specified queues in output",
     )
@@ -959,6 +987,8 @@ def main():
     R_parser.add_argument(
         "-q",
         "--queue",
+        action=FilterActionSetUpdate,
+        default=set(),
         metavar="QUEUE,...",
         help="Include only specified queues in output",
     )

t/t2350-resource-list.t

@@ -517,9 +517,7 @@ test_expect_success 'flux resource lists expected queues (every)' '
 	flux resource list -o "{state} {nnodes} {queue}" > listqueue_every.out &&
 	test $(grep "free 2" listqueue_every.out | grep -c batch) -eq 1 &&
 	test $(grep "free 2" listqueue_every.out | grep -c debug) -eq 1 &&
-	test $(grep "free 2" listqueue_every.out | grep -c every) -eq 2 &&
-	test $(grep "allocated 0" listqueue_every.out | grep -c every) -eq 0 &&
-	test $(grep "down 0" listqueue_every.out | grep -c every) -eq 0
+	test $(grep "free 2" listqueue_every.out | grep -c every) -eq 2
 '
 test_expect_success 'run a few jobs (every)' '
 	flux submit -q batch sleep 30 > job2A.id &&
@@ -532,8 +530,7 @@ test_expect_success 'flux resource lists expected queues in states (every)' '
 	test $(grep "free 1" listqueue_every2.out | grep -c every) -eq 2 &&
 	test $(grep "allocated 1" listqueue_every2.out | grep -c batch) -eq 1 &&
 	test $(grep "allocated 1" listqueue_every2.out | grep -c debug) -eq 1 &&
-	test $(grep "allocated 1" listqueue_every2.out | grep -c every) -eq 2 &&
-	test $(grep "down 0" listqueue_every2.out | grep -c every) -eq 0
+	test $(grep "allocated 1" listqueue_every2.out | grep -c every) -eq 2
 '
 test_expect_success 'flux resource list --queue works for a queue with no constraints' '
 	flux resource list --queue=every -o "{state} {nnodes} {queue}" >queue-every.out &&
@@ -542,8 +539,33 @@ test_expect_success 'flux resource list --queue works for a queue with no constr
 	test $(grep "free 1" queue-every.out | grep -c every) -eq 2 &&
 	test $(grep "allocated 1" queue-every.out | grep -c batch) -eq 1 &&
 	test $(grep "allocated 1" queue-every.out | grep -c debug) -eq 1 &&
-	test $(grep "allocated 1" queue-every.out | grep -c every) -eq 2 &&
-	test $(grep "down 0" queue-every.out | grep -c every) -eq 0
+	test $(grep "allocated 1" queue-every.out | grep -c every) -eq 2
+'
+test_expect_success 'flux resource list includes queue names for empty sets' '
+	# There are no nodes in 'down' state in this test, so use that:
+	flux resource list -s down -no "{queue} {state} {nnodes}" \
+		>queue-empty1.out &&
+	cat <<-EOF >queue-empty1.expected &&
+	every down 0
+	batch down 0
+	debug down 0
+	EOF
+	test_cmp queue-empty1.expected queue-empty1.out
+'
+test_expect_success 'flux resource list includes queue names for empty sets (single)' '
+	flux resource list -q batch -s down -no "{queue} {state} {nnodes}" \
+		>queue-batch-down.out &&
+	cat <<-EOF >queue-batch-down.expected &&
+	batch down 0
+	EOF
+	test_cmp queue-batch-down.expected queue-batch-down.out
+'
+test_expect_success 'flux resource list includes queue names for empty sets (multiple)' '
+	flux resource list -q batch,debug \
+		-s down -no "{queue} {state} {nnodes}" \
+		>queue-batch,debug-down.out &&
+	grep "debug down 0" queue-batch,debug-down.out &&
+	grep "batch down 0" queue-batch,debug-down.out
 '
 test_expect_success 'cleanup jobs' '
 	flux cancel $(cat job2A.id) $(cat job2B.id)