Merge pull request #4542 from chu11/issue4530_flux_jobs_nnodes

job-list: return nnodes if jobspec specifies nodes

Author: mergify[bot]

src/modules/job-list/job_state.c

@@ -376,8 +376,7 @@ static int parse_res_level (struct list_ctx *ctx,
 
 /* Return basename of path if there is a '/' in path.  Otherwise return
  * full path */
-static const char *
-parse_job_name (const char *path)
+static const char *parse_job_name (const char *path)
 {
     char *p = strrchr (path, '/');
     if (p) {
@@ -398,6 +397,7 @@ static int jobspec_parse (struct list_ctx *ctx,
     json_error_t error;
     json_t *jobspec = NULL;
     json_t *tasks, *resources, *command, *jobspec_job = NULL;
+    struct res_level res[3];
     int rc = -1;
 
     if (!(jobspec = json_loads (s, 0, &error))) {
@@ -489,13 +489,33 @@ static int jobspec_parse (struct list_ctx *ctx,
         goto nonfatal_error;
     }
 
+    /* For jobspec version 1, expect either:
+     * - node->slot->core->NIL
+     * - slot->core->NIL
+     */
+    memset (res, 0, sizeof (res));
+    if (parse_res_level (ctx, job, resources, &res[0]) < 0)
+        goto nonfatal_error;
+    if (res[0].with && parse_res_level (ctx, job, res[0].with, &res[1]) < 0)
+        goto nonfatal_error;
+    if (res[1].with && parse_res_level (ctx, job, res[1].with, &res[2]) < 0)
+        goto nonfatal_error;
+
+    /* Set job->nnodes if available.  In jobspec version 1, only if
+     * resources listed as node->slot->core->NIL
+     */
+    if (res[0].type != NULL && !strcmp (res[0].type, "node")
+        && res[1].type != NULL && !strcmp (res[1].type, "slot")
+        && res[2].type != NULL && !strcmp (res[2].type, "core")
+        && res[2].with == NULL)
+        job->nnodes = res[0].count;
+
     /* Set job->ntasks
      */
     if (json_unpack_ex (tasks, NULL, 0,
                         "[{s:{s:i}}]",
                         "count", "total", &job->ntasks) < 0) {
         int per_slot, slot_count = 0;
-        struct res_level res[3];
 
         if (json_unpack_ex (tasks, &error, 0,
                             "[{s:{s:i}}]",
@@ -511,18 +531,6 @@ static int jobspec_parse (struct list_ctx *ctx,
                       __FUNCTION__, (uintmax_t)job->id, per_slot);
             goto nonfatal_error;
         }
-        /* For jobspec version 1, expect either:
-         * - node->slot->core->NIL
-         * - slot->core->NIL
-         * Set job->slot_count and job->cores_per_slot.
-         */
-        memset (res, 0, sizeof (res));
-        if (parse_res_level (ctx, job, resources, &res[0]) < 0)
-            goto nonfatal_error;
-        if (res[0].with && parse_res_level (ctx, job, res[0].with, &res[1]) < 0)
-            goto nonfatal_error;
-        if (res[1].with && parse_res_level (ctx, job, res[1].with, &res[2]) < 0)
-            goto nonfatal_error;
         if (res[0].type != NULL && !strcmp (res[0].type, "slot")
             && res[1].type != NULL && !strcmp (res[1].type, "core")
             && res[1].with == NULL) {
@@ -549,7 +557,7 @@ static int jobspec_parse (struct list_ctx *ctx,
     }
 
     /* nonfatal error - jobspec illegal, but we'll continue on.  job
-     * listing will get initialized data */
+     * listing will return whatever data is available */
 nonfatal_error:
     rc = 0;
 error:

src/modules/job-list/job_util.c

@@ -91,9 +91,9 @@ static int store_attr (struct job *job,
         val = json_integer (job->ntasks);
     }
     else if (!strcmp (attr, "nnodes")) {
-        /* job->nnodes potentially < 0 if R invalid */
-        if (!(job->states_mask & FLUX_JOB_STATE_RUN)
-            || job->nnodes < 0)
+        /* job->nnodes < 0 if not set yet or R invalid, may be set in
+         * DEPEND or RUN state */
+        if (job->nnodes < 0)
             return 0;
         val = json_integer (job->nnodes);
     }

t/t2260-job-list.t

@@ -719,6 +719,22 @@ test_expect_success HAVE_JQ 'flux job list outputs nnodes/ranks/nodelist correct
         echo $obj | jq -e ".nodelist == \"${nodes}\""
 '
 
+# use flux queue to ensure jobs stay in pending state
+test_expect_success HAVE_JQ 'flux job list lists nnodes for pending jobs correctly' '
+        flux queue stop &&
+        id1=$(flux mini submit -N1 hostname | flux job id) &&
+        echo ${id1} >> nnodes.ids &&
+        id2=$(flux mini submit -N3 hostname | flux job id) &&
+        echo ${id2} >> nnodes.ids &&
+        flux job list -s pending | grep ${id1} &&
+        flux job list -s pending | grep ${id2} &&
+        flux job list-ids ${id1} | jq -e ".nnodes == 1" &&
+        flux job list-ids ${id2} | jq -e ".nnodes == 3" &&
+        flux job cancel ${id1} &&
+        flux job cancel ${id2} &&
+        flux queue start
+'
+
 #
 # job success
 #

t/t2800-jobs-cmd.t

@@ -1135,6 +1135,24 @@ test_expect_success HAVE_JQ 'flux jobs works on job with illegal R' '
         test_cmp list_illegal_R.out list_illegal_R.exp
 '
 
+#
+# special tests
+#
+
+# use flux queue to ensure jobs stay in pending state
+test_expect_success HAVE_JQ 'flux jobs lists nnodes for pending jobs correctly' '
+	flux queue stop &&
+	id1=$(flux mini submit -N1 hostname) &&
+	id2=$(flux mini submit -N3 hostname) &&
+	flux jobs -no "{state},{nnodes},{nnodes:h}" ${id1} ${id2}> nnodesP.out &&
+	echo "SCHED,1,1" >> nnodesP.exp &&
+	echo "SCHED,3,3" >> nnodesP.exp &&
+	flux job cancel ${id1} &&
+	flux job cancel ${id2} &&
+	flux queue start &&
+	test_cmp nnodesP.exp nnodesP.out
+'
+
 #
 # leave job cleanup to rc3
 #