[2964] use DataSrcClientsMgr to identify the data source for xfr.

from this point, xfrin stops auto-create a new zone; the admin must explicitly
create empty zone using b10-loadzone -e.

src/bin/xfrin/tests/xfrin_test.py

@@ -243,6 +243,23 @@ class MockDataSourceClient():
         # pretend it just succeeds
         pass
 
+class MockDataSrcClientsMgr():
+    def __init__(self):
+        # Default faked result of get_client_list, customizable by tests
+        self.found_datasrc_client_list = self
+
+        # Default faked result of find(), customizable by tests
+        self.found_datasrc_client = MockDataSourceClient()
+
+    def get_client_list(self, rrclass):
+        return self.found_datasrc_client_list
+
+    def find(self, zone_name, want_exact_match, want_finder):
+        """Pretending find method on the object returned by get_clinet_list"""
+        if issubclass(type(self.found_datasrc_client), Exception):
+            raise self.found_datasrc_client
+        return self.found_datasrc_client, None, None
+
 class MockXfrin(Xfrin):
     # This is a class attribute of a callable object that specifies a non
     # default behavior triggered in _cc_check_command().  Specific test methods
@@ -2447,6 +2464,9 @@ class TestXfrin(unittest.TestCase):
         # redirect output
         self.stderr_backup = sys.stderr
         sys.stderr = open(os.devnull, 'w')
+        self.__orig_DataSrcClientsMgr = xfrin.DataSrcClientsMgr
+        xfrin.DataSrcClientsMgr = MockDataSrcClientsMgr
+
         self.xfr = MockXfrin()
         self.args = {}
         self.args['zone_name'] = TEST_ZONE_NAME_STR
@@ -2457,6 +2477,7 @@ class TestXfrin(unittest.TestCase):
         self.args['tsig_key'] = ''
 
     def tearDown(self):
+        xfrin.DataSrcClientsMgr = self.__orig_DataSrcClientsMgr
         self.assertFalse(self.xfr._module_cc.stopped);
         self.xfr.shutdown()
         self.assertTrue(self.xfr._module_cc.stopped);
@@ -2543,9 +2564,11 @@ class TestXfrin(unittest.TestCase):
     def test_command_handler_retransfer(self):
         self.assertEqual(self.xfr.command_handler("retransfer",
                                                   self.args)['result'][0], 0)
-        self.assertEqual(self.args['master'], self.xfr.xfrin_started_master_addr)
+        self.assertEqual(self.args['master'],
-        self.assertEqual(int(self.args['port']), self.xfr.xfrin_started_master_port)
+                         self.xfr.xfrin_started_master_addr)
-        # By default we use AXFR (for now)
+        self.assertEqual(int(self.args['port']),
+                         self.xfr.xfrin_started_master_port)
+        # retransfer always uses AXFR
         self.assertEqual(ZoneInfo.REQUEST_IXFR_DISABLED,
                          self.xfr.xfrin_started_request_ixfr)
 
@@ -2650,6 +2673,37 @@ class TestXfrin(unittest.TestCase):
         # sys.modules is global, so we must recover it
         sys.modules['pydnspp'] = dns_module
 
+    def test_command_handler_retransfer_datasrc_error(self):
+        # Failure cases due to various errors at the data source (config/data)
+        # level
+
+        # No data source client list for the RR class
+        self.xfr._datasrc_clients_mgr.found_datasrc_client_list = None
+        self.assertEqual(1, self.xfr.command_handler("retransfer",
+                                                     self.args)['result'][0])
+
+        # No  data source client for the zone name
+        self.xfr._datasrc_clients_mgr.found_datasrc_client_list = \
+            self.xfr._datasrc_clients_mgr # restore the original
+        self.xfr._datasrc_clients_mgr.found_datasrc_client = None
+        self.assertEqual(1, self.xfr.command_handler("retransfer",
+                                                     self.args)['result'][0])
+
+        # list.find() raises an exception
+        self.xfr._datasrc_clients_mgr.found_datasrc_client = \
+            isc.datasrc.Error('test exception')
+        self.assertEqual(1, self.xfr.command_handler("retransfer",
+                                                     self.args)['result'][0])
+
+        # datasrc.find() raises an exception
+        class RaisingkDataSourceClient(MockDataSourceClient):
+            def find_zone(self, zone_name):
+                raise isc.datasrc.Error('test exception')
+        self.xfr._datasrc_clients_mgr.found_datasrc_client = \
+            RaisingkDataSourceClient()
+        self.assertEqual(1, self.xfr.command_handler("retransfer",
+                                                     self.args)['result'][0])
+
     def test_command_handler_refresh(self):
         # at this level, refresh is no different than retransfer.
         # just confirm the successful case with a different family of address.

src/bin/xfrin/xfrin.py.in

