[#2408] Error message when HA service terminates

src/hooks/dhcp/high_availability/communication_state.cc

@@ -381,7 +381,26 @@ CommunicationState::clockSkewShouldTerminateInternal() const {
                   .arg(logFormatClockSkewInternal());
         return (true);
     }
+    return (false);
+}
 
+bool
+CommunicationState::rejectedLeaseUpdatesShouldTerminate() const {
+    if (MultiThreadingMgr::instance().getMode()) {
+        std::lock_guard<std::mutex> lk(*mutex_);
+        return (rejectedLeaseUpdatesShouldTerminateInternal());
+    } else {
+        return (rejectedLeaseUpdatesShouldTerminateInternal());
+    }
+}
+
+bool
+CommunicationState::rejectedLeaseUpdatesShouldTerminateInternal() const {
+    if (config_->getMaxRejectedLeaseUpdates() &&
+        (config_->getMaxRejectedLeaseUpdates() <= getRejectedLeaseUpdatesCount())) {
+        LOG_ERROR(ha_logger, HA_REJECTED_LEASE_UPDATES_CAUSE_TERMINATION);
+        return (true);
+    }
     return (false);
 }
 

src/hooks/dhcp/high_availability/communication_state.h

@@ -428,6 +428,28 @@ private:
     /// number of seconds, false otherwise.
     bool isClockSkewGreater(const long seconds) const;
 
+public:
+
+    /// @brief Indicates whether the HA service should enter "terminated"
+    /// state due to excessive number of rejected lease updates.
+    ///
+    /// @return true if the number of rejected lease updates is equal or
+    /// exceeds the value of max-rejected-lease-updates, false when the
+    /// max-rejected-lease-updates is 0 or is greater than the current
+    /// number of rejected lease updates.
+    bool rejectedLeaseUpdatesShouldTerminate() const;
+
+private:
+
+    /// @brief Indicates whether the HA service should enter "terminated"
+    /// state due to excessive number of rejected lease updates.
+    ///
+    /// @return true if the number of rejected lease updates is equal or
+    /// exceeds the value of max-rejected-lease-updates, false when the
+    /// max-rejected-lease-updates is 0 or is greater than the current
+    /// number of rejected lease updates.
+    bool rejectedLeaseUpdatesShouldTerminateInternal() const;
+
 public:
 
     /// @brief Provide partner's notion of time so the new clock skew can be

src/hooks/dhcp/high_availability/ha_messages.cc

@@ -88,6 +88,7 @@ extern const isc::log::MessageID HA_MAINTENANCE_START_HANDLER_FAILED = "HA_MAINT
 extern const isc::log::MessageID HA_MISSING_CONFIGURATION = "HA_MISSING_CONFIGURATION";
 extern const isc::log::MessageID HA_PAUSE_CLIENT_LISTENER_FAILED = "HA_PAUSE_CLIENT_LISTENER_FAILED";
 extern const isc::log::MessageID HA_PAUSE_CLIENT_LISTENER_ILLEGAL = "HA_PAUSE_CLIENT_LISTENER_ILLEGAL";
+extern const isc::log::MessageID HA_REJECTED_LEASE_UPDATES_CAUSE_TERMINATION = "HA_REJECTED_LEASE_UPDATES_CAUSE_TERMINATION";
 extern const isc::log::MessageID HA_RESET_COMMUNICATIONS_FAILED = "HA_RESET_COMMUNICATIONS_FAILED";
 extern const isc::log::MessageID HA_RESET_FAILED = "HA_RESET_FAILED";
 extern const isc::log::MessageID HA_RESET_HANDLER_FAILED = "HA_RESET_HANDLER_FAILED";
