MXS-2900 Cleanup test labels handling

The labels are stored in sets for easy searching.

maxscale-system-test/CMakeLists.txt

@@ -33,7 +33,7 @@ include_directories(${CMAKE_SOURCE_DIR}/../connectors/cdc-connector/)
 include_directories(maxtest/include/maxtest)
 
 # Tool used to check backend state
-add_test_executable_notest(check_backend.cpp check_backend check_backend LABELS CONFIG REPL_BACKEND GALERA_BACKEND TWO_MAXSCALES)
+add_test_executable_notest(check_backend.cpp check_backend check_backend LABELS CONFIG REPL_BACKEND GALERA_BACKEND SECOND_MAXSCALE)
 
 # Configuration tests
 add_template_manual(bug359 bug359)
@@ -207,7 +207,7 @@ add_test_executable(mxs2355_wrong_auth.cpp mxs2355_wrong_auth maxctrl LABELS REP
 ############################################
 
 # Configures 'keepalived' on two Maxscale machines and tried failover
-#add_test_executable(keepalived_masterdown.cpp keepalived_masterdown keepalived_masterdown LABELS REPL_BACKEND TWO_MAXSCALES)
+#add_test_executable(keepalived_masterdown.cpp keepalived_masterdown keepalived_masterdown LABELS REPL_BACKEND SECOND_MAXSCALE)
 
 # MySQL Monitor with Multi-master configurations
 add_test_executable(mysqlmon_multimaster.cpp mysqlmon_multimaster mysqlmon_multimaster LABELS mysqlmon REPL_BACKEND BREAKS_REPL)
@@ -840,7 +840,7 @@ add_test_executable(kerberos_setup.cpp kerberos_setup kerberos LABELS HEAVY gssa
 add_test_derived(kerberos_setup_ssl kerberos_setup kerberos_ssl LABELS HEAVY gssapi REPL_BACKEND)
 
 # Configures 'keepalived' on two Maxscale machines and tried failover
-#add_test_executable(keepalived.cpp keepalived keepalived LABELS REPL_BACKEND TWO_MAXSCALES)
+#add_test_executable(keepalived.cpp keepalived keepalived LABELS REPL_BACKEND SECOND_MAXSCALE)
 
 # a tool to delete RDS Aurora cluster
 add_test_executable_notest(delete_rds.cpp delete_rds replication LABELS EXTERN_BACKEND)

maxscale-system-test/maxtest/include/maxtest/testconnections.h

@@ -51,6 +51,8 @@ typedef std::set<std::string> StringSet;
 class TestConnections
 {
 public:
+    using StringSet = std::set<std::string>;
+
     /**
      * @brief TestConnections constructor: reads environmental variables, copies MaxScale.cnf for MaxScale
      * machine
@@ -198,11 +200,6 @@ public:
      */
     bool use_ipv6;
 
-    /**
-     * @brief configured_labels List of lables for which nodes are configured
-     */
-    std::string configured_labels;
-
     /**
      * @brief vm_path Path to the VM Vagrant directory
      */
@@ -609,8 +606,14 @@ private:
 
     std::string m_test_name;            /**< Test name */
     std::string m_cnf_template_path;    /**< MaxScale config file template used by test */
-    std::string m_labels;               /**< Test labels */
+
-    std::string m_mdbci_labels;         /**< Labels for MDBCI */
+    std::string m_test_labels_str;  /**< Test labels as given in CMakeLists.txt and required by the test */
+    StringSet   m_test_labels;      /**< Test labels parsed to a set. */
+
+    StringSet   m_required_mdbci_labels;/**< MDBCI-labels required by test. Subset of test labels. */
+    std::string m_mdbci_labels_str;     /**< MDBCI-labels in string form. Used on the command line. */
+
+    StringSet m_configured_mdbci_labels;    /**< MDBCI-labels already configured on the VM setup */
 
     std::string m_mdbci_config_name;    /**< Name of MDBCI VMs set */
     std::string m_mdbci_vm_path;        /**< Path to directory with MDBCI VMs descriptions */
@@ -640,6 +643,9 @@ private:
 
     /** If true tests do not revert VMs after the test even if test failed (use it for debugging) */
     bool no_vm_revert {true};
+
+    std::string flatten_stringset(const StringSet& set);
+    StringSet   parse_to_stringset(const std::string& source);
 };
 
 /**

maxscale-system-test/maxtest/src/CMakeLists.txt

@@ -11,7 +11,6 @@ add_library(maxtest SHARED
         get_com_select_insert.cpp
         get_my_ip.cpp
         keepalived_func.cpp
-        labels_table.cpp
         mariadb_func.cpp
         mariadb_nodes.cpp
         maxadmin_operations.cpp

maxscale-system-test/maxtest/src/labels_table.cpp

@@ -1,43 +0,0 @@
-#include <cstdio>
-#include "testconnections.h"
-
-namespace
-{
-struct labels_table_t
-{
-    std::string test_label;
-    std::string mdbci_label;
-};
-
-const labels_table_t labels_table[] =
-{
-    {"REPL_BACKEND", "REPL_BACKEND"},
-    {"BIG_REPL_BACKEND", "BIG_REPL_BACKEND"},
-    {"GALERA_BACKEND", "GALERA_BACKEND"},
-    {"TWO_MAXSCALES", "SECOND_MAXSCALE"},
-    {"COLUMNSTORE_BACKEND", "COLUMNSTORE_BACKEND"},
-};
-}
-/**
- * Generate MDBCI labels required by test. Every test has a number of labels defined in CMakeLists.txt.
- * Some of these labels define which nodes (virtual machines) are needed for this particular test.
- * This function generates the equivalent labels for the 'mdbci up'-command.
- */
-void TestConnections::set_mdbci_labels()
-{
-    std::string mdbci_labels_str("MAXSCALE");
-    for (size_t i = 0; i < sizeof(labels_table) / sizeof(labels_table_t); i++)
-    {
-        std::string test_label = ";" + labels_table[i].test_label;
-        if (m_labels.find(test_label) != std::string::npos)
-        {
-            mdbci_labels_str += "," + labels_table[i].mdbci_label;
-        }
-    }
-
-    if (TestConnections::verbose)
-    {
-        printf("mdbci labels %s\n", mdbci_labels_str.c_str());
-    }
-    m_mdbci_labels = mdbci_labels_str;
-}

maxscale-system-test/maxtest/src/testconnections.cpp

@@ -14,6 +14,7 @@
 #include <iostream>
 #include <future>
 #include <maxbase/stacktrace.hh>
+#include <algorithm>
 
 #include "mariadb_func.h"
 #include "maxadmin_operations.h"
@@ -27,6 +28,19 @@ using std::cout;
 using std::endl;
 using std::string;
 
+namespace
+{
+// These must match the labels recognized by MDBCI.
+const string label_repl_be = "REPL_BACKEND";
+const string label_galera_be = "GALERA_BACKEND";
+const string label_big_be = "BIG_REPL_BACKEND";
+const string label_2nd_mxs = "SECOND_MAXSCALE";
+const string label_cs_be = "COLUMNSTORE_BACKEND";
+
+const StringSet recognized_mdbci_labels =
+{label_repl_be, label_big_be, label_galera_be, label_2nd_mxs, label_cs_be};
+}
+
 namespace maxscale
 {
 
@@ -254,30 +268,30 @@ TestConnections::TestConnections(int argc, char* argv[])
     m_test_name = (optind < argc) ? argv[optind] : basename(argv[0]);
     set_template_and_labels();
     tprintf("Test: '%s', config template: '%s', labels: '%s'",
-            m_test_name.c_str(), m_cnf_template_path.c_str(), m_labels.c_str());
+            m_test_name.c_str(), m_cnf_template_path.c_str(), m_test_labels_str.c_str());
     set_mdbci_labels();
 
-    std::string delimiter = std::string (",");
+    StringSet missing_mdbci_labels;
-    size_t pos_start = 0, pos_end, delim_len = delimiter.length();
+    std::set_difference(m_required_mdbci_labels.begin(), m_required_mdbci_labels.end(),
-    std::string label;
+                        m_configured_mdbci_labels.begin(), m_configured_mdbci_labels.end(),
-    std::string mdbci_labels_c = m_mdbci_labels + delimiter;
+                        std::inserter(missing_mdbci_labels, missing_mdbci_labels.begin()));
 
     bool mdbci_call_needed = false;
-
+    if (missing_mdbci_labels.empty())
-    while ((pos_end = mdbci_labels_c.find (delimiter, pos_start)) != std::string::npos)
     {
-        label = mdbci_labels_c.substr (pos_start, pos_end - pos_start);
+        if (verbose)
-        pos_start = pos_end + delim_len;
-        if (configured_labels.find(label, 0) == std::string::npos)
         {
-            mdbci_call_needed = true;
+            tprintf("Machines with all required labels '%s' are running, MDBCI UP call is not needed",
-            tprintf("Machines with label '%s' are not running, MDBCI UP call is needed", label.c_str());
+                    m_mdbci_labels_str.c_str());
-        }
-        else if (verbose)
-        {
-            tprintf("Machines with label '%s' are running, MDBCI UP call is not needed", label.c_str());
         }
     }
+    else
+    {
+        string missing_labels_str = flatten_stringset(missing_mdbci_labels);
+        tprintf("Machines with labels '%s' are not running, MDBCI UP call is needed",
+                missing_labels_str.c_str());
+        mdbci_call_needed = true;
+    }
 
     if (mdbci_call_needed)
     {
@@ -288,7 +302,7 @@ TestConnections::TestConnections(int argc, char* argv[])
 
     }
 
-    if (m_mdbci_labels.find(std::string("REPL_BACKEND")) == std::string::npos)
+    if (m_required_mdbci_labels.count(label_repl_be) == 0)
     {
         no_repl = true;
         if (verbose)
@@ -297,7 +311,7 @@ TestConnections::TestConnections(int argc, char* argv[])
         }
     }
 
-    if (m_mdbci_labels.find(std::string("GALERA_BACKEND")) == std::string::npos)
+    if (m_required_mdbci_labels.count(label_galera_be) == 0)
     {
         no_galera = true;
         if (verbose)
@@ -612,7 +626,7 @@ void TestConnections::read_mdbci_info()
         nc_file.open(vm_path + "_configured_labels");
         std::stringstream strStream1;
         strStream1 << nc_file.rdbuf();
-        configured_labels = strStream1.str();
+        m_configured_mdbci_labels = parse_to_stringset(strStream1.str());
         nc_file.close();
     }
     else
@@ -699,19 +713,19 @@ void TestConnections::set_template_and_labels()
     if (found)
     {
         m_cnf_template_path = found->config_template;
-        m_labels = found->labels;
+        m_test_labels_str = found->labels;
     }
     else
     {
-        printf("Failed to find configuration template for test '%s', using default template '%s'.\n",
+        printf("Failed to find configuration template for test '%s', using default template '%s' and "
-               m_test_name.c_str(), default_template);
+               "labels '%s'.\n",
+               m_test_name.c_str(), default_template, label_repl_be.c_str());
         m_cnf_template_path = default_template;
+        m_test_labels_str = label_repl_be;
     }
 
-    if (m_labels.empty())
+    // Parse the labels-string to a set.
-    {
+    m_test_labels = parse_to_stringset(m_test_labels_str);
-        m_labels = "REPL_BACKEND";
-    }
 }
 
 void TestConnections::process_template(int m, const string& cnf_template_path, const char* dest)
