job-manager: require duration limit to be FSD

Problem: RFC 33 states that policy durations are FSD strings, but the
duration-limit jobtap plugin permits them to be numerical also.

Change the duration-limit plugin to require FSD strings. This affects:

policy.jobspec.defaults.system.duration
policy.limits.duration

as well as the per-queue policy tables.

Update the flux-config-policy(5) man page.
Update the t2221-job-manager-limit-duration.t sharness test.

Author: garlick

doc/man5/flux-config-policy.rst

@@ -20,9 +20,9 @@ DEFAULTS
 Default values for job requests may be configured in the
 ``policy.jobspec.defaults.system`` table.  If a job request does not specify
 a value for a system attribute that is configured in the table, the configured
-value is substituted.  Some common examples are:
+value is substituted.  Currently the following values are allowed:
 
-policy.jobspec.defaults.system.duration (float seconds or FSD string)
+policy.jobspec.defaults.system.duration (FSD string)
    (optional) If a job is submitted without specifying a job duration,
    this value is substituted.
 
@@ -42,8 +42,8 @@ rejected at submission time if they violate configured limits.
    queue.  The actual value used to indicate `unlimited` varies by limit
    type and is noted below.
 
-policy.limits.duration (float seconds or FSD string)
-   (optional) Maximum duration that a job may request (0 = unlimited).
+policy.limits.duration (FSD string)
+   (optional) Maximum duration that a job may request ("0" = unlimited).
 
 .. note::
    Limit checks take place before the scheduler sees the request, so it is
@@ -81,7 +81,7 @@ EXAMPLE
 
    [policy.defaults]
    duration = "1h"
-   queue = "pbatch"
+   queue = "batch"
 
    [policy.limits]
    duration = "4h"

src/modules/job-manager/plugins/limit-duration.c

@@ -12,15 +12,15 @@
  *
  * This plugin uses the job.validate callback to accept or reject job
  * requests.  Any default jobspec values would have been applied earlier
- * (where applicable) in the job.create callback.
+ * (where applicable) at ingest.
  *
  * General limit:
  *  policy.limits.duration
  * Queue-specific limit:
  *  queues.<name>.policy.limits.duration
  *
  * N.B. a queue limit may override the general limit with a higher or lower
- * limit, even "unlimited" (0).
+ * limit, or "0" for unlimited.
  *
  * See also:
  *  RFC 33/Flux Job Queues
@@ -131,36 +131,24 @@ static int duration_parse (double *duration,
                            flux_error_t *error)
 {
     double d = DURATION_INVALID;
-    json_t *o = NULL;
+    const char *ds = NULL;
     json_error_t jerror;
     const char *name = "policy.limits.duration";
 
     if (json_unpack_ex (conf,
                         &jerror,
                         0,
-                        "{s?{s?{s?o}}}",
+                        "{s?{s?{s?s}}}",
                         "policy",
                           "limits",
-                            "duration", &o) < 0) {
+                            "duration", &ds) < 0) {
         errprintf (error, "%s: %s", name, jerror.text);
-        return -1;
+        goto inval;
     }
-    if (o) {
-        if (json_is_string (o)) {
-            if (fsd_parse_duration (json_string_value (o), &d) < 0) {
-                errprintf (error, "%s: FSD value is malformed", name);
-                return -1;
-            }
-        }
-        else if (json_is_number (o)) {
-            if ((d = json_number_value (o)) < 0) {
-                errprintf (error, "%s: must be >= 0", name);
-                goto inval;
-            }
-        }
-        else {
-            errprintf (error, "%s: must be FSD (string) or seconds", name);
-            goto inval;
+    if (ds) {
+        if (fsd_parse_duration (ds, &d) < 0) {
+            errprintf (error, "%s: FSD value is malformed", name);
+            return -1;
         }
     }
     *duration = d;

t/t2221-job-manager-limit-duration.t

@@ -10,29 +10,20 @@ test_under_flux 2 full -o,--config-path=$(pwd)/config
 
 flux setattr log-stderr-level 1
 
-test_expect_success 'unload all built-in plugins' '
-	flux jobtap remove ".*"
-'
-test_expect_success 'configure an invalid numerical duration limit' '
+test_expect_success 'configuring an invalid duration limit fails' '
 	cat >config/policy.toml <<-EOT &&
 	[policy.limits]
-	duration = -1.0
+	duration = 1.0
 	EOT
-	flux config reload
-'
-test_expect_success 'cannot load the limit-duration plugin' '
-	test_must_fail flux jobtap load .limit-duration
+	test_must_fail flux config reload
 '
-test_expect_success 'configure a valid duration limit (FSD)' '
+test_expect_success 'configure a valid duration limit' '
 	cat >config/policy.toml <<-EOT &&
 	[policy.limits]
 	duration = "1m"
 	EOT
 	flux config reload
 '
-test_expect_success 'the limit-duration plugin can now be laoded' '
-	flux jobtap load .limit-duration
-'
 test_expect_success 'a job that exceeds policy.limits.duration is rejected' '
 	test_must_fail flux mini submit -t 1h /bin/true 2>duration.err &&
 	grep "exceeds policy limit of 1m" duration.err
@@ -83,7 +74,7 @@ test_expect_success 'configure policy.limits.duration and an unlimited queue' '
 	[policy.limits]
 	duration = "1h"
 	[queues.pdebug.policy.limits]
-	duration = 0
+	duration = "0"
 	EOT
 	flux config reload
 '
@@ -100,7 +91,7 @@ test_expect_success 'a job that sets no explicit duration is accepted by the unl
 	    --setattr=system.queue=pdebug \
 	    /bin/true
 '
-test_expect_success 'configure an invalid string duration limit' '
+test_expect_success 'configure an invalid duration limit' '
 	cat >config/policy.toml <<-EOT &&
 	[policy.limits]
 	duration = "xyz123"