Merge pull request #1268 from wihobbs/new-policy-refactor

policy: expose internals of resource module policy factory to allow custom policies

doc/man5/flux-config-sched-fluxion-resource.rst

@@ -102,6 +102,45 @@ hinodex
 first
     Select the first matching resources and stop the search
 
+firstnodex
+    A node-exclusive scheduling whose behavior is identical to
+    ``first`` except each compute node is exclusively allocated.
+
+
+CUSTOM MATCH POLICIES
+=====================
+Match policies are initialized as collections of individual attributes
+that help the scheduler to select matches. For convenience, these 
+attributes are exposed to users such that they can write custom policies.
+Below is a list of match attributes which can be selected by users.
+
+policy
+    Allowed options are ``low`` or ``high``. If only ``policy=low``
+    or ``policy=high`` is specified, the behavior of the match policy is the
+    same as if ``match-policy=low`` or ``match-policy=high`` were selected,
+    respectively.
+
+node_centric
+    ``true`` or ``false`` are allowed options. Evaluate matches based on the
+    ID of the compute node first. 
+
+node_exclusive
+    ``true`` or ``false`` are allowed options. Exclusively allocate compute
+    nodes when a match is found.
+
+set_stop_on_1_matches
+    ``true`` or ``false`` are allowed options. When a match is found, take
+    it, without evaluating for potentially more optimal matches.
+
+::
+
+    [sched-fluxion-resource]
+
+    # system instance will use node-exclusive
+    # scheduling first-match (with nodes of high node IDs
+    # selected first).
+    match-policy = "policy=high node_exclusive=true set_stop_on_1_matches=true"
+
 
 EXAMPLE
 =======

resource/modules/resource_match.cpp

@@ -297,9 +297,10 @@ static const struct flux_msg_handler_spec htab[] =
 static void set_default_args (std::shared_ptr<resource_ctx_t> &ctx)
 {
     resource_opts_t ct_opts;
+    std::string e = "";
     ct_opts.set_load_format ("rv1exec");
     ct_opts.set_match_subsystems ("containment");
-    ct_opts.set_match_policy ("first");
+    ct_opts.set_match_policy ("first", e);
     ct_opts.set_prune_filters ("ALL:core");
     ct_opts.set_match_format ("rv1_nosched");
     ct_opts.set_update_interval (0);

resource/modules/resource_match_opts.cpp

@@ -90,9 +90,9 @@ void resource_prop_t::set_load_allowlist (const std::string &p)
     m_load_allowlist = p;
 }
 
-bool resource_prop_t::set_match_policy (const std::string &p)
+bool resource_prop_t::set_match_policy (const std::string &p, std::string &error_str)
 {
-    if (!known_match_policy (p))
+    if (!known_match_policy (p, error_str))
         return false;
     m_match_policy = p;
     return true;
@@ -348,9 +348,9 @@ void resource_opts_t::set_load_allowlist (const std::string &o)
     m_resource_prop.set_load_allowlist (o);
 }
 
-bool resource_opts_t::set_match_policy (const std::string &o)
+bool resource_opts_t::set_match_policy (const std::string &o, std::string &e)
 {
-    return m_resource_prop.set_match_policy (o);
+    return m_resource_prop.set_match_policy (o, e);
 }
 
 bool resource_opts_t::set_match_format (const std::string &o)
@@ -436,8 +436,10 @@ resource_opts_t &resource_opts_t::operator+= (const resource_opts_t &src)
         m_resource_prop.set_load_format (src.m_resource_prop.get_load_format ());
     if (src.m_resource_prop.is_load_allowlist_set ())
         m_resource_prop.set_load_allowlist (src.m_resource_prop.get_load_allowlist ());
-    if (src.m_resource_prop.is_match_policy_set ())
-        m_resource_prop.set_match_policy (src.m_resource_prop.get_match_policy ());
+    if (src.m_resource_prop.is_match_policy_set ()) {
+        std::string e = "";
+        m_resource_prop.set_match_policy (src.m_resource_prop.get_match_policy (), e);
+    }
     if (src.m_resource_prop.is_match_format_set ())
         m_resource_prop.set_match_format (src.m_resource_prop.get_match_format ());
     if (src.m_resource_prop.is_match_subsystems_set ())