@@ -195,6 +196,7 @@ const char* values[] = {
     "HA_MISSING_CONFIGURATION", "high-availability parameter not specified for High Availability hooks library",
     "HA_PAUSE_CLIENT_LISTENER_FAILED", "Pausing multi-threaded HTTP processing failed: %1",
     "HA_PAUSE_CLIENT_LISTENER_ILLEGAL", "Pausing multi-threaded HTTP processing failed: %1",
+    "HA_REJECTED_LEASE_UPDATES_CAUSE_TERMINATION", "too many rejected lease updates cause the HA service to terminate",
     "HA_RESET_COMMUNICATIONS_FAILED", "failed to send ha-reset command to %1: %2",
     "HA_RESET_FAILED", "failed to reset HA state machine of %1: %2",
     "HA_RESET_HANDLER_FAILED", "ha-reset command failed: %1",

src/hooks/dhcp/high_availability/ha_messages.h

@@ -89,6 +89,7 @@ extern const isc::log::MessageID HA_MAINTENANCE_START_HANDLER_FAILED;
 extern const isc::log::MessageID HA_MISSING_CONFIGURATION;
 extern const isc::log::MessageID HA_PAUSE_CLIENT_LISTENER_FAILED;
 extern const isc::log::MessageID HA_PAUSE_CLIENT_LISTENER_ILLEGAL;
+extern const isc::log::MessageID HA_REJECTED_LEASE_UPDATES_CAUSE_TERMINATION;
 extern const isc::log::MessageID HA_RESET_COMMUNICATIONS_FAILED;
 extern const isc::log::MessageID HA_RESET_FAILED;
 extern const isc::log::MessageID HA_RESET_HANDLER_FAILED;

src/hooks/dhcp/high_availability/ha_messages.mes

@@ -504,6 +504,12 @@ listener thread pools from a worker thread. This error indicates that a command
 run on the listener threads is trying to use a critical section which would
 result in a dead-lock.
 
+% HA_REJECTED_LEASE_UPDATES_CAUSE_TERMINATION too many rejected lease updates cause the HA service to terminate
+This error message is issued when the HA service terminates because the
+number of lease updates for which a conflict status code was returned
+by the partner exceeds the limit set with max-rejected-lease-updates
+configuration parameter.
+
 % HA_RESET_COMMUNICATIONS_FAILED failed to send ha-reset command to %1: %2
 This warning message indicates a problem with communication with a HA peer
 while sending the ha-reset command. The first argument specifies a remote

src/hooks/dhcp/high_availability/ha_service.cc

@@ -1115,9 +1115,7 @@ HAService::shouldTerminate() const {
         communication_state_->clockSkewShouldWarn();
         // Check if we should terminate because the number of rejected leases
         // has been exceeded.
-        should_terminate =
+        should_terminate = communication_state_->rejectedLeaseUpdatesShouldTerminate();
-            config_->getMaxRejectedLeaseUpdates() &&
-            (config_->getMaxRejectedLeaseUpdates() <= communication_state_->getRejectedLeaseUpdatesCount());
     }
 
     return (should_terminate);

src/hooks/dhcp/high_availability/tests/communication_state_unittest.cc

@@ -98,6 +98,10 @@ public:
     /// for logging.
     void logFormatClockSkewTest();
 
+    /// @brief This test verifies that too many rejected lease updates cause
+    /// the service termination.
+    void rejectedLeaseUpdatesTerminateTest();
+
     /// @brief Tests that the communication state report is correct.
     void getReportTest();
 
@@ -607,6 +611,19 @@ CommunicationStateTest::logFormatClockSkewTest() {
     EXPECT_EQ(expected, log);
 }
 
+void
+CommunicationStateTest::rejectedLeaseUpdatesTerminateTest() {
+    EXPECT_FALSE(state_.rejectedLeaseUpdatesShouldTerminate());
+    // Reject several lease updates but do not exceed the limit.
+    for (auto i = 0; i < 9; ++i) {
+        ASSERT_NO_THROW(state_.reportRejectedLeaseUpdate(createMessage4(DHCPDISCOVER, i, i, 0)));
+    }
+    EXPECT_FALSE(state_.rejectedLeaseUpdatesShouldTerminate());
+    // Add one more. It should exceed the limit.
+    ASSERT_NO_THROW(state_.reportRejectedLeaseUpdate(createMessage4(DHCPDISCOVER, 9, 9, 0)));
+    EXPECT_TRUE(state_.rejectedLeaseUpdatesShouldTerminate());
+}
+
 // Tests that the communication state report is correct.
 void
 CommunicationStateTest::getReportTest() {
@@ -932,6 +949,15 @@ TEST_F(CommunicationStateTest, clockSkewTestMultiThreading) {
     clockSkewTest();
 }
 
+TEST_F(CommunicationStateTest, rejectedLeaseUpdatesTerminateTest) {
+    rejectedLeaseUpdatesTerminateTest();
+}
+
+TEST_F(CommunicationStateTest, rejectedLeaseUpdatesTerminateTestMultiThreading) {
+    MultiThreadingMgr::instance().setMode(true);
+    rejectedLeaseUpdatesTerminateTest();
+}
+
 TEST_F(CommunicationStateTest, logFormatClockSkewTest) {
     logFormatClockSkewTest();
 }