policy: refactor options to allow custom

Problem: the current fluxion policy matching factory
doesn't allow for customizable policies.

Add a custom option and refactor existing policies to
behave similarly. Add unit testing for the string parsing
convenience functions.

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,63 @@
+/*****************************************************************************\
+ * 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 <string>
+#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::string policy_opts = policies.find ("first")->second;
+    bool first = Flux::resource_model::parse_bool_match_options ("high", policy_opts);
+    ok (first == true, "policy first uses option high");
+
+    policy_opts = policies.find ("high")->second;
+    bool second = Flux::resource_model::option_exists ("stop_on_k_matches", policy_opts);
+    ok (second == false, "policy high does not have stop_on_k_matches as substring");
+
+    policy_opts = policies.find ("firstnodex")->second;
+    bool third = Flux::resource_model::parse_bool_match_options ("node_exclusive", policy_opts);
+    ok (third == true, "policy first uses option node_centric");
+
+    bool fourth = Flux::resource_model::option_exists ("stop_on_k_matches", policy_opts);
+    ok (fourth == true, "policy firstnodex has stop_on_k_matches as substring");
+
+    int fifth = Flux::resource_model::parse_int_match_options ("stop_on_k_matches", policy_opts);
+    ok (fifth == 1, "policy firstnodex stops on first match");
+
+    return 0;
+}
+
+int main (int argc, char *argv[])
+{
+    plan (5);
+    test_parsers ();
+    done_testing ();
+    return 0;
+}
+
+/*
+ * vi: ts=4 sw=4 expandtab
+ */

resource/policies/dfu_match_policy_factory.cpp

@@ -13,7 +13,6 @@ extern "C" {
 #include <config.h>
 #endif
 }
-
 #include <string>
 #include "resource/policies/dfu_match_policy_factory.hpp"
 