@@ -512,9 +514,11 @@ int resource_opts_t::parse (const std::string &k, const std::string &v, std::str
             break;
 
         case static_cast<int> (resource_opts_key_t::MATCH_POLICY):
-            if (!m_resource_prop.set_match_policy (v)) {
+            if (!m_resource_prop.set_match_policy (v, info)) {
                 info += "Unknown match policy (" + v + ")! ";
-                info += "Using default.";
+                errno = EINVAL;
+                rc = -1;
+                return rc;
             }
             break;
 

resource/modules/resource_match_opts.hpp

@@ -40,7 +40,7 @@ class resource_prop_t {
     void set_load_file (const std::string &o);
     bool set_load_format (const std::string &o);
     void set_load_allowlist (const std::string &o);
-    bool set_match_policy (const std::string &o);
+    bool set_match_policy (const std::string &o, std::string &e);
     bool set_match_format (const std::string &o);
     void set_match_subsystems (const std::string &o);
     void set_reserve_vtx_vec (const int i);
@@ -111,7 +111,7 @@ class resource_opts_t : public optmgr_parse_t {
     void set_load_file (const std::string &o);
     bool set_load_format (const std::string &o);
     void set_load_allowlist (const std::string &o);
-    bool set_match_policy (const std::string &o);
+    bool set_match_policy (const std::string &o, std::string &e);
     bool set_match_format (const std::string &o);
     void set_match_subsystems (const std::string &o);
     void set_reserve_vtx_vec (const int i);

resource/policies/base/test/CMakeLists.txt

@@ -1,3 +1,9 @@
+add_executable(matcher_policy_factory_test
+  ${CMAKE_CURRENT_SOURCE_DIR}/matcher_policy_factory_test02.cpp
+  )
+target_link_libraries(matcher_policy_factory_test PRIVATE libtap resource)
+add_sanitizers(matcher_policy_factory_test)
+flux_add_test(NAME matcher_policy_factory_test COMMAND matcher_policy_factory_test)
 add_executable(matcher_util_api_test
   ${CMAKE_CURRENT_SOURCE_DIR}/matcher_util_api_test01.cpp
   )

resource/policies/base/test/matcher_policy_factory_test02.cpp

@@ -0,0 +1,93 @@
+/*****************************************************************************\
+ * Copyright 2024 Lawrence Livermore National Security, LLC
+ * (c.f. AUTHORS, NOTICE.LLNS, LICENSE)
+ *
+ * This file is part of the Flux resource manager framework.
+ * For details, see https://github.com/flux-framework.
+ *
+ * SPDX-License-Identifier: LGPL-3.0
+\*****************************************************************************/
+
+/*
+ * Test for the dfu_match_policy class(es). Compares shared pointers
+ * expected values to string inputs to these classes. Strings can be
+ * specified in config for custom policies, or some are provided.
+ */
+
+extern "C" {
+#if HAVE_CONFIG_H
+#include <config.h>
+#endif
+}
+#include <iostream>
+#include <string>
+#include <boost/lexical_cast.hpp>
+#include "resource/policies/dfu_match_policy_factory.hpp"
+#include "src/common/libtap/tap.h"
+
+using namespace Flux;
+using namespace Flux::resource_model;
+
+int test_parsers ()
+{
+    std::map<std::string, bool> container;
+    std::string e = "";
+    bool first = Flux::resource_model::parse_custom_match_policy (
+        "policy=high node_centric=true node_exclusive=true stop_on_1_matches=false blah=4",
+        container,
+        e);
+    ok (first == false, "blah is an unrecognized policy option");
+    ok (e == "invalid policy option: blah\n", "correct error message for invalid option");
+
+    std::map<std::string, bool> container2;
+    std::string e2 = "";
+    bool second = Flux::resource_model::parse_custom_match_policy (
+        "policy=1 node_centric=true node_exclusive=true stop_on_1_matches=false", container2, e2);
+    ok (second == false, "1 is an invalid option, must be high or low");
+    ok (e2 == "policy key within custom policy accepts only low or high\n",
+        "correct error message for high/low");
+
+    std::map<std::string, bool> container3;
+    std::string e3 = "";
+    bool third = Flux::resource_model::
+        parse_custom_match_policy ("policy=high node_centric=true stop_on_1_matches=true",
+                                   container3,
+                                   e3);
+    ok (third == true, "first is a valid policy");
+
+    bool fourth = Flux::resource_model::parse_bool_match_options ("high", container3);
+    ok (fourth == true, "policy first uses option high");
+
+    bool fifth = Flux::resource_model::parse_bool_match_options ("node_centric", container3);
+    ok (fifth == true, "policy first uses option node_centric");
+
+    bool sixth = Flux::resource_model::parse_bool_match_options ("stop_on_1_matches", container3);
+    ok (sixth == true, "policy first uses option stop_on_1_matches");
+
+    bool seventh = Flux::resource_model::parse_bool_match_options ("low", container3);
+    ok (seventh == false, "policy first does not use option low");
+
+    std::map<std::string, bool> container4;
+    std::string e4 = "";
+    bool eighth = Flux::resource_model::
+        parse_custom_match_policy ("policy=high node_centric=true node_exclusive=4",
+                                   container4,
+                                   e4);
+    ok (eighth == false, "node_exclusive accepts only true or false");
+    ok (e4 == "Policy option node_exclusive requires true or false, got 4\n",
+        "correct error msg for options");
+
+    return 0;
+}
+
+int main (int argc, char *argv[])
+{
+    plan (11);
+    test_parsers ();
+    done_testing ();
+    return 0;
+}
+
+/*
+ * vi: ts=4 sw=4 expandtab
+ */