From: Thomas Markwalder Date: Tue, 17 Jun 2014 17:44:48 +0000 (-0400) Subject: [3407] Integrated IOSignaling into CPL X-Git-Tag: trac3434_base~4^2~8 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=73f57b83a5e1496ad040481f447e5a540fc55876;p=thirdparty%2Fkea.git [3407] Integrated IOSignaling into CPL DController was extended to instantiate an IOSignalQueue and register for signals with a SignalSet. The default implementation for signal processing supports SIGHUP as config file reload, and SIGINT/SIGTERM for graceful shutdown. D2Controller inherits this support without change. A good deal of work went into the unit test classes as well, particularly DControllerTest. --- diff --git a/src/bin/d2/d2_messages.mes b/src/bin/d2/d2_messages.mes index 1a0a9545a6..bb3f312abb 100644 --- a/src/bin/d2/d2_messages.mes +++ b/src/bin/d2/d2_messages.mes @@ -32,15 +32,15 @@ new configuration. It is output during server startup, and when an updated configuration is committed by the administrator. Additional information may be provided. +% DCTL_CONFIG_FILE_LOAD_FAIL %1 configuration could not be loaded from file: %2 +This fatal error message indicates that the application attempted to load its +initial configuration from file and has failed. The service will exit. + % DCTL_CONFIG_LOAD_FAIL %1 configuration failed to load: %2 This critical error message indicates that the initial application configuration has failed. The service will start, but will not process requests until the configuration has been corrected. -% DCTL_CONFIG_FILE_LOAD_FAIL %1 configuration could not be loaded from file: %2 -This fatal error message indicates that the application attempted to load its -initial configuration from file and has failed. The service will exit. - % DCTL_CONFIG_START parsing new configuration: %1 A debug message indicating that the application process has received an updated configuration and has passed it to its configuration manager @@ -129,6 +129,16 @@ mapping additions which were received and accepted by an appropriate DNS server. This is a debug message that indicates that the application has DHCP_DDNS requests in the queue but is working as many concurrent requests as allowed. +% DHCP_DDNS_CFG_FILE_RELOAD_ERROR configuration reload failed: %1, reverting to current configuration. +This is an error message indicating that the application attempted to reload +its configuration from file and encountered an error. This is likely due to +invalid content in the configuration file. The application should continue +to operate under its current configuration. + +% DHCP_DDNS_CFG_FILE_RELOAD_SIGNAL_RECVD OS signal %1 received, reloading configurationfrom file: %2 +This is an informational message indicating the application has received a signal +instructing it to reload its configuration from file. + % DHCP_DDNS_CLEARED_FOR_SHUTDOWN application has met shutdown criteria for shutdown type: %1 This is an informational message issued when the application has been instructed to shutdown and has met the required criteria to exit. @@ -452,6 +462,15 @@ in event loop. This is informational message issued when the application has been instructed to shut down by the controller. +% DHCP_DDNS_SHUTDOWN_SIGNAL_RECVD OS signal %1 received, starting shutdown +This is an informational message indicating the application has received a signal +instructing it to shutdown. + +% DHCP_DDNS_SIGNAL_ERROR signal handler for signal %1, threw an unexpected exception: %2 +This is an error message indicating that the application encountered an unexpected error after receiving a signal. This is a programmatic error and should be +reported. While The application will likely continue to operating, it may be +unable to respond correctly to signals. + % DHCP_DDNS_STARTING_TRANSACTION Transaction Key: %1 This is a debug message issued when DHCP-DDNS has begun a transaction for a given request. @@ -468,6 +487,11 @@ message but the attempt to send it suffered a unexpected error. This is most likely a programmatic error, rather than a communications issue. Some or all of the DNS updates requested as part of this request did not succeed. +% DHCP_DDNS_UNSUPPORTED_SIGNAL ignoring reception of unsupported signal: %1 +This is a debug message indicating that the application received an unsupported +signal. This a programmatic error indicating the application has registered to +receive the signal, but for which no processing logic has been added. + % DHCP_DDNS_UPDATE_REQUEST_SENT %1 for transaction key: %2 to server: %3 This is a debug message issued when DHCP_DDNS sends a DNS request to a DNS server. @@ -475,8 +499,3 @@ server. % DHCP_DDNS_UPDATE_RESPONSE_RECEIVED for transaction key: %1 to server: %2 status: %3 This is a debug message issued when DHCP_DDNS receives sends a DNS update response from a DNS server. - -% DHCP_DDNS_SIGNAL_ERROR The signal handler for signal %1, threw an unexpected exception: %2 -This is an error message indicating that the application encountered an unexpected error after receiving a signal. This is a programmatic error and should be -reported. While The application will likely continue to operating, it may be -unable to respond correctly to signals. diff --git a/src/bin/d2/d_controller.cc b/src/bin/d2/d_controller.cc index 16fc52081d..5983ef6135 100644 --- a/src/bin/d2/d_controller.cc +++ b/src/bin/d2/d_controller.cc @@ -30,7 +30,8 @@ DControllerBasePtr DControllerBase::controller_; DControllerBase::DControllerBase(const char* app_name, const char* bin_name) : app_name_(app_name), bin_name_(bin_name), verbose_(false), spec_file_name_(""), - io_service_(new isc::asiolink::IOService()){ + io_service_(new isc::asiolink::IOService()), + signal_set_(), io_signal_queue_() { } void @@ -75,6 +76,9 @@ DControllerBase::launch(int argc, char* argv[], const bool test_mode) { "Application Process initialization failed: " << ex.what()); } + // Now that we have a proces, we can set up signal handling. + initSignalHandling(); + LOG_DEBUG(dctl_logger, DBGLVL_START_SHUT, DCTL_STANDALONE).arg(app_name_); // Step 3 is to load configuration from file. @@ -287,6 +291,79 @@ DControllerBase::shutdownProcess(isc::data::ConstElementPtr args) { return (isc::config::createAnswer(0, "Process has not been initialzed.")); } +void +DControllerBase::initSignalHandling() { + /// @todo block everything we don't handle + + // Create our signal queue. + io_signal_queue_.reset(new IOSignalQueue(io_service_)); + + // Install the on-receipt handler + util::SignalSet::setOnReceiptHandler(boost::bind(&DControllerBase:: + osSignalHandler, + this, _1)); + // Register for the signals we wish to handle. + signal_set_.reset(new util::SignalSet(SIGHUP,SIGINT,SIGTERM)); +} + +bool +DControllerBase::osSignalHandler(int signum) { + // Create a IOSignal to propagate the signal to IOService. + io_signal_queue_->pushSignal(signum, boost::bind(&DControllerBase:: + ioSignalHandler, + this, _1)); + return (true); +} + +void +DControllerBase::ioSignalHandler(IOSignalId sequence_id) { + // Pop the signal instance off the queue. This should make us + // the only one holding it, so when we leave it should be freed. + // (note that popSignal will throw if signal is not found, which + // in turn will caught, logged, and swallowed by IOSignal callback + // invocation code.) + IOSignalPtr io_signal = io_signal_queue_->popSignal(sequence_id); + + // Now call virtual signal processing method. + processSignal(io_signal->getSignum()); +} + +void +DControllerBase::processSignal(int signum) { + switch (signum) { + case SIGHUP: + { + LOG_INFO(dctl_logger, DHCP_DDNS_CFG_FILE_RELOAD_SIGNAL_RECVD) + .arg(signum).arg(getConfigFile()); + int rcode; + isc::data::ConstElementPtr comment = isc::config:: + parseAnswer(rcode, + configFromFile()); + if (rcode != 0) { + LOG_ERROR(dctl_logger, DHCP_DDNS_CFG_FILE_RELOAD_ERROR) + .arg(comment->stringValue()); + } + + break; + } + + case SIGINT: + case SIGTERM: + { + LOG_INFO(dctl_logger, DHCP_DDNS_SHUTDOWN_SIGNAL_RECVD) + .arg(signum); + isc::data::ElementPtr arg_set; + executeCommand(SHUT_DOWN_COMMAND, arg_set); + break; + } + + default: + LOG_DEBUG(dctl_logger, DBGLVL_START_SHUT, + DHCP_DDNS_UNSUPPORTED_SIGNAL).arg(signum); + break; + } +} + void DControllerBase::usage(const std::string & text) { diff --git a/src/bin/d2/d_controller.h b/src/bin/d2/d_controller.h index 5d5c0ab8fd..8c60af2e8a 100644 --- a/src/bin/d2/d_controller.h +++ b/src/bin/d2/d_controller.h @@ -19,9 +19,11 @@ #include #include #include +#include #include #include #include +#include #include #include @@ -78,9 +80,12 @@ typedef boost::shared_ptr DControllerBasePtr; /// creation. /// It provides the callback handlers for command and configuration events /// which could be triggered by an external source. Such sources are intended -/// to be registed with and monitored by the controller's IOService such that +/// to be registered with and monitored by the controller's IOService such that /// the appropriate handler can be invoked. /// +/// DControllerBase provides dynamic configuration file reloading upon receipt +/// of SIGHUP, and graceful shutdown upon receipt of either SIGINT or SIGTERM. +/// /// NOTE: Derivations must supply their own static singleton instance method(s) /// for creating and fetching the instance. The base class declares the instance /// member in order for it to be available for static callback functions. @@ -101,9 +106,10 @@ public: /// /// 1. parse command line arguments /// 2. instantiate and initialize the application process - /// 3. load the configuration file - /// 4. start and wait on the application process event loop - /// 5. exit to the caller + /// 3. initialize signal handling + /// 4. load the configuration file + /// 5. start and wait on the application process event loop + /// 6. exit to the caller /// /// It is intended to be called from main() and be given the command line /// arguments. @@ -155,7 +161,7 @@ public: /// configuration data for this controller's application /// /// module-config: a set of zero or more JSON elements which comprise - /// the application'ss configuration values + /// the application's configuration values /// @endcode /// /// The method extracts the set of configuration elements for the @@ -279,6 +285,24 @@ protected: return (""); } + /// @brief Application-level signal processing method. + /// + /// This method is the last step in processing a OS signal occurrence. It + /// is invoked when an IOSignal's internal timer callback is executed by + /// IOService. It currently supports the following signals as follows: + /// -# SIGHUP - instigates reloading the configuration file + /// -# SIGINT - instigates a graceful shutdown + /// -# SIGTERM - instigates a graceful shutdown + /// If if received any other signal, it will issue a debug statement and + /// discard it. + /// Derivations wishing to support additional signals could override this + /// method with one that: processes the signal if it is one of additional + /// signals, otherwise invoke this method (DControllerBase::processSignal()) + /// with signal value. + /// @todo Provide a convenient way for derivations to register additional + /// signals. + virtual void processSignal(int signum); + /// @brief Supplies whether or not verbose logging is enabled. /// /// @return returns true if verbose logging is enabled. @@ -372,7 +396,7 @@ protected: /// to begin its shutdown process. /// /// Note, it is assumed that the process of shutting down is neither - /// instanteneous nor synchronous. This method does not "block" waiting + /// instantaneous nor synchronous. This method does not "block" waiting /// until the process has halted. Rather it is used to convey the /// need to shutdown. A successful return indicates that the shutdown /// has successfully commenced, but does not indicate that the process @@ -383,9 +407,41 @@ protected: /// non-zero means failure), and a string explanation of the outcome. isc::data::ConstElementPtr shutdownProcess(isc::data::ConstElementPtr args); + /// @brief Initializes signal handling + /// + /// This method configures the controller to catch and handle signals. + /// It instantiates an IOSignalQueue, registers @c osSignalHandler() as + /// the SignalSet "on-receipt" handler, and lastly instantiates a SignalSet + /// which listens for SIGHUP, SIGINT, and SIGTERM. + void initSignalHandling(); + + /// @brief Handler for processing OS-level signals + /// + /// This method is installed as the SignalSet "on-receipt" handler. Upon + /// invocation, it uses the controller's IOSignalQueue to schedule an + /// IOSignal with for the given signal value. + /// + /// @param signum OS signal value (e.g. SIGINT, SIGUSR1 ...) to received + /// + /// @return SignalSet "on-receipt" handlers are required to return a + /// boolean indicating if the OS signal has been processed (true) or if it + /// should be saved for deferred processing (false). Currently this + /// method processes all received signals, so it always returns true. + bool osSignalHandler(int signum); + + /// @brief Handler for processing IOSignals + /// + /// This method is supplied as the callback when IOSignals are scheduled. + /// It fetches the IOSignal for the given sequence_id and then invokes + /// the virtual method, @c processSignal() passing it the signal value + /// obtained from the IOSignal. This allows derivations to supply a + /// custom signal processing method, while ensuring IOSignalQueue + /// integrity. + void ioSignalHandler(IOSignalId sequence_id); + /// @brief Fetches the current process /// - /// @return the a pointer to the current process instance. + /// @return a pointer to the current process instance. DProcessBasePtr getProcess() { return (process_); } @@ -403,7 +459,7 @@ private: std::string app_name_; /// @brief Name of the service executable. - /// By convention this matches the executable nam. It is also used to + /// By convention this matches the executable name. It is also used to /// establish the logger name. std::string bin_name_; @@ -422,6 +478,12 @@ private: /// @brief Shared pointer to an IOService object, used for ASIO operations. IOServicePtr io_service_; + /// @brief Set of registered signals to handle. + util::SignalSetPtr signal_set_; + + /// @brief Queue for propagating caught signals to the IOService. + IOSignalQueuePtr io_signal_queue_; + /// @brief Singleton instance value. static DControllerBasePtr controller_; diff --git a/src/bin/d2/io_service_signal.h b/src/bin/d2/io_service_signal.h index 8f91f2b2ce..d10e6bf01e 100644 --- a/src/bin/d2/io_service_signal.h +++ b/src/bin/d2/io_service_signal.h @@ -17,7 +17,6 @@ #include #include -//#include #include diff --git a/src/bin/d2/tests/d2_controller_unittests.cc b/src/bin/d2/tests/d2_controller_unittests.cc index 1d840034c0..4ffc918d23 100644 --- a/src/bin/d2/tests/d2_controller_unittests.cc +++ b/src/bin/d2/tests/d2_controller_unittests.cc @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -51,6 +52,40 @@ public: /// @brief Destructor ~D2ControllerTest() { } + + /// @brief Fetches the D2Controller's D2Process + /// + /// @return A pointer to the process which may be null if it has not yet + /// been instantiated. + D2ProcessPtr getD2Process() { + return (boost::dynamic_pointer_cast(getProcess())); + } + + /// @brief Fetches the D2Process's D2Configuration manager + /// + /// @return A pointer to the manager which may be null if it has not yet + /// been instantiated. + D2CfgMgrPtr getD2CfgMgr() { + D2CfgMgrPtr p; + if (getD2Process()) { + p = getD2Process()->getD2CfgMgr(); + } + + return (p); + } + + /// @brief Fetches the D2Configuration manager's D2CfgContext + /// + /// @return A pointer to the context which may be null if it has not yet + /// been instantiated. + D2CfgContextPtr getD2CfgContext() { + D2CfgContextPtr p; + if (getD2CfgMgr()) { + p = getD2CfgMgr()->getD2CfgContext(); + } + + return (p); + } }; /// @brief Basic Controller instantiation testing. @@ -120,34 +155,13 @@ TEST_F(D2ControllerTest, initProcessTesting) { /// This creates an interval timer to generate a normal shutdown and then /// launches with a valid, stand-alone command line and no simulated errors. TEST_F(D2ControllerTest, launchNormalShutdown) { - // command line to run standalone - char* argv[] = { const_cast("progName"), - const_cast("-c"), - const_cast(DControllerTest::CFG_TEST_FILE), - const_cast("-v") }; - int argc = 4; - - // Create a valid D2 configuration file. - writeFile(valid_d2_config); - - // Use an asiolink IntervalTimer and callback to generate the - // shutdown invocation. (Note IntervalTimer setup is in milliseconds). - isc::asiolink::IntervalTimer timer(*getIOService()); - timer.setup(genShutdownCallback, 2 * 1000); - - // Record start time, and invoke launch(). - ptime start = microsec_clock::universal_time(); - EXPECT_NO_THROW(launch(argc, argv)); + // Write valid_d2_config and then run launch() for 1000 ms. + time_duration elapsed_time; + runWithConfig(valid_d2_config, 1000, elapsed_time); - // Record stop time. - ptime stop = microsec_clock::universal_time(); - - // Verify that duration of the run invocation is the same as the - // timer duration. This demonstrates that the shutdown was driven - // by an io_service event and callback. - time_duration elapsed = stop - start; - EXPECT_TRUE(elapsed.total_milliseconds() >= 1900 && - elapsed.total_milliseconds() <= 2200); + // Give a generous margin to accomodate slower test environs. + EXPECT_TRUE(elapsed_time.total_milliseconds() >= 800 && + elapsed_time.total_milliseconds() <= 1300); } /// @brief Configuration update event testing. @@ -211,7 +225,107 @@ TEST_F(D2ControllerTest, executeCommandTests) { answer = executeCommand(SHUT_DOWN_COMMAND, arg_set); isc::config::parseAnswer(rcode, answer); EXPECT_EQ(COMMAND_SUCCESS, rcode); +} + +// Tests that the original configuration is retained after a SIGHUP triggered +// reconfiguration fails due to invalid config content. +TEST_F(D2ControllerTest, invalidConfigReload) { + // Schedule to replace the configuration file after launch. This way the + // file is updated after we have done the initial configuration. + scheduleTimedWrite("{ \"string_test\": BOGUS JSON }", 100); + + // Setup to raise SIGHUP in 200 ms. + TimedSignal sighup(*getIOService(), SIGHUP, 200); + + // Write valid_d2_config and then run launch() for a maximum of 500 ms. + time_duration elapsed_time; + runWithConfig(valid_d2_config, 500, elapsed_time); + + // Context is still available post launch. + // Check to see that our configuration matches the original per + // valid_d2_config (see d_test_stubs.cc) + D2CfgMgrPtr d2_cfg_mgr = getD2CfgMgr(); + D2ParamsPtr d2_params = d2_cfg_mgr->getD2Params(); + ASSERT_TRUE(d2_params); + + EXPECT_EQ("127.0.0.1", d2_params->getIpAddress().toText()); + EXPECT_EQ(5031, d2_params->getPort()); + EXPECT_TRUE(d2_cfg_mgr->forwardUpdatesEnabled()); + EXPECT_TRUE(d2_cfg_mgr->reverseUpdatesEnabled()); + + /// @todo add a way to trap log file and search it +} + +// Tests that the original configuration is replaced after a SIGHUP triggered +// reconfiguration succeeds. +TEST_F(D2ControllerTest, validConfigReload) { + // Define a replacement config. + const char* second_cfg = + "{" + " \"ip_address\": \"192.168.77.1\" , " + " \"port\": 777 , " + "\"tsig_keys\": [], " + "\"forward_ddns\" : {}, " + "\"reverse_ddns\" : {} " + "}"; + + // Schedule to replace the configuration file after launch. This way the + // file is updated after we have done the initial configuration. + scheduleTimedWrite(second_cfg, 100); + + // Setup to raise SIGHUP in 200 ms. + TimedSignal sighup(*getIOService(), SIGHUP, 200); + + // Write valid_d2_config and then run launch() for a maximum of 500ms. + time_duration elapsed_time; + runWithConfig(valid_d2_config, 500, elapsed_time); + + // Context is still available post launch. + // Check to see that our configuration matches the replacement config. + D2CfgMgrPtr d2_cfg_mgr = getD2CfgMgr(); + D2ParamsPtr d2_params = d2_cfg_mgr->getD2Params(); + ASSERT_TRUE(d2_params); + + EXPECT_EQ("192.168.77.1", d2_params->getIpAddress().toText()); + EXPECT_EQ(777, d2_params->getPort()); + EXPECT_FALSE(d2_cfg_mgr->forwardUpdatesEnabled()); + EXPECT_FALSE(d2_cfg_mgr->reverseUpdatesEnabled()); + + /// @todo add a way to trap log file and search it +} + +// Tests that the SIGINT triggers a normal shutdown. +TEST_F(D2ControllerTest, sigintShutdown) { + // Setup to raise SIGHUP in 1 ms. + TimedSignal sighup(*getIOService(), SIGINT, 1); + + // Write valid_d2_config and then run launch() for a maximum of 1000 ms. + time_duration elapsed_time; + runWithConfig(valid_d2_config, 1000, elapsed_time); + + // Signaled shutdown should make our elapsed time much smaller than + // the maximum run time. Give generous margin to accomodate slow + // test environs. + EXPECT_TRUE(elapsed_time.total_milliseconds() < 300); + + /// @todo add a way to trap log file and search it +} + +// Tests that the SIGTERM triggers a normal shutdown. +TEST_F(D2ControllerTest, sigtermShutdown) { + // Setup to raise SIGHUP in 1 ms. + TimedSignal sighup(*getIOService(), SIGTERM, 1); + + // Write valid_d2_config and then run launch() for a maximum of 1 s. + time_duration elapsed_time; + runWithConfig(valid_d2_config, 1000, elapsed_time); + + // Signaled shutdown should make our elapsed time much smaller than + // the maximum run time. Give generous margin to accomodate slow + // test environs. + EXPECT_TRUE(elapsed_time.total_milliseconds() < 300); + /// @todo add a way to trap log file and search it } }; // end of isc::d2 namespace diff --git a/src/bin/d2/tests/d_controller_unittests.cc b/src/bin/d2/tests/d_controller_unittests.cc index 42c9f39606..2f757e92ef 100644 --- a/src/bin/d2/tests/d_controller_unittests.cc +++ b/src/bin/d2/tests/d_controller_unittests.cc @@ -32,15 +32,19 @@ namespace d2 { /// has been constructed to exercise DControllerBase. class DStubControllerTest : public DControllerTest { public: - /// @brief Constructor. /// Note the constructor passes in the static DStubController instance /// method. DStubControllerTest() : DControllerTest (DStubController::instance) { + controller_ = boost::dynamic_pointer_cast + (DControllerTest:: + getController()); } virtual ~DStubControllerTest() { } + + DStubControllerPtr controller_; }; /// @brief Basic Controller instantiation testing. @@ -183,35 +187,15 @@ TEST_F(DStubControllerTest, launchProcessInitError) { /// launches with a valid, command line, with a valid configuration file /// and no simulated errors. TEST_F(DStubControllerTest, launchNormalShutdown) { - // command line to run standalone - char* argv[] = { const_cast("progName"), - const_cast("-c"), - const_cast(DControllerTest::CFG_TEST_FILE), - const_cast("-v") }; - int argc = 4; - - // Create a non-empty, config file. writeFile will wrap the contents - // with the module name for us. - writeFile("{}"); - - // Use an asiolink IntervalTimer and callback to generate the - // shutdown invocation. (Note IntervalTimer setup is in milliseconds). - isc::asiolink::IntervalTimer timer(*getIOService()); - timer.setup(genShutdownCallback, 2 * 1000); - - // Record start time, and invoke launch(). - ptime start = microsec_clock::universal_time(); - EXPECT_NO_THROW(launch(argc, argv)); - - // Record stop time. - ptime stop = microsec_clock::universal_time(); + // Write the valid, empty, config and then run launch() for 1000 ms + time_duration elapsed_time; + ASSERT_NO_THROW(runWithConfig("{}", 2000, elapsed_time)); // Verify that duration of the run invocation is the same as the // timer duration. This demonstrates that the shutdown was driven // by an io_service event and callback. - time_duration elapsed = stop - start; - EXPECT_TRUE(elapsed.total_milliseconds() >= 1900 && - elapsed.total_milliseconds() <= 2200); + EXPECT_TRUE(elapsed_time.total_milliseconds() >= 1900 && + elapsed_time.total_milliseconds() <= 2300); } /// @brief Tests launch with an nonexistant configuration file. @@ -255,35 +239,20 @@ TEST_F(DStubControllerTest, missingConfigFileArgument) { /// the process event loop. It launches wih a valid, stand-alone command line /// and no simulated errors. Launch should throw ProcessRunError. TEST_F(DStubControllerTest, launchRuntimeError) { - // command line to run standalone - char* argv[] = { const_cast("progName"), - const_cast("-c"), - const_cast(DControllerTest::CFG_TEST_FILE), - const_cast("-v") }; - int argc = 4; - - // Create a non-empty, config file. writeFile will wrap the contents - // with the module name for us. - writeFile("{}"); - // Use an asiolink IntervalTimer and callback to generate the // shutdown invocation. (Note IntervalTimer setup is in milliseconds). isc::asiolink::IntervalTimer timer(*getIOService()); - timer.setup(genFatalErrorCallback, 2 * 1000); + timer.setup(genFatalErrorCallback, 2000); - // Record start time, and invoke launch(). - ptime start = microsec_clock::universal_time(); - EXPECT_THROW(launch(argc, argv), ProcessRunError); - - // Record stop time. - ptime stop = microsec_clock::universal_time(); + // Write the valid, empty, config and then run launch() for 1000 ms + time_duration elapsed_time; + EXPECT_THROW(runWithConfig("{}", 2000, elapsed_time), ProcessRunError); // Verify that duration of the run invocation is the same as the // timer duration. This demonstrates that the shutdown was driven // by an io_service event and callback. - time_duration elapsed = stop - start; - EXPECT_TRUE(elapsed.total_milliseconds() >= 1900 && - elapsed.total_milliseconds() <= 2200); + EXPECT_TRUE(elapsed_time.total_milliseconds() >= 1900 && + elapsed_time.total_milliseconds() <= 2300); } /// @brief Configuration update event testing. @@ -380,5 +349,120 @@ TEST_F(DStubControllerTest, executeCommandTests) { EXPECT_EQ(COMMAND_ERROR, rcode); } +// Tests that registered signals are caught and handled. +TEST_F(DStubControllerTest, ioSignals) { + // Tell test controller just to record the signals, don't call the + // base class signal handler. + controller_->recordSignalOnly(true); + + // Setup to raise SIGHUP in 10 ms. + TimedSignal sighup(*getIOService(), SIGHUP, 10); + TimedSignal sigint(*getIOService(), SIGINT, 10); + TimedSignal sigterm(*getIOService(), SIGTERM, 10); + + // Write the valid, empty, config and then run launch() for 500 ms + time_duration elapsed_time; + runWithConfig("{}", 500, elapsed_time); + + // Verify that we caught the signals as expected. + std::vector& signals = controller_->getProcessedSignals(); + ASSERT_EQ(3, signals.size()); + EXPECT_EQ(SIGHUP, signals[0]); + EXPECT_EQ(SIGINT, signals[1]); + EXPECT_EQ(SIGTERM, signals[2]); +} + +// Tests that the original configuration is retained after a SIGHUP triggered +// reconfiguration fails due to invalid config content. +TEST_F(DStubControllerTest, invalidConfigReload) { + // Schedule to rewrite the configuration file after launch. This way the + // file is updated after we have done the initial configuration. The + // new content is invalid JSON which will cause the config parse to fail. + scheduleTimedWrite("{ \"string_test\": BOGUS JSON }", 100); + + // Setup to raise SIGHUP in 200 ms. + TimedSignal sighup(*getIOService(), SIGHUP, 200); + + // Write the config and then run launch() for 500 ms + // After startup, which will load the initial configuration this enters + // the process's runIO() loop. We will first rewrite the config file. + // Next we process the SIGHUP signal which should cause us to reconfigure. + time_duration elapsed_time; + runWithConfig("{ \"string_test\": \"first value\" }", 500, elapsed_time); + + // Context is still available post launch. Check to see that our + // configuration value is still the original value. + std::string actual_value = ""; + ASSERT_NO_THROW(getContext()->getParam("string_test", actual_value)); + EXPECT_EQ("first value", actual_value); + + // Verify that we saw the signal. + std::vector& signals = controller_->getProcessedSignals(); + ASSERT_EQ(1, signals.size()); + EXPECT_EQ(SIGHUP, signals[0]); +} + +// Tests that the original configuration is replaced after a SIGHUP triggered +// reconfiguration succeeds. +TEST_F(DStubControllerTest, validConfigReload) { + // Schedule to rewrite the configuration file after launch. This way the + // file is updated after we have done the initial configuration. + scheduleTimedWrite("{ \"string_test\": \"second value\" }", 100); + + // Setup to raise SIGHUP in 200 ms. + TimedSignal sighup(*getIOService(), SIGHUP, 200); + + // Write the config and then run launch() for 500 ms + time_duration elapsed_time; + runWithConfig("{ \"string_test\": \"first value\" }", 500, elapsed_time); + + // Context is still available post launch. + // Check to see that our configuration value is what we expect. + std::string actual_value = ""; + ASSERT_NO_THROW(getContext()->getParam("string_test", actual_value)); + EXPECT_EQ("second value", actual_value); + + // Verify that we saw the signal. + std::vector& signals = controller_->getProcessedSignals(); + ASSERT_EQ(1, signals.size()); + EXPECT_EQ(SIGHUP, signals[0]); +} + +// Tests that the SIGINT triggers a normal shutdown. +TEST_F(DStubControllerTest, sigintShutdown) { + // Setup to raise SIGHUP in 1 ms. + TimedSignal sighup(*getIOService(), SIGINT, 1); + + // Write the config and then run launch() for 1000 ms + time_duration elapsed_time; + runWithConfig("{ \"string_test\": \"first value\" }", 1000, elapsed_time); + + // Verify that we saw the signal. + std::vector& signals = controller_->getProcessedSignals(); + ASSERT_EQ(1, signals.size()); + EXPECT_EQ(SIGINT, signals[0]); + + // Duration should be significantly less than our max run time. + EXPECT_TRUE(elapsed_time.total_milliseconds() < 300); +} + +// Tests that the SIGTERM triggers a normal shutdown. +TEST_F(DStubControllerTest, sigtermShutdown) { + // Setup to raise SIGHUP in 1 ms. + TimedSignal sighup(*getIOService(), SIGTERM, 1); + + // Write the config and then run launch() for 1000 ms + time_duration elapsed_time; + runWithConfig("{ \"string_test\": \"first value\" }", 1000, elapsed_time); + + // Verify that we saw the signal. + std::vector& signals = controller_->getProcessedSignals(); + ASSERT_EQ(1, signals.size()); + EXPECT_EQ(SIGTERM, signals[0]); + + // Duration should be significantly less than our max run time. + EXPECT_TRUE(elapsed_time.total_milliseconds() < 300); +} + }; // end of isc::d2 namespace }; // end of isc namespace diff --git a/src/bin/d2/tests/d_test_stubs.cc b/src/bin/d2/tests/d_test_stubs.cc index e98beecc6d..cfab23d6fe 100644 --- a/src/bin/d2/tests/d_test_stubs.cc +++ b/src/bin/d2/tests/d_test_stubs.cc @@ -96,14 +96,14 @@ DStubProcess::shutdown(isc::data::ConstElementPtr /* args */) { } isc::data::ConstElementPtr -DStubProcess::configure(isc::data::ConstElementPtr /*config_set*/) { +DStubProcess::configure(isc::data::ConstElementPtr config_set) { if (SimFailure::shouldFailOn(SimFailure::ftProcessConfigure)) { // Simulates a process configure failure. return (isc::config::createAnswer(1, "Simulated process configuration error.")); } - return (isc::config::createAnswer(0, "Configuration accepted.")); + return (getCfgMgr()->parseConfig(config_set)); } isc::data::ConstElementPtr @@ -154,7 +154,8 @@ DStubController::instance() { } DStubController::DStubController() - : DControllerBase(stub_app_name_, stub_bin_name_) { + : DControllerBase(stub_app_name_, stub_bin_name_), + processed_signals_(), record_signal_only_(false) { if (getenv("B10_FROM_BUILD")) { setSpecFileName(std::string(getenv("B10_FROM_BUILD")) + @@ -213,9 +214,110 @@ const std::string DStubController::getCustomOpts() const { return (std::string(stub_option_x_)); } +void +DStubController::processSignal(int signum){ + processed_signals_.push_back(signum); + if (record_signal_only_) { + return; + } + + DControllerBase::processSignal(signum); +} + DStubController::~DStubController() { } +//************************** DControllerTest ************************* + +void +DControllerTest::writeFile(const std::string& content, + const std::string& module_name) { + std::ofstream out(CFG_TEST_FILE, std::ios::trunc); + ASSERT_TRUE(out.is_open()); + + out << "{ \"" << (!module_name.empty() ? module_name + : getController()->getAppName()) + << "\": " << std::endl; + + out << content; + out << " } " << std::endl; + out.close(); +} + +void +DControllerTest::timedWriteCallback() { + writeFile(new_cfg_content_); +} + +void +DControllerTest::scheduleTimedWrite(const std::string& config, + int write_time_ms) { + new_cfg_content_ = config; + write_timer_.reset(new asiolink::IntervalTimer(*getIOService())); + write_timer_->setup(boost::bind(&DControllerTest::timedWriteCallback, this), + write_time_ms, asiolink::IntervalTimer::ONE_SHOT); +} + +void +DControllerTest::runWithConfig(const std::string& config, int run_time_ms, + time_duration& elapsed_time) { + // Set up valid command line arguments + char* argv[] = { const_cast("progName"), + const_cast("-c"), + const_cast(DControllerTest::CFG_TEST_FILE), + const_cast("-v") }; + int argc = 4; + + // Create the config file. + writeFile(config); + + // Shutdown (without error) after runtime. + isc::asiolink::IntervalTimer timer(*getIOService()); + timer.setup(genShutdownCallback, run_time_ms); + + // Record start time, and invoke launch(). + // We catch and rethrow to allow testing error scenarios. + ptime start = microsec_clock::universal_time(); + try { + launch(argc, argv); + } catch (...) { + // calculate elasped time, then rethrow it + elapsed_time = microsec_clock::universal_time() - start; + throw; + } + + elapsed_time = microsec_clock::universal_time() - start; +} + +DProcessBasePtr +DControllerTest:: getProcess() { + DProcessBasePtr p; + if (getController()) { + p = getController()->getProcess(); + } + return (p); +} + +DCfgMgrBasePtr +DControllerTest::getCfgMgr() { + DCfgMgrBasePtr p; + if (getProcess()) { + p = getProcess()->getCfgMgr(); + } + + return (p); +} + +DCfgContextBasePtr +DControllerTest::getContext() { + DCfgContextBasePtr p; + if (getCfgMgr()) { + p = getCfgMgr()->getContext(); + } + + return (p); +} + // Initialize controller wrapper's static instance getter member. DControllerTest::InstanceGetter DControllerTest::instanceGetter_ = NULL; @@ -290,7 +392,7 @@ DStubCfgMgr::DStubCfgMgr() DStubCfgMgr::~DStubCfgMgr() { } -DCfgContextBasePtr +DCfgContextBasePtr DStubCfgMgr::createNewContext() { return (DCfgContextBasePtr (new DStubContext())); } @@ -300,7 +402,6 @@ DStubCfgMgr::createConfigParser(const std::string& element_id) { isc::dhcp::ParserPtr parser; DStubContextPtr context = boost::dynamic_pointer_cast(getContext()); - if (element_id == "bool_test") { parser.reset(new isc::dhcp:: BooleanParser(element_id, diff --git a/src/bin/d2/tests/d_test_stubs.h b/src/bin/d2/tests/d_test_stubs.h index cd4249d81d..a5b81ac3a3 100644 --- a/src/bin/d2/tests/d_test_stubs.h +++ b/src/bin/d2/tests/d_test_stubs.h @@ -23,6 +23,10 @@ #include #include +#include + +using namespace boost::posix_time; + #include #include @@ -213,6 +217,23 @@ public: /// @brief Defines the executable name used to construct the controller static const char* stub_bin_name_; + /// @brief Gets the list of signals that have been caught and processed. + std::vector& getProcessedSignals() { + return (processed_signals_); + } + + /// @brief Controls whether signals are processed in full or merely + /// recorded. + /// + /// If true, signal handling will stop after recording the signal. + /// Otherwise the base class signal handler, + /// DControllerBase::processSignals will also be invoked. This switch is + /// useful for ensuring that IOSignals are delivered as expected without + /// incurring the full impact such as reconfiguring or shutting down. + void recordSignalOnly(bool value) { + record_signal_only_ = value; + } + protected: /// @brief Handles additional command line options that are supported /// by DStubController. This implementation supports an option "-x". @@ -259,14 +280,25 @@ protected: /// @return returns a string containing the option letters. virtual const std::string getCustomOpts() const; + virtual void processSignal(int signum); + private: /// @brief Constructor is private to protect singleton integrity. DStubController(); + /// @brief Vector to record the signal values received. + std::vector processed_signals_; + + /// @brief Boolean for controlling if signals are merely recorded. + bool record_signal_only_; + public: virtual ~DStubController(); }; +/// @brief Defines a pointer to a DStubController. +typedef boost::shared_ptr DStubControllerPtr; + /// @brief Abstract Test fixture class that wraps a DControllerBase. This class /// is a friend class of DControllerBase which allows it access to class /// content to facilitate testing. It provides numerous wrapper methods for @@ -285,7 +317,8 @@ public: /// /// @param instance_getter is a function pointer to the static instance /// method of the DControllerBase derivation under test. - DControllerTest(InstanceGetter instance_getter) { + DControllerTest(InstanceGetter instance_getter) + : write_timer_(), new_cfg_content_() { // Set the static fetcher member, then invoke it via getController. // This ensures the singleton is instantiated. instanceGetter_ = instance_getter; @@ -447,20 +480,71 @@ public: /// @param content JSON text to be written to file /// @param module_name content content to be written to file void writeFile(const std::string& content, - const std::string& module_name = "") { - std::ofstream out(CFG_TEST_FILE, std::ios::trunc); - ASSERT_TRUE(out.is_open()); + const std::string& module_name = ""); - out << "{ \"" << (!module_name.empty() ? module_name : - getController()->getAppName()) - << "\": " << std::endl; + /// @brief Method used as timer callback to invoke writeFile. + /// + /// Wraps a call to writeFile passing in new_cfg_content_. This allows + /// the method to be bound as an IntervalTimer callback. + virtual void timedWriteCallback(); - out << content; - out << " } " << std::endl; - out.close(); - } + /// @brief Schedules the given content to overwrite the config file. + /// + /// Creates a one-shot IntervalTimer whose callback will overwrite the + /// configuration with the given content. This allows the configuration + /// file to replaced write_time_ms after DControllerBase::launch() has + /// invoked runProcess(). + /// + /// @param config JSON string containing the deisred content for the config + /// file. + /// @param write_time_ms time in milliseconds to delay before writing the + /// file. + void scheduleTimedWrite(const std::string& config, int write_time_ms); + + /// @brief Convenience method for invoking standard, valid launch + /// + /// This method sets up a timed run of the DController::launch. It does + /// the following: + /// - It creates command line argument variables argc/argv + /// - Invokes writeFile to create the config file with the given content + /// - Schedules a shutdown time timer to call DController::executeShutdown + /// after the interval + /// - Records the start time + /// - Invokes DController::launch() with the command line arguments + /// - After launch returns, it calculates the elapsed time and returns it + /// + /// @param config configuration file content to write before calling launch + /// @param run_time_ms maximum amount of time to allow runProcess() to + /// continue. + /// @param[out] elapsed_time the actual time in ms spent in launch(). + void runWithConfig(const std::string& config, int run_time_ms, + time_duration& elapsed_time); + + /// @brief Fetches the controller's process + /// + /// @return A pointer to the process which may be null if it has not yet + /// been instantiated. + DProcessBasePtr getProcess(); + + /// @brief Fetches the process's configuration manager + /// + /// @return A pointer to the manager which may be null if it has not yet + /// been instantiated. + DCfgMgrBasePtr getCfgMgr(); + + /// @brief Fetches the configuration manager's context + /// + /// @return A pointer to the context which may be null if it has not yet + /// been instantiated. + DCfgContextBasePtr getContext(); + + /// @brief Timer used for delayed configuration file writing. + asiolink::IntervalTimerPtr write_timer_; + + /// @brief String which contains the content delayed file writing will use. + std::string new_cfg_content_; - /// Name of a config file used during tests + /// @brief Name of a config file used during tests static const char* CFG_TEST_FILE; };