Commit cf7f6c97 authored by Marcin Siodelski's avatar Marcin Siodelski

[#1041] Several improvements to status-get (HA)

- Included boolean parameter in-touch to indicate if the status has been
  actually received from the partner.
- Renamed last-known-status (scopes) to last-status (scopes) which is
  shorter.
- Always include all parameters in the status-get response, even if they
  are not set to anything meaningful.
- Return partner's scopes in the response to status-get.
parent 0bbaae02
......@@ -7,6 +7,7 @@
#include <config.h>
#include <communication_state.h>
#include <exceptions/exceptions.h>
#include <ha_service_states.h>
#include <exceptions/exceptions.h>
#include <dhcp/dhcp4.h>
......@@ -22,6 +23,7 @@
#include <utility>
using namespace isc::asiolink;
using namespace isc::data;
using namespace isc::dhcp;
using namespace isc::http;
using namespace boost::posix_time;
......@@ -46,8 +48,9 @@ CommunicationState::CommunicationState(const IOServicePtr& io_service,
const HAConfigPtr& config)
: io_service_(io_service), config_(config), timer_(), interval_(0),
poke_time_(boost::posix_time::microsec_clock::universal_time()),
heartbeat_impl_(0), partner_state_(-1), clock_skew_(0, 0, 0, 0),
last_clock_skew_warn_(), my_time_at_skew_(), partner_time_at_skew_() {
heartbeat_impl_(0), partner_state_(-1), partner_scopes_(),
clock_skew_(0, 0, 0, 0), last_clock_skew_warn_(),
my_time_at_skew_(), partner_time_at_skew_() {
}
CommunicationState::~CommunicationState() {
......@@ -78,6 +81,28 @@ CommunicationState::setPartnerState(const std::string& state) {
}
}
void
CommunicationState::setPartnerScopes(ConstElementPtr new_scopes) {
if (!new_scopes || (new_scopes->getType() != Element::list)) {
isc_throw(BadValue, "unable to record partner's HA scopes because"
" the received value is not a valid JSON list");
}
std::set<std::string> partner_scopes;
for (auto i = 0; i < new_scopes->size(); ++i) {
auto scope = new_scopes->get(i);
if (scope->getType() != Element::string) {
isc_throw(BadValue, "unable to record partner's HA scopes because"
" the received scope value is not a valid JSON string");
}
auto scope_str = scope->stringValue();
if (!scope_str.empty()) {
partner_scopes.insert(scope_str);
}
}
partner_scopes_ = partner_scopes;
}
void
CommunicationState::startHeartbeat(const long interval,
const boost::function<void()>& heartbeat_impl) {
......
......@@ -11,6 +11,7 @@
#include <ha_service_states.h>
#include <asiolink/interval_timer.h>
#include <asiolink/io_service.h>
#include <cc/data.h>
#include <dhcp/pkt.h>
#include <boost/date_time/posix_time/posix_time.hpp>
#include <boost/function.hpp>
......@@ -100,6 +101,12 @@ public:
/// @throw BadValue if unsupported state value was provided.
void setPartnerState(const std::string& state);
std::set<std::string> getPartnerScopes() const {
return (partner_scopes_);
}
void setPartnerScopes(data::ConstElementPtr new_scopes);
/// @brief Starts recurring heartbeat (public interface).
///
/// @param interval heartbeat interval in milliseconds.
......@@ -313,6 +320,9 @@ protected:
/// Negative value means that the partner's state is unknown.
int partner_state_;
/// @brief Last known set of scopes served by the partner server.
std::set<std::string> partner_scopes_;
/// @brief Clock skew between the active servers.
boost::posix_time::time_duration clock_skew_;
......
......@@ -965,8 +965,10 @@ HAService::processStatusGet() const {
int state = getCurrState();
try {
local->set("state", Element::create(stateToString(state)));
} catch (const std::exception&) {
/* ignore errors */
} catch (...) {
// Empty string on error.
local->set("state", Element::create(std::string()));
}
std::set<std::string> scopes = query_filter_.getServedScopes();
ElementPtr list = Element::createList();
......@@ -978,24 +980,39 @@ HAService::processStatusGet() const {
// Remote part
ElementPtr remote = Element::createMap();
// Add the in-touch boolean flag to indicate whether there was any
// communication between the HA peers. Based on that, the user
// may determine if the status returned for the peer is based on
// the heartbeat or is to be determined.
remote->set("in-touch", Element::create(communication_state_->getPartnerState() > 0));
try {
role = config_->getFailoverPeerConfig()->getRole();
std::string role_txt = HAConfig::PeerConfig::roleToString(role);
remote->set("role", Element::create(role_txt));
} catch (const std::exception&) {
/* ignore errors */
} catch (...) {
remote->set("role", Element::create(std::string()));
}
try {
state = getPartnerState();
remote->set("last-known-state", Element::create(stateToString(state)));
} catch (const std::exception&) {
/* ignore errors */
remote->set("last-state", Element::create(stateToString(state)));
} catch (...) {
remote->set("last-state", Element::create(std::string()));
}
// No current way to get remote scopes.
// Add remote if not empty.
if (remote->size() > 0) {
ha_servers->set("remote", remote);
// Remote server's scopes.
scopes = communication_state_->getPartnerScopes();
list = Element::createList();
for (auto scope : scopes) {
list->add(Element::create(scope));
}
remote->set("last-scopes", list);
ha_servers->set("remote", remote);
return (ha_servers);
}
......@@ -1085,6 +1102,19 @@ HAService::asyncSendHeartbeat() {
// Note the time returned by the partner to calculate the clock skew.
communication_state_->setPartnerTime(date_time->stringValue());
// Remember the scopes served by the partner.
try {
auto scopes = args->get("scopes");
communication_state_->setPartnerScopes(scopes);
} catch (...) {
// We don't want to fail if the scopes are missing because
// this would be incompatible with old HA hook library
// versions. We may make it mandatory one day, but during
// upgrades of existing HA setup it would be a real issue
// if we failed here.
}
} catch (const std::exception& ex) {
LOG_WARN(ha_logger, HA_HEARTBEAT_FAILED)
.arg(partner_config->getLogLabel())
......
......@@ -510,8 +510,10 @@ public:
/// @brief Processes status-get command and returns a response.
///
/// @c HAImpl::commandProcessed calls this to add HA servers info
/// into the status-get response.
///
///
/// @c HAImpl::commandProcessed calls this to add information about the
/// HA servers status into the status-get response.
data::ConstElementPtr processStatusGet() const;
protected:
......
......@@ -22,6 +22,7 @@
using namespace isc;
using namespace isc::asiolink;
using namespace isc::data;
using namespace isc::dhcp;
using namespace isc::ha;
using namespace isc::ha::test;
......@@ -102,7 +103,42 @@ TEST_F(CommunicationStateTest, partnerState) {
// An attempt to set unsupported value should result in exception.
EXPECT_THROW(state_.setPartnerState("unsupported"), BadValue);
}
// Verifies that the partner's scopes are set and retrieved correctly.
TEST_F(CommunicationStateTest, partnerScopes) {
// Initially, the scopes should be empty.
ASSERT_TRUE(state_.getPartnerScopes().empty());
// Set new partner scopes.
ASSERT_NO_THROW(
state_.setPartnerScopes(Element::fromJSON("[ \"server1\", \"server2\" ]"))
);
// Get them back.
auto returned = state_.getPartnerScopes();
EXPECT_EQ(2, returned.size());
EXPECT_EQ(1, returned.count("server1"));
EXPECT_EQ(1, returned.count("server2"));
// Override the scopes.
ASSERT_NO_THROW(
state_.setPartnerScopes(Element::fromJSON("[ \"server1\" ]"))
);
returned = state_.getPartnerScopes();
EXPECT_EQ(1, returned.size());
EXPECT_EQ(1, returned.count("server1"));
// Clear the scopes.
ASSERT_NO_THROW(
state_.setPartnerScopes(Element::fromJSON("[ ]"))
);
returned = state_.getPartnerScopes();
EXPECT_TRUE(returned.empty());
// An attempt to set invalid JSON should fail.
EXPECT_THROW(state_.setPartnerScopes(Element::fromJSON("{ \"not-a-list\": 1 }")),
BadValue);
}
// Verifies that the object is poked right after construction.
......
......@@ -570,6 +570,9 @@ TEST_F(HAImplTest, statusGet) {
" \"state\": \"waiting\""
" },"
" \"remote\": {"
" \"in-touch\": false,"
" \"last-scopes\": [ ],"
" \"last-state\": \"\","
" \"role\": \"secondary\""
" }"
" },"
......
......@@ -40,6 +40,7 @@
#include <gtest/gtest.h>
#include <functional>
#include <sstream>
#include <set>
#include <string>
#include <vector>
......@@ -1068,12 +1069,20 @@ TEST_F(HAServiceTest, hotStandbyScopeSelectionThisPrimary) {
// Check the reported info about servers.
ConstElementPtr ha_servers = service.processStatusGet();
ASSERT_TRUE(ha_servers);
std::string expected = "{ "
"\"local\": { \"role\": \"primary\", "
"\"scopes\": [ \"server1\" ], "
"\"state\": \"hot-standby\" }, "
"\"remote\": { \"role\": \"standby\" } }";
EXPECT_EQ(expected, ha_servers->str());
std::string expected = "{"
" \"local\": {"
" \"role\": \"primary\","
" \"scopes\": [ \"server1\" ],"
" \"state\": \"hot-standby\""
" }, "
" \"remote\": {"
" \"in-touch\": false,"
" \"role\": \"standby\","
" \"last-scopes\": [ ],"
" \"last-state\": \"\""
" }"
"}";
EXPECT_TRUE(isEquivalent(Element::fromJSON(expected), ha_servers));
// Set the test size - 65535 queries.
const unsigned queries_num = 65535;
......@@ -1106,11 +1115,21 @@ TEST_F(HAServiceTest, hotStandbyScopeSelectionThisStandby) {
// Check the reported info about servers.
ConstElementPtr ha_servers = service.processStatusGet();
ASSERT_TRUE(ha_servers);
std::string expected = "{ "
"\"local\": { \"role\": \"standby\", "
"\"scopes\": [ ], \"state\": \"waiting\" }, "
"\"remote\": { \"role\": \"primary\" } }";
EXPECT_EQ(expected, ha_servers->str());
std::string expected = "{"
" \"local\": {"
" \"role\": \"standby\","
" \"scopes\": [ ],"
" \"state\": \"waiting\""
" }, "
" \"remote\": {"
" \"in-touch\": false,"
" \"role\": \"primary\","
" \"last-scopes\": [ ],"
" \"last-state\": \"\""
" }"
"}";
EXPECT_TRUE(isEquivalent(Element::fromJSON(expected), ha_servers));
// Set the test size - 65535 queries.
const unsigned queries_num = 65535;
......@@ -2576,7 +2595,7 @@ public:
const TestHttpResponseCreatorFactoryPtr& factory,
const std::string& initial_state = "waiting")
: listener_(listener), factory_(factory), running_(false),
static_date_time_() {
static_date_time_(), static_scopes_() {
transition(initial_state);
}
......@@ -2595,6 +2614,13 @@ public:
static_date_time_ = static_date_time;
}
/// @brief Sets static scopes to be used in responses.
///
/// @param scopes Fixed scopes set.
void setScopes(const std::set<std::string>& scopes) {
static_scopes_ = scopes;
}
/// @brief Enable response to commands required for leases synchronization.
///
/// Enables dhcp-disable, dhcp-enable and lease4-get-page commands. The last
......@@ -2644,6 +2670,13 @@ public:
if (!static_date_time_.empty()) {
response_arguments->set("date-time", Element::create(static_date_time_));
}
if (!static_scopes_.empty()) {
auto json_scopes = Element::createList();
for (auto scope : static_scopes_) {
json_scopes->add(Element::create(scope));
}
response_arguments->set("scopes", json_scopes);
}
factory_->getResponseCreator()->setArguments(response_arguments);
}
......@@ -2663,6 +2696,9 @@ private:
/// @brief Static date-time value to be returned.
std::string static_date_time_;
/// @brief Static scopes to be reported.
std::set<std::string> static_scopes_;
};
/// @brief Shared pointer to a partner.
......@@ -3041,7 +3077,9 @@ TEST_F(HAServiceStateMachineTest, waitingParterDownLoadBalancingPartnerDown) {
EXPECT_EQ(HA_PARTNER_DOWN_ST, service_->getCurrState());
// Partner shows up and (eventually) transitions to READY state.
HAPartner partner(listener2_, factory2_, "ready");
HAPartner partner(listener2_, factory2_);
partner.setScopes({ "server1", "server2" });
partner.transition("ready");
partner.startup();
// PARTNER DOWN state: receive a response from the partner indicating that
......@@ -3056,14 +3094,21 @@ TEST_F(HAServiceStateMachineTest, waitingParterDownLoadBalancingPartnerDown) {
// Check the reported info about servers.
ConstElementPtr ha_servers = service_->processStatusGet();
ASSERT_TRUE(ha_servers);
std::cout << ha_servers->str() << "\n";
std::string expected = "{ "
"\"local\": { \"role\": \"primary\", "
"\"scopes\": [ \"server1\", \"server2\" ], "
"\"state\": \"load-balancing\" }, "
"\"remote\": { \"last-known-state\": \"ready\", "
"\"role\": \"secondary\" } }";
EXPECT_EQ(expected, ha_servers->str());
std::string expected = "{"
" \"local\": {"
" \"role\": \"primary\","
" \"scopes\": [ \"server1\", \"server2\" ], "
" \"state\": \"load-balancing\""
" }, "
" \"remote\": {"
" \"in-touch\": true,"
" \"role\": \"secondary\","
" \"last-scopes\": [ \"server1\", \"server2\" ],"
" \"last-state\": \"ready\""
" }"
"}";
EXPECT_TRUE(isEquivalent(Element::fromJSON(expected), ha_servers));
// Crash the partner and see whether our server can return to the partner
// down state.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment