[2020] make sure datasrc client is created only when auth DB is updated.

b10-ddns now maintains the created datasrc until next config update,
and keeps using it for update requests.  this will make the update process
a bit more efficient.

get_datasrc_client() is now effectively a private subroutine within ddns.py,
so the test cases were updated so they will be tested indirectly.

src/bin/ddns/ddns.py.in

@@ -141,15 +141,15 @@ def get_datasrc_client(cc_session):
         file = os.environ["B10_FROM_BUILD"] + "/bind10_zones.sqlite3"
     datasrc_config = '{ "database_file": "' + file + '"}'
     try:
-        return HARDCODED_DATASRC_CLASS, DataSourceClient('sqlite3',
-                                                         datasrc_config)
+        return (HARDCODED_DATASRC_CLASS,
+                DataSourceClient('sqlite3', datasrc_config), file)
     except isc.datasrc.Error as ex:
         class DummyDataSourceClient:
             def __init__(self, ex):
                 self.__ex = ex
             def find_zone(self, zone_name):
                 raise isc.datasrc.Error(self.__ex)
-        return HARDCODED_DATASRC_CLASS, DummyDataSourceClient(ex)
+        return (HARDCODED_DATASRC_CLASS, DummyDataSourceClient(ex), file)
 
 class DDNSServer:
     # The number of TCP clients that can be handled by the server at the same
@@ -179,11 +179,16 @@ class DDNSServer:
             self._cc.get_default_value('zones'))
         self._cc.start()
 
+        # Datasource client used for handling update requests: when set,
+        # should a tuple of RRClass and DataSourceClient.  Constructed and
+        # maintained based on auth configuration.
+        self._datasrc_info = None
         # A set of secondary zones, retrieved from zonemgr configuration.
         self._secondary_zones = None
 
         # Get necessary configurations from remote modules.
-        self._cc.add_remote_config_by_name(AUTH_MODULE_NAME)
+        self._cc.add_remote_config_by_name(AUTH_MODULE_NAME,
+                                           self.__auth_config_handler)
         self._cc.add_remote_config_by_name(ZONEMGR_MODULE_NAME,
                                            self.__zonemgr_config_handler)
         isc.server_common.tsig_keyring.init_keyring(self._cc)
@@ -259,17 +264,30 @@ class DDNSServer:
             answer = create_answer(1, "Unknown command: " + str(cmd))
         return answer
 
-    def __zonemgr_config_handler(self, new_config, module_config):
-        logger.info(DDNS_RECEIVED_ZONEMGR_UPDATE)
+    def __auth_config_handler(self, new_config, module_config):
+        logger.info(DDNS_RECEIVED_AUTH_UPDATE)
 
         # If we've got the config before and the new config doesn't update
-        # the secondary zone list, there's nothing we should do with it.
+        # the DB file, there's nothing we should do with it.
         # Note: there seems to be a bug either in bindctl or cfgmgr, and
-        # new_config can contain 'secondary_zones' even if it's not really
+        # new_config can contain 'database_file' even if it's not really
         # updated.  We still perform the check so we can avoid redundant
         # resetting when the bug is fixed.  The redundant reset itself is not
         # good, but such configuration update should not happen so often and
         # it should be acceptable in practice.
+        if self._datasrc_info is not None and \
+                not 'database_file' in new_config:
+            return
+        rrclass, client, db_file = get_datasrc_client(self._cc)
+        self._datasrc_info = (rrclass, client)
+        logger.info(DDNS_AUTH_DBFILE_UPDATE, db_file)
+
+    def __zonemgr_config_handler(self, new_config, module_config):
+        logger.info(DDNS_RECEIVED_ZONEMGR_UPDATE)
+
+        # If we've got the config before and the new config doesn't update
+        # the secondary zone list, there's nothing we should do with it.
+        # (Same note as that for auth's config applies)
         if self._secondary_zones is not None and \
                 not 'secondary_zones' in new_config:
             return
@@ -412,9 +430,8 @@ class DDNSServer:
         # Let an update session object handle the request.  Note: things around
         # ZoneConfig will soon be substantially revised.  For now we don't
         # bother to generalize it.
-        datasrc_class, datasrc_client = get_datasrc_client(self._cc)
-        zone_cfg = ZoneConfig(self._secondary_zones, datasrc_class,
-                              datasrc_client, self._zone_config)
+        zone_cfg = ZoneConfig(self._secondary_zones, self._datasrc_info[0],
+                              self._datasrc_info[1], self._zone_config)
         update_session = self._UpdateSessionClass(self.__request_msg,
                                                   remote_addr, zone_cfg)
         result, zname, zclass = update_session.handle()

src/bin/ddns/ddns_messages.mes

@@ -187,3 +187,14 @@ b10-ddns has successfully updated the internal copy of secondary zones
 obtained from b10-zonemgr, based on a latest update to zonemgr's
 configuration.  The number of newly configured (unique) secondary
 zones is logged.
