BIND 10 trac1976, updated. c3badfe37d74047f7b6b1f7f4ec03d30e7164296 [1976] Preserve configuration in the ClientList
BIND 10 source code commits
bind10-changes at lists.isc.org
Thu Jun 21 12:42:30 UTC 2012
The branch, trac1976 has been updated
via c3badfe37d74047f7b6b1f7f4ec03d30e7164296 (commit)
via 327c71c61633f0d60163ac0cf38ce3954547c60f (commit)
from 1124a637dfa3b123519bbd0e5e28d3b370c0bf4e (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit c3badfe37d74047f7b6b1f7f4ec03d30e7164296
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Thu Jun 21 14:40:50 2012 +0200
[1976] Preserve configuration in the ClientList
And change the parameter type in configure, since we want to preserve
the exact instance and we don't want to do a (deep) copy.
commit 327c71c61633f0d60163ac0cf38ce3954547c60f
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Thu Jun 21 14:20:10 2012 +0200
[1976] Configuration rollbacks
-----------------------------------------------------------------------
Summary of changes:
src/bin/auth/datasrc_configurator.h | 77 ++++++++++++++------
.../auth/tests/datasrc_configurator_unittest.cc | 71 +++++++++++++++++-
src/lib/datasrc/client_list.cc | 10 ++-
src/lib/datasrc/client_list.h | 18 ++++-
src/lib/datasrc/tests/client_list_unittest.cc | 26 ++++---
5 files changed, 161 insertions(+), 41 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/auth/datasrc_configurator.h b/src/bin/auth/datasrc_configurator.h
index 681d265..014ce69 100644
--- a/src/bin/auth/datasrc_configurator.h
+++ b/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;
}
}
};
diff --git a/src/bin/auth/tests/datasrc_configurator_unittest.cc b/src/bin/auth/tests/datasrc_configurator_unittest.cc
index 85c413e..9f92638 100644
--- a/src/bin/auth/tests/datasrc_configurator_unittest.cc
+++ b/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());
+}
}
diff --git a/src/lib/datasrc/client_list.cc b/src/lib/datasrc/client_list.cc
index 549b216..ce4f0a4 100644
--- a/src/lib/datasrc/client_list.cc
+++ b/src/lib/datasrc/client_list.cc
@@ -26,15 +26,18 @@ namespace isc {
namespace datasrc {
void
-ConfigurableClientList::configure(const Element& config, bool) {
+ConfigurableClientList::configure(const ConstElementPtr& config, bool) {
+ if (!config) {
+ isc_throw(isc::BadValue, "NULL configuration passed");
+ }
// TODO: Implement the cache
// TODO: Implement recycling from the old configuration.
size_t i(0); // Outside of the try to be able to access it in the catch
try {
vector<DataSourceInfo> new_data_sources;
- for (; i < config.size(); ++i) {
+ for (; i < config->size(); ++i) {
// Extract the parameters
- const ConstElementPtr dconf(config.get(i));
+ const ConstElementPtr dconf(config->get(i));
const ConstElementPtr typeElem(dconf->get("type"));
if (typeElem == ConstElementPtr()) {
isc_throw(ConfigurationError, "Missing the type option in "
@@ -56,6 +59,7 @@ ConfigurableClientList::configure(const Element& config, bool) {
// ready. So just put it there and let the old one die when we exit
// the scope.
data_sources_.swap(new_data_sources);
+ configuration_ = config;
} catch (const TypeError& te) {
isc_throw(ConfigurationError, "Malformed configuration at data source "
"no. " << i << ": " << te.what());
diff --git a/src/lib/datasrc/client_list.h b/src/lib/datasrc/client_list.h
index 599dca8..c705ecf 100644
--- a/src/lib/datasrc/client_list.h
+++ b/src/lib/datasrc/client_list.h
@@ -185,6 +185,9 @@ typedef boost::shared_ptr<const ClientList> ConstClientListPtr;
/// inherited except for tests.
class ConfigurableClientList : public ClientList {
public:
+ ConfigurableClientList() :
+ configuration_(new isc::data::ListElement)
+ {}
/// \brief Exception thrown when there's an error in configuration.
class ConfigurationError : public Exception {
public:
@@ -212,7 +215,17 @@ public:
/// client.
/// \throw ConfigurationError if the configuration is invalid in some
/// sense.
- void configure(const data::Element& configuration, bool allow_cache);
+ /// \throw BadValue if configuration is NULL
+ void configure(const isc::data::ConstElementPtr& configuration,
+ bool allow_cache);
+
+ /// \brief Returns the currently active configuration.
+ ///
+ /// In case configure was not called yet, it returns an empty
+ /// list, which corresponds to the default content.
+ const isc::data::ConstElementPtr& getConfiguration() const {
+ return (configuration_);
+ }
/// \brief Implementation of the ClientList::find.
virtual FindResult find(const dns::Name& zone,
@@ -273,6 +286,9 @@ protected:
virtual DataSourcePair getDataSourceClient(const std::string& type,
const data::ConstElementPtr&
configuration);
+private:
+ /// \brief Currently active configuration.
+ isc::data::ConstElementPtr configuration_;
public:
/// \brief Access to the data source clients.
///
diff --git a/src/lib/datasrc/tests/client_list_unittest.cc b/src/lib/datasrc/tests/client_list_unittest.cc
index 4fed961..f3137c1 100644
--- a/src/lib/datasrc/tests/client_list_unittest.cc
+++ b/src/lib/datasrc/tests/client_list_unittest.cc
@@ -353,8 +353,10 @@ TEST_F(ListTest, multiBestMatch) {
// Check the configuration is empty when the list is empty
TEST_F(ListTest, configureEmpty) {
ConstElementPtr elem(new ListElement);
- list_->configure(*elem, true);
+ list_->configure(elem, true);
EXPECT_TRUE(list_->getDataSources().empty());
+ // Check the exact configuration is preserved
+ EXPECT_EQ(elem, list_->getConfiguration());
}
// Check we can get multiple data sources and they are in the right order.
@@ -371,10 +373,12 @@ TEST_F(ListTest, configureMulti) {
" \"params\": {}"
"}]"
));
- list_->configure(*elem, true);
+ list_->configure(elem, true);
EXPECT_EQ(2, list_->getDataSources().size());
checkDS(0, "type1", "{}");
checkDS(1, "type2", "{}");
+ // Check the exact configuration is preserved
+ EXPECT_EQ(elem, list_->getConfiguration());
}
// Check we can pass whatever we want to the params
@@ -397,7 +401,7 @@ TEST_F(ListTest, configureParams) {
" \"cache\": \"off\","
" \"params\": ") + *param +
"}]"));
- list_->configure(*elem, true);
+ list_->configure(elem, true);
EXPECT_EQ(1, list_->getDataSources().size());
checkDS(0, "t", *param);
}
@@ -425,12 +429,12 @@ TEST_F(ListTest, wrongConfig) {
NULL
};
// Put something inside to see it survives the exception
- list_->configure(*config_elem_, true);
+ list_->configure(config_elem_, true);
checkDS(0, "test_type", "{}");
for (const char** config(configs); *config; ++config) {
SCOPED_TRACE(*config);
ConstElementPtr elem(Element::fromJSON(*config));
- EXPECT_THROW(list_->configure(*elem, true),
+ EXPECT_THROW(list_->configure(elem, true),
ConfigurableClientList::ConfigurationError);
// Still untouched
checkDS(0, "test_type", "{}");
@@ -444,7 +448,7 @@ TEST_F(ListTest, defaults) {
"{"
" \"type\": \"type1\""
"}]"));
- list_->configure(*elem, true);
+ list_->configure(elem, true);
EXPECT_EQ(1, list_->getDataSources().size());
checkDS(0, "type1", "null");
}
@@ -452,11 +456,11 @@ TEST_F(ListTest, defaults) {
// Check we can call the configure multiple times, to change the configuration
TEST_F(ListTest, reconfigure) {
ConstElementPtr empty(new ListElement);
- list_->configure(*config_elem_, true);
+ list_->configure(config_elem_, true);
checkDS(0, "test_type", "{}");
- list_->configure(*empty, true);
+ list_->configure(empty, true);
EXPECT_TRUE(list_->getDataSources().empty());
- list_->configure(*config_elem_, true);
+ list_->configure(config_elem_, true);
checkDS(0, "test_type", "{}");
}
@@ -466,9 +470,9 @@ TEST_F(ListTest, dataSrcError) {
"{"
" \"type\": \"error\""
"}]"));
- list_->configure(*config_elem_, true);
+ list_->configure(config_elem_, true);
checkDS(0, "test_type", "{}");
- EXPECT_THROW(list_->configure(*elem, true), DataSourceError);
+ EXPECT_THROW(list_->configure(elem, true), DataSourceError);
checkDS(0, "test_type", "{}");
}
More information about the bind10-changes
mailing list