diff --git a/octodns/provider/dyn.py b/octodns/provider/dyn.py index 1b9c868..08cc915 100644 --- a/octodns/provider/dyn.py +++ b/octodns/provider/dyn.py @@ -398,10 +398,10 @@ class DynProvider(BaseProvider): for td in get_all_dsf_services(): try: _, fqdn, _type = td.label.split(':', 2) - except ValueError as e: + except ValueError: try: fqdn, _type = td.label.split(':', 1) - except ValueError as e: + except ValueError: self.log.warn("Unsupported TrafficDirector '%s'", td.label) continue @@ -451,7 +451,8 @@ class DynProvider(BaseProvider): return record - def _populate_dynamic_traffic_director(self, zone, fqdn, _type, td, lenient): + def _populate_dynamic_traffic_director(self, zone, fqdn, _type, td, + lenient): # critical to call rulesets once, each call loads them :-( rulesets = td.rulesets @@ -488,8 +489,6 @@ class DynProvider(BaseProvider): 'ruleset.label': ruleset.label, 'ruleset.criteria_type': ruleset.criteria_type, 'ruleset.criterial': ruleset.criteria, - -# 'records': [r.__dict__ for r in record_set.records], }) if ruleset.label.startswith('default:'): @@ -551,7 +550,6 @@ class DynProvider(BaseProvider): return record - def _populate_traffic_directors(self, zone, lenient): self.log.debug('_populate_traffic_directors: zone=%s, lenient=%s', zone.name, lenient) @@ -779,8 +777,31 @@ class DynProvider(BaseProvider): self._traffic_director_monitors[label] = monitor return monitor - def _find_or_create_pool(self, td, pools, label, _type, values=[], - monitor_id=None, record_extras={}): + def _find_or_create_geo_pool(self, td, pools, label, _type, values, + monitor_id=None): + for pool in pools: + if pool.label != label: + continue + records = pool.rs_chains[0].record_sets[0].records + record_values = sorted([r.address for r in records]) + if record_values == values: + # it's a match + return pool + # we need to create the pool + _class = { + 'A': DSFARecord, + 'AAAA': DSFAAAARecord + }[_type] + records = [_class(v) for v in values] + record_set = DSFRecordSet(_type, label, serve_count=len(records), + records=records, dsf_monitor_id=monitor_id) + chain = DSFFailoverChain(label, record_sets=[record_set]) + pool = DSFResponsePool(label, rs_chains=[chain]) + pool.create(td) + return pool + + def _find_or_create_dynamic_pool(self, td, pools, label, _type, values, + monitor_id=None, record_extras={}): # TODO: move this somewhere better def weighted_keyer(d): @@ -884,8 +905,8 @@ class DynProvider(BaseProvider): label = 'default:{}'.format(uuid4().hex) ruleset = DSFRuleset(label, 'always', []) ruleset.create(td, index=insert_at) - pool = self._find_or_create_pool(td, pools, 'default', new._type, - new.values) + pool = self._find_or_create_geo_pool(td, pools, 'default', new._type, + new.values) # There's no way in the client lib to create a ruleset with an existing # pool (ref'd by id) so we have to do this round-a-bout. active_pools = { @@ -919,8 +940,8 @@ class DynProvider(BaseProvider): ruleset.create(td, index=insert_at) first = geo.values[0] - pool = self._find_or_create_pool(td, pools, first, new._type, - geo.values, monitor_id) + pool = self._find_or_create_geo_pool(td, pools, first, new._type, + geo.values, monitor_id) active_pools[geo.code] = pool.response_pool_id ruleset.add_response_pool(pool.response_pool_id) @@ -1051,11 +1072,12 @@ class DynProvider(BaseProvider): } for v in new.values] # For these defaults we need to set them to always be served and to # ignore any health checking (since they won't have one) - pool = self._find_or_create_pool(td, pools, 'default', new._type, - values, record_extras={ - 'automation': 'manual', - 'eligible': True, - }) + pool = self._find_or_create_dynamic_pool(td, pools, 'default', + new._type, values, + record_extras={ + 'automation': 'manual', + 'eligible': True, + }) # There's no way in the client lib to create a ruleset with an existing # pool (ref'd by id) so we have to do this round-a-bout. active_pools = { @@ -1076,8 +1098,9 @@ class DynProvider(BaseProvider): 'weight': v.get('weight', 1), 'value': v['value'], } for v in pool.data['values']] - pool = self._find_or_create_pool(td, pools, _id, new._type, values, - monitor_id) + pool = self._find_or_create_dynamic_pool(td, pools, _id, + new._type, values, + monitor_id) active_pools[_id] = pool.response_pool_id # Run through and configure our rules @@ -1093,7 +1116,8 @@ class DynProvider(BaseProvider): criteria['geoip']['province'] \ .append(geo['subdivision_code'].lower()) elif geo['country_code']: - criteria['geoip']['country'].append(geo['country_code']) + criteria['geoip']['country'] \ + .append(geo['country_code']) else: criteria['geoip']['region'] \ .append(self.REGION_CODES[geo['continent_code']]) @@ -1117,7 +1141,8 @@ class DynProvider(BaseProvider): while fallback: # looking at client lib code, index > exists appends ruleset.add_response_pool(active_pools[fallback], index=999) - fallback = new.dynamic.pools[fallback].data.get('fallback', None) + fallback = new.dynamic.pools[fallback].data.get('fallback', + None) # and always add default as the last ruleset.add_response_pool(active_pools['default'], index=999) @@ -1205,7 +1230,8 @@ class DynProvider(BaseProvider): # we only mess with changes that have geo info somewhere if getattr(c.new, 'dynamic', False) or getattr(c.existing, 'dynamic', False): - mod = getattr(self, '_mod_dynamic_{}'.format(c.__class__.__name__)) + mod = getattr(self, '_mod_dynamic_{}' + .format(c.__class__.__name__)) mod(dyn_zone, c) elif getattr(c.new, 'geo', False) or getattr(c.existing, 'geo', False): diff --git a/tests/test_octodns_provider_dyn.py b/tests/test_octodns_provider_dyn.py index ac56477..8c46f3c 100644 --- a/tests/test_octodns_provider_dyn.py +++ b/tests/test_octodns_provider_dyn.py @@ -671,10 +671,8 @@ class TestDynProviderGeo(TestCase): set(tds.keys())) self.assertEquals(['A'], tds['unit.tests.'].keys()) self.assertEquals(['A'], tds['geo.unit.tests.'].keys()) - provider.log.warn.assert_called_with("Failed to load TrafficDirector " - "'%s': %s", 'something else', - 'need more than 1 value to ' - 'unpack') + provider.log.warn.assert_called_with("Unsupported TrafficDirector " + "'%s'", 'something else') @patch('dyn.core.SessionEngine.execute') def test_traffic_director_monitor(self, mock): @@ -1159,7 +1157,7 @@ class TestDynProviderGeo(TestCase): traffic_directors_enabled=True) # will be tested separately - provider._mod_rulesets = MagicMock() + provider._mod_geo_rulesets = MagicMock() mock.side_effect = [ # create traffic director @@ -1171,7 +1169,7 @@ class TestDynProviderGeo(TestCase): # td now lives in cache self.assertTrue('A' in provider.traffic_directors['unit.tests.']) # should have seen 1 gen call - provider._mod_rulesets.assert_called_once() + provider._mod_geo_rulesets.assert_called_once() def test_mod_geo_update_geo_geo(self): provider = DynProvider('test', 'cust', 'user', 'pass', @@ -1185,8 +1183,8 @@ class TestDynProviderGeo(TestCase): 'A': 42, } } - # mock _mod_rulesets - provider._mod_rulesets = MagicMock() + # mock _mod_geo_rulesets + provider._mod_geo_rulesets = MagicMock() geo = self.geo_record change = Update(geo, geo) @@ -1194,7 +1192,7 @@ class TestDynProviderGeo(TestCase): # still in cache self.assertTrue('A' in provider.traffic_directors['unit.tests.']) # should have seen 1 gen call - provider._mod_rulesets.assert_called_once_with(42, change) + provider._mod_geo_rulesets.assert_called_once_with(42, change) @patch('dyn.core.SessionEngine.execute') def test_mod_geo_update_geo_regular(self, _): @@ -1248,7 +1246,7 @@ class TestDynProviderGeo(TestCase): self.assertFalse('A' in provider.traffic_directors['unit.tests.']) @patch('dyn.tm.services.DSFResponsePool.create') - def test_find_or_create_pool(self, mock): + def test_find_or_create_geo_pool(self, mock): provider = DynProvider('test', 'cust', 'user', 'pass', traffic_directors_enabled=True) @@ -1256,7 +1254,8 @@ class TestDynProviderGeo(TestCase): # no candidates cache miss, so create values = ['1.2.3.4', '1.2.3.5'] - pool = provider._find_or_create_pool(td, [], 'default', 'A', values) + pool = provider._find_or_create_geo_pool(td, [], 'default', 'A', + values) self.assertIsInstance(pool, DSFResponsePool) self.assertEquals(1, len(pool.rs_chains)) records = pool.rs_chains[0].record_sets[0].records @@ -1266,15 +1265,15 @@ class TestDynProviderGeo(TestCase): # cache hit, use the one we just created mock.reset_mock() pools = [pool] - cached = provider._find_or_create_pool(td, pools, 'default', 'A', - values) + cached = provider._find_or_create_geo_pool(td, pools, 'default', 'A', + values) self.assertEquals(pool, cached) mock.assert_not_called() # cache miss, non-matching label mock.reset_mock() - miss = provider._find_or_create_pool(td, pools, 'NA-US-CA', 'A', - values) + miss = provider._find_or_create_geo_pool(td, pools, 'NA-US-CA', 'A', + values) self.assertNotEquals(pool, miss) self.assertEquals('NA-US-CA', miss.label) mock.assert_called_once_with(td) @@ -1282,7 +1281,8 @@ class TestDynProviderGeo(TestCase): # cache miss, non-matching label mock.reset_mock() values = ['2.2.3.4.', '2.2.3.5'] - miss = provider._find_or_create_pool(td, pools, 'default', 'A', values) + miss = provider._find_or_create_geo_pool(td, pools, 'default', 'A', + values) self.assertNotEquals(pool, miss) mock.assert_called_once_with(td) @@ -1290,19 +1290,19 @@ class TestDynProviderGeo(TestCase): @patch('dyn.tm.services.DSFRuleset.create') # just lets us ignore the pool.create calls @patch('dyn.tm.services.DSFResponsePool.create') - def test_mod_rulesets_create(self, _, ruleset_create_mock, - add_response_pool_mock): + def test_mod_geo_rulesets_create(self, _, ruleset_create_mock, + add_response_pool_mock): provider = DynProvider('test', 'cust', 'user', 'pass', traffic_directors_enabled=True) td_mock = MagicMock() td_mock._rulesets = [] provider._traffic_director_monitor = MagicMock() - provider._find_or_create_pool = MagicMock() + provider._find_or_create_geo_pool = MagicMock() td_mock.all_response_pools = [] - provider._find_or_create_pool.side_effect = [ + provider._find_or_create_geo_pool.side_effect = [ _DummyPool('default'), _DummyPool(1), _DummyPool(2), @@ -1311,7 +1311,7 @@ class TestDynProviderGeo(TestCase): ] change = Create(self.geo_record) - provider._mod_rulesets(td_mock, change) + provider._mod_geo_rulesets(td_mock, change) ruleset_create_mock.assert_has_calls(( call(td_mock, index=0), call(td_mock, index=0), @@ -1343,9 +1343,9 @@ class TestDynProviderGeo(TestCase): @patch('dyn.tm.services.DSFRuleset.create') # just lets us ignore the pool.create calls @patch('dyn.tm.services.DSFResponsePool.create') - def test_mod_rulesets_existing(self, _, ruleset_create_mock, - add_response_pool_mock, - get_response_pool_mock): + def test_mod_geo_rulesets_existing(self, _, ruleset_create_mock, + add_response_pool_mock, + get_response_pool_mock): provider = DynProvider('test', 'cust', 'user', 'pass', traffic_directors_enabled=True) @@ -1357,14 +1357,14 @@ class TestDynProviderGeo(TestCase): ruleset_mock, ] provider._traffic_director_monitor = MagicMock() - provider._find_or_create_pool = MagicMock() + provider._find_or_create_geo_pool = MagicMock() unused_pool = _DummyPool('unused') td_mock.all_response_pools = \ ruleset_mock.response_pools + [unused_pool] get_response_pool_mock.return_value = unused_pool - provider._find_or_create_pool.side_effect = [ + provider._find_or_create_geo_pool.side_effect = [ _DummyPool('default'), _DummyPool(1), _DummyPool(2), @@ -1373,7 +1373,7 @@ class TestDynProviderGeo(TestCase): ] change = Create(self.geo_record) - provider._mod_rulesets(td_mock, change) + provider._mod_geo_rulesets(td_mock, change) ruleset_create_mock.assert_has_calls(( call(td_mock, index=2), call(td_mock, index=2),