@@ -22,50 +21,98 @@ namespace resource_model {
 
 bool known_match_policy (const std::string &policy)
 {
-    bool rc = true;
-    if (policy != FIRST_MATCH && policy != FIRST_NODEX_MATCH && policy != HIGH_ID_FIRST
-        && policy != LOW_ID_FIRST && policy != LOW_NODE_FIRST && policy != HIGH_NODE_FIRST
-        && policy != LOW_NODEX_FIRST && policy != HIGH_NODEX_FIRST && policy != LOCALITY_AWARE
-        && policy != VAR_AWARE)
-        rc = false;
-
-    return rc;
+    if (policies.contains (policy)) {
+        return true;
+    }
+    return false;
 }
 
-std::shared_ptr<dfu_match_cb_t> create_match_cb (const std::string &policy)
+bool parse_bool_match_options (const std::string match_option, const std::string policy_options)
 {
-    std::shared_ptr<dfu_match_cb_t> matcher = nullptr;
+    // Return anything from after the = and before space or newline
+    size_t spot = policy_options.find (match_option, 0);
+    size_t start_pos = policy_options.find ("=", spot);
+    size_t end_pos = policy_options.find (" ", spot);
+    size_t end_str = policy_options.length ();
+    std::string return_opt;
+    if ((end_pos == std::string::npos) && (start_pos != std::string::npos)) {
+        return_opt = policy_options.substr ((start_pos + 1), (end_str - start_pos - 1));
+    } else {
+        return_opt = policy_options.substr ((start_pos + 1), (end_pos - start_pos - 1));
+    }
+    if (return_opt == "true") {
+        return true;
+    }
+    return false;
+}
+
+bool option_exists (const std::string match_option, const std::string policy_options)
+{
+    size_t found = policy_options.find (match_option, 0);
+    if (found == std::string::npos) {
+        return false;
+    }
+    return true;
+}
+
+int parse_int_match_options (const std::string match_option, const std::string policy_options)
+{
+    size_t spot = policy_options.find (match_option, 0);
+    size_t start_pos = policy_options.find ("=", spot);
+    size_t end_pos = policy_options.find (" ", spot);
+    size_t end_str = policy_options.length ();
+    int return_opt;
+    if ((end_pos == std::string::npos) && (start_pos != std::string::npos)) {
+        return_opt = stoi (policy_options.substr ((start_pos + 1), (end_str - start_pos - 1)));
+    } else {
+        return_opt = stoi (policy_options.substr ((start_pos + 1), (end_pos - start_pos - 1)));
+    }
+    return return_opt;
+}
 
-    resource_type_t node_rt ("node");
+std::shared_ptr<dfu_match_cb_t> create_match_cb (const std::string &policy_requested)
+{
+    std::string policy = policies.find (policy_requested)->second;
+    std::shared_ptr<dfu_match_cb_t> matcher = nullptr;
     try {
-        if (policy == FIRST_MATCH || policy == FIRST_NODEX_MATCH) {
+        if (policy_requested == "locality") {
+            matcher = std::make_shared<greater_interval_first_t> ();
+        }
+        if (policy_requested == "variation") {
+            matcher = std::make_shared<var_aware_t> ();
+        }
+
+        if (parse_bool_match_options ("high", policy)) {
             std::shared_ptr<high_first_t> ptr = std::make_shared<high_first_t> ();
-            ptr->add_score_factor (node_rt, 1, 10000);
-            ptr->set_stop_on_k_matches (1);
-            if (policy == FIRST_NODEX_MATCH)
+            if (parse_bool_match_options ("node_centric", policy)) {
+                ptr->add_score_factor (node_rt, 1, 10000);
+            }
+
+            if (parse_bool_match_options ("node_exclusive", policy)) {
                 ptr->add_exclusive_resource_type (node_rt);
+            }
+
+            if (option_exists ("stop_on_k_matches", policy)) {
+                ptr->set_stop_on_k_matches (parse_int_match_options ("stop_on_k_matches", policy));
+            }
             matcher = ptr;
-        } else if (policy == HIGH_ID_FIRST) {
-            matcher = std::make_shared<high_first_t> ();
-        } else if (policy == LOW_ID_FIRST) {
-            matcher = std::make_shared<low_first_t> ();
-        } else if (policy == LOW_NODE_FIRST || policy == LOW_NODEX_FIRST) {
+
+        } else if (parse_bool_match_options ("low", policy)) {
             std::shared_ptr<low_first_t> ptr = std::make_shared<low_first_t> ();
-            ptr->add_score_factor (node_rt, 1, 10000);
-            if (policy == LOW_NODEX_FIRST)
-                ptr->add_exclusive_resource_type (node_rt);
-            matcher = ptr;
-        } else if (policy == HIGH_NODE_FIRST || policy == HIGH_NODEX_FIRST) {
-            std::shared_ptr<high_first_t> ptr = std::make_shared<high_first_t> ();
-            ptr->add_score_factor (node_rt, 1, 10000);
-            if (policy == HIGH_NODEX_FIRST)
+            if (parse_bool_match_options ("node_centric", policy)) {
+                ptr->add_score_factor (node_rt, 1, 10000);
+            }
+
+            if (parse_bool_match_options ("node_exclusive", policy)) {
                 ptr->add_exclusive_resource_type (node_rt);
+            }
+
+            if (option_exists ("stop_on_k_matches", policy)) {
+                ptr->set_stop_on_k_matches (parse_int_match_options ("stop_on_k_matches", policy));
+            }
             matcher = ptr;
-        } else if (policy == LOCALITY_AWARE) {
-            matcher = std::make_shared<greater_interval_first_t> ();
-        } else if (policy == VAR_AWARE) {
-            matcher = std::make_shared<var_aware_t> ();
         }
+
     } catch (std::bad_alloc &e) {
         errno = ENOMEM;
         matcher = nullptr;

resource/policies/dfu_match_policy_factory.hpp

@@ -13,6 +13,7 @@
 
 #include <string>
 #include <memory>
+#include <map>
 #include "resource/policies/base/dfu_match_cb.hpp"
 #include "resource/policies/dfu_match_high_id_first.hpp"
 #include "resource/policies/dfu_match_low_id_first.hpp"
@@ -24,19 +25,28 @@
 namespace Flux {
 namespace resource_model {
 
-const std::string FIRST_MATCH = "first";
-const std::string FIRST_NODEX_MATCH = "firstnodex";
-const std::string HIGH_ID_FIRST = "high";
-const std::string LOW_ID_FIRST = "low";
-const std::string LOW_NODE_FIRST = "lonode";
-const std::string HIGH_NODE_FIRST = "hinode";
-const std::string LOW_NODEX_FIRST = "lonodex";
-const std::string HIGH_NODEX_FIRST = "hinodex";
-const std::string LOCALITY_AWARE = "locality";
-const std::string VAR_AWARE = "variation";
-
 bool known_match_policy (const std::string &policy);
 
+const std::map<std::string, std::string> policies =
+    {{"first", "name=FIRST_MATCH high=true node_centric=true stop_on_k_matches=1"},
+     {"firstnodex",
+      "name=FIRST_NODEX_MATCH high=true node_centric=true node_exclusive=true stop_on_k_matches=1"},
+     {"high", "name=HIGH_ID_FIRST high=true"},
+     {"low", "name=LOW_ID_FIRST low=true"},
+     {"lonode", "name=LOW_NODE_FIRST low=true node_centric=true"},
+     {"hinode", "name=HIGH_NODE_FIRST high=true node_centric=true"},
+     {"lonodex", "name=LOW_NODEX_FIRST low=true node_centric=true node_exclusive=true"},
+     {"hinodex", "name=HIGH_NODEX_FIRST high=true node_centric=true node_exclusive=true"},
+     {"locality", "name=LOCALITY_AWARE"},
+     {"variation", "name=VAR_AWARE"},
+     {"custom", ""}};
+
+bool parse_bool_match_options (const std::string match_option, const std::string policy_options);
+
+bool option_exists (const std::string match_option, const std::string policy_options);
+
+int parse_int_match_options (const std::string match_option, const std::string policy_options);
+
 /*! Factory method for creating a matching callback
  *  object, representing a matching policy.
  */