From: JINMEI Tatuya Date: Wed, 22 May 2013 04:12:49 +0000 (-0700) Subject: [2911] use request_ixfr, not request_type, just before do_xfrin(). X-Git-Tag: bind10-1.2.0beta1-release~418^2~9^2~1^2~17 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=d43299cb0e6680458c2e831048aaa16acc0d3017;p=thirdparty%2Fkea.git [2911] use request_ixfr, not request_type, just before do_xfrin(). no behavior change yet. also, use AXFR by default, for now, to preserve the behavior. --- diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index d38371e9f8..3572a9170a 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -269,15 +269,15 @@ class MockXfrin(Xfrin): MockXfrin.check_command_hook() def xfrin_start(self, zone_name, rrclass, db_file, master_addrinfo, - tsig_key, request_type, check_soa=True): + tsig_key, request_ixfr, check_soa=True): # store some of the arguments for verification, then call this # method in the superclass self.xfrin_started_master_addr = master_addrinfo[2][0] self.xfrin_started_master_port = master_addrinfo[2][1] - self.xfrin_started_request_type = request_type + self.xfrin_started_request_ixfr = request_ixfr return Xfrin.xfrin_start(self, zone_name, rrclass, None, master_addrinfo, tsig_key, - request_type, check_soa) + request_ixfr, check_soa) class MockXfrinConnection(XfrinConnection): def __init__(self, sock_map, zone_name, rrclass, datasrc_client, @@ -2419,7 +2419,7 @@ class TestXfrinProcess(unittest.TestCase): # Normal, successful case. We only check that things are cleaned up # at the tearDown time. process_xfrin(self, self, TEST_ZONE_NAME, TEST_RRCLASS, None, None, - self.master, False, None, RRType.AXFR, + self.master, False, None, ZoneInfo.REQUEST_IXFR_DISABLED, self.create_xfrinconn) def test_process_xfrin_exception_on_connect(self): @@ -2427,7 +2427,7 @@ class TestXfrinProcess(unittest.TestCase): # cleaned up. self.do_raise_on_connect = True process_xfrin(self, self, TEST_ZONE_NAME, TEST_RRCLASS, None, None, - self.master, False, None, RRType.AXFR, + self.master, False, None, ZoneInfo.REQUEST_IXFR_DISABLED, self.create_xfrinconn) def test_process_xfrin_exception_on_close(self): @@ -2437,7 +2437,7 @@ class TestXfrinProcess(unittest.TestCase): self.do_raise_on_connect = True self.do_raise_on_close = True process_xfrin(self, self, TEST_ZONE_NAME, TEST_RRCLASS, None, None, - self.master, False, None, RRType.AXFR, + self.master, False, None, ZoneInfo.REQUEST_IXFR_DISABLED, self.create_xfrinconn) def test_process_xfrin_exception_on_publish(self): @@ -2445,7 +2445,7 @@ class TestXfrinProcess(unittest.TestCase): # everything must still be cleaned up. self.do_raise_on_publish = True process_xfrin(self, self, TEST_ZONE_NAME, TEST_RRCLASS, None, None, - self.master, False, None, RRType.AXFR, + self.master, False, None, ZoneInfo.REQUEST_IXFR_DISABLED, self.create_xfrinconn) class TestXfrin(unittest.TestCase): @@ -2545,7 +2545,8 @@ class TestXfrin(unittest.TestCase): self.assertEqual(self.args['master'], self.xfr.xfrin_started_master_addr) self.assertEqual(int(self.args['port']), self.xfr.xfrin_started_master_port) # By default we use AXFR (for now) - self.assertEqual(RRType.AXFR, self.xfr.xfrin_started_request_type) + self.assertEqual(ZoneInfo.REQUEST_IXFR_DISABLED, + self.xfr.xfrin_started_request_ixfr) def test_command_handler_retransfer_short_command1(self): # try it when only specifying the zone name (of unknown zone) @@ -2659,7 +2660,8 @@ class TestXfrin(unittest.TestCase): self.assertEqual(int(TEST_MASTER_PORT), self.xfr.xfrin_started_master_port) # By default we use AXFR (for now) - self.assertEqual(RRType.AXFR, self.xfr.xfrin_started_request_type) + self.assertEqual(ZoneInfo.REQUEST_IXFR_DISABLED, + self.xfr.xfrin_started_request_ixfr) def test_command_handler_notify(self): # at this level, refresh is no different than retransfer. @@ -2759,7 +2761,8 @@ class TestXfrin(unittest.TestCase): elif cfg_val == 'only': self.assertEqual(ZoneInfo.REQUEST_IXFR_ONLY, val) else: # check the default - self.assertEqual(ZoneInfo.REQUEST_IXFR_FIRST, + #self.assertEqual(ZoneInfo.REQUEST_IXFR_FIRST, + self.assertEqual(ZoneInfo.REQUEST_IXFR_DISABLED, zone_info.request_ixfr) def test_config_handler_zones(self): @@ -2929,7 +2932,7 @@ class TestXfrin(unittest.TestCase): self.assertEqual(self.xfr.config_handler(config)['result'][0], 0) self._check_zones_config(config) - def common_ixfr_setup(self, xfr_mode, use_ixfr, tsig_key_str = None): + def common_ixfr_setup(self, xfr_mode, request_ixfr, tsig_key_str=None): # This helper method explicitly sets up a zone configuration with # use_ixfr, and invokes either retransfer or refresh. # Shared by some of the following test cases. @@ -2937,59 +2940,65 @@ class TestXfrin(unittest.TestCase): {'name': 'example.com.', 'master_addr': '192.0.2.1', 'tsig_key': tsig_key_str, - 'use_ixfr': use_ixfr}]} + 'request_ixfr': request_ixfr}]} self.assertEqual(self.xfr.config_handler(config)['result'][0], 0) self.assertEqual(self.xfr.command_handler(xfr_mode, self.args)['result'][0], 0) def test_command_handler_retransfer_ixfr_enabled(self): # If IXFR is explicitly enabled in config, IXFR will be used - self.common_ixfr_setup('retransfer', True) - self.assertEqual(RRType.IXFR, self.xfr.xfrin_started_request_type) + self.common_ixfr_setup('retransfer', 'yes') + self.assertEqual(ZoneInfo.REQUEST_IXFR_FIRST, + self.xfr.xfrin_started_request_ixfr) def test_command_handler_refresh_ixfr_enabled(self): # Same for refresh - self.common_ixfr_setup('refresh', True) - self.assertEqual(RRType.IXFR, self.xfr.xfrin_started_request_type) + self.common_ixfr_setup('refresh', 'yes') + self.assertEqual(ZoneInfo.REQUEST_IXFR_FIRST, + self.xfr.xfrin_started_request_ixfr) def test_command_handler_retransfer_with_tsig(self): - self.common_ixfr_setup('retransfer', False, 'example.com.key') - self.assertEqual(RRType.AXFR, self.xfr.xfrin_started_request_type) + self.common_ixfr_setup('retransfer', 'no', 'example.com.key') + self.assertEqual(ZoneInfo.REQUEST_IXFR_DISABLED, + self.xfr.xfrin_started_request_ixfr) def test_command_handler_retransfer_with_tsig_bad_key(self): # bad keys should not reach xfrin, but should they somehow, # they are ignored (and result in 'key not found' + error log). self.assertRaises(XfrinZoneInfoException, self.common_ixfr_setup, - 'retransfer', False, 'bad.key') + 'retransfer', 'no', 'bad.key') def test_command_handler_retransfer_with_tsig_unknown_key(self): self.assertRaises(XfrinZoneInfoException, self.common_ixfr_setup, - 'retransfer', False, 'no.such.key') + 'retransfer', 'no', 'no.such.key') def test_command_handler_refresh_with_tsig(self): - self.common_ixfr_setup('refresh', False, 'example.com.key') - self.assertEqual(RRType.AXFR, self.xfr.xfrin_started_request_type) + self.common_ixfr_setup('refresh', 'no', 'example.com.key') + self.assertEqual(ZoneInfo.REQUEST_IXFR_DISABLED, + self.xfr.xfrin_started_request_ixfr) def test_command_handler_refresh_with_tsig_bad_key(self): # bad keys should not reach xfrin, but should they somehow, # they are ignored (and result in 'key not found' + error log). self.assertRaises(XfrinZoneInfoException, self.common_ixfr_setup, - 'refresh', False, 'bad.key') + 'refresh', 'no', 'bad.key') def test_command_handler_refresh_with_tsig_unknown_key(self): self.assertRaises(XfrinZoneInfoException, self.common_ixfr_setup, - 'refresh', False, 'no.such.key') + 'refresh', 'no', 'no.such.key') def test_command_handler_retransfer_ixfr_disabled(self): # Similar to the previous case, but explicitly disabled. AXFR should # be used. - self.common_ixfr_setup('retransfer', False) - self.assertEqual(RRType.AXFR, self.xfr.xfrin_started_request_type) + self.common_ixfr_setup('retransfer', 'no') + self.assertEqual(ZoneInfo.REQUEST_IXFR_DISABLED, + self.xfr.xfrin_started_request_ixfr) def test_command_handler_refresh_ixfr_disabled(self): # Same for refresh - self.common_ixfr_setup('refresh', False) - self.assertEqual(RRType.AXFR, self.xfr.xfrin_started_request_type) + self.common_ixfr_setup('refresh', 'no') + self.assertEqual(ZoneInfo.REQUEST_IXFR_DISABLED, + self.xfr.xfrin_started_request_ixfr) class TestXfrinMemoryZones(unittest.TestCase): def setUp(self): @@ -3256,7 +3265,7 @@ class TestXfrinProcess(unittest.TestCase): """ pass - def __do_test(self, rets, transfers, request_type): + def __do_test(self, rets, transfers, request_ixfr): """ Do the actual test. The request type, prepared sucesses/failures and expected sequence of transfers is passed to specify what test @@ -3266,7 +3275,7 @@ class TestXfrinProcess(unittest.TestCase): published = rets[-1] xfrin.process_xfrin(self, XfrinRecorder(), Name("example.org."), RRClass.IN, None, None, None, True, None, - request_type, self.__get_connection) + request_ixfr, self.__get_connection) self.assertEqual([], self.__rets) self.assertEqual(transfers, self.__transfers) # Create a connection for each attempt @@ -3277,7 +3286,7 @@ class TestXfrinProcess(unittest.TestCase): """ Everything OK the first time, over IXFR. """ - self.__do_test([XFRIN_OK], [RRType.IXFR], RRType.IXFR) + self.__do_test([XFRIN_OK], [RRType.IXFR], ZoneInfo.REQUEST_IXFR_FIRST) # Check there was loadzone command self.assertTrue(self._send_cc_session.send_called) self.assertTrue(self._send_cc_session.send_called_correctly) @@ -3288,23 +3297,27 @@ class TestXfrinProcess(unittest.TestCase): """ Everything OK the first time, over AXFR. """ - self.__do_test([XFRIN_OK], [RRType.AXFR], RRType.AXFR) + self.__do_test([XFRIN_OK], [RRType.AXFR], + ZoneInfo.REQUEST_IXFR_DISABLED) def test_axfr_fail(self): """ The transfer failed over AXFR. Should not be retried (we don't expect - to fail on AXFR, but succeed on IXFR and we didn't use IXFR in the first - place for some reason. + to fail on AXFR, but succeed on IXFR and we didn't use IXFR in the + first place for some reason. + """ - self.__do_test([XFRIN_FAIL], [RRType.AXFR], RRType.AXFR) + self.__do_test([XFRIN_FAIL], [RRType.AXFR], + ZoneInfo.REQUEST_IXFR_DISABLED) def test_ixfr_fallback(self): """ - The transfer fails over IXFR, but suceeds over AXFR. It should fall back - to it and say everything is OK. + The transfer fails over IXFR, but suceeds over AXFR. It should fall + back to it and say everything is OK. + """ self.__do_test([XFRIN_FAIL, XFRIN_OK], [RRType.IXFR, RRType.AXFR], - RRType.IXFR) + ZoneInfo.REQUEST_IXFR_FIRST) def test_ixfr_fail(self): """ @@ -3312,13 +3325,14 @@ class TestXfrinProcess(unittest.TestCase): (only once) and should try both before giving up. """ self.__do_test([XFRIN_FAIL, XFRIN_FAIL], - [RRType.IXFR, RRType.AXFR], RRType.IXFR) + [RRType.IXFR, RRType.AXFR], ZoneInfo.REQUEST_IXFR_FIRST) def test_send_loadzone(self): """ Check the loadzone command is sent after successful transfer. """ - self.__do_test([XFRIN_OK], [RRType.IXFR], RRType.IXFR) + self.__do_test([XFRIN_OK], [RRType.IXFR], + ZoneInfo.REQUEST_IXFR_FIRST) self.assertTrue(self._send_cc_session.send_called) self.assertTrue(self._send_cc_session.send_called_correctly) self.assertTrue(self._send_cc_session.recv_called) diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index d4a0f496ba..2faff0cf30 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -1117,14 +1117,14 @@ def _get_zone_soa(datasrc_client, zone_name, zone_class): def __process_xfrin(server, zone_name, rrclass, db_file, shutdown_event, master_addrinfo, check_soa, tsig_key, - request_type, conn_class): + request_ixfr, conn_class): conn = None exception = None ret = XFRIN_FAIL try: # Create a data source client used in this XFR session. Right now we - # still assume an sqlite3-based data source, and use both the old and new - # data source APIs. We also need to use a mock client for tests. + # still assume an sqlite3-based data source, and use both the old and + # 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. @@ -1132,19 +1132,26 @@ def __process_xfrin(server, zone_name, rrclass, db_file, if db_file is not None: # temporary hardcoded sqlite initialization. Once we decide on # the config specification, we need to update this (TODO) - # this may depend on #1207, or any follow-up ticket created for #1207 + # this may depend on #1207, or any follow-up ticket created for + # #1207 datasrc_type = "sqlite3" datasrc_config = "{ \"database_file\": \"" + db_file + "\"}" datasrc_client = DataSourceClient(datasrc_type, datasrc_config) - # Create a TCP connection for the XFR session and perform the operation. + # Create a TCP connection for the XFR session and perform the + # operation. sock_map = {} - # In case we were asked to do IXFR and that one fails, we try again with - # AXFR. But only if we could actually connect to the server. + # In case we were asked to do IXFR and that one fails, we try again + # with AXFR. But only if we could actually connect to the server. # - # So we start with retry as True, which is set to false on each attempt. - # In the case of connected but failed IXFR, we set it to true once again. + # So we start with retry as True, which is set to false on each + # attempt. In the case of connected but failed IXFR, we set it to true + # once again. retry = True + if request_ixfr == ZoneInfo.REQUEST_IXFR_DISABLED: + request_type = RRType.AXFR + else: + request_type = RRType.IXFR while retry: retry = False conn = conn_class(sock_map, zone_name, rrclass, datasrc_client, @@ -1154,10 +1161,10 @@ def __process_xfrin(server, zone_name, rrclass, db_file, if conn.connect_to_master(): ret = conn.do_xfrin(check_soa, request_type) if ret == XFRIN_FAIL and request_type == RRType.IXFR: - # IXFR failed for some reason. It might mean the server can't - # handle it, or we don't have the zone or we are out of sync or - # whatever else. So we retry with with AXFR, as it may succeed - # in many such cases. + # IXFR failed for some reason. It might mean the server + # can't handle it, or we don't have the zone or we are out + # of sync or whatever else. So we retry with with AXFR, as + # it may succeed in many such cases. retry = True request_type = RRType.AXFR logger.warn(XFRIN_XFR_TRANSFER_FALLBACK, conn.zone_str()) @@ -1188,7 +1195,7 @@ def __process_xfrin(server, zone_name, rrclass, db_file, def process_xfrin(server, xfrin_recorder, zone_name, rrclass, db_file, shutdown_event, master_addrinfo, check_soa, tsig_key, - request_type, conn_class=XfrinConnection): + request_ixfr, conn_class=XfrinConnection): # Even if it should be rare, the main process of xfrin session can # raise an exception. In order to make sure the lock in xfrin_recorder # is released in any cases, we delegate the main part to the helper @@ -1198,14 +1205,17 @@ def process_xfrin(server, xfrin_recorder, zone_name, rrclass, db_file, try: __process_xfrin(server, zone_name, rrclass, db_file, shutdown_event, master_addrinfo, check_soa, tsig_key, - request_type, conn_class) + request_ixfr, conn_class) except Exception as ex: # don't log it until we complete decrement(). exception = ex xfrin_recorder.decrement(zone_name) if exception is not None: - typestr = "AXFR" if request_type == RRType.AXFR else "IXFR" + if request_ixfr == ZoneInfo.REQUEST_IXFR_DISABLED: + typestr = "AXFR" + else: + typestr = "IXFR" logger.error(XFRIN_XFR_PROCESS_FAILURE, typestr, zone_name.to_text(), str(rrclass), str(exception)) @@ -1363,7 +1373,7 @@ class ZoneInfo: def set_request_ixfr(self, request_ixfr): if request_ixfr is None: - request_ixfr = 'yes' # if unspecified, use the default + request_ixfr = 'no' # if unspecified, use the default cfg_to_val = { 'yes': self.REQUEST_IXFR_FIRST, 'only': self.REQUEST_IXFR_ONLY, 'no': self.REQUEST_IXFR_DISABLED } @@ -1595,9 +1605,9 @@ class Xfrin: (zone_name, rrclass) = self._parse_zone_name_and_class(args) master_addr = self._parse_master_and_port(args, zone_name, rrclass) zone_info = self._get_zone_info(zone_name, rrclass) - request_type = RRType.AXFR - if zone_info is not None and zone_info.use_ixfr: - request_type = RRType.IXFR + request_ixfr = ZoneInfo.REQUEST_IXFR_DISABLED + if zone_info is not None: + request_ixfr = zone_info.request_ixfr tsig_key = None if zone_info is None else zone_info.get_tsig_key() db_file = arg_db or self._get_db_file() zone_str = format_zone_str(zone_name, rrclass) # for logging @@ -1605,7 +1615,7 @@ class Xfrin: if answer is not None: return answer ret = self.xfrin_start(zone_name, rrclass, db_file, master_addr, - tsig_key, request_type, check_soa) + tsig_key, request_ixfr, check_soa) return create_answer(ret[0], ret[1]) def command_handler(self, command, args): @@ -1769,7 +1779,7 @@ class Xfrin: self._cc_check_command() def xfrin_start(self, zone_name, rrclass, db_file, master_addrinfo, - tsig_key, request_type, check_soa=True): + tsig_key, request_ixfr, check_soa=True): if "pydnspp" not in sys.modules: return (1, "xfrin failed, can't load dns message python library: 'pydnspp'") @@ -1788,7 +1798,7 @@ class Xfrin: db_file, self._shutdown_event, master_addrinfo, check_soa, - tsig_key, request_type)) + tsig_key, request_ixfr)) xfrin_thread.start() return (0, 'zone xfrin is started') diff --git a/src/bin/xfrin/xfrin.spec b/src/bin/xfrin/xfrin.spec index 42dd8745df..f6f0fa9254 100644 --- a/src/bin/xfrin/xfrin.spec +++ b/src/bin/xfrin/xfrin.spec @@ -52,7 +52,7 @@ { "item_name": "request_ixfr", "item_type": "string", "item_optional": false, - "item_default": "yes" + "item_default": "no" } ] }