Skip to content

Commit

Permalink
resource: enforce co-dependency of range keys
Browse files Browse the repository at this point in the history
Problem: recent change to RFC 14 explicitly makes max/operator/operand
co-dependent; i.e., if one is given then all three must be, but this is
not currently checked or enforced

Add checks ensuring that if one of max/operator/operand is specified
then all are, and adjusts the testsuite to match
  • Loading branch information
sam-maloney committed Feb 14, 2025
1 parent c9c8d8b commit 3a46225
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 41 deletions.
30 changes: 24 additions & 6 deletions resource/libjobspec/jobspec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,32 @@ void parse_yaml_count (Resource &res, const YAML::Node &cnode)
if (!cnode["min"].IsScalar ()) {
throw parse_error (cnode["min"], "Value of \"min\" must be a scalar");
}
if (cnode["max"] && !cnode["max"].IsScalar ()) {
throw parse_error (cnode["max"], "Value of \"max\" must be a scalar");
if (cnode["max"]) {
if (!cnode["max"].IsScalar ()) {
throw parse_error (cnode["max"], "Value of \"max\" must be a scalar");

Check warning on line 71 in resource/libjobspec/jobspec.cpp

View check run for this annotation

Codecov / codecov/patch

resource/libjobspec/jobspec.cpp#L71

Added line #L71 was not covered by tests
}
if (!cnode["operator"] || !cnode["operand"]) {
throw parse_error (cnode,
"\"operator\" and \"operand\" required when \"max\" given");
}
}
if (cnode["operator"] && !cnode["operator"].IsScalar ()) {
throw parse_error (cnode["operator"], "Value of \"operator\" must be a scalar");
if (cnode["operator"]) {
if (!cnode["operator"].IsScalar ()) {
throw parse_error (cnode["operator"], "Value of \"operator\" must be a scalar");

Check warning on line 80 in resource/libjobspec/jobspec.cpp

View check run for this annotation

Codecov / codecov/patch

resource/libjobspec/jobspec.cpp#L80

Added line #L80 was not covered by tests
}
if (!cnode["max"] || !cnode["operand"]) {
throw parse_error (cnode,
"\"max\" and \"operand\" required when \"operator\" given");
}
}
if (cnode["operand"] && !cnode["operand"].IsScalar ()) {
throw parse_error (cnode["operand"], "Value of \"operand\" must be a scalar");
if (cnode["operand"]) {
if (!cnode["operand"].IsScalar ()) {
throw parse_error (cnode["operand"], "Value of \"operand\" must be a scalar");

Check warning on line 89 in resource/libjobspec/jobspec.cpp

View check run for this annotation

Codecov / codecov/patch

resource/libjobspec/jobspec.cpp#L89

Added line #L89 was not covered by tests
}
if (!cnode["max"] || !cnode["operator"]) {
throw parse_error (cnode,
"\"max\" and \"operator\" required when \"operand\" given");

Check warning on line 93 in resource/libjobspec/jobspec.cpp

View check run for this annotation

Codecov / codecov/patch

resource/libjobspec/jobspec.cpp#L92-L93

Added lines #L92 - L93 were not covered by tests
}
}

/* Validate values of entries */
Expand Down
36 changes: 1 addition & 35 deletions resource/policies/base/test/matcher_util_api_test01.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ using namespace Flux::resource_model;
class test_jobspec_template {
public:
explicit test_jobspec_template (unsigned int min);
explicit test_jobspec_template (unsigned int min, unsigned int max);
explicit test_jobspec_template (unsigned int min, unsigned int max, std::string oper);
explicit test_jobspec_template (unsigned int min,
unsigned int max,
std::string oper,
Expand All @@ -42,8 +40,6 @@ class test_jobspec_template {
private:
int emit (json_t *count_obj);
json_t *emit_count (unsigned int min);
json_t *emit_count (unsigned int min, unsigned int max);
json_t *emit_count (unsigned int min, unsigned int max, const std::string &oper);
json_t *emit_count (unsigned int min,
unsigned int max,
const std::string &oper,
Expand Down Expand Up @@ -114,18 +110,6 @@ json_t *test_jobspec_template::emit_count (unsigned int min)
return json_pack ("{s:i}", "min", min);
}

json_t *test_jobspec_template::emit_count (unsigned int min, unsigned int max)
{
return json_pack ("{s:i s:i}", "min", min, "max", max);
}

json_t *test_jobspec_template::emit_count (unsigned int min,
unsigned int max,
const std::string &oper)
{
return json_pack ("{s:i s:i s:s}", "min", min, "max", max, "operator", oper.c_str ());
}

json_t *test_jobspec_template::emit_count (unsigned int min,
unsigned int max,
const std::string &oper,
Expand Down Expand Up @@ -155,15 +139,6 @@ test_jobspec_template::test_jobspec_template (unsigned int min)
throw std::bad_alloc ();
}

test_jobspec_template::test_jobspec_template (unsigned int min, unsigned int max)
{
json_t *count_obj{nullptr};
if ((count_obj = emit_count (min, max)) == nullptr)
throw std::bad_alloc ();
if ((emit (count_obj)) < 0)
throw std::bad_alloc ();
}

test_jobspec_template::test_jobspec_template (unsigned int min,
unsigned int max,
std::string oper,
Expand All @@ -176,15 +151,6 @@ test_jobspec_template::test_jobspec_template (unsigned int min,
throw std::bad_alloc ();
}

test_jobspec_template::test_jobspec_template (unsigned int min, unsigned int max, std::string oper)
{
json_t *count_obj{nullptr};
if ((count_obj = emit_count (min, max, oper)) == nullptr)
throw std::bad_alloc ();
if ((emit (count_obj)) < 0)
throw std::bad_alloc ();
}

const std::string &test_jobspec_template::get () const
{
return m_job_spec_string;
Expand All @@ -200,7 +166,7 @@ void test_plus_op ()
matcher_util_api_t api;

// {min=1, max=10, operator='+', operand=1}
test_jobspec_template jobin_001 (1, 10);
test_jobspec_template jobin_001 (1, 10, "+", 1);
Flux::Jobspec::Jobspec job_001{jobin_001.get ()};
count = api.calc_count (job_001.resources[0], std::numeric_limits<unsigned int>::max ());
ok (count == 10, "{min=1, max=10, oper='+', op=1} is 10");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
version: 9999
resources:
- type: slot
count:
min: 1
label: foo
with:
- type: node
count: 1
tasks:
- command: [ "app" ]
slot: foo
count:
per_slot: 1
attributes:

0 comments on commit 3a46225

Please sign in to comment.