[430-configure-location-of-datadir] Reverted to CfgMgr. Made Optional

diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.cc b/src/bin/dhcp4/tests/dhcp4_test_utils.cc
index 0f23420a64d4c6c3f7d9cf84aa7c21d02bafee61..2083ad9270fb48e8d47670ab5f119ad9208152af 100644 (file)
--- a/src/bin/dhcp4/tests/dhcp4_test_utils.cc
+++ b/src/bin/dhcp4/tests/dhcp4_test_utils.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2019 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2018 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -36,19 +36,18 @@ namespace dhcp {
 namespace test {
 
 BaseServerTest::BaseServerTest()
-    : original_datadir_(CfgMgr::instance().getCurrentCfg()->getDataDir()) {
-    CfgMgr::instance().getStagingCfg()->setDataDir(TEST_DATA_BUILDDIR);
+    : original_datadir_(CfgMgr::instance().getDataDir()) {
+    CfgMgr::instance().setDataDir(TEST_DATA_BUILDDIR);
 }
 
 BaseServerTest::~BaseServerTest() {
     // Remove default lease file.
     std::ostringstream s2;
-    s2 << CfgMgr::instance().getStagingCfg()->getDataDir()
-       << "/" << "kea-leases4.csv";
+    s2 << CfgMgr::instance().getDataDir() << "/" << "kea-leases4.csv";
     static_cast<void>(::remove(s2.str().c_str()));
 
     // Revert to original data directory.
-    CfgMgr::instance().getStagingCfg()->setDataDir(original_datadir_);
+    CfgMgr::instance().setDataDir(original_datadir_);
 
     // Revert to unit test logging, in case the test reconfigured it.
     isc::log::initLogger();

diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc
index f3014957ff8cd3556177c65f3e594b83cbd95c2e..93c89a58f44577d8a408cf5239cf6a811496d9bf 100644 (file)
--- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc
+++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc
@@ -618,7 +618,7 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) {
     // Regenerate server identifier if needed.
     try {
         const std::string duid_file =
-            CfgMgr::instance().getStagingCfg()->getDataDir() + "/" +
+            std::string(CfgMgr::instance().getDataDir()) + "/" +
             std::string(SERVER_DUID_FILE);
         DuidPtr duid = CfgMgr::instance().getStagingCfg()->getCfgDUID()->create(duid_file);
         server_->serverid_.reset(new Option(Option::V6, D6O_SERVERID, duid->getDuid()));

diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc
index bb62c2f3050013c6b3ec2158b3ab512acb3dfda3..d714a15212a3054a53ae2c0c7f87901c3b83f028 100644 (file)
--- a/src/bin/dhcp6/json_config_parser.cc
+++ b/src/bin/dhcp6/json_config_parser.cc
@@ -174,7 +174,8 @@ public:
 
         // Set the data directory for server id file.
         if (global->contains("data-directory")) {
-            srv_config->setDataDir(getString(global, "data-directory"));
+          CfgMgr::instance().setDataDir(getString(global, "data-directory"),
+                                        false);
         }
 
         // Set the probation period for decline handling.

diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc
index bc90d4d657dfdade039c0e5b20db7a1d02e2c260..a32a1e97d698ed107f392b694ec1e06860d064bb 100644 (file)
--- a/src/bin/dhcp6/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp6/tests/config_parser_unittest.cc
@@ -5497,6 +5497,8 @@ TEST_F(Dhcp6ParserTest, notDirDataDir) {
 /// Check that a valid data directory is accepted.
 TEST_F(Dhcp6ParserTest, testDataDir) {
 
+    EXPECT_TRUE(CfgMgr::instance().getDataDir().unspecified());
+    string original_datadir(CfgMgr::instance().getDataDir());
     string datadir(TEST_DATA_BUILDDIR);
     string config_txt = "{\n"
         "\"data-directory\": \"" + datadir + "\"\n"
@@ -5512,8 +5514,9 @@ TEST_F(Dhcp6ParserTest, testDataDir) {
     checkResult(status, 0);
 
     // The value of data-directory was updated.
-    EXPECT_EQ(datadir, CfgMgr::instance().getStagingCfg()->getDataDir());
-    EXPECT_NE(datadir, CfgMgr::instance().getCurrentCfg()->getDataDir());
+    EXPECT_FALSE(CfgMgr::instance().getDataDir().unspecified());
+    EXPECT_EQ(datadir, string(CfgMgr::instance().getDataDir()));
+    EXPECT_NE(original_datadir, string(CfgMgr::instance().getDataDir()));
 }
 
 /// Check that the decline-probation-period value has a default value if not

diff --git a/src/bin/dhcp6/tests/dhcp6_test_utils.cc b/src/bin/dhcp6/tests/dhcp6_test_utils.cc
index da0ed3bbd253925072f4d0c0b7be8f80b36adbd4..f25e164586057124be688ccb18ab50ba28307075 100644 (file)
--- a/src/bin/dhcp6/tests/dhcp6_test_utils.cc
+++ b/src/bin/dhcp6/tests/dhcp6_test_utils.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2019 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2018 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -30,25 +30,23 @@ namespace test {
 const char* BaseServerTest::DUID_FILE = "kea-dhcp6-serverid";
 
 BaseServerTest::BaseServerTest()
-    : original_datadir_(CfgMgr::instance().getCurrentCfg()->getDataDir()) {
-    CfgMgr::instance().getStagingCfg()->setDataDir(TEST_DATA_BUILDDIR);
+    : original_datadir_(CfgMgr::instance().getDataDir()) {
+    CfgMgr::instance().setDataDir(TEST_DATA_BUILDDIR);
 }
 
 BaseServerTest::~BaseServerTest() {
     // Remove test DUID file.
     std::ostringstream s;
-    s << CfgMgr::instance().getStagingCfg()->getDataDir()
-      << "/" << DUID_FILE;
+    s << CfgMgr::instance().getDataDir() << "/" << DUID_FILE;
     static_cast<void>(::remove(s.str().c_str()));
 
     // Remove default lease file.
     std::ostringstream s2;
-    s2 << CfgMgr::instance().getStagingCfg()->getDataDir()
-       << "/" << "kea-leases6.csv";
+    s2 << CfgMgr::instance().getDataDir() << "/" << "kea-leases6.csv";
     static_cast<void>(::remove(s2.str().c_str()));
 
     // Revert to original data directory.
-    CfgMgr::instance().getStagingCfg()->setDataDir(original_datadir_);
+    CfgMgr::instance().setDataDir(original_datadir_);
 
     // Revert to unit test logging in case the test reconfigured logging.
     isc::log::initLogger();

diff --git a/src/lib/dhcpsrv/cfgmgr.cc b/src/lib/dhcpsrv/cfgmgr.cc
index eb93c5985ece87478da3029f0673247b18bca8ce..a1cc231a4defa3f6f3c766390564253e1eb6143d 100644 (file)
--- a/src/lib/dhcpsrv/cfgmgr.cc
+++ b/src/lib/dhcpsrv/cfgmgr.cc
@@ -27,6 +27,16 @@ CfgMgr::instance() {
     return (cfg_mgr);
 }
 
+Optional<std::string>
+CfgMgr::getDataDir() const {
+    return (datadir_);
+}
+
+void
+CfgMgr::setDataDir(const std::string& datadir, bool unspecified) {
+    datadir_ = Optional<std::string>(datadir, unspecified);
+}
+
 void
 CfgMgr::setD2ClientConfig(D2ClientConfigPtr& new_config) {
     ensureCurrentAllocated();
@@ -194,7 +204,11 @@ CfgMgr::mergeIntoCfg(const SrvConfigPtr& target_config, const uint32_t seq) {
     }
 }
 
-CfgMgr::CfgMgr() : d2_client_mgr_(), family_(AF_INET) {
+CfgMgr::CfgMgr()
+    : datadir_(DHCP_DATA_DIR, true), d2_client_mgr_(), family_(AF_INET) {
+    // DHCP_DATA_DIR must be set set with -DDHCP_DATA_DIR="..." in Makefile.am
+    // Note: the definition of DHCP_DATA_DIR needs to include quotation marks
+    // See AM_CPPFLAGS definition in Makefile.am
 }
 
 CfgMgr::~CfgMgr() {

diff --git a/src/lib/dhcpsrv/cfgmgr.h b/src/lib/dhcpsrv/cfgmgr.h
index 8960bbd6c250f6beb76c384ef10391b9fbd59811..0302cf54e19892b85fa2b53ce986484bb2b2e4f3 100644 (file)
--- a/src/lib/dhcpsrv/cfgmgr.h
+++ b/src/lib/dhcpsrv/cfgmgr.h
@@ -15,6 +15,7 @@
 #include <dhcpsrv/pool.h>
 #include <dhcpsrv/srv_config.h>
 #include <util/buffer.h>
+#include <util/optional.h>
 
 #include <boost/shared_ptr.hpp>
 #include <boost/noncopyable.hpp>
@@ -80,6 +81,19 @@ public:
     /// accessing it.
     static CfgMgr& instance();
 
+    /// @brief returns path do the data directory
+    ///
+    /// This method returns a path to writable directory that DHCP servers
+    /// can store data in.
+    /// @return data directory
+    util::Optional<std::string> getDataDir() const;
+
+    /// @brief Sets new data directory.
+    ///
+    /// @param datadir New data directory.
+    /// @param unspecified Initial state. Default is "unspecified".
+    void setDataDir(const std::string& datadir, bool unspecified = true);
+
     /// @brief Updates the DHCP-DDNS client configuration to the given value.
     ///
     /// Passes the new configuration to the D2ClientMgr instance,
@@ -300,6 +314,9 @@ private:
     /// @param seq Source configuration sequence number.
     void mergeIntoCfg(const SrvConfigPtr& taget_config, const uint32_t seq);
 
+    /// @brief directory where data files (e.g. server-id) are stored
+    util::Optional<std::string> datadir_;
+
     /// @brief Manages the DHCP-DDNS client and its configuration.
     D2ClientMgr d2_client_mgr_;
 

diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.cc b/src/lib/dhcpsrv/memfile_lease_mgr.cc
index 0eafe27254491ebf4b154bbf5525c899b603ae14..d060e220574083994f3563c49d962eaf9c020468 100644 (file)
--- a/src/lib/dhcpsrv/memfile_lease_mgr.cc
+++ b/src/lib/dhcpsrv/memfile_lease_mgr.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2019 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2018 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -1347,10 +1347,7 @@ Memfile_LeaseMgr::appendSuffix(const std::string& file_name,
 std::string
 Memfile_LeaseMgr::getDefaultLeaseFilePath(Universe u) const {
     std::ostringstream s;
-    // Use the staging configuration because either it is called
-    // during (re)configuration or with a fresh configuration.
-    // In both cases the right value is at least at this place.
-    s << CfgMgr::instance().getStagingCfg()->getDataDir() << "/kea-leases";
+    s << CfgMgr::instance().getDataDir() << "/kea-leases";
     s << (u == V4 ? "4" : "6");
     s << ".csv";
     return (s.str());

diff --git a/src/lib/dhcpsrv/srv_config.cc b/src/lib/dhcpsrv/srv_config.cc
index d011cc16900ab63916b67d7fc201ac17f038cdef..34274d1c7a36bc257fdd4b73d7f41df562fab9b1 100644 (file)
--- a/src/lib/dhcpsrv/srv_config.cc
+++ b/src/lib/dhcpsrv/srv_config.cc
@@ -25,14 +25,8 @@ using namespace isc::process;
 namespace isc {
 namespace dhcp {
 
-// DHCP_DATA_DIR must be set set with -DDHCP_DATA_DIR="..." in Makefile.am
-// Note: the definition of DHCP_DATA_DIR needs to include quotation marks
-// See AM_CPPFLAGS definition in Makefile.am
-const std::string
-SrvConfig::DEFAULT_DATA_DIR = DHCP_DATA_DIR;
-
 SrvConfig::SrvConfig()
-    : sequence_(0), datadir_(DEFAULT_DATA_DIR), cfg_iface_(new CfgIface()),
+    : sequence_(0), cfg_iface_(new CfgIface()),
       cfg_option_def_(new CfgOptionDef()), cfg_option_(new CfgOption()),
       cfg_subnets4_(new CfgSubnets4()), cfg_subnets6_(new CfgSubnets6()),
       cfg_shared_networks4_(new CfgSharedNetworks4()),
@@ -51,8 +45,7 @@ SrvConfig::SrvConfig()
 }
 
 SrvConfig::SrvConfig(const uint32_t sequence)
-    : sequence_(sequence),
-      datadir_(DEFAULT_DATA_DIR),cfg_iface_(new CfgIface()),
+    : sequence_(sequence), cfg_iface_(new CfgIface()),
       cfg_option_def_(new CfgOptionDef()), cfg_option_(new CfgOption()),
       cfg_subnets4_(new CfgSubnets4()), cfg_subnets6_(new CfgSubnets6()),
       cfg_shared_networks4_(new CfgSharedNetworks4()),
@@ -120,8 +113,6 @@ void
 SrvConfig::copy(SrvConfig& new_config) const {
     ConfigBase::copy(new_config);
 
-    // Replace data directory.
-    new_config.datadir_ = datadir_;
     // Replace interface configuration.
     new_config.cfg_iface_.reset(new CfgIface(*cfg_iface_));
     // Replace option definitions.
@@ -150,8 +141,7 @@ SrvConfig::equals(const SrvConfig& other) const {
     }
 
     // Common information is equal between objects, so check other values.
-    if ((datadir_ != other.datadir_) ||
-        (*cfg_iface_ != *other.cfg_iface_) ||
+    if ((*cfg_iface_ != *other.cfg_iface_) ||
         (*cfg_option_def_ != *other.cfg_option_def_) ||
         (*cfg_option_ != *other.cfg_option_) ||
         (*class_dictionary_ != *other.class_dictionary_) ||
@@ -214,15 +204,13 @@ SrvConfig::mergeGlobals4(SrvConfig& other) {
         addConfiguredGlobal(other_global.first, other_global.second);
     }
 
-    // A handful of values are stored as members in SrvConfig. So we'll
+    // A handful of values are stored as members in SrvConfig. So we'll 
     // iterate over the merged globals, setting approprate members.
     for (auto merged_global : getConfiguredGlobals()->mapValue()) {
         std::string name = merged_global.first;
         ConstElementPtr element = merged_global.second;
         try {
-            if (name == "data-directory") {
-                setDataDir(element->stringValue());
-            } if (name == "decline-probation-period") {
+            if (name == "decline-probation-period") {
                 setDeclinePeriod(element->intValue());
             }
             else if (name == "echo-client-id") {
@@ -296,9 +284,13 @@ SrvConfig::toElement() const {
     // Set user-context
     contextToElement(dhcp);
 
-    // Set data directory if DHCPv6 and not default.
-    if ((family == AF_INET6) && (getDataDir() != DEFAULT_DATA_DIR)) {
-        dhcp->set("data-directory", Element::create(getDataDir()));
+    // Set data directory if DHCPv6 and specified.
+    if (family == AF_INET6) {
+        const util::Optional<std::string>& datadir =
+            CfgMgr::instance().getDataDir();
+        if (!datadir.unspecified()) {
+            dhcp->set("data-directory", Element::create(datadir));
+        }
     }
 
     // Set decline-probation-period

diff --git a/src/lib/dhcpsrv/srv_config.h b/src/lib/dhcpsrv/srv_config.h
index 18c21d17e45181cc86969ca253a683996cafdf7f..7dcb361c78bb8434ab122989661ee300ecf06c18 100644 (file)
--- a/src/lib/dhcpsrv/srv_config.h
+++ b/src/lib/dhcpsrv/srv_config.h
@@ -119,25 +119,6 @@ public:
     /// @return true if sequence numbers are equal.
     bool sequenceEquals(const SrvConfig& other);
 
-    /// @brief Default data directory.
-    static const std::string DEFAULT_DATA_DIR;
-
-    /// @brief returns path do the data directory.
-    ///
-    /// This method returns a path to writable directory that DHCP servers
-    /// can store data in.
-    /// @return data directory.
-    std::string getDataDir() const {
-        return (datadir_);
-    }
-
-    /// @brief Sets new data directory.
-    ///
-    /// @param datadir New data directory.
-    void setDataDir(const std::string& datadir) {
-        datadir_ = datadir;
-    }
-
     /// @brief Returns non-const pointer to interface configuration.
     ///
     /// This function returns a non-const pointer to the interface
@@ -711,9 +692,6 @@ private:
     /// @brief Sequence number identifying the configuration.
     uint32_t sequence_;
 
-    /// @brief directory where data files (e.g. server-id) are stored.
-    std::string datadir_;
-
     /// @brief Interface configuration.
     ///
     /// Used to select interfaces on which the DHCP server will listen to