+
+% DDNS_RECEIVED_AUTH_UPDATE received configuration updates from auth server
+b10-ddns is notified of updates to b10-auth configuration
+(including a report of the initial configuration) that b10-ddns might
+be interested in.
+
+% DDNS_AUTH_DBFILE_UPDATE updated auth DB file to %1
+b10-ddns was notified of updates to the SQLite3 DB file that b10-auth
+uses for the underlying data source and on which b10-ddns needs to
+make updates.  b10-ddns then updated its internal setup so further
+updates would be made on the new DB.

src/bin/ddns/tests/ddns_test.py

@@ -217,6 +217,7 @@ class MyCCSession(isc.config.ConfigData):
 
         # Attributes to handle (faked) remote configurations
         self.__callbacks = {}   # record callbacks for updates to remote confs
+        self._auth_config = {}  # faked auth cfg, settable by tests
         self._zonemgr_config = {} # faked zonemgr cfg, settable by tests
 
     def start(self):
@@ -236,6 +237,11 @@ class MyCCSession(isc.config.ConfigData):
     def add_remote_config_by_name(self, module_name, update_callback=None):
         if update_callback is not None:
             self.__callbacks[module_name] = update_callback
+        if module_name is 'Auth':
+            if module_name in self.__callbacks:
+                # ddns implementation doesn't use the 2nd element, so just
+                # setting it to None
+                self.__callbacks[module_name](self._auth_config, None)
         if module_name is 'Zonemgr':
             if module_name in self.__callbacks:
                 self.__callbacks[module_name](self._zonemgr_config,
@@ -446,12 +452,41 @@ class TestDDNSServer(unittest.TestCase):
         acl = self.ddns_server._zone_config[(TEST_ZONE_NAME, TEST_RRCLASS)]
         self.assertEqual(ACCEPT, acl.execute(TEST_ACL_CONTEXT))
 
+    def test_datasrc_config(self):
+        # By default (in our faked config) it should be derived from the
+        # test data source
+        rrclass, datasrc_client = self.ddns_server._datasrc_info
+        self.assertEqual(RRClass.IN(), rrclass)
+        self.assertEqual(DataSourceClient.SUCCESS,
+                         datasrc_client.find_zone(Name('example.org'))[0])
+
+        # emulating an update.  calling add_remote_config_by_name is a
+        # convenient faked way to invoke the callback.  We set the db file
+        # to a bogus one; the current implementation will create an unusable
+        # data source client.
+        self.__cc_session.auth_db_file = './notexistentdir/somedb.sqlite3'
+        self.__cc_session._auth_config = \
+            {'database_file': './notexistentdir/somedb.sqlite3'}
+        self.__cc_session.add_remote_config_by_name('Auth')
+        rrclass, datasrc_client = self.ddns_server._datasrc_info
+        self.assertEqual(RRClass.IN(), rrclass)
+        self.assertRaises(isc.datasrc.Error,
+                          datasrc_client.find_zone, Name('example.org'))
+
+        # Check the current info isn't changed if the new config doesn't
+        # update it.
+        info_orig = self.ddns_server._datasrc_info
+        self.ddns_server._datasrc_info = 42 # dummy value, should be kept.
+        self.__cc_session._auth_config = {'other_config': 'value'}
+        self.__cc_session.add_remote_config_by_name('Auth')
+        self.assertEqual(42, self.ddns_server._datasrc_info)
+        self.ddns_server._datasrc_info = info_orig
+
     def test_secondary_zones_config(self):
         # By default it should be an empty list
         self.assertEqual(set(), self.ddns_server._secondary_zones)
 
-        # emulating an update.  calling add_remote_config_by_name is a
-        # convenient faked way to invoke the callback.
+        # emulating an update.
         self.__cc_session._zonemgr_config = {'secondary_zones': [
                 {'name': TEST_ZONE_NAME_STR, 'class': TEST_RRCLASS_STR}]}
         self.__cc_session.add_remote_config_by_name('Zonemgr')
@@ -1249,23 +1284,6 @@ class TestConfig(unittest.TestCase):
         self.assertEqual(os.environ["B10_FROM_SOURCE"] +
                          "/src/bin/ddns/ddns.spec", ddns.SPECFILE_LOCATION)
 
-    def test_get_datasrc_client(self):
-        # The test sqlite DB should contain the example.org zone.
-        rrclass, datasrc_client = ddns.get_datasrc_client(self.__ccsession)
-        self.assertEqual(RRClass.IN(), rrclass)
-        self.assertEqual(DataSourceClient.SUCCESS,
-                         datasrc_client.find_zone(Name('example.org'))[0])
-
-    def test_get_datasrc_client_fail(self):
-        # DB file is in a non existent directory, and creatng the client
-        # will fail.  get_datasrc_client will return a dummy client, which
-        # will subsequently make find_zone() fail.
-        self.__ccsession.auth_db_file = './notexistentdir/somedb.sqlite3'
-        rrclass, datasrc_client = ddns.get_datasrc_client(self.__ccsession)
-        self.assertEqual(RRClass.IN(), rrclass)
-        self.assertRaises(isc.datasrc.Error,
-                          datasrc_client.find_zone, Name('example.org'))
-
 if __name__== "__main__":
     isc.log.resetUnitTestRootLogger()
     unittest.main()