[#2749] adapt unit tests to default HA+MT

5 files changed, 64 insertions, 48 deletions

diff --git a/src/hooks/dhcp/high_availability/libloadtests/close_unittests.cc b/src/hooks/dhcp/high_availability/libloadtests/close_unittests.cc
index 1c122d12ad..424f9628f8 100644
--- a/src/hooks/dhcp/high_availability/libloadtests/close_unittests.cc
+++ b/src/hooks/dhcp/high_availability/libloadtests/close_unittests.cc
@@ -24,6 +24,7 @@
 #include <dhcp/pkt4.h>
 #include <dhcp/pkt6.h>
 #include <dhcpsrv/cfgmgr.h>
+#include <dhcpsrv/cfg_multi_threading.h>
 #include <dhcpsrv/network_state.h>
 #include <hooks/hooks.h>
 #include <hooks/hooks_manager.h>
@@ -102,6 +103,8 @@ class CloseHATest : public ::testing::Test {
 public:
     /// @brief Constructor
     CloseHATest() {
+        // Simulate the application of MT config such as in ControlledDhcpvXSrv::processConfig().
+        CfgMultiThreading::apply(CfgMgr::instance().getStagingCfg()->getDHCPMultiThreading());
     }
 
     /// @brief Destructor
@@ -122,7 +125,10 @@ public:
     ///
     /// Simulate partners by accepting connections. The HA will send
     /// lease updates and waits for answers so will own the query.
-    void runPartners();
+    ///
+    /// @param backup whether to run partner1 as a backup.
+    ///     If false, partner1 runs as primary.
+    void runPartners(bool const backup = true);
 
     /// @brief The watched thread.
     WatchedThreadPtr wthread_;
@@ -134,7 +140,7 @@ CloseHATest::createValidJsonConfiguration(bool backup) const {
     config_text <<
         "["
         "     {"
-        "         \"this-server-name\": \"" << (!backup ? "server1" : "server2") << "\","
+        "         \"this-server-name\": \"" << (backup ? "server2" : "server1") << "\","
         "         \"mode\": \"passive-backup\","
         "         \"wait-backup-ack\": true,"
         "         \"peers\": ["
@@ -161,9 +167,10 @@ CloseHATest::createValidJsonConfiguration(bool backup) const {
 }
 
 void
-CloseHATest::runPartners() {
+CloseHATest::runPartners(bool const backup /* = true */) {
     int accept_partner1 = -1;
     int accept_partner2 = -1;
+    int reuse_addr = 1;
     std::map<int, bool> readers;
 
     try {
@@ -177,6 +184,7 @@ CloseHATest::runPartners() {
         socklen_t slen = sizeof(partner);
 
 #define SA(x)   reinterpret_cast<const sockaddr*>(x)
+
         accept_partner1 = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP);
         if (accept_partner1 < 0) {
             isc_throw(Unexpected, "socket1 " << strerror(errno));
@@ -185,13 +193,12 @@ CloseHATest::runPartners() {
             isc_throw(Unexpected, "fcntl1 " << strerror(errno));
         }
 
-        int reuse_addr = 1;
         if (setsockopt(accept_partner1, SOL_SOCKET, SO_REUSEADDR,
                (char *)&reuse_addr, sizeof(reuse_addr)) < 0) {
             isc_throw(Unexpected, "partner1 setsocketopt SO_REUSEADDR failed: " << strerror(errno));
         }
 
-        partner.sin_port = htons(18124);
+        partner.sin_port = htons(backup ? 18124 : 18123);
         if (::bind(accept_partner1, SA(&partner), slen) < 0) {
             isc_throw(Unexpected, "bind1 " << strerror(errno));
         }
@@ -447,7 +454,7 @@ TEST_F(CloseHATest, close4) {
     }
 
     // Done: purge I/Os.
-    io_service->poll();
+    EXPECT_NO_THROW(io_service->poll());
     io_service->stop();
     io_service.reset();
 
@@ -477,13 +484,14 @@ TEST_F(CloseHATest, close4) {
 // 4. Unload the library which should not have any dangling resources.
 // 5. Verify that the network state is reset on unload.
 TEST_F(CloseHATest, close4Backup) {
-    // Start partners.
+    // Start the second backup server.
+    // The first backup server will be started when libraries are loaded.
     wthread_.reset(new WatchedThread());
-    wthread_->start([this] () { runPartners(); });
+    wthread_->start([this] () { runPartners(/* backup = */ false); });
 
     // Prepare parameters,
     ElementPtr params = Element::createMap();
-    params->set("high-availability", createValidJsonConfiguration(true));
+    params->set("high-availability", createValidJsonConfiguration(/* backup = */ true));
 
     // Set family and proc name.
     CfgMgr::instance().setFamily(AF_INET);
@@ -541,7 +549,7 @@ TEST_F(CloseHATest, close4Backup) {
     ASSERT_FALSE(network_state->isServiceEnabled());
 
     // Done: purge I/Os.
-    io_service->poll();
+    EXPECT_NO_THROW(io_service->poll());
     io_service->stop();
     io_service.reset();
 
@@ -685,7 +693,7 @@ TEST_F(CloseHATest, close6) {
     }
 
     // Done: purge I/Os.
-    io_service->poll();
+    EXPECT_NO_THROW(io_service->poll());
     io_service->stop();
     io_service.reset();
 
@@ -715,13 +723,14 @@ TEST_F(CloseHATest, close6) {
 // 4. Unload the library which should not have any dangling resources.
 // 5. Verify that the network state is reset on unload.
 TEST_F(CloseHATest, close6Backup) {
-    // Start partners.
+    // Start the second backup server.
+    // The first backup server will be started when libraries are loaded.
     wthread_.reset(new WatchedThread());
-    wthread_->start([this] () { runPartners(); });
+    wthread_->start([this] () { runPartners(/* backup = */ false); });
 
     // Prepare parameters,
     ElementPtr params = Element::createMap();
-    params->set("high-availability", createValidJsonConfiguration(true));
+    params->set("high-availability", createValidJsonConfiguration(/* backup = */ true));
 
     // Set family and proc name.
     CfgMgr::instance().setFamily(AF_INET6);
@@ -779,7 +788,7 @@ TEST_F(CloseHATest, close6Backup) {
     ASSERT_FALSE(network_state->isServiceEnabled());
 
     // Done: purge I/Os.
-    io_service->poll();
+    EXPECT_NO_THROW(io_service->poll());
     io_service->stop();
     io_service.reset();
 
@@ -795,4 +804,4 @@ TEST_F(CloseHATest, close6Backup) {
     wthread_->stop();
 }
 
-}
+}  // namespace
diff --git a/src/hooks/dhcp/high_availability/tests/ha_config_unittest.cc b/src/hooks/dhcp/high_availability/tests/ha_config_unittest.cc
index 660a90d5b3..865211e718 100644
--- a/src/hooks/dhcp/high_availability/tests/ha_config_unittest.cc
+++ b/src/hooks/dhcp/high_availability/tests/ha_config_unittest.cc
@@ -32,10 +32,8 @@ namespace {
 /// configuration.
 class HAConfigTest : public HATest {
 public:
-
     /// @brief Constructor.
-    HAConfigTest()
-        : HATest() {
+    HAConfigTest() : HATest(), hardware_threads_(MultiThreadingMgr::detectThreadCount()) {
     }
 
     /// @brief Verifies if an exception is thrown if provided HA
@@ -59,6 +57,9 @@ public:
                 " exception type";
         }
     }
+
+    /// @brief number of threads the system reports as supported
+    uint32_t hardware_threads_;
 };
 
 // Verifies that load balancing configuration is parsed correctly.
@@ -209,11 +210,12 @@ TEST_F(HAConfigTest, configureLoadBalancing) {
     ASSERT_TRUE(state_cfg);
     EXPECT_EQ(STATE_PAUSE_ONCE, state_cfg->getPausing());
 
-    // Verify multi-threading default values.
-    EXPECT_FALSE(impl->getConfig()->getEnableMultiThreading());
-    EXPECT_FALSE(impl->getConfig()->getHttpDedicatedListener());
-    EXPECT_EQ(0, impl->getConfig()->getHttpListenerThreads());
-    EXPECT_EQ(0, impl->getConfig()->getHttpClientThreads());
+    // Verify multi-threading default values. Default is 0 for the listener and client threads, but
+    // after MT is applied, HAImpl resolves them to the auto-detected values.
+    EXPECT_TRUE(impl->getConfig()->getEnableMultiThreading());
+    EXPECT_TRUE(impl->getConfig()->getHttpDedicatedListener());
+    EXPECT_EQ(hardware_threads_, impl->getConfig()->getHttpListenerThreads());
+    EXPECT_EQ(hardware_threads_, impl->getConfig()->getHttpClientThreads());
 }
 
 // Verifies that hot standby configuration is parsed correctly.
@@ -324,11 +326,12 @@ TEST_F(HAConfigTest, configureHotStandby) {
     ASSERT_TRUE(state_cfg);
     EXPECT_EQ(STATE_PAUSE_NEVER, state_cfg->getPausing());
 
-    // Verify multi-threading default values.
-    EXPECT_FALSE(impl->getConfig()->getEnableMultiThreading());
-    EXPECT_FALSE(impl->getConfig()->getHttpDedicatedListener());
-    EXPECT_EQ(0, impl->getConfig()->getHttpListenerThreads());
-    EXPECT_EQ(0, impl->getConfig()->getHttpClientThreads());
+    // Verify multi-threading default values. Default is 0 for the listener and client threads, but
+    // after MT is applied, HAImpl resolves them to the auto-detected values.
+    EXPECT_TRUE(impl->getConfig()->getEnableMultiThreading());
+    EXPECT_TRUE(impl->getConfig()->getHttpDedicatedListener());
+    EXPECT_EQ(hardware_threads_, impl->getConfig()->getHttpListenerThreads());
+    EXPECT_EQ(hardware_threads_, impl->getConfig()->getHttpClientThreads());
 }
 
 // Verifies that passive-backup configuration is parsed correctly.
@@ -391,11 +394,12 @@ TEST_F(HAConfigTest, configurePassiveBackup) {
     ASSERT_TRUE(cfg->getBasicAuth());
     EXPECT_EQ("a2VhdGVzdDpLZWFUZXN0", cfg->getBasicAuth()->getCredential());
 
-    // Verify multi-threading default values.
-    EXPECT_FALSE(impl->getConfig()->getEnableMultiThreading());
-    EXPECT_FALSE(impl->getConfig()->getHttpDedicatedListener());
-    EXPECT_EQ(0, impl->getConfig()->getHttpListenerThreads());
-    EXPECT_EQ(0, impl->getConfig()->getHttpClientThreads());
+    // Verify multi-threading default values. Default is 0 for the listener and client threads, but
+    // after MT is applied, HAImpl resolves them to the auto-detected values.
+    EXPECT_TRUE(impl->getConfig()->getEnableMultiThreading());
+    EXPECT_TRUE(impl->getConfig()->getHttpDedicatedListener());
+    EXPECT_EQ(hardware_threads_, impl->getConfig()->getHttpListenerThreads());
+    EXPECT_EQ(hardware_threads_, impl->getConfig()->getHttpClientThreads());
 }
 
 // This server name must not be empty.
@@ -1730,15 +1734,12 @@ TEST_F(HAConfigTest, multiThreadingPermutations) {
     bool ha_mt = true;
     bool listener = true;
 
-    // Number of threads the system reports as supported.
-    uint32_t sys_threads = MultiThreadingMgr::detectThreadCount();
-
     std::vector<Scenario> scenarios {
         {
-            "1 no ha+mt/default",
+            "1 ha+mt by default",
             "",
             dhcp_mt, 4,
-            !ha_mt, !listener, 0, 0
+            ha_mt, listener, 4, 4
         },
         {
             "2 dhcp mt enabled, ha mt disabled",
@@ -1784,7 +1785,7 @@ TEST_F(HAConfigTest, multiThreadingPermutations) {
             // reported value.
             makeHAMtJson(ha_mt, listener, 0, 0),
             dhcp_mt, 0,
-            (sys_threads > 0), listener, sys_threads, sys_threads
+            (hardware_threads_ > 0), listener, hardware_threads_, hardware_threads_
         }
     };
 
@@ -1875,4 +1876,4 @@ TEST_F(HAConfigTest, ipv6Url) {
     EXPECT_EQ(impl->getConfig()->getThisServerConfig()->getUrl().toText(), "http://[2001:db8::1]:8080/");
 }
 
-} // end of anonymous namespace
+}  // namespace
diff --git a/src/hooks/dhcp/high_availability/tests/ha_mt_unittest.cc b/src/hooks/dhcp/high_availability/tests/ha_mt_unittest.cc
index 958155f6a3..180dbbac02 100644
--- a/src/hooks/dhcp/high_availability/tests/ha_mt_unittest.cc
+++ b/src/hooks/dhcp/high_availability/tests/ha_mt_unittest.cc
@@ -409,10 +409,10 @@ TEST_F(HAMtServiceTest, multiThreadingConfigStartup) {
 
     std::vector<Scenario> scenarios {
         {
-            "1 no ha+mt/default",
+            "1 ha+mt by default",
             "",
             dhcp_mt, 4,
-            !ha_mt, !listener, 0, 0
+            ha_mt, listener, 4, 4
         },
         {
             "2 dhcp mt enabled, ha mt disabled",
@@ -558,4 +558,4 @@ TEST_F(HAMtServiceTest, multiThreadingConfigStartup) {
     }
 }
 
-} // end of anonymous namespace
+}  // namespace
diff --git a/src/hooks/dhcp/high_availability/tests/ha_test.cc b/src/hooks/dhcp/high_availability/tests/ha_test.cc
index af63e96efb..27a29defbb 100644
--- a/src/hooks/dhcp/high_availability/tests/ha_test.cc
+++ b/src/hooks/dhcp/high_availability/tests/ha_test.cc
@@ -156,6 +156,9 @@ HATest::createValidJsonConfiguration(const HAConfig::HAMode& ha_mode) const {
         "         \"max-ack-delay\": 10000,"
         "         \"max-unacked-clients\": 10,"
         "         \"max-rejected-clients\": 10,"
+        "         \"multi-threading\": {"
+        "             \"enable-multi-threading\": false"
+        "         },"
         "         \"wait-backup-ack\": false,"
         "         \"peers\": ["
         "             {"
@@ -192,6 +195,9 @@ HATest::createValidPassiveBackupJsonConfiguration() const {
         "     {"
         "         \"this-server-name\": \"server1\","
         "         \"mode\": \"passive-backup\","
+        "         \"multi-threading\": {"
+        "             \"enable-multi-threading\": false"
+        "         },"
         "         \"wait-backup-ack\": false,"
         "         \"peers\": ["
         "             {"
@@ -367,6 +373,6 @@ HATest::makeHAMtJson(bool enable_multi_threading, bool http_dedicated_listener,
     return (ss.str());
 }
 
-} // end of namespace isc::ha::test
-} // end of namespace isc::ha
-} // end of namespace isc
+}  // namespace test
+}  // namespace ha
+}  // namespace isc
diff --git a/src/lib/config/cmd_http_listener.cc b/src/lib/config/cmd_http_listener.cc
index c06e4c10f9..aa917c7824 100644
--- a/src/lib/config/cmd_http_listener.cc
+++ b/src/lib/config/cmd_http_listener.cc
@@ -84,7 +84,7 @@ CmdHttpListener::start() {
         thread_io_service_.reset();
         http_listener_.reset();
         thread_pool_.reset();
-        isc_throw(Unexpected, "CmdHttpListener::run failed:" << ex.what());
+        isc_throw(Unexpected, "CmdHttpListener::run failed: " << ex.what());
     }
 }