[1976] Configuration rollbacks

src/bin/auth/datasrc_configurator.h

@@ -117,32 +117,63 @@ public:
             isc_throw(isc::InvalidOperation,
                       "Can't reconfigure while not inited");
         }
-        // Get the configuration and current state.
         typedef std::map<std::string, isc::data::ConstElementPtr> Map;
-        const Map& map(config->mapValue());
-        const std::vector<isc::dns::RRClass>
-            activeVector(server_->getClientListClasses());
-        std::set<isc::dns::RRClass> active(activeVector.begin(),
-                                           activeVector.end());
-        // Go through the configuration and change everything.
-        for (Map::const_iterator it(map.begin()); it != map.end(); ++it) {
-            isc::dns::RRClass rrclass(it->first);
-            active.erase(rrclass);
-            ListPtr list(server_->getClientList(rrclass));
-            bool need_set(false);
-            if (!list) {
-                list.reset(new List);
-                need_set = true;
+        typedef std::pair<isc::dns::RRClass, ListPtr> RollbackPair;
+        typedef std::pair<isc::dns::RRClass, isc::data::ConstElementPtr>
+            RollbackConfiguration;
+        // Some structures to be able to perform a rollback
+        std::vector<RollbackPair> rollback_sets;
+        std::vector<RollbackConfiguration> rollback_configurations;
+        try {
+            // Get the configuration and current state.
+            const Map& map(config->mapValue());
+            const std::vector<isc::dns::RRClass>
+                activeVector(server_->getClientListClasses());
+            std::set<isc::dns::RRClass> active(activeVector.begin(),
+                                               activeVector.end());
+            // Go through the configuration and change everything.
+            for (Map::const_iterator it(map.begin()); it != map.end(); ++it) {
+                isc::dns::RRClass rrclass(it->first);
+                active.erase(rrclass);
+                ListPtr list(server_->getClientList(rrclass));
+                bool need_set(false);
+                if (list) {
+                    rollback_configurations.
+                        push_back(RollbackConfiguration(rrclass,
+                            list->getConfiguration()));
+                } else {
+                    list.reset(new List);
+                    need_set = true;
+                    rollback_sets.push_back(RollbackPair(rrclass, ListPtr()));
+                }
+                list->configure(it->second, true);
+                if (need_set) {
+                    server_->setClientList(rrclass, list);
+                }
             }
-            list->configure(*it->second, true);
-            if (need_set) {
-                server_->setClientList(rrclass, list);
+            // Remove the ones that are not in the configuration.
+            for (std::set<isc::dns::RRClass>::iterator it(active.begin());
+                 it != active.end(); ++it) {
+                // There seems to be no way the setClientList could throw.
+                // But this is just to make sure in case it did to restore
+                // the original.
+                rollback_sets.push_back(
+                    RollbackPair(*it, server_->getClientList(*it)));
+                server_->setClientList(*it, ListPtr());
             }
-        }
-        // Remove the ones that are not in the configuration.
-        for (std::set<isc::dns::RRClass>::iterator it(active.begin());
-             it != active.end(); ++it) {
-            server_->setClientList(*it, ListPtr());
+        } catch (...) {
+            // Perform a rollback of the changes. The old configuration should
+            // work.
+            for (typename std::vector<RollbackPair>::const_iterator
+                 it(rollback_sets.begin()); it != rollback_sets.end(); ++it) {
+                server_->setClientList(it->first, it->second);
+            }
+            for (typename std::vector<RollbackConfiguration>::const_iterator
+                 it(rollback_configurations.begin());
+                 it != rollback_configurations.end(); ++it) {
+                server_->getClientList(it->first)->configure(it->second, true);
+            }
+            throw;
         }
     }
 };

src/bin/auth/tests/datasrc_configurator_unittest.cc

@@ -35,15 +35,23 @@ class DatasrcConfiguratorTest;
 
 class FakeList {
 public:
-    void configure(const Element& configuration, bool allow_cache) {
+    FakeList() :
+        configuration_(new ListElement)
+    {}
+    void configure(const ConstElementPtr& configuration, bool allow_cache) {
         EXPECT_TRUE(allow_cache);
-        conf_ = configuration.get(0)->get("type")->stringValue();
+        conf_ = configuration->get(0)->get("type")->stringValue();
+        configuration_ = configuration;
     }
     const string& getConf() const {
         return (conf_);
     }
+    ConstElementPtr getConfiguration() const {
+        return (configuration_);
+    }
 private:
     string conf_;
+    ConstElementPtr configuration_;
 };
 
 typedef shared_ptr<FakeList> ListPtr;
@@ -213,9 +221,66 @@ TEST_F(DatasrcConfiguratorTest, updateDelete) {
                        "*");
     log_ = "";
     mccs->checkCommand();
-    EXPECT_EQ("set IN \n", log_);
+    EXPECT_EQ("get IN\nset IN \n", log_);
     EXPECT_FALSE(lists_[RRClass::IN()]);
 }
 
+// Check that we can rollback an addition if something else fails
+TEST_F(DatasrcConfiguratorTest, rollbackAddition) {
+    doInInit();
+    // The configuration is wrong. However, the CH one will get done first.
+    const ElementPtr
+        config(Element::fromJSON("{\"IN\": [{\"type\": 13}], "
+                                 "\"CH\": [{\"type\": \"xxx\"}]}"));
+    session.addMessage(createCommand("config_update", config), "data_sources",
+                       "*");
+    log_ = "";
+    // It does not throw, as it is handled in the ModuleCCSession.
+    // Throwing from the reconfigure is checked in other tests.
+    EXPECT_NO_THROW(mccs->checkCommand());
+    // Anyway, the result should not contain CH now and the original IN should
+    // be there.
+    EXPECT_EQ("xxx", lists_[RRClass::IN()]->getConf());
+    EXPECT_FALSE(lists_[RRClass::CH()]);
+}
+
+// Check that we can rollback a deletion if something else fails
+TEST_F(DatasrcConfiguratorTest, rollbackDeletion) {
+    doInInit();
+    // Put the CH there
+    const ElementPtr
+        config1(Element::fromJSON("{\"IN\": [{\"type\": \"yyy\"}], "
+                                  "\"CH\": [{\"type\": \"xxx\"}]}"));
+    Configurator::reconfigure(config1);
+    const ElementPtr
+        config2(Element::fromJSON("{\"IN\": [{\"type\": 13}]}"));
+    // This would delete CH. However, the IN one fails.
+    // As the deletions happen after the additions/settings
+    // and there's no known way to cause an exception during the
+    // deletions, it is not a true rollback, but the result should
+    // be the same.
+    EXPECT_THROW(Configurator::reconfigure(config2), TypeError);
+    EXPECT_EQ("yyy", lists_[RRClass::IN()]->getConf());
+    EXPECT_EQ("xxx", lists_[RRClass::CH()]->getConf());
+}
+
+// Check that we can roll back configuration change if something
+// fails later on.
+TEST_F(DatasrcConfiguratorTest, rollbackConfiguration) {
+    doInInit();
+    // Put the CH there
+    const ElementPtr
+        config1(Element::fromJSON("{\"IN\": [{\"type\": \"yyy\"}], "
+                                  "\"CH\": [{\"type\": \"xxx\"}]}"));
+    Configurator::reconfigure(config1);
+    // Now, the CH happens first. But nevertheless, it should be
+    // restored to the previoeus version.
+    const ElementPtr
+        config2(Element::fromJSON("{\"IN\": [{\"type\": 13}], "
+                                  "\"CH\": [{\"type\": \"yyy\"}]}"));
+    EXPECT_THROW(Configurator::reconfigure(config2), TypeError);
+    EXPECT_EQ("yyy", lists_[RRClass::IN()]->getConf());
+    EXPECT_EQ("xxx", lists_[RRClass::CH()]->getConf());
+}
 
 }