From: Marcin Siodelski Date: Thu, 25 Oct 2018 15:24:53 +0000 (+0200) Subject: [#78,!85] Send dhcp-disable every time we ask for a page of leases. X-Git-Tag: 66-authoritative-flag-in-kea_base~9 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=055eec3bac9abf6c7be5a73d5501dba348671501;p=thirdparty%2Fkea.git [#78,!85] Send dhcp-disable every time we ask for a page of leases. --- diff --git a/src/hooks/dhcp/high_availability/ha_messages.mes b/src/hooks/dhcp/high_availability/ha_messages.mes index 1f752bcaf8..cbb93f476b 100644 --- a/src/hooks/dhcp/high_availability/ha_messages.mes +++ b/src/hooks/dhcp/high_availability/ha_messages.mes @@ -181,6 +181,12 @@ servers to resume the HA service. This informational message indicates that the High Availability hooks library has been loaded successfully. +% HA_LEASES_SYNC_LEASE_PAGE_RECEIVED received %1 leases from %2 +This informational message is issued during lease database synchronization +to indicate that a bulk of leases have been received. The first argument +holds the count of leases received. The second argument specifies the +partner server name. + % HA_LEASES4_COMMITTED_FAILED leases4_committed callout failed: %1 This error message is issued when the callout for the leases4_committed hook point failed. This includes unexpected errors like wrong arguments provided to diff --git a/src/hooks/dhcp/high_availability/ha_service.cc b/src/hooks/dhcp/high_availability/ha_service.cc index 761f270842..9256de5d52 100644 --- a/src/hooks/dhcp/high_availability/ha_service.cc +++ b/src/hooks/dhcp/high_availability/ha_service.cc @@ -359,7 +359,7 @@ HAService::syncingStateHandler() { std::string status_message; int sync_status = synchronize(status_message, config_->getFailoverPeerConfig()->getName(), - 60, config_->getSyncPageLimit()); + 60); // If the leases synchronization was successful, let's transition // to the ready state. @@ -1018,7 +1018,7 @@ void HAService::asyncDisable(HttpClient& http_client, const std::string& server_name, const unsigned int max_period, - const PostRequestCallback& post_request_action) { + PostRequestCallback post_request_action) { HAConfig::PeerConfigPtr remote_config = config_->getPeerConfig(server_name); // Create HTTP/1.1 request including our command. @@ -1084,7 +1084,7 @@ HAService::asyncDisable(HttpClient& http_client, void HAService::asyncEnable(HttpClient& http_client, const std::string& server_name, - const PostRequestCallback& post_request_action) { + PostRequestCallback post_request_action) { HAConfig::PeerConfigPtr remote_config = config_->getPeerConfig(server_name); // Create HTTP/1.1 request including our command. @@ -1159,15 +1159,50 @@ HAService::localEnable() { void HAService::asyncSyncLeases() { - PostRequestCallback null_action; - asyncSyncLeases(client_, LeasePtr(), config_->getSyncPageLimit(), null_action); + PostSyncCallback null_action; + asyncSyncLeases(client_, config_->getFailoverPeerConfig()->getName(), + 60, LeasePtr(), null_action); } void HAService::asyncSyncLeases(http::HttpClient& http_client, + const std::string& server_name, + const unsigned int max_period, const dhcp::LeasePtr& last_lease, - const uint32_t limit, - const PostRequestCallback& post_sync_action) { + PostSyncCallback post_sync_action, + const bool dhcp_disabled) { + // Synchronization starts with a command to disable DHCP service of the + // peer from which we're fetching leases. We don't want the other server + // to allocate new leases while we fetch from it. The DHCP service will + // be disabled for a certain amount of time and will be automatically + // re-enabled if we die during the synchronization. + asyncDisable(http_client, server_name, max_period, + [this, &http_client, server_name, max_period, last_lease, + post_sync_action, dhcp_disabled] + (const bool success, const std::string& error_message) { + + // If we have successfully disabled the DHCP service on the peer, + // we can start fetching the leases. + if (success) { + // The last argument indicates that disabling the DHCP + // service on the partner server was successful. + asyncSyncLeasesInternal(http_client, server_name, max_period, + last_lease, post_sync_action, true); + + } else { + post_sync_action(success, error_message, dhcp_disabled); + } + }); +} + +void +HAService::asyncSyncLeasesInternal(http::HttpClient& http_client, + const std::string& server_name, + const unsigned int max_period, + const dhcp::LeasePtr& last_lease, + PostSyncCallback post_sync_action, + const bool dhcp_disabled) { + HAConfig::PeerConfigPtr partner_config = config_->getFailoverPeerConfig(); // Create HTTP/1.1 request including our command. @@ -1175,11 +1210,11 @@ HAService::asyncSyncLeases(http::HttpClient& http_client, (HttpRequest::Method::HTTP_POST, "/", HttpVersion::HTTP_11()); if (server_type_ == HAServerType::DHCPv4) { request->setBodyAsJson(CommandCreator::createLease4GetPage( - boost::dynamic_pointer_cast(last_lease), limit)); + boost::dynamic_pointer_cast(last_lease), config_->getSyncPageLimit())); } else { request->setBodyAsJson(CommandCreator::createLease6GetPage( - boost::dynamic_pointer_cast(last_lease), limit)); + boost::dynamic_pointer_cast(last_lease), config_->getSyncPageLimit())); } request->finalize(); @@ -1189,7 +1224,8 @@ HAService::asyncSyncLeases(http::HttpClient& http_client, // Schedule asynchronous HTTP request. http_client.asyncSendRequest(partner_config->getUrl(), request, response, - [this, partner_config, post_sync_action, &http_client, limit] + [this, partner_config, post_sync_action, &http_client, server_name, + max_period, dhcp_disabled] (const boost::system::error_code& ec, const HttpResponsePtr& response, const std::string& error_str) { @@ -1234,6 +1270,10 @@ HAService::asyncSyncLeases(http::HttpClient& http_client, // Iterate over the leases and update the database as appropriate. const auto& leases_element = leases->listValue(); + LOG_INFO(ha_logger, HA_LEASES_SYNC_LEASE_PAGE_RECEIVED) + .arg(leases_element.size()) + .arg(server_name); + for (auto l = leases_element.begin(); l != leases_element.end(); ++l) { try { @@ -1260,7 +1300,8 @@ HAService::asyncSyncLeases(http::HttpClient& http_client, // If we're not on the last page and we're processing final lease on // this page, let's record the lease as input to the next // lease4-get-page command. - if ((leases_element.size() >= limit) && (l + 1 == leases_element.end())) { + if ((leases_element.size() >= config_->getSyncPageLimit()) && + (l + 1 == leases_element.end())) { last_lease = boost::dynamic_pointer_cast(lease); } @@ -1288,7 +1329,8 @@ HAService::asyncSyncLeases(http::HttpClient& http_client, // If we're not on the last page and we're processing final lease on // this page, let's record the lease as input to the next // lease6-get-page command. - if ((leases_element.size() >= limit) && (l + 1 == leases_element.end())) { + if ((leases_element.size() >= config_->getSyncPageLimit()) && + (l + 1 == leases_element.end())) { last_lease = boost::dynamic_pointer_cast(lease); } @@ -1317,14 +1359,16 @@ HAService::asyncSyncLeases(http::HttpClient& http_client, } else if (last_lease) { // This indicates that there are more leases to be fetched. // Therefore, we have to send another leaseX-get-page command. - asyncSyncLeases(http_client, last_lease, limit, post_sync_action); + asyncSyncLeases(http_client, server_name, max_period, last_lease, + post_sync_action, dhcp_disabled); return; } // Invoke post synchronization action if it was specified. if (post_sync_action) { post_sync_action(error_message.empty(), - error_message); + error_message, + dhcp_disabled); } }, HttpClient::RequestTimeout(config_->getSyncTimeout())); } @@ -1333,59 +1377,49 @@ ConstElementPtr HAService::processSynchronize(const std::string& server_name, const unsigned int max_period) { std::string answer_message; - int sync_status = synchronize(answer_message, server_name, max_period, - config_->getSyncPageLimit()); + int sync_status = synchronize(answer_message, server_name, max_period); return (createAnswer(sync_status, answer_message)); } int HAService::synchronize(std::string& status_message, const std::string& server_name, - const unsigned int max_period, const uint32_t page_limit) { + const unsigned int max_period) { IOService io_service; HttpClient client(io_service); - // Synchronization starts with a command to disable DHCP service of the - // peer from which we're fetching leases. We don't want the other server - // to allocate new leases while we fetch from it. The DHCP service will - // be disabled for a certain amount of time and will be automatically - // re-enabled if we die during the synchronization. - asyncDisable(client, server_name, max_period, - [&](const bool success, const std::string& error_message) { - // If we have successfully disabled the DHCP service on the peer, - // we can start fetching the leases. - if (success) { - asyncSyncLeases(client, Lease4Ptr(), page_limit, - [&](const bool success, const std::string& error_message) { - // If there was a fatal error while fetching the leases, let's - // log an error message so as it can be included in the response - // to the controlling client. - if (!success) { + asyncSyncLeases(client, server_name, max_period, Lease4Ptr(), + [&](const bool success, const std::string& error_message, + const bool dhcp_disabled) { + // If there was a fatal error while fetching the leases, let's + // log an error message so as it can be included in the response + // to the controlling client. + if (!success) { + status_message = error_message; + } + + // Whether or not there was an error while fetching the leases, + // we need to re-enable the DHCP service on the peer if the + // DHCP service was disabled in the course of synchronization. + if (dhcp_disabled) { + asyncEnable(client, server_name, + [&](const bool success, + const std::string& error_message) { + // It is possible that we have already recorded an error + // message while synchronizing the lease database. Don't + // override the existing error message. + if (!success && status_message.empty()) { status_message = error_message; } - // Whether or not there was an error while fetching the leases, - // we need to re-enable the DHCP service on the peer. - asyncEnable(client, server_name, - [&](const bool success, - const std::string& error_message) { - // It is possible that we have already recorded an error - // message while synchronizing the lease database. Don't - // override the existing error message. - if (!success && status_message.empty()) { - status_message = error_message; - } - // The synchronization process is completed, so let's break - // the IO service so as we can return the response to the - // controlling client. - io_service.stop(); - }); + // The synchronization process is completed, so let's break + // the IO service so as we can return the response to the + // controlling client. + io_service.stop(); }); } else { - // We have failed to disable the DHCP service of the peer. Let's - // record the error message and break the IO service so as we can - // return the response to the controlling client. - status_message = error_message; + // Also stop IO service if there is no need to enable DHCP + // service. io_service.stop(); } }); diff --git a/src/hooks/dhcp/high_availability/ha_service.h b/src/hooks/dhcp/high_availability/ha_service.h index 709c63967e..bacef1fd99 100644 --- a/src/hooks/dhcp/high_availability/ha_service.h +++ b/src/hooks/dhcp/high_availability/ha_service.h @@ -55,9 +55,18 @@ protected: /// @brief Callback invoked when request was sent and a response received /// or an error occurred. /// - /// The first arguments indicates if the operation passed (when true). + /// The first argument indicates if the operation passed (when true). /// The second argument holds error message. - typedef std::function PostRequestCallback; + typedef std::function PostRequestCallback; + + /// @brief Callback invoked when lease database synchronization is complete. + /// + /// The first argument indicates if the operation passed (when true). + /// The second argument holds error message. + /// The third argument indicates whether the synchronization resulted in + /// disabling DHCP service on the partner server and has to be + /// re-enabled. + typedef std::function PostSyncCallback; public: @@ -509,7 +518,7 @@ protected: void asyncDisable(http::HttpClient& http_client, const std::string& server_name, const unsigned int max_period, - const PostRequestCallback& post_request_action); + PostRequestCallback post_request_action); /// @brief Schedules asynchronous "dhcp-enable" command to the specified /// server. @@ -522,7 +531,7 @@ protected: /// the request is completed. void asyncEnable(http::HttpClient& http_client, const std::string& server_name, - const PostRequestCallback& post_request_action); + PostRequestCallback post_request_action); /// @brief Disables local DHCP service. void localDisable(); @@ -533,7 +542,7 @@ protected: /// @brief Asynchronously reads leases from a peer and updates local /// lease database. /// - /// This method asynchronously sends lease4-get-all command to fetch all + /// This method asynchronously sends lease4-get-page command to fetch /// leases from the HA peer database. When the response is received, the /// callback function iterates over the returned leases and inserts those /// that are not present in the local database and replaces any existing @@ -550,12 +559,32 @@ protected: /// @brief Asynchronously reads leases from a peer and updates local /// lease database using a provided client instance. /// - /// This method asynchronously sends lease4-get-all command to fetch all - /// leases from the HA peer database. When the response is received, the - /// callback function iterates over the returned leases and inserts those - /// that are not present in the local database and replaces any existing - /// leases if the fetched lease instance is newer (based on cltt) than - /// the instance in the local lease database. + /// This method first sends dhcp-disable command to the server from which + /// it will be fetching leases to disable its DHCP function while database + /// synchronization is in progress. If the command is successful, it then + /// sends lease4-get-page command to fetch a page of leases from the + /// partner's database. Depending on the configured page size, it may + /// be required to send multiple lease4-get-page or lease6-get-page + /// commands to fetch all leases. If the lease database is large, + /// the database synchronization may even take several minutes. + /// Therefore, dhcp-disable command is sent prior to fetching each page, + /// in order to reset the timeout for automatic re-enabling of the + /// DHCP service on the remote server. Such timeout must only occur + /// if there was no communication from the synchronizing server for + /// longer period of time. If the synchronization is progressing the + /// timeout must be deferred. + /// + /// The @c asyncSyncLeases method calls itself recursively when the + /// previous @c lease4-get-page or @c lease6-get-page command has + /// completed successfully. If the last page of leases was fetched or + /// if any error occurred, the synchronization is terminated and the + /// @c post_sync_action callback is invoked. + /// + /// The last parameter passed to the @c post_sync_action callback indicates + /// whether this server has successfully disabled DHCP service on + /// the partner server at least once. If that's the case, the DHCP + /// service must be re-enabled by sending dhcp-enable command. This + /// is done in the @c HAService::synchronize method. /// /// If there is an error while inserting or updating any of the leases /// a warning message is logged and the process continues for the @@ -563,19 +592,60 @@ protected: /// /// @param http_client reference to the client to be used to communicate /// with the other server. + /// @param server_name name of the server to fetch leases from. + /// @param max_period maximum number of seconds to disable DHCP service /// @param lease Pointer to the last lease returned on the previous /// page of leases. This lease is used to set the value of the "from" - /// parameter in the lease4-get-page and lease6-get-page commands. If this + /// parameter in the @c lease4-get-page and @c lease6-get-page commands. If this /// command is sent to fetch the first page, the @c last_lease parameter /// should be set to null. - /// @param limit Limit of leases on the page. /// @param post_sync_action pointer to the function to be executed when /// lease database synchronization is complete. If this is null, no /// post synchronization action is invoked. + /// @param dhcp_disabled Boolean flag indicating if the remote DHCP + /// server is disabled. This flag propagates down to the + /// @c post_sync_action to indicate whether the DHCP service has to + /// be enabled after the leases synchronization. void asyncSyncLeases(http::HttpClient& http_client, + const std::string& server_name, + const unsigned int max_period, const dhcp::LeasePtr& last_lease, - const uint32_t limit, - const PostRequestCallback& post_sync_action); + PostSyncCallback post_sync_action, + const bool dhcp_disabled = false); + + /// @brief Implements fetching one page of leases during synchronization. + /// + /// This method implements the actual lease fetching from the partner + /// and synchronization of the database. It excludes sending @c dhcp-disable + /// command. This command is sent by @c HAService::asyncSyncLeases. + /// + /// When the page of leases is successfully synchronized, this method + /// will call @c HAService::asyncSyncLeases to schedule synchronization of + /// the next page of leases. + /// + /// @param http_client reference to the client to be used to communicate + /// with the other server. + /// @param server_name name of the server to fetch leases from. + /// @param max_period maximum number of seconds to disable DHCP service + /// @param lease Pointer to the last lease returned on the previous + /// page of leases. This lease is used to set the value of the "from" + /// parameter in the lease4-get-page and lease6-get-page commands. If this + /// command is sent to fetch the first page, the @c last_lease parameter + /// should be set to null. + /// @param post_sync_action pointer to the function to be executed when + /// lease database synchronization is complete. If this is null, no + /// post synchronization action is invoked. + /// @param dhcp_disabled Boolean flag indicating if the remote DHCP + /// server is disabled. This flag propagates down to the + /// @c post_sync_action to indicate whether the DHCP service has to + /// be enabled after the leases synchronization. + void asyncSyncLeasesInternal(http::HttpClient& http_client, + const std::string& server_name, + const unsigned int max_period, + const dhcp::LeasePtr& last_lease, + PostSyncCallback post_sync_action, + const bool dhcp_disabled); + public: @@ -595,7 +665,7 @@ public: /// @param server_name name of the server to fetch leases from. /// @param max_period maximum number of seconds to disable DHCP service /// of the peer. This value is used in dhcp-disable command issued to - /// the peer before the lease4-get-all command. + /// the peer before the lease4-get-page command. /// /// @return Pointer to the response to the ha-sync command. data::ConstElementPtr processSynchronize(const std::string& server_name, @@ -612,13 +682,12 @@ protected: /// @param server_name name of the server to fetch leases from. /// @param max_period maximum number of seconds to disable DHCP service /// of the peer. This value is used in dhcp-disable command issued to - /// the peer before the lease4-get-all command. - /// @param page_limit Maximum size of a single page of leases to be returned. + /// the peer before the lease4-get-page command. /// /// @return Synchronization result according to the status codes returned /// in responses to control commands. int synchronize(std::string& status_message, const std::string& server_name, - const unsigned int max_period, const uint32_t page_limit); + const unsigned int max_period); public: