From e897f24ac63aa47888161c2336aea03d930fad6c Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Wed, 1 Jul 2015 16:20:40 -0400 Subject: [PATCH 1/8] [3769] Added support for creating PIDFiles to dhcpsrv::Daemon src/lib/dhcpsrv/daemon.h/cc New methods: static void setConfigFile(const std::string& config_file); std::string getProcName() const; setProcName(const std::string& proc_name); std::string getPIDFileDir() const; void setPIDFileDir(const std::string& pid_file_dir); std::string getPIDFileName() const; void setPIDFileName(const std::string& pid_file_name); void createPIDFile(int pid = 0); std::string makePIDFileName() const; New members: std::string proc_name_; std::string pid_file_dir_; isc::util::PIDFilePtr pid_file_; src/lib/dhcpsrv/tests/daemon_unittest.cc New tests: TEST_F(DaemonTest, getSetConfigFile) TEST_F(DaemonTest, getSetProcName) TEST_F(DaemonTest, getSetPIDFileDir) TEST_F(DaemonTest, setPIDFileName) TEST_F(DaemonTest, makePIDFileName) TEST_F(DaemonTest, createPIDFile) TEST_F(DaemonTest, createPIDFileOverwrite) TEST_F(DaemonTest, PIDFileCleanup) src/lib/util/pid_file.h Added typedef boost::shared_ptr PIDFilePtr; --- src/lib/dhcpsrv/daemon.cc | 117 ++++++++++++++- src/lib/dhcpsrv/daemon.h | 71 ++++++++- src/lib/dhcpsrv/tests/daemon_unittest.cc | 182 +++++++++++++++++++++-- src/lib/util/pid_file.h | 4 + 4 files changed, 357 insertions(+), 17 deletions(-) diff --git a/src/lib/dhcpsrv/daemon.cc b/src/lib/dhcpsrv/daemon.cc index 2733b926cd..9d89bbc098 100644 --- a/src/lib/dhcpsrv/daemon.cc +++ b/src/lib/dhcpsrv/daemon.cc @@ -13,14 +13,18 @@ // PERFORMANCE OF THIS SOFTWARE. #include +#include #include #include #include -#include -#include -#include #include #include +#include +#include + +#include + +#include #include /// @brief provides default implementation for basic daemon operations @@ -34,10 +38,14 @@ namespace dhcp { std::string Daemon::config_file_ = ""; Daemon::Daemon() - : signal_set_(), signal_handler_() { + : signal_set_(), signal_handler_(), proc_name_(""), + pid_file_dir_(DHCP_DATA_DIR), pid_file_() { } Daemon::~Daemon() { + if (pid_file_) { + pid_file_->deleteFile(); + } } void Daemon::init(const std::string& config_file) { @@ -96,5 +104,106 @@ std::string Daemon::getVersion(bool /*extended*/) { isc_throw(isc::NotImplemented, "Daemon::getVersion() called"); } +void +Daemon::setConfigFile(const std::string& config_file) { + config_file_ = config_file; +} + +std::string +Daemon::getProcName() const { + return (proc_name_); +}; + +void +Daemon::setProcName(const std::string& proc_name) { + proc_name_ = proc_name; +} + +std::string +Daemon::getPIDFileDir() const { + return(pid_file_dir_); +} + +void +Daemon::setPIDFileDir(const std::string& pid_file_dir) { + pid_file_dir_ = pid_file_dir; +} + +std::string +Daemon::getPIDFileName() const { + if (pid_file_) { + return (pid_file_->getFilename()); + } + + return (""); +}; + +void +Daemon::setPIDFileName(const std::string& pid_file_name) { + if (pid_file_) { + isc_throw(isc::InvalidOperation, "Daemon::setConfigFile" + " file name already set to:" << pid_file_->getFilename()); + } + + if (pid_file_name.empty()) { + isc_throw(isc::BadValue, "Daemon::setPIDFileName" + " file name may not be empty"); + } + + pid_file_.reset(new util::PIDFile(pid_file_name)); +}; + +std::string +Daemon::makePIDFileName() const { + if (config_file_.empty()) { + isc_throw(isc::InvalidOperation, + "Daemon::makePIDFileName config file name is not set"); + } + + if (proc_name_.empty()) { + isc_throw(isc::InvalidOperation, + "Daemon::makePIDFileName process name is not set"); + } + + // Create Filename instance from the config_file_ pathname, so we can + // extract the fname component. + isc::util::Filename file(config_file_); + if (file.name().empty()) { + isc_throw(isc::BadValue, "Daemon::makePIDFileName config file:" + << config_file_ << " is missing file name"); + } + + // Make the pathname for the PID file from the runtime directory, + // configuration name and process name. + std::ostringstream stream; + stream << pid_file_dir_ << "/" << file.name() + << "." << proc_name_ << ".pid"; + + return(stream.str()); +}; + +void +Daemon::createPIDFile(int pid) { + // If pid_file_ hasn't been instantiated explicitly, then do so + // using the default name. + if (!pid_file_) { + setPIDFileName(makePIDFileName()); + } + + // If we find a pre-existing file containing a live PID we bail. + if (pid_file_->check()) { + isc_throw(DaemonPIDExists, "Daemon::createPIDFile " << proc_name_ + << " already running?, PID file: " << getPIDFileName()); + } + + if (pid == 0) { + // Write the PID of the current process + pid_file_->write(); + } else { + // Write the PID we were given + pid_file_->write(pid); + } +} + }; }; diff --git a/src/lib/dhcpsrv/daemon.h b/src/lib/dhcpsrv/daemon.h index 7766472053..bd20e14a05 100644 --- a/src/lib/dhcpsrv/daemon.h +++ b/src/lib/dhcpsrv/daemon.h @@ -17,6 +17,7 @@ #include #include +#include #include #include #include @@ -24,6 +25,13 @@ namespace isc { namespace dhcp { +/// @brief Exception thrown when a the PID file points to a live PID +class DaemonPIDExists : public Exception { +public: + DaemonPIDExists(const char* file, size_t line, const char* what) : + isc::Exception(file, line, what) { }; +}; + /// @brief Base class for all services /// /// This is the base class that all daemons (DHCPv4, DHCPv6, D2 and possibly @@ -161,6 +169,56 @@ public: /// @return text string static std::string getVersion(bool extended); + /// @brief Sets the configuration file name + /// + /// @param config_file pathname of the configuration file + static void setConfigFile(const std::string& config_file); + + /// @brief returns the process name + /// This value is used as when forming the default PID file name + /// @return text string + std::string getProcName() const; + + /// @brief Sets the process name + /// @param proc_name name the process by which the process is recognized + void setProcName(const std::string& proc_name); + + /// @brief Returns the directory used when forming default PID file name + /// @return text string + std::string getPIDFileDir() const; + + /// @brief Sets the PID file directory + /// @param pid_file_dir path into which the PID file should be written + /// Note the value should not include a trailing slash, '/' + void setPIDFileDir(const std::string& pid_file_dir); + + /// @brief Returns the current PID file name + /// @return text string + std::string getPIDFileName() const; + + /// @brief Sets PID file name + /// + /// If this method is called prior to calling createPIDFile, + /// the value passed in will be treated as the full file name + /// for the PID file. This provides a means to override the + /// default file name with an explicit value. + /// + /// @param pid_file_name file name to be used as the PID file + void setPIDFileName(const std::string& pid_file_name); + + /// @brief Creates the PID file + /// + /// If the PID file name has not been previously set, the method + /// uses manufacturePIDFileName() to set it. If the PID file + /// name refers to an existing file whose contents are a PID whose + /// process is still alive, the method will throw a DaemonPIDExists + /// exception. Otherwise, the file created (or truncated) and + /// the given pid (if not zero) is written to the file. + /// + /// @param pid PID to write to the file if not zero, otherwise the + /// PID of the current process is used. + void createPIDFile(int pid = 0); + protected: /// @brief Invokes handler for the next received signal. @@ -189,11 +247,22 @@ protected: /// it not initialized, the signals will not be handled. isc::util::SignalHandler signal_handler_; -private: + /// @brief Manufacture the pid file name + std::string makePIDFileName() const; +private: /// @brief Config file name or empty if config file not used. static std::string config_file_; + /// @brief Name of this process, used when creating its pid file + std::string proc_name_; + + /// @brief Pointer to the directory where PID file(s) are written + /// It defaults to --localstatedir + std::string pid_file_dir_; + + /// @brief Pointer to the PID file for this process + isc::util::PIDFilePtr pid_file_; }; }; // end of isc::dhcp namespace diff --git a/src/lib/dhcpsrv/tests/daemon_unittest.cc b/src/lib/dhcpsrv/tests/daemon_unittest.cc index 89fb9354cb..f28819a371 100644 --- a/src/lib/dhcpsrv/tests/daemon_unittest.cc +++ b/src/lib/dhcpsrv/tests/daemon_unittest.cc @@ -34,6 +34,8 @@ namespace dhcp { class DaemonImpl : public Daemon { public: static std::string getVersion(bool extended); + + using Daemon::makePIDFileName; }; std::string DaemonImpl::getVersion(bool extended) { @@ -63,6 +65,8 @@ public: /// the default after each test completes. ~DaemonTest() { isc::log::setDefaultLoggingOutput(); + // Since it's static we need to clear it between tests + Daemon::setConfigFile(""); } }; @@ -75,6 +79,172 @@ TEST_F(DaemonTest, constructor) { // Check that the verbose mode is not set by default. Daemon instance2; EXPECT_FALSE(instance2.getVerbose()); + + EXPECT_EQ("",instance2.getConfigFile()); + EXPECT_EQ("",instance2.getProcName()); + EXPECT_EQ(CfgMgr::instance().getDataDir(),instance2.getPIDFileDir()); + EXPECT_EQ("",instance2.getPIDFileName()); +} + +// Verify config file accessors +TEST_F(DaemonTest, getSetConfigFile) { + Daemon instance; + + EXPECT_NO_THROW(instance.setConfigFile("test.txt")); + EXPECT_EQ("test.txt", instance.getConfigFile()); +} + +// Verify process name accessors +TEST_F(DaemonTest, getSetProcName) { + Daemon instance; + + EXPECT_NO_THROW(instance.setProcName("myproc")); + EXPECT_EQ("myproc", instance.getProcName()); +} + +// Verify PID file directory name accessors +TEST_F(DaemonTest, getSetPIDFileDir) { + Daemon instance; + + EXPECT_NO_THROW(instance.setPIDFileDir("/tmp")); + EXPECT_EQ("/tmp", instance.getPIDFileDir()); +} + +// Verify PID file name accessors. +TEST_F(DaemonTest, setPIDFileName) { + Daemon instance; + + // Verify that PID file name may not be set to empty + EXPECT_THROW(instance.setPIDFileName(""), BadValue); + + EXPECT_NO_THROW(instance.setPIDFileName("myproc")); + EXPECT_EQ("myproc", instance.getPIDFileName()); + + // Verify that setPIDFileName cannot be called twice on the same instance. + EXPECT_THROW(instance.setPIDFileName("again"), InvalidOperation); +} + +// Test the getVersion() redefinition +TEST_F(DaemonTest, getVersion) { + EXPECT_THROW(Daemon::getVersion(false), NotImplemented); + + ASSERT_NO_THROW(DaemonImpl::getVersion(false)); + + EXPECT_EQ(DaemonImpl::getVersion(false), "BASIC"); + + ASSERT_NO_THROW(DaemonImpl::getVersion(true)); + + EXPECT_EQ(DaemonImpl::getVersion(true), "EXTENDED"); +} + +// Verify makePIDFileName method +TEST_F(DaemonTest, makePIDFileName) { + DaemonImpl instance; + + // Verify that config file cannot be blank + instance.setProcName("notblank"); + EXPECT_THROW(instance.makePIDFileName(), InvalidOperation); + + // Verify that proc name cannot be blank + instance.setProcName(""); + instance.setConfigFile("notblank"); + EXPECT_THROW(instance.makePIDFileName(), InvalidOperation); + + // Verify that config file must contain a file name + instance.setProcName("myproc"); + instance.setConfigFile(".txt"); + EXPECT_THROW(instance.makePIDFileName(), BadValue); + instance.setConfigFile("/tmp/"); + EXPECT_THROW(instance.makePIDFileName(), BadValue); + + // Given a valid config file name and proc name we should good to go + instance.setConfigFile("/tmp/test.conf"); + std::string name; + EXPECT_NO_THROW(name = instance.makePIDFileName()); + + // Make sure the name is as we expect + std::ostringstream stream; + stream << CfgMgr::instance().getDataDir() << "/test.myproc.pid"; + EXPECT_EQ(stream.str(), name); + + // Verify that the default directory can be overridden + instance.setPIDFileDir("/tmp"); + EXPECT_NO_THROW(name = instance.makePIDFileName()); + EXPECT_EQ("/tmp/test.myproc.pid", name); +} + +// Verifies the creation a PID file and that a pre-existing PID file +// which points to a live PID causes a throw. +TEST_F(DaemonTest, createPIDFile) { + DaemonImpl instance; + + instance.setConfigFile("test.conf"); + instance.setProcName("daemon_test"); + instance.setPIDFileDir(TEST_DATA_BUILDDIR); + + EXPECT_NO_THROW(instance.createPIDFile()); + + std::ostringstream stream; + stream << TEST_DATA_BUILDDIR << "/test.daemon_test.pid"; + EXPECT_EQ(stream.str(), instance.getPIDFileName()); + + // If we try again, we should see our own PID file and fail + EXPECT_THROW(instance.createPIDFile(), DaemonPIDExists); +} + +// Verifies that a pre-existing PID file which points to a dead PID +// is overwritten. +TEST_F(DaemonTest, createPIDFileOverwrite) { + DaemonImpl instance; + + // We're going to use fork to generate a PID we KNOW is dead. + int pid = fork(); + ASSERT_GE(pid, 0); + + if (pid == 0) { + // This is the child, die right away. Tragic, no? + exit (0); + } + + // Back in the parent test, we need to wait for the child to die + int stat; + int ret = waitpid(pid, &stat, 0); + ASSERT_EQ(ret, pid); + + // Ok, so we should now have a PID that we know to be dead. + // Let's use it to create a PID file. + instance.setConfigFile("test.conf"); + instance.setProcName("daemon_test"); + instance.setPIDFileDir(TEST_DATA_BUILDDIR); + EXPECT_NO_THROW(instance.createPIDFile(pid)); + + // If we try to create the PID file again, this should work. + EXPECT_NO_THROW(instance.createPIDFile()); +} + +// Verifies that Daemon destruction deletes the PID file +TEST_F(DaemonTest, PIDFileCleanup) { + boost::shared_ptr instance; + instance.reset(new DaemonImpl); + + instance->setConfigFile("test.conf"); + instance->setProcName("daemon_test"); + instance->setPIDFileDir(TEST_DATA_BUILDDIR); + EXPECT_NO_THROW(instance->createPIDFile()); + + // If we try again, we should see our own PID file + EXPECT_THROW(instance->createPIDFile(), DaemonPIDExists); + + // Save the pid file name + std::string pid_file_name = instance->getPIDFileName(); + + // Now delete the Daemon instance. This should remove the + // PID file. + instance.reset(); + + struct stat stat_buf; + ASSERT_EQ(-1, stat(pid_file_name.c_str(), &stat_buf)); + EXPECT_EQ(errno, ENOENT); } // Checks that configureLogger method is behaving properly. @@ -117,18 +287,6 @@ TEST_F(DaemonTest, parsingConsoleOutput) { EXPECT_EQ("stdout" , storage->getLoggingInfo()[0].destinations_[0].output_); } -// Test the getVersion() redefinition -TEST_F(DaemonTest, getVersion) { - EXPECT_THROW(Daemon::getVersion(false), NotImplemented); - - ASSERT_NO_THROW(DaemonImpl::getVersion(false)); - - EXPECT_EQ(DaemonImpl::getVersion(false), "BASIC"); - - ASSERT_NO_THROW(DaemonImpl::getVersion(true)); - - EXPECT_EQ(DaemonImpl::getVersion(true), "EXTENDED"); -} // More tests will appear here as we develop Daemon class. diff --git a/src/lib/util/pid_file.h b/src/lib/util/pid_file.h index 8910c4355e..58bc93dd1b 100644 --- a/src/lib/util/pid_file.h +++ b/src/lib/util/pid_file.h @@ -16,6 +16,7 @@ #define PID_FILE_H #include +#include #include #include #include @@ -95,6 +96,9 @@ private: std::string filename_; }; +/// @brief Defines a shared pointer to a PIDFile +typedef boost::shared_ptr PIDFilePtr; + } // namespace isc::util } // namespace isc From d743c5f2743e3e07d83a8f8fa5cf8d7dee8379e5 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Thu, 2 Jul 2015 14:42:58 -0400 Subject: [PATCH 2/8] [3769] Added env var,KEA_PIDFILE_DIR; D2 now uses a PIDFile src/lib/dhcpsrv/daemon.c/h Daemon::Daemon() - Constructor will now override the default PID directory with the value of env variable KEA_PIDFILE_DIR. This provides a simple means to alter the value for tests. Added am_file_author_ flag so Daemon instances will only delete a file they have written. src/lib/testutils/dhcp_test_lib.sh.in - verify_server_pid() - new function which verifies that a server has a PID file and that it contains the server's PID, and that the process is alive. src/bin/keactrl/tests/Makefile.am - added export of KEA_PIDFILE_DIR to override default PID directory during tests Added PID file creation to D2 src/bin/d2/d_controller.cc - DControllerBase::launch() - Added block to createPIDFile() -DControllerBase::parseArgs() Replaced call to Daemon::init() with call to Daemon::setConfigFile() src/bin/d2/tests/Makefile.am - added export of KEA_PIDFILE_DIR to override default PID directory during tests src/bin/d2/tests/d2_process_tests.sh.in - dupcliate_server_start_test() - new test which verifies that D2 cannot be started twice (with the same configuration file) src/bin/d2/tests/d2_unittests.cc - main(int argc, char* argv[]) sets environment variable KEA_PIDFILE_DIR to override default PID diretory during tests src/lib/util/pid_file.cc/h src/lib/util/tests/pid_file_unittest.cc Changed PIDFile::check() to return either the PID contained in the PID file if the process is alive, or 0, rather than bool. This permits callers to see/log the PID. --- src/bin/d2/d2_messages.mes | 17 ++++++++ src/bin/d2/d_controller.cc | 16 +++++++- src/bin/d2/d_controller.h | 6 +++ src/bin/d2/tests/Makefile.am | 1 + src/bin/d2/tests/d2_process_tests.sh.in | 44 +++++++++++++++++++- src/bin/d2/tests/d2_unittests.cc | 3 ++ src/bin/keactrl/tests/Makefile.am | 1 + src/lib/dhcpsrv/daemon.cc | 20 ++++++--- src/lib/dhcpsrv/daemon.h | 3 ++ src/lib/testutils/dhcp_test_lib.sh.in | 54 +++++++++++++++++++++++++ src/lib/util/pid_file.cc | 6 +-- src/lib/util/pid_file.h | 4 +- src/lib/util/tests/pid_file_unittest.cc | 6 +-- 13 files changed, 166 insertions(+), 15 deletions(-) diff --git a/src/bin/d2/d2_messages.mes b/src/bin/d2/d2_messages.mes index 42516d13fa..a0193a7683 100644 --- a/src/bin/d2/d2_messages.mes +++ b/src/bin/d2/d2_messages.mes @@ -492,3 +492,20 @@ server. % DHCP_DDNS_UPDATE_RESPONSE_RECEIVED Request ID %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_ALREADY_RUNNING %1 already running? %2 +This is an error message that occurs when DHCP_DDNS encounters a pre-existing +PID file which contains the PID of a running process. This most likely +indicates an attempt to start a second instance of DHCP_DDNS using the +same configuration file. It is possible, the unlikely that the PID file +is a remnant left behind by a server crash or power failure and the PID +it contains refers to a process other than DHCP_DDNS. In such an event, +it would be necessary to manually remove the PID file. + +% DHCP_DDNS_PID_FILE_ERROR %1 could not create a PID file: %2 +This is an error message that occurs when DHCP_DDNS is unable to create +its PID file. The log message should contain details sufficient to +determine the underlying cause. The most likely culprits are that +some portion of the pathname does not exist or a permissions issue. The +default path is determined by --localstatedir configure paramter but +may be overridden by setting environment variable, KEA_PIDFILE_DIR. diff --git a/src/bin/d2/d_controller.cc b/src/bin/d2/d_controller.cc index 1cbd9aa598..6bbc304453 100644 --- a/src/bin/d2/d_controller.cc +++ b/src/bin/d2/d_controller.cc @@ -70,6 +70,8 @@ DControllerBase::launch(int argc, char* argv[], const bool test_mode) { throw; // rethrow it } + Daemon::setProcName(bin_name_); + // It is important that we set a default logger name because this name // will be used when the user doesn't provide the logging configuration // in the Kea configuration file. @@ -87,6 +89,18 @@ DControllerBase::launch(int argc, char* argv[], const bool test_mode) { Daemon::loggerInit(bin_name_.c_str(), verbose_); } + try { + createPIDFile(); + } catch (const dhcp::DaemonPIDExists& ex) { + LOG_FATAL(dctl_logger, DHCP_DDNS_ALREADY_RUNNING) + .arg(bin_name_).arg(ex.what()); + isc_throw (LaunchError, "Launch Failed: " << ex.what()); + } catch (const std::exception& ex) { + LOG_FATAL(dctl_logger, DHCP_DDNS_PID_FILE_ERROR) + .arg(app_name_).arg(ex.what()); + isc_throw (LaunchError, "Launch failed: " << ex.what()); + } + // Log the starting of the service. Although this is the controller // module, use a "DHCP_DDNS_" prefix to the module (to conform to the // principle of least astonishment). @@ -173,7 +187,7 @@ DControllerBase::parseArgs(int argc, char* argv[]) isc_throw(InvalidUsage, "configuration file name missing"); } - Daemon::init(optarg); + Daemon::setConfigFile(optarg); break; case '?': { diff --git a/src/bin/d2/d_controller.h b/src/bin/d2/d_controller.h index 61731872c5..19b42c65f4 100644 --- a/src/bin/d2/d_controller.h +++ b/src/bin/d2/d_controller.h @@ -50,6 +50,12 @@ public: isc::Exception(file, line, what) { }; }; +/// @brief Exception thrown when the controller launch fails. +class LaunchError: public isc::Exception { +public: + LaunchError (const char* file, size_t line, const char* what) : + isc::Exception(file, line, what) { }; +}; /// @brief Exception thrown when the application process fails. class ProcessInitError: public isc::Exception { diff --git a/src/bin/d2/tests/Makefile.am b/src/bin/d2/tests/Makefile.am index febe54b8ae..588654f18c 100644 --- a/src/bin/d2/tests/Makefile.am +++ b/src/bin/d2/tests/Makefile.am @@ -13,6 +13,7 @@ check-local: for shtest in $(SHTESTS) ; do \ echo Running test: $$shtest ; \ export KEA_LOCKFILE_DIR=$(abs_top_builddir); \ + export KEA_PIDFILE_DIR=$(abs_top_builddir); \ ${SHELL} $(abs_builddir)/$$shtest || exit ; \ done diff --git a/src/bin/d2/tests/d2_process_tests.sh.in b/src/bin/d2/tests/d2_process_tests.sh.in index 48f67ef5ff..1233b30701 100755 --- a/src/bin/d2/tests/d2_process_tests.sh.in +++ b/src/bin/d2/tests/d2_process_tests.sh.in @@ -1,4 +1,4 @@ -# Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC") +# Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC") # # Permission to use, copy, modify, and/or distribute this software for any # purpose with or without fee is hereby granted, provided that the above @@ -235,8 +235,50 @@ shutdown_test() { test_finish 0 } +# This test verifies if only one D2 per config can be started. +dupcliate_server_start_test() { + # Log the start of the test and print test name. + test_start "dhcp_ddns.duplicate_server_start_test" + # Remove dangling D2 instances and remove log files. + cleanup + # Create new configuration file. + create_config "${CONFIG}" + # Instruct D2 to log to the specific file. + set_logger + # Start D2. + start_kea ${bin_path}/${bin} + # Wait up to 20s for D2 to start. + wait_for_kea 20 + if [ ${_WAIT_FOR_KEA} -eq 0 ]; then + printf "ERROR: timeout waiting for D2 to start.\n" + clean_exit 1 + fi + + # Verify server is still running + verify_server_pid ${bin} ${CFG_FILE} + + printf "PID file is [%s], PID is [%d]" ${_SERVER_PID_FILE} ${_SERVER_PID} + + # Now try to start a second one + start_kea ${bin_path}/${bin} + + wait_for_message 10 "DHCP_DDNS_ALREADY_RUNNING" 1 + if [ ${_WAIT_FOR_MESSAGE} -eq 0 ]; then + printf "ERROR: Second D2 instance started? PID conflict not reported.\n" + clean_exit 1 + fi + + # Verify server is still running + verify_server_pid ${bin} ${CFG_FILE} + + # All ok. Shut down D2 and exit. + test_finish 0 +} + + dynamic_reconfiguration_test shutdown_test "dhcp-ddns.sigterm_test" 15 shutdown_test "dhcp-ddns.sigint_test" 2 version_test "dhcp-ddns.version" logger_vars_test "dhcp-ddns.variables" +dupcliate_server_start_test diff --git a/src/bin/d2/tests/d2_unittests.cc b/src/bin/d2/tests/d2_unittests.cc index 413abdf527..a5e986dec5 100644 --- a/src/bin/d2/tests/d2_unittests.cc +++ b/src/bin/d2/tests/d2_unittests.cc @@ -25,6 +25,9 @@ main(int argc, char* argv[]) { // src/lib/log/README for info on how to tweak logging isc::log::initLogger(); + // Override --localstatedir value for PID files + setenv("KEA_PIDFILE_DIR", TEST_DATA_BUILDDIR, 1); + int result = RUN_ALL_TESTS(); return (result); diff --git a/src/bin/keactrl/tests/Makefile.am b/src/bin/keactrl/tests/Makefile.am index b692cbe41a..ae0ec665ac 100644 --- a/src/bin/keactrl/tests/Makefile.am +++ b/src/bin/keactrl/tests/Makefile.am @@ -16,6 +16,7 @@ check-local: chmod +x $(abs_builddir)/$$shtest ; \ export KEA_LOCKFILE_DIR=$(abs_top_builddir); \ export KEACTRL_BUILD_DIR=$(abs_top_builddir); \ + export KEA_PIDFILE_DIR=$(abs_top_builddir); \ export KEACTRL_CONF=$(abs_top_builddir)/src/bin/keactrl/tests/keactrl_test.conf; \ ${SHELL} $(abs_builddir)/$$shtest || exit ; \ done diff --git a/src/lib/dhcpsrv/daemon.cc b/src/lib/dhcpsrv/daemon.cc index 9d89bbc098..6fe0172057 100644 --- a/src/lib/dhcpsrv/daemon.cc +++ b/src/lib/dhcpsrv/daemon.cc @@ -39,11 +39,18 @@ std::string Daemon::config_file_ = ""; Daemon::Daemon() : signal_set_(), signal_handler_(), proc_name_(""), - pid_file_dir_(DHCP_DATA_DIR), pid_file_() { + pid_file_dir_(DHCP_DATA_DIR), pid_file_(), am_file_author_(false) { + + // The pid_file_dir can be overridden via environment variable + // This is primarily intended to simplify testing + const char* const env = getenv("KEA_PIDFILE_DIR"); + if (env) { + pid_file_dir_ = env; + } } Daemon::~Daemon() { - if (pid_file_) { + if (pid_file_ && am_file_author_) { pid_file_->deleteFile(); } } @@ -191,9 +198,10 @@ Daemon::createPIDFile(int pid) { } // If we find a pre-existing file containing a live PID we bail. - if (pid_file_->check()) { - isc_throw(DaemonPIDExists, "Daemon::createPIDFile " << proc_name_ - << " already running?, PID file: " << getPIDFileName()); + int chk_pid = pid_file_->check(); + if (chk_pid > 0) { + isc_throw(DaemonPIDExists, "Daemon::createPIDFile: PID: " << chk_pid + << " exists, PID file: " << getPIDFileName()); } if (pid == 0) { @@ -203,6 +211,8 @@ Daemon::createPIDFile(int pid) { // Write the PID we were given pid_file_->write(pid); } + + am_file_author_ = true; } }; diff --git a/src/lib/dhcpsrv/daemon.h b/src/lib/dhcpsrv/daemon.h index bd20e14a05..7d8ffe7673 100644 --- a/src/lib/dhcpsrv/daemon.h +++ b/src/lib/dhcpsrv/daemon.h @@ -263,6 +263,9 @@ private: /// @brief Pointer to the PID file for this process isc::util::PIDFilePtr pid_file_; + + /// @brief Flag indicating if this instance created the file + bool am_file_author_; }; }; // end of isc::dhcp namespace diff --git a/src/lib/testutils/dhcp_test_lib.sh.in b/src/lib/testutils/dhcp_test_lib.sh.in index b02e88a261..f81a3a63ed 100644 --- a/src/lib/testutils/dhcp_test_lib.sh.in +++ b/src/lib/testutils/dhcp_test_lib.sh.in @@ -426,6 +426,60 @@ must be a number" kill -${sig} ${_GET_PIDS} } +# Verifies that a server is up running by its PID file +# The PID file is constructed from the given config file name and +# binary name. If it exists and the PID it contains refers to a +# live process it sets _SERVER_PID_FILE and _SERVER_PID to the +# corresponding values. Otherwise, it emits an error and exits. +verify_server_pid() { + local bin_name="${1}" # binary name of the server + local cfg_file="${2}" # config file name + + # We will construct the PID file name based on the server config + # and binary name + if [ -z ${bin_name} ]; then + test_lib_error "verify_server_pid requires binary name" + clean_exit 1 + fi + + if [ -z ${cfg_file} ]; then + test_lib_error "verify_server_pid requires config file name" + clean_exit 1 + fi + + # Only the file name portio of the config file is used, try and + # extract it. NOTE if this "algorithm" changes this code will need + # to be updated. + fname=`basename ${cfg_file}` + fname=`echo $fname | cut -f1 -d'.'` + + if [ -z ${fname} ]; then + test_lib_error "verify_server_pid could not extract config name" + clean_exit 1 + fi + + # Now we can build the name: + pid_file="$KEA_PIDFILE_DIR/$fname.$bin_name.pid" + + if [ ! -e ${pid_file} ]; then + printf "ERROR: PID file:[%s] does not exist\n" ${pid_file} + clean_exit 1 + fi + + # File exists, does its PID point to a live process? + pid=`cat ${pid_file}` + kill -0 ${pid} + if [ $? -ne 0 ]; then + printf "ERROR: PID file:[%s] exists but PID:[%d] does not\n" \ + ${pid_file} ${pid} + clean_exit 1 + fi + + # Make the values accessible to the caller + _SERVER_PID="${pid}" + _SERVER_PID_FILE="${pid_file}" +} + # This test verifies that the binary is reporting its version properly. version_test() { test_name=${1} # Test name diff --git a/src/lib/util/pid_file.cc b/src/lib/util/pid_file.cc index 8f10b0ae1f..9ebd1be73d 100644 --- a/src/lib/util/pid_file.cc +++ b/src/lib/util/pid_file.cc @@ -28,7 +28,7 @@ PIDFile::PIDFile(const std::string& filename) PIDFile::~PIDFile() { } -bool +int PIDFile::check() const { std::ifstream fs(filename_.c_str()); int pid; @@ -53,11 +53,11 @@ PIDFile::check() const { // If the process is still running return true if (kill(pid, 0) == 0) { - return (true); + return (pid); } // No process - return (false); + return (0); } void diff --git a/src/lib/util/pid_file.h b/src/lib/util/pid_file.h index 58bc93dd1b..236b966ecc 100644 --- a/src/lib/util/pid_file.h +++ b/src/lib/util/pid_file.h @@ -63,11 +63,11 @@ public: /// If the file exists but can't be read or it doesn't have /// the proper format treat it as the process existing. /// - /// @return true if the PID is in use, false otherwise + /// @return returns the PID if it is in, 0 otherwise. /// /// @throw throws PIDCantReadPID if it was able to open the file /// but was unable to read the PID from it. - bool check() const; + int check() const; /// @brief Write the PID to the file. /// diff --git a/src/lib/util/tests/pid_file_unittest.cc b/src/lib/util/tests/pid_file_unittest.cc index a0df57d161..cdad04881d 100644 --- a/src/lib/util/tests/pid_file_unittest.cc +++ b/src/lib/util/tests/pid_file_unittest.cc @@ -127,7 +127,7 @@ TEST_F(PIDFileTest, pidInUse) { pid_file.write(); // Check if we think the process is running - EXPECT_TRUE(pid_file.check()); + EXPECT_EQ(getpid(), pid_file.check()); } /// @brief Test checking a PID. Write a PID that isn't in use @@ -148,7 +148,7 @@ TEST_F(PIDFileTest, pidNotInUse) { pid_file.write(pid); // Check to see if we think the process is running - if (!pid_file.check()) { + if (pid_file.check() == 0) { return; } @@ -159,7 +159,7 @@ TEST_F(PIDFileTest, pidNotInUse) { pid_file.write(pid); // Check to see if we think the process is running - EXPECT_FALSE(pid_file.check()); + EXPECT_EQ(0, pid_file.check()); } /// @brief Test checking a PID. Write garbage to the PID file From 24267d20dd09ab1432b099d33b7e0597e18d040d Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Thu, 2 Jul 2015 16:49:53 -0400 Subject: [PATCH 3/8] [3769] DHPCv4 now uses PID file, made pid test common src/lib/testutils/dhcp_test_lib.sh.in - server_pid_file_test() - common test for any server to verify PID file management src/bin/d2/tests/d2_process_tests.sh.in remmoved duplicate_server_start_test now calls server_pid_file_test Added PID file creation to DHCP4 src/bin/dhcp4/dhcp4_messages.mes - added log DHCP4_ALREADY_RUNNING src/bin/dhcp4/main.cc - added logic to create the PID and catch exception specific to PID conflict src/bin/dhcp4/tests/Makefile.am - exports KEA_PIDFILE_DIR src/bin/dhcp4/tests/dhcp4_process_tests.sh.in - added call to server_pid_file_test src/bin/dhcp4/tests/dhcp4_unittests.cc - main(int argc, char* argv[]) sets env var KEA_PIDFILE_DIR --- src/bin/d2/d2_messages.mes | 34 +++++++------- src/bin/d2/tests/d2_process_tests.sh.in | 43 +---------------- src/bin/dhcp4/dhcp4_messages.mes | 10 ++++ src/bin/dhcp4/kea_controller.cc | 3 -- src/bin/dhcp4/main.cc | 18 +++++++- src/bin/dhcp4/tests/Makefile.am | 1 + src/bin/dhcp4/tests/dhcp4_process_tests.sh.in | 1 + src/bin/dhcp4/tests/dhcp4_unittests.cc | 1 + src/lib/dhcpsrv/daemon.cc | 8 ++-- src/lib/dhcpsrv/daemon.h | 6 +-- src/lib/dhcpsrv/tests/daemon_unittest.cc | 3 +- src/lib/testutils/dhcp_test_lib.sh.in | 46 +++++++++++++++++++ 12 files changed, 103 insertions(+), 71 deletions(-) diff --git a/src/bin/d2/d2_messages.mes b/src/bin/d2/d2_messages.mes index a0193a7683..383c64f13c 100644 --- a/src/bin/d2/d2_messages.mes +++ b/src/bin/d2/d2_messages.mes @@ -100,6 +100,15 @@ documented in preceding log entries. This is an informational message issued after DHCP_DDNS has submitted DNS mapping additions which were received and accepted by an appropriate DNS server. +% DHCP_DDNS_ALREADY_RUNNING %1 already running? %2 +This is an error message that occurs when DHCP_DDNS encounters a pre-existing +PID file which contains the PID of a running process. This most likely +indicates an attempt to start a second instance of DHCP_DDNS using the +same configuration file. It is possible, though unlikely, that the PID file +is a remnant left behind by a server crash or power failure and the PID +it contains refers to a process other than DHCP_DDNS. In such an event, +it would be necessary to manually remove the PID file. + % DHCP_DDNS_AT_MAX_TRANSACTIONS application has %1 queued requests but has reached maximum number of %2 concurrent transactions 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. @@ -277,6 +286,14 @@ no configured DDNS domains in the DHCP_DDNS configuration. Either the DHCP_DDNS configuration needs to be updated or the source of the FQDN itself should be investigated. +% DHCP_DDNS_PID_FILE_ERROR %1 could not create a PID file: %2 +This is an error message that occurs when DHCP_DDNS is unable to create +its PID file. The log message should contain details sufficient to +determine the underlying cause. The most likely culprits are that +some portion of the pathname does not exist or a permissions issue. The +default path is determined by --localstatedir configure paramter but +may be overridden by setting environment variable, KEA_PIDFILE_DIR. + % DHCP_DDNS_PROCESS_INIT application init invoked This is a debug message issued when the DHCP-DDNS application enters its initialization method. @@ -492,20 +509,3 @@ server. % DHCP_DDNS_UPDATE_RESPONSE_RECEIVED Request ID %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_ALREADY_RUNNING %1 already running? %2 -This is an error message that occurs when DHCP_DDNS encounters a pre-existing -PID file which contains the PID of a running process. This most likely -indicates an attempt to start a second instance of DHCP_DDNS using the -same configuration file. It is possible, the unlikely that the PID file -is a remnant left behind by a server crash or power failure and the PID -it contains refers to a process other than DHCP_DDNS. In such an event, -it would be necessary to manually remove the PID file. - -% DHCP_DDNS_PID_FILE_ERROR %1 could not create a PID file: %2 -This is an error message that occurs when DHCP_DDNS is unable to create -its PID file. The log message should contain details sufficient to -determine the underlying cause. The most likely culprits are that -some portion of the pathname does not exist or a permissions issue. The -default path is determined by --localstatedir configure paramter but -may be overridden by setting environment variable, KEA_PIDFILE_DIR. diff --git a/src/bin/d2/tests/d2_process_tests.sh.in b/src/bin/d2/tests/d2_process_tests.sh.in index 1233b30701..3a6aa83d3b 100755 --- a/src/bin/d2/tests/d2_process_tests.sh.in +++ b/src/bin/d2/tests/d2_process_tests.sh.in @@ -235,50 +235,9 @@ shutdown_test() { test_finish 0 } -# This test verifies if only one D2 per config can be started. -dupcliate_server_start_test() { - # Log the start of the test and print test name. - test_start "dhcp_ddns.duplicate_server_start_test" - # Remove dangling D2 instances and remove log files. - cleanup - # Create new configuration file. - create_config "${CONFIG}" - # Instruct D2 to log to the specific file. - set_logger - # Start D2. - start_kea ${bin_path}/${bin} - # Wait up to 20s for D2 to start. - wait_for_kea 20 - if [ ${_WAIT_FOR_KEA} -eq 0 ]; then - printf "ERROR: timeout waiting for D2 to start.\n" - clean_exit 1 - fi - - # Verify server is still running - verify_server_pid ${bin} ${CFG_FILE} - - printf "PID file is [%s], PID is [%d]" ${_SERVER_PID_FILE} ${_SERVER_PID} - - # Now try to start a second one - start_kea ${bin_path}/${bin} - - wait_for_message 10 "DHCP_DDNS_ALREADY_RUNNING" 1 - if [ ${_WAIT_FOR_MESSAGE} -eq 0 ]; then - printf "ERROR: Second D2 instance started? PID conflict not reported.\n" - clean_exit 1 - fi - - # Verify server is still running - verify_server_pid ${bin} ${CFG_FILE} - - # All ok. Shut down D2 and exit. - test_finish 0 -} - - +server_pid_file_test "${CONFIG}" DHCP_DDNS_ALREADY_RUNNING dynamic_reconfiguration_test shutdown_test "dhcp-ddns.sigterm_test" 15 shutdown_test "dhcp-ddns.sigint_test" 2 version_test "dhcp-ddns.version" logger_vars_test "dhcp-ddns.variables" -dupcliate_server_start_test diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes index d820896b6d..9c358abbf4 100644 --- a/src/bin/dhcp4/dhcp4_messages.mes +++ b/src/bin/dhcp4/dhcp4_messages.mes @@ -19,6 +19,16 @@ This message is printed when DHCPv4 server enabled an interface to be used to receive DHCPv4 traffic. IPv4 socket on this interface will be opened once Interface Manager starts up procedure of opening sockets. +% DHCP4_ALREADY_RUNNING %1 already running? %2 +This is an error message that occurs when the DHCPv4 server encounters +a pre-existing PID file which contains the PID of a running process. +This most likely indicates an attempt to start a second instance of +the server using the same configuration file. It is possible, though +unlikely that the PID file is a remnant left behind by a server crash or +power failure and the PID it contains refers to a process other than +the server. In such an event, it would be necessary to manually remove +the PID file. + % DHCP4_BUFFER_RECEIVED received buffer from %1:%2 to %3:%4 over interface %5 This debug message is logged when the server has received a packet over the socket. When the message is logged the contents of the received diff --git a/src/bin/dhcp4/kea_controller.cc b/src/bin/dhcp4/kea_controller.cc index ea43d88935..92503d5cf0 100644 --- a/src/bin/dhcp4/kea_controller.cc +++ b/src/bin/dhcp4/kea_controller.cc @@ -168,9 +168,6 @@ namespace dhcp { void ControlledDhcpv4Srv::init(const std::string& file_name) { - // Call parent class's init to initialize file name. - Daemon::init(file_name); - // Configure the server using JSON file. configure(file_name); diff --git a/src/bin/dhcp4/main.cc b/src/bin/dhcp4/main.cc index 0635086950..ca49d30eee 100644 --- a/src/bin/dhcp4/main.cc +++ b/src/bin/dhcp4/main.cc @@ -118,6 +118,7 @@ main(int argc, char* argv[]) { usage(); } + // Configuration file is required. if (config_file.empty()) { cerr << "Configuration file not specified." << endl; @@ -125,7 +126,6 @@ main(int argc, char* argv[]) { } int ret = EXIT_SUCCESS; - try { // It is important that we set a default logger name because this name // will be used when the user doesn't provide the logging configuration @@ -145,6 +145,10 @@ main(int argc, char* argv[]) { // Remember verbose-mode server.setVerbose(verbose_mode); + Daemon::setProcName(DHCP4_NAME); + Daemon::setConfigFile(config_file); + server.createPIDFile(); + try { // Initialize the server. server.init(config_file); @@ -173,8 +177,18 @@ main(int argc, char* argv[]) { LOG_INFO(dhcp4_logger, DHCP4_SHUTDOWN); - } catch (const std::exception& ex) { + } catch (const isc::dhcp::DaemonPIDExists& ex) { + // First, we print the error on stderr (that should always work) + cerr << DHCP4_NAME << " already running? " << ex.what() + << endl; + // Let's also try to log it using logging system, but we're not + // sure if it's usable (the exception may have been thrown from + // the logger subsystem) + LOG_FATAL(dhcp4_logger, DHCP4_ALREADY_RUNNING) + .arg(DHCP4_NAME).arg(ex.what()); + ret = EXIT_FAILURE; + } catch (const std::exception& ex) { // First, we print the error on stderr (that should always work) cerr << DHCP4_NAME << ": Fatal error during start up: " << ex.what() << endl; diff --git a/src/bin/dhcp4/tests/Makefile.am b/src/bin/dhcp4/tests/Makefile.am index 2752ecdef9..65098c98f5 100644 --- a/src/bin/dhcp4/tests/Makefile.am +++ b/src/bin/dhcp4/tests/Makefile.am @@ -12,6 +12,7 @@ check-local: for shtest in $(SHTESTS) ; do \ echo Running test: $$shtest ; \ export KEA_LOCKFILE_DIR=$(abs_top_builddir); \ + export KEA_PIDFILE_DIR=$(abs_top_builddir); \ ${SHELL} $(abs_builddir)/$$shtest || exit ; \ done diff --git a/src/bin/dhcp4/tests/dhcp4_process_tests.sh.in b/src/bin/dhcp4/tests/dhcp4_process_tests.sh.in index 4bf8fb67b1..91bca88538 100755 --- a/src/bin/dhcp4/tests/dhcp4_process_tests.sh.in +++ b/src/bin/dhcp4/tests/dhcp4_process_tests.sh.in @@ -272,6 +272,7 @@ shutdown_test() { test_finish 0 } +server_pid_file_test "${CONFIG}" DHCP4_ALREADY_RUNNING dynamic_reconfiguration_test shutdown_test "dhcpv4.sigterm_test" 15 shutdown_test "dhcpv4.sigint_test" 2 diff --git a/src/bin/dhcp4/tests/dhcp4_unittests.cc b/src/bin/dhcp4/tests/dhcp4_unittests.cc index f57cfa429f..2beea84252 100644 --- a/src/bin/dhcp4/tests/dhcp4_unittests.cc +++ b/src/bin/dhcp4/tests/dhcp4_unittests.cc @@ -25,6 +25,7 @@ main(int argc, char* argv[]) { // src/lib/log/README for info on how to tweak logging isc::log::initLogger(); + setenv("KEA_PIDFILE_DIR", TEST_DATA_BUILDDIR, 1); int result = RUN_ALL_TESTS(); return (result); diff --git a/src/lib/dhcpsrv/daemon.cc b/src/lib/dhcpsrv/daemon.cc index 6fe0172057..49868c3ee1 100644 --- a/src/lib/dhcpsrv/daemon.cc +++ b/src/lib/dhcpsrv/daemon.cc @@ -37,9 +37,11 @@ namespace dhcp { // This is an initial config file location. std::string Daemon::config_file_ = ""; +std::string Daemon::proc_name_ = ""; + Daemon::Daemon() - : signal_set_(), signal_handler_(), proc_name_(""), - pid_file_dir_(DHCP_DATA_DIR), pid_file_(), am_file_author_(false) { + : signal_set_(), signal_handler_(), pid_file_dir_(DHCP_DATA_DIR), + pid_file_(), am_file_author_(false) { // The pid_file_dir can be overridden via environment variable // This is primarily intended to simplify testing @@ -117,7 +119,7 @@ Daemon::setConfigFile(const std::string& config_file) { } std::string -Daemon::getProcName() const { +Daemon::getProcName() { return (proc_name_); }; diff --git a/src/lib/dhcpsrv/daemon.h b/src/lib/dhcpsrv/daemon.h index 7d8ffe7673..77035dbb69 100644 --- a/src/lib/dhcpsrv/daemon.h +++ b/src/lib/dhcpsrv/daemon.h @@ -177,11 +177,11 @@ public: /// @brief returns the process name /// This value is used as when forming the default PID file name /// @return text string - std::string getProcName() const; + static std::string getProcName(); /// @brief Sets the process name /// @param proc_name name the process by which the process is recognized - void setProcName(const std::string& proc_name); + static void setProcName(const std::string& proc_name); /// @brief Returns the directory used when forming default PID file name /// @return text string @@ -255,7 +255,7 @@ private: static std::string config_file_; /// @brief Name of this process, used when creating its pid file - std::string proc_name_; + static std::string proc_name_; /// @brief Pointer to the directory where PID file(s) are written /// It defaults to --localstatedir diff --git a/src/lib/dhcpsrv/tests/daemon_unittest.cc b/src/lib/dhcpsrv/tests/daemon_unittest.cc index f28819a371..b6618e539b 100644 --- a/src/lib/dhcpsrv/tests/daemon_unittest.cc +++ b/src/lib/dhcpsrv/tests/daemon_unittest.cc @@ -65,8 +65,9 @@ public: /// the default after each test completes. ~DaemonTest() { isc::log::setDefaultLoggingOutput(); - // Since it's static we need to clear it between tests + // Since they're static we need to clear them between tests Daemon::setConfigFile(""); + Daemon::setProcName(""); } }; diff --git a/src/lib/testutils/dhcp_test_lib.sh.in b/src/lib/testutils/dhcp_test_lib.sh.in index f81a3a63ed..a07cc47339 100644 --- a/src/lib/testutils/dhcp_test_lib.sh.in +++ b/src/lib/testutils/dhcp_test_lib.sh.in @@ -590,3 +590,49 @@ logger_vars_test() { test_finish 0 } + +# This test verifies server PID file management +# 1. It verifies that upon startup, the server creates a PID file +# 2. It verifies the an attempt to start a second instance fails +# due to pre-existing PID File/PID detection +server_pid_file_test() { + local server_cfg="${1}" + local log_id="${2}" + + # Log the start of the test and print test name. + test_start "${bin}.server_pid_file_test" + # Remove dangling DHCP4 instances and remove log files. + cleanup + # Create new configuration file. + create_config "${CONFIG}" + # Instruct server to log to the specific file. + set_logger + # Start server + start_kea ${bin_path}/${bin} + # Wait up to 20s for server to start. + wait_for_kea 20 + if [ ${_WAIT_FOR_KEA} -eq 0 ]; then + printf "ERROR: timeout waiting for %s to start.\n" ${bin} + clean_exit 1 + fi + + # Verify server is still running + verify_server_pid ${bin} ${CFG_FILE} + + printf "PID file is [%s], PID is [%d]" ${_SERVER_PID_FILE} ${_SERVER_PID} + + # Now try to start a second one + start_kea ${bin_path}/${bin} + + wait_for_message 10 "${log_id}" 1 + if [ ${_WAIT_FOR_MESSAGE} -eq 0 ]; then + printf "ERROR: Second %s instance started? PID conflict not reported.\n" ${bin} + clean_exit 1 + fi + + # Verify server is still running + verify_server_pid ${bin} ${CFG_FILE} + + # All ok. Shut down the server and exit. + test_finish 0 +} From 5776ee0cc7f7614c83c5395f5c97f443f9c39ce5 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Thu, 2 Jul 2015 17:17:08 -0400 Subject: [PATCH 4/8] [3769] DHCPv6 now uses PID file Added PID file creation to DHCP6 src/bin/dhcp6/dhcp6_messages.mes - added log DHCP6_ALREADY_RUNNING src/bin/dhcp6/main.cc - added logic to create the PID and catch exception specific to PID conflict src/bin/dhcp6/tests/Makefile.am - exports KEA_PIDFILE_DIR src/bin/dhcp6/tests/dhcp6_process_tests.sh.in - added call to server_pid_file_test src/bin/dhcp6/tests/dhcp6_unittests.cc - main(int argc, char* argv[]) sets env var KEA_PIDFILE_DIR --- src/bin/d2/d_controller.cc | 4 ++-- src/bin/dhcp4/main.cc | 5 +++-- src/bin/dhcp6/dhcp6_messages.mes | 10 ++++++++++ src/bin/dhcp6/kea_controller.cc | 3 --- src/bin/dhcp6/main.cc | 16 ++++++++++++++++ src/bin/dhcp6/tests/Makefile.am | 1 + src/bin/dhcp6/tests/dhcp6_process_tests.sh.in | 1 + src/bin/dhcp6/tests/dhcp6_unittests.cc | 1 + 8 files changed, 34 insertions(+), 7 deletions(-) diff --git a/src/bin/d2/d_controller.cc b/src/bin/d2/d_controller.cc index 6bbc304453..8aa18c77f9 100644 --- a/src/bin/d2/d_controller.cc +++ b/src/bin/d2/d_controller.cc @@ -70,7 +70,7 @@ DControllerBase::launch(int argc, char* argv[], const bool test_mode) { throw; // rethrow it } - Daemon::setProcName(bin_name_); + setProcName(bin_name_); // It is important that we set a default logger name because this name // will be used when the user doesn't provide the logging configuration @@ -187,7 +187,7 @@ DControllerBase::parseArgs(int argc, char* argv[]) isc_throw(InvalidUsage, "configuration file name missing"); } - Daemon::setConfigFile(optarg); + setConfigFile(optarg); break; case '?': { diff --git a/src/bin/dhcp4/main.cc b/src/bin/dhcp4/main.cc index ca49d30eee..f428c3f3f8 100644 --- a/src/bin/dhcp4/main.cc +++ b/src/bin/dhcp4/main.cc @@ -145,8 +145,9 @@ main(int argc, char* argv[]) { // Remember verbose-mode server.setVerbose(verbose_mode); - Daemon::setProcName(DHCP4_NAME); - Daemon::setConfigFile(config_file); + // Create our PID file. + server.setProcName(DHCP4_NAME); + server.setConfigFile(config_file); server.createPIDFile(); try { diff --git a/src/bin/dhcp6/dhcp6_messages.mes b/src/bin/dhcp6/dhcp6_messages.mes index 6643da3f3b..b82e6475e9 100644 --- a/src/bin/dhcp6/dhcp6_messages.mes +++ b/src/bin/dhcp6/dhcp6_messages.mes @@ -19,6 +19,16 @@ This message is printed when DHCPv6 server enabled an interface to be used to receive DHCPv6 traffic. IPv6 socket on this interface will be opened once Interface Manager starts up procedure of opening sockets. +% DHCP6_ALREADY_RUNNING %1 already running? %2 +This is an error message that occurs when the DHCPv6 server encounters +a pre-existing PID file which contains the PID of a running process. +This most likely indicates an attempt to start a second instance of +the server using the same configuration file. It is possible, though +unlikely that the PID file is a remnant left behind by a server crash or +power failure and the PID it contains refers to a process other than +the server. In such an event, it would be necessary to manually remove +the PID file. + % DHCP6_ADD_GLOBAL_STATUS_CODE %1: adding Status Code to DHCPv6 packet: %2 This message is logged when the server is adding the top-level Status Code option. The first argument includes the client and the diff --git a/src/bin/dhcp6/kea_controller.cc b/src/bin/dhcp6/kea_controller.cc index 18d416dc5c..f1fc901f97 100644 --- a/src/bin/dhcp6/kea_controller.cc +++ b/src/bin/dhcp6/kea_controller.cc @@ -174,9 +174,6 @@ namespace dhcp { void ControlledDhcpv6Srv::init(const std::string& file_name) { - // Call parent class's init to initialize file name. - Daemon::init(file_name); - // Configure the server using JSON file. configure(file_name); diff --git a/src/bin/dhcp6/main.cc b/src/bin/dhcp6/main.cc index d780d395c2..6e1fb5c707 100644 --- a/src/bin/dhcp6/main.cc +++ b/src/bin/dhcp6/main.cc @@ -148,6 +148,11 @@ main(int argc, char* argv[]) { // Remember verbose-mode server.setVerbose(verbose_mode); + // Create our PID file + server.setProcName(DHCP6_NAME); + server.setConfigFile(config_file); + server.createPIDFile(); + try { // Initialize the server, e.g. establish control session // Read a configuration file @@ -177,6 +182,17 @@ main(int argc, char* argv[]) { LOG_INFO(dhcp6_logger, DHCP6_SHUTDOWN); + } catch (const isc::dhcp::DaemonPIDExists& ex) { + // First, we print the error on stderr (that should always work) + cerr << DHCP6_NAME << " already running? " << ex.what() + << endl; + + // Let's also try to log it using logging system, but we're not + // sure if it's usable (the exception may have been thrown from + // the logger subsystem) + LOG_FATAL(dhcp6_logger, DHCP6_ALREADY_RUNNING) + .arg(DHCP6_NAME).arg(ex.what()); + ret = EXIT_FAILURE; } catch (const std::exception& ex) { // First, we print the error on stderr (that should always work) diff --git a/src/bin/dhcp6/tests/Makefile.am b/src/bin/dhcp6/tests/Makefile.am index b4157c72e1..2d444bc2c1 100644 --- a/src/bin/dhcp6/tests/Makefile.am +++ b/src/bin/dhcp6/tests/Makefile.am @@ -12,6 +12,7 @@ check-local: for shtest in $(SHTESTS) ; do \ echo Running test: $$shtest ; \ export KEA_LOCKFILE_DIR=$(abs_top_builddir); \ + export KEA_PIDFILE_DIR=$(abs_top_builddir); \ ${SHELL} $(abs_builddir)/$$shtest || exit ; \ done diff --git a/src/bin/dhcp6/tests/dhcp6_process_tests.sh.in b/src/bin/dhcp6/tests/dhcp6_process_tests.sh.in index fd3e6065db..e9748b8419 100755 --- a/src/bin/dhcp6/tests/dhcp6_process_tests.sh.in +++ b/src/bin/dhcp6/tests/dhcp6_process_tests.sh.in @@ -273,6 +273,7 @@ returned %d." test_finish 0 } +server_pid_file_test "${CONFIG}" DHCP6_ALREADY_RUNNING dynamic_reconfiguration_test shutdown_test "dhcpv6.sigterm_test" 15 shutdown_test "dhcpv6.sigint_test" 2 diff --git a/src/bin/dhcp6/tests/dhcp6_unittests.cc b/src/bin/dhcp6/tests/dhcp6_unittests.cc index d9695e9d6b..0c15300d6f 100644 --- a/src/bin/dhcp6/tests/dhcp6_unittests.cc +++ b/src/bin/dhcp6/tests/dhcp6_unittests.cc @@ -22,6 +22,7 @@ main(int argc, char* argv[]) { ::testing::InitGoogleTest(&argc, argv); isc::log::initLogger(); + setenv("KEA_PIDFILE_DIR", TEST_DATA_BUILDDIR, 1); int result = RUN_ALL_TESTS(); return result; From 1c6b2862c9cac1e6486a256a17385174528325a1 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Mon, 6 Jul 2015 09:35:38 -0400 Subject: [PATCH 5/8] [3769] Removed static from Daemon::config_file_ and proc_name_ Rendered config_file_ and proc_name_ instance members as there is no reason for them to be static. --- src/bin/d2/tests/d_controller_unittests.cc | 2 +- src/lib/dhcpsrv/daemon.cc | 16 +++++------ src/lib/dhcpsrv/daemon.h | 33 +++++++++------------- src/lib/dhcpsrv/tests/daemon_unittest.cc | 3 -- 4 files changed, 22 insertions(+), 32 deletions(-) diff --git a/src/bin/d2/tests/d_controller_unittests.cc b/src/bin/d2/tests/d_controller_unittests.cc index f5612b5c38..e882bd4c5c 100644 --- a/src/bin/d2/tests/d_controller_unittests.cc +++ b/src/bin/d2/tests/d_controller_unittests.cc @@ -229,7 +229,7 @@ TEST_F(DStubControllerTest, missingConfigFileArgument) { int argc = 2; // Record start time, and invoke launch(). - EXPECT_THROW(launch(argc, argv), ProcessInitError); + EXPECT_THROW(launch(argc, argv), LaunchError); } /// @brief Tests launch with an operational error during application execution. diff --git a/src/lib/dhcpsrv/daemon.cc b/src/lib/dhcpsrv/daemon.cc index 49868c3ee1..a020b0abb2 100644 --- a/src/lib/dhcpsrv/daemon.cc +++ b/src/lib/dhcpsrv/daemon.cc @@ -34,14 +34,9 @@ namespace isc { namespace dhcp { -// This is an initial config file location. -std::string Daemon::config_file_ = ""; - -std::string Daemon::proc_name_ = ""; - Daemon::Daemon() - : signal_set_(), signal_handler_(), pid_file_dir_(DHCP_DATA_DIR), - pid_file_(), am_file_author_(false) { + : signal_set_(), signal_handler_(), config_file_(""), proc_name_(""), + pid_file_dir_(DHCP_DATA_DIR), pid_file_(), am_file_author_(false) { // The pid_file_dir can be overridden via environment variable // This is primarily intended to simplify testing @@ -113,13 +108,18 @@ std::string Daemon::getVersion(bool /*extended*/) { isc_throw(isc::NotImplemented, "Daemon::getVersion() called"); } +std::string +Daemon::getConfigFile() const { + return (config_file_); +} + void Daemon::setConfigFile(const std::string& config_file) { config_file_ = config_file; } std::string -Daemon::getProcName() { +Daemon::getProcName() const { return (proc_name_); }; diff --git a/src/lib/dhcpsrv/daemon.h b/src/lib/dhcpsrv/daemon.h index 77035dbb69..94a6e38ab9 100644 --- a/src/lib/dhcpsrv/daemon.h +++ b/src/lib/dhcpsrv/daemon.h @@ -46,16 +46,6 @@ public: /// Dhcpv6Srv) in tests, without going through the hassles of implemeting stub /// methods. /// -/// This class comprises a static object holding a location of the configuration -/// file. The object must be static because it is instantiated by the signal -/// handler functions, which are static by their nature. The signal handlers -/// are used to reconfigure a running server and they need access to the -/// configuration file location. They get this access by calling -/// @c Daemon::getConfigFile function. -/// -/// By default, the configuration file location is empty and its actual value -/// is assigned to the static object in @c Daemon::init function. -/// /// Classes derived from @c Daemon may install custom signal handlers using /// @c isc::util::SignalSet class. This base class provides a declaration /// of the @c SignalSet object that should be initialized in the derived @@ -81,6 +71,10 @@ public: /// @brief Initializes the server. /// + /// @todo #3753 - This method should be revisited as its original purpose + /// has been lost. As of #3769, it has been superseded with setConfigFile(). + /// None of the following is currently accurate. + /// /// Depending on the configuration backend, it establishes msgq session, /// or reads the configuration file. /// @@ -111,11 +105,6 @@ public: /// @brief Initiates shutdown procedure for the whole DHCPv6 server. virtual void shutdown(); - /// @brief Returns config file name. - static std::string getConfigFile() { - return (config_file_); - } - /// @brief Initializes logger /// /// This method initializes logging system. It also sets the default @@ -169,19 +158,23 @@ public: /// @return text string static std::string getVersion(bool extended); + /// @brief Returns config file name. + /// @return text string + std::string getConfigFile() const; + /// @brief Sets the configuration file name /// /// @param config_file pathname of the configuration file - static void setConfigFile(const std::string& config_file); + void setConfigFile(const std::string& config_file); /// @brief returns the process name /// This value is used as when forming the default PID file name /// @return text string - static std::string getProcName(); + std::string getProcName() const; /// @brief Sets the process name /// @param proc_name name the process by which the process is recognized - static void setProcName(const std::string& proc_name); + void setProcName(const std::string& proc_name); /// @brief Returns the directory used when forming default PID file name /// @return text string @@ -252,10 +245,10 @@ protected: private: /// @brief Config file name or empty if config file not used. - static std::string config_file_; + std::string config_file_; /// @brief Name of this process, used when creating its pid file - static std::string proc_name_; + std::string proc_name_; /// @brief Pointer to the directory where PID file(s) are written /// It defaults to --localstatedir diff --git a/src/lib/dhcpsrv/tests/daemon_unittest.cc b/src/lib/dhcpsrv/tests/daemon_unittest.cc index b6618e539b..afc791fe70 100644 --- a/src/lib/dhcpsrv/tests/daemon_unittest.cc +++ b/src/lib/dhcpsrv/tests/daemon_unittest.cc @@ -65,9 +65,6 @@ public: /// the default after each test completes. ~DaemonTest() { isc::log::setDefaultLoggingOutput(); - // Since they're static we need to clear them between tests - Daemon::setConfigFile(""); - Daemon::setProcName(""); } }; From ac9aec7eb17f6949f8ca33411053581dd778cf51 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Mon, 6 Jul 2015 10:30:54 -0400 Subject: [PATCH 6/8] [3769] Updated Kea Admin guide with PID file info Added discussion of PID files to start up section for each of the servers. --- doc/guide/ddns.xml | 29 +++++++++++++++++++++++++++++ doc/guide/dhcp4-srv.xml | 27 +++++++++++++++++++++++++++ doc/guide/dhcp6-srv.xml | 26 ++++++++++++++++++++++++++ 3 files changed, 82 insertions(+) diff --git a/doc/guide/ddns.xml b/doc/guide/ddns.xml index 9b0159b98a..95d1582026 100644 --- a/doc/guide/ddns.xml +++ b/doc/guide/ddns.xml @@ -128,6 +128,35 @@ strings path/kea-dhcp-ddns | sed -n 's/;;;; //p' Upon start up the module will load its configuration and begin listening for NCRs based on that configuration. + + + During startup the server will attempt to create a PID file of the + form: [localstatedir]/[conf name].kea-dhcp-ddns.pid + where: + + + localstatedir: The value as passed into the + build configure script. It defaults defaults to "/usr/local/var". Note + that this value may be overridden at run time by setting the environment + variable KEA_PIDFILE_DIR. This is intended primarily for testing purposes. + + + + conf name: The confguration file name + used to start the server, minus all preceding path and file extension. + For example, given a pathname of "/usr/local/etc/kea/myconf.txt", the + portion used would be "myconf". + + + + If the file already exists and contains the PID of a live process, + the server will issue a DHCP_DDNS_ALREADY_RUNNING log message and exit. It + is possible, though unlikely, that the file is a remnant of a system crash + and the process to which the PID belongs is unrelated to Kea. In such a + case it would be necessary to manually delete the PID file. + + +
Configuring the DHCP-DDNS Server diff --git a/doc/guide/dhcp4-srv.xml b/doc/guide/dhcp4-srv.xml index ab353edd63..583faf653a 100644 --- a/doc/guide/dhcp4-srv.xml +++ b/doc/guide/dhcp4-srv.xml @@ -106,6 +106,33 @@ strings path/kea-dhcp4 | sed -n 's/;;;; //p' access. Make sure you run this daemon as root. + + During startup the server will attempt to create a PID file of the + form: [localstatedir]/[conf name].kea-dhcp4.pid + where: + + + localstatedir: The value as passed into the + build configure script. It defaults defaults to "/usr/local/var". Note + that this value may be overridden at run time by setting the environment + variable KEA_PIDFILE_DIR. This is intended primarily for testing purposes. + + + + conf name: The confguration file name + used to start the server, minus all preceding path and file extension. + For example, given a pathname of "/usr/local/etc/kea/myconf.txt", the + portion used would be "myconf". + + + + If the file already exists and contains the PID of a live process, + the server will issue a DHCP4_ALREADY_RUNNING log message and exit. It + is possible, though unlikely, that the file is a remnant of a system crash + and the process to which the PID belongs is unrelated to Kea. In such a + case it would be necessary to manually delete the PID file. + +
diff --git a/doc/guide/dhcp6-srv.xml b/doc/guide/dhcp6-srv.xml index 89aa135336..a35ab85b2d 100644 --- a/doc/guide/dhcp6-srv.xml +++ b/doc/guide/dhcp6-srv.xml @@ -104,6 +104,32 @@ strings path/kea-dhcp6 | sed -n 's/;;;; //p' access. Make sure you run this daemon as root. + + During startup the server will attempt to create a PID file of the + form: [localstatedir]/[conf name].kea-dhcp6.pid + where: + + + localstatedir: The value as passed into the + build configure script. It defaults defaults to "/usr/local/var". Note + that this value may be overridden at run time by setting the environment + variable KEA_PIDFILE_DIR. This is intended primarily for testing purposes. + + + + conf name: The confguration file name + used to start the server, minus all preceding path and file extension. + For example, given a pathname of "/usr/local/etc/kea/myconf.txt", the + portion used would be "myconf". + + + + If the file already exists and contains the PID of a live process, + the server will issue a DHCP6_ALREADY_RUNNING log message and exit. It + is possible, though unlikely, that the file is a remnant of a system crash + and the process to which the PID belongs is unrelated to Kea. In such a + case it would be necessary to manually delete the PID file. +
From a6d898ec4bb9418c7a675f358c05d4801b6b1cdf Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Wed, 8 Jul 2015 09:01:53 -0400 Subject: [PATCH 7/8] [3769] Addressed review comments Removed Daemon::init() method, improved server log message descpriptions, and minor typos. --- src/bin/d2/d2_messages.mes | 6 ++++-- src/bin/dhcp4/dhcp4_messages.mes | 3 ++- src/bin/dhcp6/dhcp6_messages.mes | 3 ++- src/lib/dhcpsrv/daemon.cc | 4 ---- src/lib/dhcpsrv/daemon.h | 23 ----------------------- src/lib/dhcpsrv/tests/daemon_unittest.cc | 6 +++--- src/lib/testutils/dhcp_test_lib.sh.in | 2 +- src/lib/util/pid_file.cc | 2 +- 8 files changed, 13 insertions(+), 36 deletions(-) diff --git a/src/bin/d2/d2_messages.mes b/src/bin/d2/d2_messages.mes index 383c64f13c..ee7e995f97 100644 --- a/src/bin/d2/d2_messages.mes +++ b/src/bin/d2/d2_messages.mes @@ -107,7 +107,8 @@ indicates an attempt to start a second instance of DHCP_DDNS using the same configuration file. It is possible, though unlikely, that the PID file is a remnant left behind by a server crash or power failure and the PID it contains refers to a process other than DHCP_DDNS. In such an event, -it would be necessary to manually remove the PID file. +it would be necessary to manually remove the PID file. The first argument is +the DHCP_DDNS process name, the second contains the PID and and PID file. % DHCP_DDNS_AT_MAX_TRANSACTIONS application has %1 queued requests but has reached maximum number of %2 concurrent transactions This is a debug message that indicates that the application has DHCP_DDNS @@ -292,7 +293,8 @@ its PID file. The log message should contain details sufficient to determine the underlying cause. The most likely culprits are that some portion of the pathname does not exist or a permissions issue. The default path is determined by --localstatedir configure paramter but -may be overridden by setting environment variable, KEA_PIDFILE_DIR. +may be overridden by setting environment variable, KEA_PIDFILE_DIR. The +first argument is the DHCP_DDNS process name. % DHCP_DDNS_PROCESS_INIT application init invoked This is a debug message issued when the DHCP-DDNS application enters diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes index 9c358abbf4..fb96e90882 100644 --- a/src/bin/dhcp4/dhcp4_messages.mes +++ b/src/bin/dhcp4/dhcp4_messages.mes @@ -27,7 +27,8 @@ the server using the same configuration file. It is possible, though unlikely that the PID file is a remnant left behind by a server crash or power failure and the PID it contains refers to a process other than the server. In such an event, it would be necessary to manually remove -the PID file. +the PID file. The first argument is the DHCPv4 process name, the +second contains the PID and and PID file. % DHCP4_BUFFER_RECEIVED received buffer from %1:%2 to %3:%4 over interface %5 This debug message is logged when the server has received a packet diff --git a/src/bin/dhcp6/dhcp6_messages.mes b/src/bin/dhcp6/dhcp6_messages.mes index b82e6475e9..9d0b4fb3d4 100644 --- a/src/bin/dhcp6/dhcp6_messages.mes +++ b/src/bin/dhcp6/dhcp6_messages.mes @@ -27,7 +27,8 @@ the server using the same configuration file. It is possible, though unlikely that the PID file is a remnant left behind by a server crash or power failure and the PID it contains refers to a process other than the server. In such an event, it would be necessary to manually remove -the PID file. +the PID file. The first argument is the DHCPv6 process name, the second +contains the PID and and PID file. % DHCP6_ADD_GLOBAL_STATUS_CODE %1: adding Status Code to DHCPv6 packet: %2 This message is logged when the server is adding the top-level diff --git a/src/lib/dhcpsrv/daemon.cc b/src/lib/dhcpsrv/daemon.cc index a020b0abb2..4ff010a93d 100644 --- a/src/lib/dhcpsrv/daemon.cc +++ b/src/lib/dhcpsrv/daemon.cc @@ -52,10 +52,6 @@ Daemon::~Daemon() { } } -void Daemon::init(const std::string& config_file) { - config_file_ = config_file; -} - void Daemon::cleanup() { } diff --git a/src/lib/dhcpsrv/daemon.h b/src/lib/dhcpsrv/daemon.h index 94a6e38ab9..0147401aa6 100644 --- a/src/lib/dhcpsrv/daemon.h +++ b/src/lib/dhcpsrv/daemon.h @@ -69,29 +69,6 @@ public: /// virtual destructor as well. virtual ~Daemon(); - /// @brief Initializes the server. - /// - /// @todo #3753 - This method should be revisited as its original purpose - /// has been lost. As of #3769, it has been superseded with setConfigFile(). - /// None of the following is currently accurate. - /// - /// Depending on the configuration backend, it establishes msgq session, - /// or reads the configuration file. - /// - /// Note: This function may throw to report enountered problems. It may - /// also return false if the initialization was skipped. That may seem - /// redundant, but the idea here is that in some cases the configuration - /// was read, understood and the decision was made to not start. One - /// case where such capability could be needed is when we have a single - /// config file for Kea4 and D2, but the DNS Update is disabled. It is - /// likely that the D2 will be started, it will analyze its config file, - /// decide that it is not needed and will shut down. - /// - /// @note this method may throw - /// - /// @param config_file Config file name (may be empty if unused). - virtual void init(const std::string& config_file); - /// @brief Performs final deconfiguration. /// /// Performs configuration backend specific final clean-up. This is called diff --git a/src/lib/dhcpsrv/tests/daemon_unittest.cc b/src/lib/dhcpsrv/tests/daemon_unittest.cc index afc791fe70..e47b332cbc 100644 --- a/src/lib/dhcpsrv/tests/daemon_unittest.cc +++ b/src/lib/dhcpsrv/tests/daemon_unittest.cc @@ -78,10 +78,10 @@ TEST_F(DaemonTest, constructor) { Daemon instance2; EXPECT_FALSE(instance2.getVerbose()); - EXPECT_EQ("",instance2.getConfigFile()); - EXPECT_EQ("",instance2.getProcName()); + EXPECT_TRUE(instance2.getConfigFile().empty()); + EXPECT_TRUE(instance2.getProcName().empty()); EXPECT_EQ(CfgMgr::instance().getDataDir(),instance2.getPIDFileDir()); - EXPECT_EQ("",instance2.getPIDFileName()); + EXPECT_TRUE(instance2.getPIDFileName().empty()); } // Verify config file accessors diff --git a/src/lib/testutils/dhcp_test_lib.sh.in b/src/lib/testutils/dhcp_test_lib.sh.in index a07cc47339..dd542324ee 100644 --- a/src/lib/testutils/dhcp_test_lib.sh.in +++ b/src/lib/testutils/dhcp_test_lib.sh.in @@ -447,7 +447,7 @@ verify_server_pid() { clean_exit 1 fi - # Only the file name portio of the config file is used, try and + # Only the file name portion of the config file is used, try and # extract it. NOTE if this "algorithm" changes this code will need # to be updated. fname=`basename ${cfg_file}` diff --git a/src/lib/util/pid_file.cc b/src/lib/util/pid_file.cc index 9ebd1be73d..42bde7e3cc 100644 --- a/src/lib/util/pid_file.cc +++ b/src/lib/util/pid_file.cc @@ -51,7 +51,7 @@ PIDFile::check() const { << filename_ << "'"); } - // If the process is still running return true + // If the process is still running return its pid. if (kill(pid, 0) == 0) { return (pid); } From 3437d50c61f7452061bccd867436b4a617a45cd4 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Wed, 8 Jul 2015 15:19:15 +0200 Subject: [PATCH 8/8] [3769] Fixed typos in the message files. --- src/bin/d2/d2_messages.mes | 2 +- src/bin/dhcp4/dhcp4_messages.mes | 2 +- src/bin/dhcp6/dhcp6_messages.mes | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/bin/d2/d2_messages.mes b/src/bin/d2/d2_messages.mes index ee7e995f97..6b22b293a3 100644 --- a/src/bin/d2/d2_messages.mes +++ b/src/bin/d2/d2_messages.mes @@ -108,7 +108,7 @@ same configuration file. It is possible, though unlikely, that the PID file is a remnant left behind by a server crash or power failure and the PID it contains refers to a process other than DHCP_DDNS. In such an event, it would be necessary to manually remove the PID file. The first argument is -the DHCP_DDNS process name, the second contains the PID and and PID file. +the DHCP_DDNS process name, the second contains the PID and PID file. % DHCP_DDNS_AT_MAX_TRANSACTIONS application has %1 queued requests but has reached maximum number of %2 concurrent transactions This is a debug message that indicates that the application has DHCP_DDNS diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes index fb96e90882..f8e471bbdb 100644 --- a/src/bin/dhcp4/dhcp4_messages.mes +++ b/src/bin/dhcp4/dhcp4_messages.mes @@ -28,7 +28,7 @@ unlikely that the PID file is a remnant left behind by a server crash or power failure and the PID it contains refers to a process other than the server. In such an event, it would be necessary to manually remove the PID file. The first argument is the DHCPv4 process name, the -second contains the PID and and PID file. +second contains the PID and PID file. % DHCP4_BUFFER_RECEIVED received buffer from %1:%2 to %3:%4 over interface %5 This debug message is logged when the server has received a packet diff --git a/src/bin/dhcp6/dhcp6_messages.mes b/src/bin/dhcp6/dhcp6_messages.mes index 9d0b4fb3d4..d0d7aa15ec 100644 --- a/src/bin/dhcp6/dhcp6_messages.mes +++ b/src/bin/dhcp6/dhcp6_messages.mes @@ -28,7 +28,7 @@ unlikely that the PID file is a remnant left behind by a server crash or power failure and the PID it contains refers to a process other than the server. In such an event, it would be necessary to manually remove the PID file. The first argument is the DHCPv6 process name, the second -contains the PID and and PID file. +contains the PID and PID file. % DHCP6_ADD_GLOBAL_STATUS_CODE %1: adding Status Code to DHCPv6 packet: %2 This message is logged when the server is adding the top-level