Commit 92e085f8 authored by Marcin Siodelski's avatar Marcin Siodelski
Browse files

[5078] Handle CA reconfiguration gracefully.

parent a2624b3a
......@@ -5,6 +5,7 @@
// file, You can obtain one at http://mozilla.org/MPL/2.0/.
#include <config.h>
#include <asiolink/asio_wrapper.h>
#include <agent/ca_process.h>
#include <agent/ca_controller.h>
#include <agent/ca_response_creator_factory.h>
......@@ -12,10 +13,10 @@
#include <asiolink/io_address.h>
#include <asiolink/io_error.h>
#include <cc/command_interpreter.h>
#include <http/listener.h>
#include <boost/pointer_cast.hpp>
using namespace isc::asiolink;
using namespace isc::data;
using namespace isc::http;
using namespace isc::process;
......@@ -32,7 +33,8 @@ namespace agent {
CtrlAgentProcess::CtrlAgentProcess(const char* name,
const asiolink::IOServicePtr& io_service)
: DProcessBase(name, io_service, DCfgMgrBasePtr(new CtrlAgentCfgMgr())) {
: DProcessBase(name, io_service, DCfgMgrBasePtr(new CtrlAgentCfgMgr())),
http_listeners_() {
}
CtrlAgentProcess::~CtrlAgentProcess() {
......@@ -47,53 +49,19 @@ CtrlAgentProcess::run() {
LOG_INFO(agent_logger, CTRL_AGENT_STARTED).arg(VERSION);
try {
// Register commands.
CtrlAgentControllerPtr controller =
boost::dynamic_pointer_cast<CtrlAgentController>(
CtrlAgentController::instance());
controller->registerCommands();
// Create response creator factory first. It will be used to generate
// response creators. Each response creator will be used to generate
// answer to specific request.
HttpResponseCreatorFactoryPtr rcf(new CtrlAgentResponseCreatorFactory());
DCfgContextBasePtr base_ctx = getCfgMgr()->getContext();
CtrlAgentCfgContextPtr ctx =
boost::dynamic_pointer_cast<CtrlAgentCfgContext>(base_ctx);
if (!ctx) {
isc_throw(Unexpected, "Interal logic error: bad context type");
}
/// @todo: If the parameter is a hostname, we need to resolve it.
IOAddress server_address("::");
try {
server_address = IOAddress(ctx->getHttpHost());
} catch (const IOError& e) {
isc_throw(BadValue, "Failed to convert " << ctx->getHttpHost()
<< " to IP address:" << e.what());
}
uint16_t server_port = ctx->getHttpPort();
// Create http listener. It will open up a TCP socket and be prepared
// to accept incoming connection.
HttpListener http_listener(*getIoService(), server_address,
server_port, rcf, REQUEST_TIMEOUT);
// Instruct the http listener to actually open socket, install callback
// and start listening.
http_listener.start();
// Ok, seems we're good to go.
LOG_INFO(agent_logger, CTRL_AGENT_HTTP_SERVICE_STARTED)
.arg(server_address.toText()).arg(server_port);
// Let's process incoming data or expiring timers in a loop until
// shutdown condition is detected.
while (!shouldShutdown()) {
getIoService()->run_one();
garbageCollectListeners();
runIO();
}
stopIOService();
} catch (const std::exception& ex) {
......@@ -120,6 +88,15 @@ CtrlAgentProcess::run() {
LOG_DEBUG(agent_logger, isc::log::DBGLVL_START_SHUT, CTRL_AGENT_RUN_EXIT);
}
size_t
CtrlAgentProcess::runIO() {
size_t cnt = getIoService()->get_io_service().poll();
if (!cnt) {
cnt = getIoService()->get_io_service().run_one();
}
return (cnt);
}
isc::data::ConstElementPtr
CtrlAgentProcess::shutdown(isc::data::ConstElementPtr /*args*/) {
setShutdownFlag(true);
......@@ -129,17 +106,113 @@ CtrlAgentProcess::shutdown(isc::data::ConstElementPtr /*args*/) {
isc::data::ConstElementPtr
CtrlAgentProcess::configure(isc::data::ConstElementPtr config_set,
bool check_only) {
// System reconfiguration often poses an interesting issue whereby the
// configuration parsing is successful, but an attempt to use a new
// configuration is not. This will leave us in the inconsistent state
// when the configuration is in fact only partially applied and the
// system's ability to operate is impaired. The use of C++ lambda is
// a way to resolve this problem by injecting the code to the
// simpleParseConfig which performs an attempt to open new instance
// of the listener (if required). The lambda code will throw an
// exception if it fails and cause the simpleParseConfig to rollback
// configuration changes and report an error.
ConstElementPtr answer = getCfgMgr()->simpleParseConfig(config_set,
check_only,
[this]() {
DCfgContextBasePtr base_ctx = getCfgMgr()->getContext();
CtrlAgentCfgContextPtr
ctx = boost::dynamic_pointer_cast<CtrlAgentCfgContext>(base_ctx);
if (!ctx) {
isc_throw(Unexpected, "Interal logic error: bad context type");
}
/// @todo: If the parameter is a hostname, we need to resolve it.
IOAddress server_address("::");
try {
server_address = IOAddress(ctx->getHttpHost());
} catch (const IOError& e) {
isc_throw(BadValue, "Failed to convert " << ctx->getHttpHost()
<< " to IP address:" << e.what());
}
uint16_t server_port = ctx->getHttpPort();
// Only open a new listener if the configuration has changed.
if (http_listeners_.empty() ||
(http_listeners_.back()->getLocalAddress() != server_address) ||
(http_listeners_.back()->getLocalPort() != server_port)) {
// Create response creator factory first. It will be used to
// generate response creators. Each response creator will be
// used to generate answer to specific request.
HttpResponseCreatorFactoryPtr rcf(new CtrlAgentResponseCreatorFactory());
// Create http listener. It will open up a TCP socket and be
// prepared to accept incoming connection.
HttpListenerPtr http_listener(new HttpListener(*getIoService(),
server_address,
server_port, rcf,
REQUEST_TIMEOUT));
// Instruct the http listener to actually open socket, install
// callback and start listening.
http_listener->start();
// The new listener is running so add it to the collection of
// active listeners. The next step will be to remove all other
// active listeners, but we do it inside the main process loop.
http_listeners_.push_back(http_listener);
}
// Ok, seems we're good to go.
LOG_INFO(agent_logger, CTRL_AGENT_HTTP_SERVICE_STARTED)
.arg(server_address.toText()).arg(server_port);
});
int rcode = 0;
isc::data::ConstElementPtr answer = getCfgMgr()->simpleParseConfig(config_set,
check_only);
config::parseAnswer(rcode, answer);
return (answer);
}
void
CtrlAgentProcess::garbageCollectListeners() {
// We expect only one active listener. If there are more (most likely 2),
// it means we have just reconfigured the server and need to shut down all
// listeners execept the most recently added.
if (http_listeners_.size() > 1) {
// Stop no longer used listeners.
for (auto l = http_listeners_.begin(); l != http_listeners_.end() - 1;
++l) {
(*l)->stop();
}
// We have stopped listeners but there may be some pending handlers
// related to these listeners. Need to invoke these handlers.
getIoService()->get_io_service().poll();
// Finally, we're ready to remove no longer used listeners.
http_listeners_.erase(http_listeners_.cbegin(),
http_listeners_.cend() - 1);
}
}
CtrlAgentCfgMgrPtr
CtrlAgentProcess::getCtrlAgentCfgMgr() {
return(boost::dynamic_pointer_cast<CtrlAgentCfgMgr>(getCfgMgr()));
return (boost::dynamic_pointer_cast<CtrlAgentCfgMgr>(getCfgMgr()));
}
ConstHttpListenerPtr
CtrlAgentProcess::getHttpListener() const {
// Return the most recent listener or null.
return (http_listeners_.empty() ? ConstHttpListenerPtr() :
http_listeners_.back());
}
bool
CtrlAgentProcess::isListening() const {
// If there are is a listener, we're listening.
return (static_cast<bool>(getHttpListener()));
}
} // namespace isc::agent
......
......@@ -8,7 +8,9 @@
#define CTRL_AGENT_PROCESS_H
#include <agent/ca_cfg_mgr.h>
#include <http/listener.h>
#include <process/d_process.h>
#include <vector>
namespace isc {
namespace agent {
......@@ -78,6 +80,23 @@ public:
/// processing errors and return a success or failure answer as described
/// below.
///
/// A usual problem related to the system reconfiguration is how to preserve
/// configuration integrity in case of errors. In this case, when the
/// HTTP listener's configuration is modified there is a need to close all
/// existing connections and gracefully shutdown the listener's instance.
/// This, however, makes it possible that the control agent looses
/// connectivity if opening a new listener is unsuccessful. In fact, this
/// is quite possible scenario when the user is setting up the listener to
/// use a restricted port range or non-existing IP address. In this case,
/// the configuration parser will not signal the problem because IP address
/// and/or port are syntactically correcect.
///
/// This method deals with this problem by opening a new listener aside of
/// the currently running listener (if the new listener settings are
/// different than current settings). Both instances are held until the
/// @ref CtrlAgentProcess::garbageCollectListeners is invoked, which
/// removes any listeners which are no longer used.
///
/// @param config_set a new configuration (JSON) for the process
/// @param check_only true if configuration is to be verified only, not applied
/// @return an Element that contains the results of configuration composed
......@@ -89,6 +108,39 @@ public:
/// @brief Returns a pointer to the configuration manager.
CtrlAgentCfgMgrPtr getCtrlAgentCfgMgr();
/// @brief Returns a const pointer to the HTTP listener used by the process.
///
/// @return Const pointer to the currently used listener or null pointer if
/// we're not listening. In fact, the latter should never be the case given
/// that we provide default listener configuration.
http::ConstHttpListenerPtr getHttpListener() const;
/// @brief Checks if the process is listening to the HTTP requests.
///
/// @return true if the process is listening.
bool isListening() const;
private:
/// @brief Removes listeners which are no longer in use.
///
/// This method should be called after executing
/// @ref CtrlAgentProcess::configure to remove listeners used previously
/// (no longer used because the listening address and port has changed as
// a result of the reconfiguration). If there are no listeners additional
/// to the one that is currently in use, the method has no effect.
void garbageCollectListeners();
/// @brief Polls all ready handlers and then runs one handler if none
/// handlers have been executed as a result of polling.
///
/// @return Number of executed handlers.
size_t runIO();
/// @brief Holds a list of pointers to the active listeners.
std::vector<http::HttpListenerPtr> http_listeners_;
};
/// @brief Defines a shared pointer to CtrlAgentProcess.
......
......@@ -14,6 +14,7 @@
using namespace isc::agent;
using namespace isc::data;
using namespace isc::http;
using namespace isc::process;
using namespace boost::posix_time;
......@@ -198,8 +199,9 @@ TEST_F(CtrlAgentControllerTest, sigtermShutdown) {
EXPECT_TRUE(elapsed_time.total_milliseconds() < 300);
}
// Tests that the configuration is updated as a result of agent reconfiguration.
TEST_F(CtrlAgentControllerTest, configUpdate) {
// Tests that the sockets settings are updated upon successful reconfiguration.
TEST_F(CtrlAgentControllerTest, successfulConfigUpdate) {
// This configuration should be used to override the initial conifguration.
const char* second_config =
"{"
" \"http-host\": \"127.0.0.1\","
......@@ -216,21 +218,144 @@ TEST_F(CtrlAgentControllerTest, configUpdate) {
" }"
"}";
// Schedule reconfiguration.
scheduleTimedWrite(second_config, 100);
// Schedule SIGHUP signal to trigger reconfiguration.
TimedSignal sighup(*getIOService(), SIGHUP, 200);
// Start the server.
time_duration elapsed_time;
runWithConfig(valid_agent_config, 500, elapsed_time);
CtrlAgentCfgContextPtr ctx = getCtrlAgentCfgContext();
ASSERT_TRUE(ctx);
// The server should now hold the new listener configuration.
EXPECT_EQ("127.0.0.1", ctx->getHttpHost());
EXPECT_EQ(8080, ctx->getHttpPort());
// The forwarding configuration should have been updated too.
testUnixSocketInfo(CtrlAgentCfgContext::TYPE_DHCP4, "/second/dhcp4/socket");
testUnixSocketInfo(CtrlAgentCfgContext::TYPE_DHCP6, "/second/dhcp6/socket");
CtrlAgentProcessPtr process = getCtrlAgentProcess();
ASSERT_TRUE(process);
// Check that the HTTP listener still exists after reconfiguration.
ConstHttpListenerPtr listener = process->getHttpListener();
ASSERT_TRUE(listener);
EXPECT_TRUE(process->isListening());
// The listener should have been reconfigured to use new address and port.
EXPECT_EQ("127.0.0.1", listener->getLocalAddress().toText());
EXPECT_EQ(8080, listener->getLocalPort());
}
// Tests that the server continues to use an old configuration when the listener
// reconfiguration is unsuccessful.
TEST_F(CtrlAgentControllerTest, unsuccessfulConfigUpdate) {
// This is invalid configuration. We're using restricted port number and
// IP address of 1.1.1.1.
const char* second_config =
"{"
" \"http-host\": \"1.1.1.1\","
" \"http-port\": 1,"
" \"control-sockets\": {"
" \"dhcp4-server\": {"
" \"socket-type\": \"unix\","
" \"socket-name\": \"/second/dhcp4/socket\""
" },"
" \"dhcp6-server\": {"
" \"socket-type\": \"unix\","
" \"socket-name\": \"/second/dhcp6/socket\""
" }"
" }"
"}";
// Schedule reconfiguration.
scheduleTimedWrite(second_config, 100);
// Schedule SIGHUP signal to trigger reconfiguration.
TimedSignal sighup(*getIOService(), SIGHUP, 200);
// Start the server.
time_duration elapsed_time;
runWithConfig(valid_agent_config, 500, elapsed_time);
CtrlAgentCfgContextPtr ctx = getCtrlAgentCfgContext();
ASSERT_TRUE(ctx);
// The reconfiguration should have been unsuccessful, and the server should
// still use the original configuration.
EXPECT_EQ("127.0.0.1", ctx->getHttpHost());
EXPECT_EQ(8081, ctx->getHttpPort());
// Same for forwarding.
testUnixSocketInfo(CtrlAgentCfgContext::TYPE_DHCP4, "/first/dhcp4/socket");
testUnixSocketInfo(CtrlAgentCfgContext::TYPE_DHCP6, "/first/dhcp6/socket");
CtrlAgentProcessPtr process = getCtrlAgentProcess();
ASSERT_TRUE(process);
// We should still be using an original listener.
ConstHttpListenerPtr listener = process->getHttpListener();
ASSERT_TRUE(listener);
EXPECT_TRUE(process->isListening());
EXPECT_EQ("127.0.0.1", listener->getLocalAddress().toText());
EXPECT_EQ(8081, listener->getLocalPort());
}
// Tests that it is possible to update the configuration in such a way that the
// listener configuration remains the same. The server should continue using the
// listener instance it has been using prior to the reconfiguration.
TEST_F(CtrlAgentControllerTest, noListenerChange) {
// This configuration should be used to override the initial conifguration.
const char* second_config =
"{"
" \"http-host\": \"127.0.0.1\","
" \"http-port\": 8081,"
" \"control-sockets\": {"
" \"dhcp4-server\": {"
" \"socket-type\": \"unix\","
" \"socket-name\": \"/second/dhcp4/socket\""
" },"
" \"dhcp6-server\": {"
" \"socket-type\": \"unix\","
" \"socket-name\": \"/second/dhcp6/socket\""
" }"
" }"
"}";
// Schedule reconfiguration.
scheduleTimedWrite(second_config, 100);
// Schedule SIGHUP signal to trigger reconfiguration.
TimedSignal sighup(*getIOService(), SIGHUP, 200);
// Start the server.
time_duration elapsed_time;
runWithConfig(valid_agent_config, 500, elapsed_time);
CtrlAgentCfgContextPtr ctx = getCtrlAgentCfgContext();
ASSERT_TRUE(ctx);
// The server should use a correct listener configuration.
EXPECT_EQ("127.0.0.1", ctx->getHttpHost());
EXPECT_EQ(8081, ctx->getHttpPort());
// The forwarding configuration should have been updated.
testUnixSocketInfo(CtrlAgentCfgContext::TYPE_DHCP4, "/second/dhcp4/socket");
testUnixSocketInfo(CtrlAgentCfgContext::TYPE_DHCP6, "/second/dhcp6/socket");
CtrlAgentProcessPtr process = getCtrlAgentProcess();
ASSERT_TRUE(process);
// The listener should keep listening.
ConstHttpListenerPtr listener = process->getHttpListener();
ASSERT_TRUE(listener);
EXPECT_TRUE(process->isListening());
EXPECT_EQ("127.0.0.1", listener->getLocalAddress().toText());
EXPECT_EQ(8081, listener->getLocalPort());
}
}
......@@ -46,6 +46,9 @@ public:
const HttpResponseCreatorFactoryPtr& creator_factory,
const long request_timeout);
/// @brief Returns reference to the current listener endpoint.
const TCPEndpoint& getEndpoint() const;
/// @brief Starts accepting new connections.
///
/// This method starts accepting and handling new HTTP connections on
......@@ -126,6 +129,11 @@ HttpListenerImpl::HttpListenerImpl(IOService& io_service,
}
}
const TCPEndpoint&
HttpListenerImpl::getEndpoint() const {
return (*endpoint_);
}
void
HttpListenerImpl::start() {
try {
......@@ -186,6 +194,16 @@ HttpListener::~HttpListener() {
stop();
}
IOAddress
HttpListener::getLocalAddress() const {
return (impl_->getEndpoint().getAddress());
}
uint16_t
HttpListener::getLocalPort() const {
return (impl_->getEndpoint().getPort());
}
void
HttpListener::start() {
impl_->start();
......
......@@ -12,6 +12,7 @@
#include <exceptions/exceptions.h>
#include <http/response_creator_factory.h>
#include <boost/shared_ptr.hpp>
#include <stdint.h>
namespace isc {
namespace http {
......@@ -80,6 +81,12 @@ public:
/// Stops all active connections and closes TCP acceptor service.
~HttpListener();
/// @brief Returns local address on which server is listening.
asiolink::IOAddress getLocalAddress() const;
/// @brief Returns local port on which server is listening.
uint16_t getLocalPort() const;
/// @brief Starts accepting new connections.
///
/// This method starts accepting and handling new HTTP connections on
......@@ -101,6 +108,12 @@ private:
};
/// @brief Pointer to the @ref HttpListener.
typedef boost::shared_ptr<HttpListener> HttpListenerPtr;
/// @brief Pointer to the const @ref HttpListener.
typedef boost::shared_ptr<const HttpListener> ConstHttpListenerPtr;
} // end of namespace isc::http
} // end of namespace isc
......
......@@ -309,6 +309,8 @@ TEST_F(HttpListenerTest, listen) {
HttpListener listener(io_service_, IOAddress(SERVER_ADDRESS), SERVER_PORT,
factory_, REQUEST_TIMEOUT);
ASSERT_NO_THROW(listener.start());
ASSERT_EQ(SERVER_ADDRESS, listener.getLocalAddress().toText());
ASSERT_EQ(SERVER_PORT, listener.getLocalPort());
ASSERT_NO_THROW(startRequest(request));
ASSERT_NO_THROW(io_service_.run());
ASSERT_EQ(1, clients_.size());
......
......@@ -275,7 +275,8 @@ DCfgMgrBase::parseConfig(isc::data::ConstElementPtr config_set,
isc::data::ConstElementPtr
DCfgMgrBase::simpleParseConfig(isc::data::ConstElementPtr config_set,
bool check_only) {
bool check_only,
const std::function<void()>& post_config_cb) {
if (!config_set) {
return (isc::config::createAnswer(1,
std::string("Can't parse NULL config")));
......@@ -301,6 +302,10 @@ DCfgMgrBase::simpleParseConfig(isc::data::ConstElementPtr config_set,
// Everything was fine. Configuration set processed successfully.
if (!check_only) {
if (post_config_cb) {
post_config_cb();
}
LOG_INFO(dctl_logger, DCTL_CONFIG_COMPLETE).arg(getConfigSummary(0));
answer = isc::config::createAnswer(0, "Configuration committed.");
} else {
......
......@@ -11,6 +11,7 @@
#include <cc/cfg_to_element.h>
#include <exceptions/exceptions.h>
#include <dhcpsrv/parsers/dhcp_parsers.h>
#include <functional>
#include <stdint.h>
#include <string>
......@@ -326,12 +327,19 @@ public:
///
/// @param config set of configuration elements to be parsed
/// @param check_only true if the config is to be checked only, but not applied
/// @param post_config_cb Callback to be executed after the usual parsing stage.
/// This can be specified as a C++ lambda which configures other parts of the
/// system based on the parsed configuration information. The callback should
/// throw an exception to signal an error. This method will catch this
/// exception and place an exception string within the result returned.
///
/// @return an Element that contains the results of configuration composed
/// of an integer status value (0 means successful, non-zero means failure),
/// and a string explanation of the outcome.
isc::data::ConstElementPtr
simpleParseConfig(isc::data::ConstElementPtr config,
bool check_only = false);
bool check_only = false,
const std::function<void()>& post_config_cb = nullptr);
/// @brief Adds a given element id to the end of the parse order list.
///
......
......@@ -8,6 +8,7 @@
#include <cc/command_interpreter.h>
#include <config/module_spec.h>
#include <exceptions/exceptions.h>
#include <dhcpsrv/parsers/dhcp_parsers.h>
#include <process/testutils/d_test_stubs.h>
#include <process/d_cfg_mgr.h>
......@@ -563,4 +564,19 @@ TEST_F(DStubCfgMgrTest, simpleParseConfig) {
EXPECT_TRUE(checkAnswer(0));
}
// This test checks that the post configuration callback function is
// executed by the simpleParseConfig function.
TEST_F(DStubCfgMgrTest, simpleParseConfigWithCallback) {
string config = "{ \"bool_test\": true , \n"
" \"uint32_test\": 77 , \n"
" \"string_test\": \"hmmm chewy\" }";
ASSERT_NO_THROW(fromJSON(config));
answer_ = cfg_mgr_->simpleParseConfig(config_set_, false,
[this]() {
isc_throw(Unexpected, "unexpected configuration error");
});
EXPECT_TRUE(checkAnswer(1));
}
} // end of anonymous namespace
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