[2854] corrected DataSrcClientsMgr.reconfigure() so it'll work for the default.

'new_config' can be empty for the initial default and won't work as expected.

src/lib/python/isc/server_common/datasrc_clients_mgr.py

@@ -117,7 +117,7 @@ class DataSrcClientsMgr:
             client_list = self.__clients_map.get(rrclass)
         return client_list
 
-    def reconfigure(self, config):
+    def reconfigure(self, new_config, config_data):
         """(Re)configure the set of client lists.
 
         This method takes a new set of data source configuration, builds
@@ -142,12 +142,20 @@ class DataSrcClientsMgr:
         at the same time.
 
         Parameter:
-          config (dict): configuration data for the data_sources module.
+          new_config (dict): configuration data for the data_sources module
+            (actually unused in this method).
+          config_data (isc.config.ConfigData): the latest full config data
+            for the data_sources module.  Usually the second parameter of
+            the (remote) configuration update callback for the module.
 
         """
         try:
             new_map = {}
-            for rrclass_cfg, class_cfg in config.get('classes').items():
+            # We only refer to config_data, not new_config (diff from the
+            # previous).  the latter may be empty for the initial default
+            # configuration while the former works for all cases.
+            for rrclass_cfg, class_cfg in \
+                    config_data.get_value('classes')[0].items():
                 rrclass = isc.dns.RRClass(rrclass_cfg)
                 new_client_list = isc.datasrc.ConfigurableClientList(rrclass)
                 new_client_list.configure(json.dumps(class_cfg),

src/lib/python/isc/server_common/tests/datasrc_clients_mgr_test.py

@@ -32,6 +32,8 @@ class DataSrcClientsMgrTest(unittest.TestCase):
         # We construct the manager with enabling in-memory cache for easier
         # tests.  There should be no risk of inter-thread issues in the tests.
         self.__mgr = DataSrcClientsMgr(use_cache=True)
+        self.__datasrc_cfg = isc.config.ConfigData(
+            isc.config.module_spec_from_file(DATASRC_SPECFILE))
 
     def test_init(self):
         """Check some initial state.
@@ -52,31 +54,33 @@ class DataSrcClientsMgrTest(unittest.TestCase):
         # There should be at least in-memory only data for the static
         # bind/CH zone. (We don't assume the existence of SQLite3 datasrc,
         # so it'll still work if and when we make the default DB-independent).
-        self.__mgr.reconfigure(DEFAULT_CONFIG)
+        self.__mgr.reconfigure({}, self.__datasrc_cfg)
         clist = self.__mgr.get_client_list(RRClass.CH)
         self.assertIsNotNone(clist)
         self.assertTrue(clist.find(Name('bind'), True, False)[2])
 
         # Reconfigure it with a simple new config: the list for CH will be
         # gone, and and an empty list for IN will be installed.
-        self.__mgr.reconfigure({"classes": {"IN": []}})
+        self.__datasrc_cfg.set_local_config({"classes": {"IN": []}})
+        self.__mgr.reconfigure({}, self.__datasrc_cfg)
         self.assertIsNone(self.__mgr.get_client_list(RRClass.CH))
         self.assertIsNotNone(self.__mgr.get_client_list(RRClass.IN))
 
     def test_reconfigure_error(self):
         """Check reconfigure failure preserves the old config."""
         # Configure it with the default
-        self.__mgr.reconfigure(DEFAULT_CONFIG)
+        self.__mgr.reconfigure({}, self.__datasrc_cfg)
         self.assertIsNotNone(self.__mgr.get_client_list(RRClass.CH))
 
         # Then try invalid configuration
-        self.assertRaises(ConfigError, self.__mgr.reconfigure, 42)
+        self.assertRaises(ConfigError, self.__mgr.reconfigure, {}, 42)
         self.assertIsNotNone(self.__mgr.get_client_list(RRClass.CH))
 
         # Another type of invalid configuration: exception would come from
         # the C++ wrapper.
+        self.__datasrc_cfg.set_local_config({"classes": {"IN": 42}})
         self.assertRaises(ConfigError,
-                          self.__mgr.reconfigure, {"classes": {"IN": 42}})
+                          self.__mgr.reconfigure, {}, self.__datasrc_cfg)
         self.assertIsNotNone(self.__mgr.get_client_list(RRClass.CH))
 
     def check_client_list_content(self, clist):
@@ -105,9 +109,11 @@ class DataSrcClientsMgrTest(unittest.TestCase):
 
     def test_reconfig_while_using_old(self):
         """Check datasrc client and finder can work even after list is gone."""
-        self.__mgr.reconfigure(DEFAULT_CONFIG)
+        self.__mgr.reconfigure({}, self.__datasrc_cfg)
         clist = self.__mgr.get_client_list(RRClass.CH)
-        self.__mgr.reconfigure({"classes": {"IN": []}})
+
+        self.__datasrc_cfg.set_local_config({"classes": {"IN": []}})
+        self.__mgr.reconfigure({}, self.__datasrc_cfg)
         self.check_client_list_content(clist)
 
     def test_get_clients_map(self):
@@ -117,13 +123,14 @@ class DataSrcClientsMgrTest(unittest.TestCase):
         # Initially map iss empty, the generation ID is 0.
         self.assertEqual((0, {}), self.__mgr.get_clients_map())
 
-        self.__mgr.reconfigure(DEFAULT_CONFIG)
+        self.__mgr.reconfigure({}, self.__datasrc_cfg)
         genid, clients_map = self.__mgr.get_clients_map()
         self.assertEqual(1, genid)
         self.assertEqual(2, len(clients_map)) # should contain 'IN' and 'CH'
 
         # Check the retrieved map is usable even after further reconfig().
-        self.__mgr.reconfigure({"classes": {"IN": []}})
+        self.__datasrc_cfg.set_local_config({"classes": {"IN": []}})
+        self.__mgr.reconfigure({}, self.__datasrc_cfg)
         self.check_client_list_content(clients_map[RRClass.CH])
 
         # generation ID should be incremented again