[#3178] Handle no subnet selected in HA

diff --git a/src/hooks/dhcp/high_availability/ha_impl.cc b/src/hooks/dhcp/high_availability/ha_impl.cc
index c00a393b9511526b8e44159b6ef3e22f42a984ae..c6f546156a19a30c69f657e5cbb6ea9642c10d46 100644 (file)
--- a/src/hooks/dhcp/high_availability/ha_impl.cc
+++ b/src/hooks/dhcp/high_availability/ha_impl.cc
@@ -144,6 +144,19 @@ HAImpl::subnet4Select(hooks::CalloutHandle& callout_handle) {
     Subnet4Ptr subnet4;
     callout_handle.getArgument("subnet4", subnet4);
 
+    // If the server failed to select the subnet this pointer is null.
+    // There is nothing we can do with this packet because we don't know
+    // which relationship it belongs to. We're even unable to check if the
+    // server is responsible for this packet.
+    if (!subnet4) {
+        // Log at debug level because that's the level at which the server
+        // logs the subnet selection failure.
+        LOG_DEBUG(ha_logger, DBGLVL_TRACE_BASIC, HA_SUBNET4_SELECT_NO_SUBNET_SELECTED)
+            .arg(query4->getLabel());
+        callout_handle.setStatus(CalloutHandle::NEXT_STEP_DROP);
+        return;
+    }
+
     // The subnet configuration should contain a user context
     // and this context should contain a mapping of the subnet to a
     // relationship. If the context doesn't exist there is no way
@@ -393,6 +406,19 @@ HAImpl::subnet6Select(hooks::CalloutHandle& callout_handle) {
     Subnet6Ptr subnet6;
     callout_handle.getArgument("subnet6", subnet6);
 
+    // If the server failed to select the subnet this pointer is null.
+    // There is nothing we can do with this packet because we don't know
+    // which relationship it belongs to. We're even unable to check if the
+    // server is responsible for this packet.
+    if (!subnet6) {
+        // Log at debug level because that's the level at which the server
+        // logs the subnet selection failure.
+        LOG_DEBUG(ha_logger, DBGLVL_TRACE_BASIC, HA_SUBNET6_SELECT_NO_SUBNET_SELECTED)
+            .arg(query6->getLabel());
+        callout_handle.setStatus(CalloutHandle::NEXT_STEP_DROP);
+        return;
+    }
+
     // The subnet configuration should contain a user context
     // and this context should contain a mapping of the subnet to a
     // relationship. If the context doesn't exist there is no way
@@ -401,7 +427,7 @@ HAImpl::subnet6Select(hooks::CalloutHandle& callout_handle) {
     try {
         server_name = HAConfig::getSubnetServerName(subnet6);
         if (server_name.empty()) {
-            LOG_ERROR(ha_logger, HA_SUBNET4_SELECT_NO_RELATIONSHIP_SELECTOR_FOR_SUBNET)
+            LOG_ERROR(ha_logger, HA_SUBNET6_SELECT_NO_RELATIONSHIP_SELECTOR_FOR_SUBNET)
                 .arg(query6->getLabel())
                 .arg(subnet6->toText());
             callout_handle.setStatus(CalloutHandle::NEXT_STEP_DROP);

diff --git a/src/hooks/dhcp/high_availability/ha_messages.cc b/src/hooks/dhcp/high_availability/ha_messages.cc
index cfff463142b773e9c0ba7f6bca4fa7fb6a71f7d3..be2e3705336ed7040cc35e7516650c6b1d99b840 100644 (file)
--- a/src/hooks/dhcp/high_availability/ha_messages.cc
+++ b/src/hooks/dhcp/high_availability/ha_messages.cc
@@ -107,11 +107,13 @@ extern const isc::log::MessageID HA_SUBNET4_SELECT_INVALID_HA_SERVER_NAME = "HA_
 extern const isc::log::MessageID HA_SUBNET4_SELECT_NOT_FOR_US = "HA_SUBNET4_SELECT_NOT_FOR_US";
 extern const isc::log::MessageID HA_SUBNET4_SELECT_NO_RELATIONSHIP_FOR_SUBNET = "HA_SUBNET4_SELECT_NO_RELATIONSHIP_FOR_SUBNET";
 extern const isc::log::MessageID HA_SUBNET4_SELECT_NO_RELATIONSHIP_SELECTOR_FOR_SUBNET = "HA_SUBNET4_SELECT_NO_RELATIONSHIP_SELECTOR_FOR_SUBNET";
+extern const isc::log::MessageID HA_SUBNET4_SELECT_NO_SUBNET_SELECTED = "HA_SUBNET4_SELECT_NO_SUBNET_SELECTED";
 extern const isc::log::MessageID HA_SUBNET6_SELECT_FAILED = "HA_SUBNET6_SELECT_FAILED";
 extern const isc::log::MessageID HA_SUBNET6_SELECT_INVALID_HA_SERVER_NAME = "HA_SUBNET6_SELECT_INVALID_HA_SERVER_NAME";
 extern const isc::log::MessageID HA_SUBNET6_SELECT_NOT_FOR_US = "HA_SUBNET6_SELECT_NOT_FOR_US";
 extern const isc::log::MessageID HA_SUBNET6_SELECT_NO_RELATIONSHIP_FOR_SUBNET = "HA_SUBNET6_SELECT_NO_RELATIONSHIP_FOR_SUBNET";
 extern const isc::log::MessageID HA_SUBNET6_SELECT_NO_RELATIONSHIP_SELECTOR_FOR_SUBNET = "HA_SUBNET6_SELECT_NO_RELATIONSHIP_SELECTOR_FOR_SUBNET";
+extern const isc::log::MessageID HA_SUBNET6_SELECT_NO_SUBNET_SELECTED = "HA_SUBNET6_SELECT_NO_SUBNET_SELECTED";
 extern const isc::log::MessageID HA_SYNC_COMPLETE_NOTIFY_COMMUNICATIONS_FAILED = "HA_SYNC_COMPLETE_NOTIFY_COMMUNICATIONS_FAILED";
 extern const isc::log::MessageID HA_SYNC_COMPLETE_NOTIFY_FAILED = "HA_SYNC_COMPLETE_NOTIFY_FAILED";
 extern const isc::log::MessageID HA_SYNC_COMPLETE_NOTIFY_HANDLER_FAILED = "HA_SYNC_COMPLETE_NOTIFY_HANDLER_FAILED";
@@ -228,11 +230,13 @@ const char* values[] = {
     "HA_SUBNET4_SELECT_NOT_FOR_US", "%1: dropping query in relationship %2 to be processed by another server",
     "HA_SUBNET4_SELECT_NO_RELATIONSHIP_FOR_SUBNET", "%1: HA relationship not found for %2",
     "HA_SUBNET4_SELECT_NO_RELATIONSHIP_SELECTOR_FOR_SUBNET", "%1: unable to determine HA relationship because selected subnet %2 lacks the ha-server-name",
+    "HA_SUBNET4_SELECT_NO_SUBNET_SELECTED", "%1: unable to determine HA relationship because no subnet has been selected for the client",
     "HA_SUBNET6_SELECT_FAILED", "subnet6_select callout failed: %1",
     "HA_SUBNET6_SELECT_INVALID_HA_SERVER_NAME", "%1: invalid ha-server-name value for subnet %2",
     "HA_SUBNET6_SELECT_NOT_FOR_US", "%1: dropping query in relationship %2 to be processed by another server",
     "HA_SUBNET6_SELECT_NO_RELATIONSHIP_FOR_SUBNET", "%1: HA relationship not found for %2",
     "HA_SUBNET6_SELECT_NO_RELATIONSHIP_SELECTOR_FOR_SUBNET", "%1: unable to determine HA relationship because selected subnet %2 lacks the ha-server-name",
+    "HA_SUBNET6_SELECT_NO_SUBNET_SELECTED", "%1: unable to determine HA relationship because no subnet has been selected for the client",
     "HA_SYNC_COMPLETE_NOTIFY_COMMUNICATIONS_FAILED", "%1: failed to send ha-sync-complete-notify to %2: %3",
     "HA_SYNC_COMPLETE_NOTIFY_FAILED", "%1: error processing ha-sync-complete-notify command on %2: %3",
     "HA_SYNC_COMPLETE_NOTIFY_HANDLER_FAILED", "ha-sync-complete-notify command failed: %1",

diff --git a/src/hooks/dhcp/high_availability/ha_messages.h b/src/hooks/dhcp/high_availability/ha_messages.h
index e4b693fd086746791583aadfbd2319c122c26ecc..e035793b29ed8c105b5962f7921a194d53b2be94 100644 (file)
--- a/src/hooks/dhcp/high_availability/ha_messages.h
+++ b/src/hooks/dhcp/high_availability/ha_messages.h
@@ -108,11 +108,13 @@ extern const isc::log::MessageID HA_SUBNET4_SELECT_INVALID_HA_SERVER_NAME;
 extern const isc::log::MessageID HA_SUBNET4_SELECT_NOT_FOR_US;
 extern const isc::log::MessageID HA_SUBNET4_SELECT_NO_RELATIONSHIP_FOR_SUBNET;
 extern const isc::log::MessageID HA_SUBNET4_SELECT_NO_RELATIONSHIP_SELECTOR_FOR_SUBNET;
+extern const isc::log::MessageID HA_SUBNET4_SELECT_NO_SUBNET_SELECTED;
 extern const isc::log::MessageID HA_SUBNET6_SELECT_FAILED;
 extern const isc::log::MessageID HA_SUBNET6_SELECT_INVALID_HA_SERVER_NAME;
 extern const isc::log::MessageID HA_SUBNET6_SELECT_NOT_FOR_US;
 extern const isc::log::MessageID HA_SUBNET6_SELECT_NO_RELATIONSHIP_FOR_SUBNET;
 extern const isc::log::MessageID HA_SUBNET6_SELECT_NO_RELATIONSHIP_SELECTOR_FOR_SUBNET;
+extern const isc::log::MessageID HA_SUBNET6_SELECT_NO_SUBNET_SELECTED;
 extern const isc::log::MessageID HA_SYNC_COMPLETE_NOTIFY_COMMUNICATIONS_FAILED;
 extern const isc::log::MessageID HA_SYNC_COMPLETE_NOTIFY_FAILED;
 extern const isc::log::MessageID HA_SYNC_COMPLETE_NOTIFY_HANDLER_FAILED;

diff --git a/src/hooks/dhcp/high_availability/ha_messages.mes b/src/hooks/dhcp/high_availability/ha_messages.mes
index 7d4b4743a81e760737e00903f7d80a3e76d07457..09cb61f0abf170af12300d70f6e1ba004ae6be5c 100644 (file)
--- a/src/hooks/dhcp/high_availability/ha_messages.mes
+++ b/src/hooks/dhcp/high_availability/ha_messages.mes
@@ -595,6 +595,13 @@ for any of the subnets it is a configuration error. The first argument is
 the client identification information. The second argument is a subnet
 prefix.
 
+% HA_SUBNET4_SELECT_NO_SUBNET_SELECTED %1: unable to determine HA relationship because no subnet has been selected for the client
+This debug message is issued when the received DHCPv4 query is dropped
+by this server because it could not select a subnet for this client.
+Selecting the subnet is required to find a suitable HA relationship.
+This message is not emitted when the server has only one relationship.
+The argument is the client identification information.
+
 % HA_SUBNET4_SELECT_NOT_FOR_US %1: dropping query in relationship %2 to be processed by another server
 This debug message is issued when the received DHCPv4 query is dropped
 by this server because it should be served by another server. This
@@ -633,6 +640,13 @@ for any of the subnets it is a configuration error. The first argument is
 the client identification information. The second argument is a subnet
 prefix.
 
+% HA_SUBNET6_SELECT_NO_SUBNET_SELECTED %1: unable to determine HA relationship because no subnet has been selected for the client
+This debug message is issued when the received DHCPv6 query is dropped
+by this server because it could not select a subnet for this client.
+Selecting the subnet is required to find a suitable HA relationship.
+This message is not emitted when the server has only one relationship.
+The argument is the client identification information.
+
 % HA_SUBNET6_SELECT_NOT_FOR_US %1: dropping query in relationship %2 to be processed by another server
 This debug message is issued when the received DHCPv6 query is dropped
 by this server because it should be served by another server. This

diff --git a/src/hooks/dhcp/high_availability/tests/ha_impl_unittest.cc b/src/hooks/dhcp/high_availability/tests/ha_impl_unittest.cc
index 5a7576fcce4343b656e0b5c1f527319847f8bdbf..e61664ce1a3eda90622fac1f6df5f7b9d4433337 100644 (file)
--- a/src/hooks/dhcp/high_availability/tests/ha_impl_unittest.cc
+++ b/src/hooks/dhcp/high_availability/tests/ha_impl_unittest.cc
@@ -539,6 +539,47 @@ TEST_F(HAImplTest, subnet4SelectDropNotInScope) {
     EXPECT_TRUE(query4->inClass("HA_server3"));
 }
 
+// Tests that the subnet4_select drops a packet when no subnet has been selected.
+TEST_F(HAImplTest, subnet4SelectNoSubnet) {
+    ConstElementPtr ha_config = createValidHubJsonConfiguration();
+
+    // Create implementation object and configure it.
+    TestHAImpl ha_impl;
+    ASSERT_NO_THROW(ha_impl.configure(ha_config));
+
+    // Starting the service is required before any callouts.
+    NetworkStatePtr network_state(new NetworkState(NetworkState::DHCPv4));
+    ASSERT_NO_THROW(ha_impl.startServices(io_service_, network_state,
+                                          HAServerType::DHCPv4));
+
+    ha_impl.services_->get("server2")->serveFailoverScopes();
+    ha_impl.services_->get("server4")->serveFailoverScopes();
+
+    // Create callout handle to be used for passing arguments to the
+    // callout.
+    CalloutHandlePtr callout_handle = HooksManager::createCalloutHandle();
+    ASSERT_TRUE(callout_handle);
+    ASSERT_EQ(CalloutHandle::NEXT_STEP_CONTINUE, callout_handle->getStatus());
+
+    // Create the query.
+    Pkt4Ptr query4(new Pkt4(DHCPDISCOVER, 1234));
+    callout_handle->setArgument("query4", query4);
+
+    // Create null subnet.
+    Subnet4Ptr subnet4;
+    callout_handle->setArgument("subnet4", subnet4);
+
+    // Invoke the subnet4_select callout.
+    ASSERT_NO_THROW(ha_impl.subnet4Select(*callout_handle));
+
+    // No subnet has been selected and we have multiple relationships. We
+    // expect that the server drops the packet.
+    EXPECT_EQ(CalloutHandle::NEXT_STEP_DROP, callout_handle->getStatus());
+
+    // No HA-specific classes should be assigned.
+    EXPECT_TRUE(query4->getClasses().empty());
+}
+
 // Tests for buffer6_receive callout implementation.
 TEST_F(HAImplTest, buffer6Receive) {
     // Use hot-standby mode to make sure that this server instance is selected
@@ -913,6 +954,47 @@ TEST_F(HAImplTest, subnet6SelectDropNotInScope) {
     EXPECT_TRUE(query6->inClass("HA_server3"));
 }
 
+// Tests that the subnet6_select drops a packet when no subnet has been selected.
+TEST_F(HAImplTest, subnet6SelectNoSubnet) {
+    ConstElementPtr ha_config = createValidHubJsonConfiguration();
+
+    // Create implementation object and configure it.
+    TestHAImpl ha_impl;
+    ASSERT_NO_THROW(ha_impl.configure(ha_config));
+
+    // Starting the service is required before any callouts.
+    NetworkStatePtr network_state(new NetworkState(NetworkState::DHCPv6));
+    ASSERT_NO_THROW(ha_impl.startServices(io_service_, network_state,
+                                          HAServerType::DHCPv6));
+
+    ha_impl.services_->get("server2")->serveFailoverScopes();
+    ha_impl.services_->get("server4")->serveFailoverScopes();
+
+    // Create callout handle to be used for passing arguments to the
+    // callout.
+    CalloutHandlePtr callout_handle = HooksManager::createCalloutHandle();
+    ASSERT_TRUE(callout_handle);
+    ASSERT_EQ(CalloutHandle::NEXT_STEP_CONTINUE, callout_handle->getStatus());
+
+    // Create the query.
+    Pkt6Ptr query6(new Pkt6(DHCPV6_SOLICIT, 1234));
+    callout_handle->setArgument("query6", query6);
+
+    // Create null subnet.
+    Subnet6Ptr subnet6;
+    callout_handle->setArgument("subnet6", subnet6);
+
+    // Invoke the subnet6_select callout.
+    ASSERT_NO_THROW(ha_impl.subnet6Select(*callout_handle));
+
+    // No subnet has been selected and we have multiple relationships. We
+    // expect that the server drops the packet.
+    EXPECT_EQ(CalloutHandle::NEXT_STEP_DROP, callout_handle->getStatus());
+
+    // No HA-specific classes should be assigned.
+    EXPECT_TRUE(query6->getClasses().empty());
+}
+
 // Tests leases4_committed callout implementation.
 TEST_F(HAImplTest, leases4Committed) {
     // Create implementation object and configure it.