@@ -2193,7 +2207,7 @@ int TestConnections::call_mdbci(const char * options)
     if (system((std::string("mdbci up ") +
                m_mdbci_config_name +
                std::string(" --labels ") +
-               m_mdbci_labels +
+               m_mdbci_labels_str +
                std::string(" ") +
                std::string(options)).c_str() ))
     {
@@ -2304,5 +2318,59 @@ int TestConnections::reinstall_maxscales()
 
 bool TestConnections::too_many_maxscales() const
 {
-    return maxscales->N < 2 && m_mdbci_labels.find("SECOND_MAXSCALE") != std::string::npos;
+    return maxscales->N < 2 && m_required_mdbci_labels.count(label_2nd_mxs) > 0;
+}
+
+std::string TestConnections::flatten_stringset(const StringSet& set)
+{
+    string rval;
+    string sep;
+    for (auto& elem : set)
+    {
+        rval += sep;
+        rval += elem;
+        sep = ",";
+    }
+    return rval;
+}
+
+StringSet TestConnections::parse_to_stringset(const string& source)
+{
+    string copy = source;
+    StringSet rval;
+    if (!copy.empty())
+    {
+        char* ptr = &copy[0];
+        char* save_ptr = nullptr;
+        // mdbci uses ',' and cmake uses ';'. Add ' ' as well to ensure trimming.
+        const char delim[] = ",; ";
+        char* token = strtok_r(ptr, delim, &save_ptr);
+        while (token)
+        {
+            rval.insert(token);
+            token = strtok_r(nullptr, delim, &save_ptr);
+        }
+    }
+    return rval;
+}
+
+/**
+ * MDBCI recognizes labels which affect backend configuration. Save those labels to a separate field.
+ * Also save a string version.
+ */
+void TestConnections::set_mdbci_labels()
+{
+    StringSet mdbci_labels;
+    mdbci_labels.insert("MAXSCALE");
+    std::set_intersection(recognized_mdbci_labels.begin(), recognized_mdbci_labels.end(),
+                          m_test_labels.begin(), m_test_labels.end(),
+                          std::inserter(mdbci_labels, mdbci_labels.begin()));
+
+    std::string mdbci_labels_str = flatten_stringset(mdbci_labels);
+    if (TestConnections::verbose)
+    {
+        printf("mdbci-labels: %s\n", mdbci_labels_str.c_str());
+    }
+    m_required_mdbci_labels = mdbci_labels;
+    m_mdbci_labels_str = mdbci_labels_str;
 }