@@ -37,6 +37,7 @@ import isc.net.parse
 from isc.xfrin.diff import Diff
 from isc.server_common.auth_command import auth_loadzone_command
 from isc.server_common.tsig_keyring import init_keyring, get_keyring
+from isc.server_common.datasrc_clients_mgr import DataSrcClientsMgr, ConfigError
 from isc.log_messages.xfrin_messages import *
 from isc.dns import *
 
@@ -1383,6 +1384,8 @@ class Xfrin:
         self.recorder = XfrinRecorder()
         self._shutdown_event = threading.Event()
         self._counters = Counters(SPECFILE_LOCATION)
+        # This is essentially private, but we allow tests to customize it.
+        self._datasrc_clients_mgr = DataSrcClientsMgr()
 
         # Initial configuration
         self._cc_setup()
@@ -1439,7 +1442,8 @@ class Xfrin:
         old_max_transfers_in = self._max_transfers_in
         old_zones = self._zones
 
-        self._max_transfers_in = new_config.get("transfers_in") or self._max_transfers_in
+        self._max_transfers_in = \
+            new_config.get("transfers_in") or self._max_transfers_in
 
         if 'zones' in new_config:
             self._clear_zone_info()
@@ -1459,7 +1463,7 @@ class Xfrin:
         self._set_db_file()
 
     def _datasrc_config_handler(self, new_config, config_data):
-        pass
+        self._datasrc_clients_mgr.reconfigure(new_config)
 
     def shutdown(self):
         ''' shutdown the xfrin process. the thread which is doing xfrin should be
@@ -1732,24 +1736,24 @@ class Xfrin:
         if self.recorder.xfrin_in_progress(zone_name):
             return (1, 'zone xfrin is in progress')
 
-        # Create a data source client used in this XFR session.  Right now we
+        # Identify the data source to which the zone content is transferred,
-        # still assume an sqlite3-based data source, and use both the old and
+        # and get the current zone SOA from the data source (if available).
-        # new data source APIs.  We also need to use a mock client for tests.
-        # For a temporary workaround to deal with these situations, we skip the
-        # creation when the given file is none (the test case).  Eventually
-        # this code will be much cleaner.
         datasrc_client = None
-        if db_file is not None:
+        clist = self._datasrc_clients_mgr.get_client_list(rrclass)
-            # temporary hardcoded sqlite initialization. Once we decide on
+        if clist is None:
-            # the config specification, we need to update this (TODO)
+            return (1, 'no data source is configured for class %s' % rrclass)
-            # this may depend on #1207, or any follow-up ticket created for
+
-            # #1207
+        try:
-            datasrc_type = "sqlite3"
+            datasrc_client = clist.find(zone_name, True, False)[0]
-            datasrc_config = "{ \"database_file\": \"" + db_file + "\"}"
+            if datasrc_client is None: # can happen, so log it separately.
-            datasrc_client = DataSourceClient(datasrc_type, datasrc_config)
+                logger.error(XFRIN_NO_DATASRC,
-
+                             format_zone_str(zone_name, rrclass))
-        # Get the current zone SOA (if available).
+                return (1, 'no data source for transferring %s' %
-        zone_soa = _get_zone_soa(datasrc_client, zone_name, rrclass)
+                        format_zone_str(zone_name, rrclass))
+            zone_soa = _get_zone_soa(datasrc_client, zone_name, rrclass)
+        except isc.datasrc.Error as ex: # rare case error, convert (so logged)
+            raise XfrinException('unexpected failure in datasrc module: ' +
+                                 str(ex))
 
         xfrin_thread = threading.Thread(target=process_xfrin,
                                         args=(self, self.recorder,
@@ -1790,6 +1794,7 @@ def _get_zone_soa(datasrc_client, zone_name, zone_class):
     """
     # datasrc_client should never be None in production case (only tests could
     # specify None)
+    # TBD: this CAN NOW BE CLEANED UP
     if datasrc_client is None:
         return None
 

src/bin/xfrin/xfrin_messages.mes

@@ -153,6 +153,17 @@ from does not match the master address in the Xfrin configuration. The notify
 is ignored. This may indicate that the configuration for the master is wrong,
 that a wrong machine is sending notifies, or that fake notifies are being sent.
 
+% XFRIN_NO_DATASRC no data source for transferring %1
+The xfrin daemon received a command that would trigger a transfer,
+but could not find a data source where the specified zone belong.
+There can be several reasons for this error: it may be a simple
+misspelling in the xfrin or zonemgr configuration, or in the user
+supplied parameter if it is triggered by an external command (such as
+from bindctl).  Another possibility is that this is the initial transfer
+for newly setup secondary zone.  In this case at least an initial empty zone
+must be created in one of configured data sources.  This can be done by
+the -e option of b10-loadzone.
+
 % XFRIN_RECEIVED_COMMAND received command: %1
 The xfrin daemon received a command on the command channel.