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,75 @@
+/*****************************************************************************\
+ * 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 <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;
+    bool first = Flux::resource_model::parse_custom_match_policy("high=true node_centric=true node_exclusive=true stop_on_1_matches=false blah=4", container);
+    ok (first == false, "blah is an unrecognized policy option");
+
+    std::map<std::string, bool> container2;
+    bool second = Flux::resource_model::parse_custom_match_policy("high=1 node_centric=true node_exclusive=true stop_on_1_matches=false", container2);
+    ok (second == false, "1 is an invalid option, must be true or false");
+
+    std::map<std::string, bool> container3;
+    bool third = Flux::resource_model::parse_custom_match_policy("high=true node_centric=true stop_on_1_matches=true", container3);
+    ok (third == true, "first is a valid policy");
+
+    // little debugging helper for printing the container
+    // std::map<std::string, bool>::iterator it;
+    // for (it=container.begin(); it != container.end(); it++) {
+    //     std::cout << it->first << ": " << it->second << std::endl;
+    // }
+    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");
+
+    return 0;
+}
+
+int main (int argc, char *argv[])
+{
+    plan (7);
+    test_parsers ();
+    done_testing ();
+    return 0;
+}
+
+/*
+ * vi: ts=4 sw=4 expandtab
+ */

resource/policies/dfu_match_policy_factory.cpp

@@ -13,59 +13,146 @@ extern "C" {
 #include <config.h>
 #endif
 }
-
+#include <array>
 #include <string>
+#include <boost/algorithm/string.hpp>
 #include "resource/policies/dfu_match_policy_factory.hpp"
 
 namespace Flux {
 namespace resource_model {
 
+policies = {{"first", "high=true node_centric=true stop_on_1_matches=true"},
+            {"firstnodex",
+             "high=true node_centric=true node_exclusive=true stop_on_1_matches=true"},
+            {"high", "high=true"},
+            {"low", "low=true"},
+            {"lonode", "low=true node_centric=true"},
+            {"hinode", "high=true node_centric=true"},
+            {"lonodex", "low=true node_centric=true node_exclusive=true"},
+            {"hinodex", "high=true node_centric=true node_exclusive=true"},
+            {"locality", ""},
+            {"variation", ""}};
+
+const std::vector<std::string> policy_options =
+    {"name", "high", "node_centric", "stop_on_1_matches", "node_exclusive", "low"};
+
+/* This takes a string of match policy options, deliniated by
+ * spaces and equals signs, and turns them into a std::map.
+ * It validates that each option is set to either "true" or "false"
+ * and puts the corresponding boolean in the map.
+ */
+bool parse_custom_match_policy (const std::string long_string, std::map<std::string, bool> &split)
+{
+    std::vector<std::string> temp;
+    boost::split (temp, long_string, boost::is_any_of (" "));
+    /* After splitting based on term, validate the terms. If invalid,
+     * abort.
+     */
+    for (int i = 0; i < temp.size (); i++) {
+        std::vector<std::string> temp_two;
+        /* This split should take an entire string and split it
+         * on the equals sign. If there's no equals sign, maybe it will
+         * get caught later?
+         */
+        boost::split (temp_two, temp.at (i), boost::is_any_of ("="));
+        /* Place the terms in a map for ease of access. Lots of gross
+         * temporary structures needed here, hopefully they'll get
+         * garbage collected soon.
+         */
+        if (std::find (policy_options.begin (), policy_options.end (), temp_two.at (0))
+            == policy_options.end ()) {
+            // better way to log something?
+            std::cout << "invalid policy option: " << temp_two.at (0) << std::endl;
+            return false;
+        } else {
+            if (temp_two.at (1).compare ("true") == 0) {
+                const auto [it, success] = split.insert ({temp_two.at (0), true});
+                if (success == false) {
+                    std::cout << "Insertion of " << it->first << " failed" << std::endl;
+                }
+            } else if (temp_two.at (1).compare ("false") == 0) {
+                const auto [it, success] = split.insert ({temp_two.at (0), false});
+                if (success == false) {
+                    std::cout << "Insertion of " << it->first << " failed" << std::endl;
+                    return false;
+                }
+            } else {
+                std::cout << "Policy option " << temp_two.at (0) << " requires true or false, got "
+                          << temp_two.at (1) << std::endl;
+                return false;
+            }
+        }
+    }
+    return true;
+}
+
 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;
+    std::map<std::string, bool> throw_away;
+    if (policies.contains (policy)) {
+        return true;
+    } else if (parse_custom_match_policy (policy, throw_away) == true) {
+        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, std::map<std::string, bool> &policy)
 {
-    std::shared_ptr<dfu_match_cb_t> matcher = nullptr;
+    if ((policy.find (match_option) != policy.end ()) && (policy[match_option] == true)) {
+        return true;
+    }
+    return false;
+}
 
-    resource_type_t node_rt ("node");
+std::shared_ptr<dfu_match_cb_t> create_match_cb (const std::string &policy_requested)
+{
+    std::map<std::string, bool> policy;
+    if (policies.contains (policy_requested)) {
+        parse_custom_match_policy (policies.find (policy_requested)->second, policy);
+    } else {
+        parse_custom_match_policy (policy_requested, policy);
+    }
+    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 (parse_bool_match_options ("stop_on_1_matches", policy)) {
+                ptr->set_stop_on_k_matches (1);
+            }
             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 (parse_bool_match_options ("stop_on_1_matches", policy)) {
+                ptr->set_stop_on_k_matches (1);
+            }
             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,24 @@
 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";
+extern const std::map<std::string, std::string> policies;
 
 bool known_match_policy (const std::string &policy);
 
+bool get_match_options (const std::string match_option, std::map<std::string, bool> &options);
+
+/* Returns true if the custom match policy is valid.
+ * Returns false if it is invalid.
+ * Stores all of the options requested in the map under
+ * key-value pairs.
+ */
+bool parse_custom_match_policy (const std::string long_string, std::map<std::string, bool> &split);
+
+/* Included in the header for unit testing.
+ * This is how the map keys get accessed and returned easily.
+ */
+bool parse_bool_match_options (const std::string match_option, std::map<std::string, bool> &policy);
+
 /*! Factory method for creating a matching callback
  *  object, representing a matching policy.
  */