[3534] Implemented (in)equality operators for CfgIface.

src/lib/dhcpsrv/cfg_iface.cc

@@ -34,6 +34,13 @@ CfgIface::closeSockets() const {
     IfaceMgr::instance().closeSockets();
 }
 
+bool
+CfgIface::equals(const CfgIface& other) const {
+    return (iface_set_ == other.iface_set_ &&
+            unicast_map_ == other.unicast_map_ &&
+            wildcard_used_ == other.wildcard_used_);
+}
+
 void
 CfgIface::openSockets(const Family& family, const uint16_t port,
                       const bool use_bcast) const {

src/lib/dhcpsrv/cfg_iface.h

@@ -86,6 +86,13 @@ public:
     /// @brief Convenience function which closes all open sockets.
     void closeSockets() const;
 
+    /// @brief Compares two @c CfgIface objects for equality.
+    ///
+    /// @param other An object to be compared with this object.
+    ///
+    /// @return true if objects are equal, false otherwise.
+    bool equals(const CfgIface& other) const;
+
     /// @brief Tries to open sockets on selected interfaces.
     ///
     /// This function opens sockets bound to link-local address as well as
@@ -139,6 +146,24 @@ public:
     /// @c CfgIface::use has been already called for this interface.
     void use(const Family& family, const std::string& iface_name);
 
+    /// @brief Equality operator.
+    ///
+    /// @param other Object to be compared with this object.
+    ///
+    /// @return true if objects are equal, false otherwise.
+    bool operator==(const CfgIface& other) const {
+        return (equals(other));
+    }
+
+    /// @brief Inequality operator.
+    ///
+    /// @param other Object to be compared with this object.
+    ///
+    /// @return true if objects are not equal, false otherwise.
+    bool operator!=(const CfgIface& other) const {
+        return (!equals(other));
+    }
+
 private:
 
     /// @brief Selects or deselects interfaces.
@@ -165,9 +190,6 @@ private:
     /// @param errmsg Error message being logged by the function.
     static void socketOpenErrorHandler(const std::string& errmsg);
 
-    /// @brief Protocol family.
-    Family family_;
-
     /// @brief Represents a set of interface names.
     typedef std::set<std::string> IfaceSet;
 

src/lib/dhcpsrv/cfgmgr.cc

@@ -25,6 +25,8 @@ using namespace isc::util;
 namespace isc {
 namespace dhcp {
 
+const size_t CfgMgr::CONFIG_LIST_SIZE = 10;
+
 CfgMgr&
 CfgMgr::instance() {
     static CfgMgr cfg_mgr;
@@ -376,6 +378,13 @@ CfgMgr::commit() {
     ensureCurrentAllocated();
     if (!configs_.back()->sequenceEquals(*configuration_)) {
         configuration_ = configs_.back();
+        // Keep track of the maximum size of the configs history. Before adding
+        // new element, we have to remove the oldest one.
+        if (configs_.size() > CONFIG_LIST_SIZE) {
+            ConfigurationList::iterator it = configs_.begin();
+            std::advance(it, configs_.size() - CONFIG_LIST_SIZE);
+            configs_.erase(configs_.begin(), it);
+        }
     }
 }
 

src/lib/dhcpsrv/cfgmgr.h

@@ -95,6 +95,11 @@ public:
 class CfgMgr : public boost::noncopyable {
 public:
 
+    /// @brief A number of configurations held by @c CfgMgr.
+    ///
+    /// @todo Make it configurable.
+    static const size_t CONFIG_LIST_SIZE;
+
     /// @brief returns a single instance of Configuration Manager
     ///
     /// CfgMgr is a singleton and this method is the only way of
@@ -385,6 +390,8 @@ public:
     /// @todo Migrate all configuration parameters to use the model supported
     /// by these functions.
     ///
+    /// @todo Make the size of the configurations history configurable.
+    ///
     //@{
 
     /// @brief Removes current, staging and all previous configurations.
@@ -399,7 +406,9 @@ public:
     /// @brief Commits the staging configuration.
     ///
     /// The staging configuration becomes current configuration when this
-    /// function is called.
+    /// function is called. It removes the oldest configurations held in the
+    /// history so as the size of the list of configuration does not excide
+    /// the @c CONFIG_LIST_SIZE.
     ///
     /// This function is exception safe.
     void commit();

src/lib/dhcpsrv/tests/cfg_iface_unittest.cc

@@ -218,9 +218,60 @@ TEST_F(CfgIfaceTest, invalidValues) {
     ASSERT_THROW(cfg.use(CfgIface::V6, "eth0/fe80::3a60:77ff:fed5:cdef"),
                  InvalidIfaceName);
     ASSERT_THROW(cfg.use(CfgIface::V6, "eth0/2001:db8:1::2"), NoSuchAddress);
-
     ASSERT_NO_THROW(cfg.use(CfgIface::V6, "*"));
     ASSERT_THROW(cfg.use(CfgIface::V6, "*"), DuplicateIfaceName);
 }
 
+// Test that the equality and inequality operators work fine for CfgIface.
+TEST_F(CfgIfaceTest, equality) {
+    CfgIface cfg1;
+    CfgIface cfg2;
+
+    // Initially objects must be equal.
+    EXPECT_TRUE(cfg1 == cfg2);
+    EXPECT_FALSE(cfg1 != cfg2);
+
+    // Differ by one interface.
+    cfg1.use(CfgIface::V4, "eth0");
+    EXPECT_FALSE(cfg1 == cfg2);
+    EXPECT_TRUE(cfg1 != cfg2);
+
+    // Now interfaces should be equal.
+    cfg2.use(CfgIface::V4, "eth0");
+    EXPECT_TRUE(cfg1 == cfg2);
+    EXPECT_FALSE(cfg1 != cfg2);
+
+    // Differ by unicast address.
+    cfg1.use(CfgIface::V6, "eth0/2001:db8:1::1");
+    EXPECT_FALSE(cfg1 == cfg2);
+    EXPECT_TRUE(cfg1 != cfg2);
+
+    // Differ by unicast address and one interface.
+    cfg2.use(CfgIface::V6, "eth1");
+    EXPECT_FALSE(cfg1 == cfg2);
+    EXPECT_TRUE(cfg1 != cfg2);
+
+    // Now, the unicast addresses are equal but still differ by one interface.
+    cfg2.use(CfgIface::V6, "eth0/2001:db8:1::1");
+    EXPECT_FALSE(cfg1 == cfg2);
+    EXPECT_TRUE(cfg1 != cfg2);
+
+    // They should be now back to equal.
+    cfg1.use(CfgIface::V6, "eth1");
+    EXPECT_TRUE(cfg1 == cfg2);
+    EXPECT_FALSE(cfg1 != cfg2);
+
+    // Even though the wildcard doesn't change anything because all interfaces
+    // are already in use, the fact that the wildcard is specified should
+    // cause them to be not equal.
+    cfg1.use(CfgIface::V6, "*");
+    EXPECT_FALSE(cfg1 == cfg2);
+    EXPECT_TRUE(cfg1 != cfg2);
+
+    // Finally, both are equal as they use wildacard.
+    cfg2.use(CfgIface::V4, "*");
+    EXPECT_TRUE(cfg1 == cfg2);
+    EXPECT_FALSE(cfg1 != cfg2);
+}
+
 } // end of anonymous namespace

src/lib/dhcpsrv/tests/cfgmgr_unittest.cc

@@ -1125,7 +1125,7 @@ TEST_F(CfgMgrTest, subnet6Duplication) {
 }
 
 
-// This test verifies that the configuration staging and commit works
+// This test verifies that the configuration staging, commit and rollback works
 // as expected.
 TEST_F(CfgMgrTest, staging) {
     CfgMgr& cfg_mgr = CfgMgr::instance();
@@ -1159,7 +1159,7 @@ TEST_F(CfgMgrTest, staging) {
 
     // This should change the staging configuration so as it becomes a current
     // one.
-    CfgMgr::instance().commit();
+    cfg_mgr.commit();
     const_config = cfg_mgr.getCurrentCfg();
     ASSERT_TRUE(const_config);
     // Sequence id equal to 1 indicates that the current configuration points
@@ -1176,7 +1176,7 @@ TEST_F(CfgMgrTest, staging) {
     // changes the configuration having sequence 2 to current configuration.
     // Other commits are no-op.
     for (int i = 0; i < 5; ++i) {
-        CfgMgr::instance().commit();
+        cfg_mgr.commit();
     }
 
     // The current configuration now have sequence number 2.
@@ -1185,15 +1185,29 @@ TEST_F(CfgMgrTest, staging) {
     EXPECT_EQ(2, const_config->getSequence());
 
     // Clear configuration along with a history.
-    CfgMgr::instance().clear();
+    cfg_mgr.clear();
 
     // After clearing configuration we should successfully get the
     // new staging configuration.
-    const_config = cfg_mgr.getStagingCfg();
-    ASSERT_TRUE(const_config);
-    EXPECT_EQ(1, const_config->getSequence());
-}
+    config = cfg_mgr.getStagingCfg();
+    ASSERT_TRUE(config);
+    EXPECT_EQ(1, config->getSequence());
 
+    // Modify the staging configuration.
+    config->addLoggingInfo(LoggingInfo());
+    ASSERT_TRUE(config);
+    // The modified staging configuration should have one logger configured.
+    ASSERT_EQ(1, config->getLoggingInfo().size());
+
+    // Rollback should remove a staging configuration, including the logger.
+    ASSERT_NO_THROW(cfg_mgr.rollback());
+
+    // Make sure that the logger is not set. This is an indication that the
+    // rollback worked.
+    config = cfg_mgr.getStagingCfg();
+    ASSERT_TRUE(config);
+    EXPECT_EQ(0, config->getLoggingInfo().size());
+}
 
 /// @todo Add unit-tests for testing:
 /// - addActiveIface() with invalid interface name