Merge pull request #723 from viranch/azuredns-optimize

minimize Azure Traffic Manager hops for dynamic records

octodns/provider/azuredns.py

@@ -281,12 +281,29 @@ def _profile_is_match(have, desired):
     if have is None or desired is None:
         return False
 
+    log = logging.getLogger('azuredns._profile_is_match').debug
+
+    def false(have, desired, name=None):
+        prefix = 'profile={}'.format(name) if name else ''
+        attr = have.__class__.__name__
+        log('%s have.%s = %s', prefix, attr, have)
+        log('%s desired.%s = %s', prefix, attr, desired)
+        return False
+
     # compare basic attributes
     if have.name != desired.name or \
        have.traffic_routing_method != desired.traffic_routing_method or \
-       have.dns_config.ttl != desired.dns_config.ttl or \
        len(have.endpoints) != len(desired.endpoints):
-        return False
+        return false(have, desired)
+
+    # compare dns config
+    dns_have = have.dns_config
+    dns_desired = desired.dns_config
+    if dns_have.ttl != dns_desired.ttl or \
+       dns_have.relative_name is None or \
+       dns_desired.relative_name is None or \
+       dns_have.relative_name != dns_desired.relative_name:
+        return false(dns_have, dns_desired, have.name)
 
     # compare monitoring configuration
     monitor_have = have.monitor_config
@@ -295,7 +312,7 @@ def _profile_is_match(have, desired):
        monitor_have.port != monitor_desired.port or \
        monitor_have.path != monitor_desired.path or \
        monitor_have.custom_headers != monitor_desired.custom_headers:
-        return False
+        return false(monitor_have, monitor_desired, have.name)
 
     # compare endpoints
     method = have.traffic_routing_method
@@ -313,26 +330,26 @@ def _profile_is_match(have, desired):
     for have_endpoint, desired_endpoint in endpoints:
         if have_endpoint.name != desired_endpoint.name or \
            have_endpoint.type != desired_endpoint.type:
-            return False
+            return false(have_endpoint, desired_endpoint, have.name)
         target_type = have_endpoint.type.split('/')[-1]
         if target_type == 'externalEndpoints':
             # compare value, weight, priority
             if have_endpoint.target != desired_endpoint.target:
-                return False
+                return false(have_endpoint, desired_endpoint, have.name)
             if method == 'Weighted' and \
                have_endpoint.weight != desired_endpoint.weight:
-                return False
+                return false(have_endpoint, desired_endpoint, have.name)
         elif target_type == 'nestedEndpoints':
             # compare targets
             if have_endpoint.target_resource_id != \
                desired_endpoint.target_resource_id:
-                return False
+                return false(have_endpoint, desired_endpoint, have.name)
             # compare geos
             if method == 'Geographic':
                 have_geos = sorted(have_endpoint.geo_mapping)
                 desired_geos = sorted(desired_endpoint.geo_mapping)
                 if have_geos != desired_geos:
-                    return False
+                    return false(have_endpoint, desired_endpoint, have.name)
         else:
             # unexpected, give up
             return False
@@ -629,14 +646,23 @@ class AzureProvider(BaseProvider):
         pools = defaultdict(lambda: {'fallback': None, 'values': []})
         rules = []
 
-        # top level geo profile
+        # top level profile
-        geo_profile = self._get_tm_profile_by_id(azrecord.target_resource.id)
+        root_profile = self._get_tm_profile_by_id(azrecord.target_resource.id)
-        for geo_ep in geo_profile.endpoints:
+        if root_profile.traffic_routing_method != 'Geographic':
+            # This record does not use geo fencing, so we skip the Geographic
+            # profile hop; let's pretend to be a geo-profile's only endpoint
+            geo_ep = Endpoint(target_resource_id=root_profile.id)
+            geo_ep.target_resource = root_profile
+            endpoints = [geo_ep]
+        else:
+            endpoints = root_profile.endpoints
+
+        for geo_ep in endpoints:
             rule = {}
 
             # resolve list of regions
-            geo_map = list(geo_ep.geo_mapping)
+            geo_map = list(geo_ep.geo_mapping or [])
-            if geo_map != ['WORLD']:
+            if geo_map and geo_map != ['WORLD']:
                 if 'GEO-ME' in geo_map:
                     # Azure treats Middle East as a separate group, but
                     # its part of Asia in octoDNS, so we need to remove GEO-ME
@@ -645,7 +671,7 @@ class AzureProvider(BaseProvider):
                     # profile was generated by octoDNS
                     if 'GEO-AS' not in geo_map:
                         msg = 'Profile={} for record {}: '.format(
-                            geo_profile.name, azrecord.fqdn)
+                            root_profile.name, azrecord.fqdn)
                         msg += 'Middle East (GEO-ME) is not supported by ' + \
                                'octoDNS. It needs to be either paired ' + \
                                'with Asia (GEO-AS) or expanded into ' + \
@@ -673,18 +699,35 @@ class AzureProvider(BaseProvider):
                         # country
                         geos.append(GeoCodes.country_to_code(code))
 
-            # second level priority profile
+            # build fallback chain from second level priority profile
+            if geo_ep.target_resource_id:
+                target = geo_ep.target_resource
+                if target.traffic_routing_method == 'Priority':
+                    rule_endpoints = target.endpoints
+                    rule_endpoints.sort(key=lambda e: e.priority)
+                else:
+                    # Weighted
+                    geo_ep.name = target.endpoints[0].name.split('--', 1)[0]
+                    rule_endpoints = [geo_ep]
+            else:
+                # this geo directly points to the default, so we skip the
+                # Priority profile hop and directly use an external endpoint;
+                # let's pretend to be a Priority profile's only endpoint
+                rule_endpoints = [geo_ep]
+
             pool = None
-            rule_endpoints = geo_ep.target_resource.endpoints
-            rule_endpoints = sorted(rule_endpoints, key=lambda e: e.priority)
             for rule_ep in rule_endpoints:
                 pool_name = rule_ep.name
 
                 # last/default pool
-                if pool_name == '--default--':
+                if pool_name.endswith('--default--'):
                     default.add(rule_ep.target)
-                    # this should be the last one, so let's break here
+                    if pool_name == '--default--':
-                    break
+                        # this should be the last one, so let's break here
+                        break
+                    # last pool is a single value pool and its value is same
+                    # as record's default value
+                    pool_name = pool_name[:-len('--default--')]
 
                 # set first priority endpoint as the rule's primary pool
                 if 'pool' not in rule:
@@ -694,32 +737,32 @@ class AzureProvider(BaseProvider):
                     # set current pool as fallback of the previous pool
                     pool['fallback'] = pool_name
 
+                if pool_name in pools:
+                    # we've already populated the pool
+                    continue
+
+                # populate the pool from Weighted profile
+                # these should be leaf node entries with no further nesting
                 pool = pools[pool_name]
                 endpoints = []
-                # these should be leaf node entries with no further nesting
+
                 if rule_ep.target_resource_id:
                     # third (and last) level weighted RR profile
                     endpoints = rule_ep.target_resource.endpoints
                 else:
-                    # single-value pool
+                    # single-value pool, so we skip the Weighted profile hop
+                    # and directly use an external endpoint; let's pretend to
+                    # be a Weighted profile's only endpoint
                     endpoints = [rule_ep]
 
                 for pool_ep in endpoints:
                     val = pool_ep.target
-                    value_dict = {
+                    pool['values'].append({
                         'value': _check_endswith_dot(val),
                         'weight': pool_ep.weight or 1,
-                    }
+                    })
-                    if value_dict not in pool['values']:
+                    if pool_ep.name.endswith('--default--'):
-                        pool['values'].append(value_dict)
+                        default.add(val)
-
-            if 'pool' not in rule or not default:
-                # this will happen if the priority profile does not have
-                # enough endpoints
-                msg = 'Expected at least 2 endpoints in {}, got {}'.format(
-                    geo_ep.target_resource.name, len(rule_endpoints)
-                )
-                raise AzureException(msg)
 
             rules.append(rule)
 
@@ -792,7 +835,7 @@ class AzureProvider(BaseProvider):
             elif ep.target:
                 ep.type = endpoint_type_prefix + 'externalEndpoints'
             else:
-                msg = ('Invalid endpoint {} in profile {}, needs to have' +
+                msg = ('Invalid endpoint {} in profile {}, needs to have ' +
                        'either target or target_resource_id').format(
                            ep.name, name)
                 raise AzureException(msg)
@@ -811,39 +854,83 @@ class AzureProvider(BaseProvider):
             location='global',
         )
 
+    def _update_tm_name(self, profile, new_name):
+        profile.name = new_name
+        profile.id = self._profile_name_to_id(new_name)
+        profile.dns_config.relative_name = new_name
+
+        return profile
+
     def _generate_traffic_managers(self, record):
         traffic_managers = []
         pools = record.dynamic.pools
 
+        default = record.value[:-1]
         tm_suffix = _traffic_manager_suffix(record)
         profile = self._generate_tm_profile
 
         geo_endpoints = []
+        pool_profiles = {}
 
         for rule in record.dynamic.rules:
+            # Prepare the list of Traffic manager geos
+            rule_geos = rule.data.get('geos', [])
+            geos = []
+            for geo in rule_geos:
+                if '-' in geo:
+                    # country/state
+                    geos.append(geo.split('-', 1)[-1])
+                else:
+                    # continent
+                    if geo == 'AS':
+                        # Middle East is part of Asia in octoDNS, but
+                        # Azure treats it as a separate "group", so let's
+                        # add it in the list of geo mappings. We will drop
+                        # it when we later parse the list of regions.
+                        geos.append('GEO-ME')
+                    elif geo == 'OC':
+                        # Azure uses Australia/Pacific (AP) instead of
+                        # Oceania
+                        geo = 'AP'
+
+                    geos.append('GEO-{}'.format(geo))
+            if not geos:
+                geos.append('WORLD')
+
             pool_name = rule.data['pool']
             rule_endpoints = []
             priority = 1
+            default_seen = False
 
             while pool_name:
                 # iterate until we reach end of fallback chain
+                default_seen = False
                 pool = pools[pool_name].data
-                profile_name = 'pool-{}--{}'.format(pool_name, tm_suffix)
                 if len(pool['values']) > 1:
                     # create Weighted profile for multi-value pool
-                    endpoints = []
+                    pool_profile = pool_profiles.get(pool_name)
-                    for val in pool['values']:
+                    if pool_profile is None:
-                        target = val['value']
+                        endpoints = []
-                        # strip trailing dot from CNAME value
+                        for val in pool['values']:
-                        target = target[:-1]
+                            target = val['value']
-                        endpoints.append(Endpoint(
+                            # strip trailing dot from CNAME value
-                            name=target,
+                            target = target[:-1]
-                            target=target,
+                            ep_name = '{}--{}'.format(pool_name, target)
-                            weight=val.get('weight', 1),
+                            if target == default:
-                        ))
+                                # mark default
-                    pool_profile = profile(profile_name, 'Weighted', endpoints,
+                                ep_name += '--default--'
-                                           record)
+                                default_seen = True
-                    traffic_managers.append(pool_profile)
+                            endpoints.append(Endpoint(
+                                name=ep_name,
+                                target=target,
+                                weight=val.get('weight', 1),
+                            ))
+                        profile_name = 'pool-{}--{}'.format(
+                            pool_name, tm_suffix)
+                        pool_profile = profile(profile_name, 'Weighted',
+                                               endpoints, record)
+                        traffic_managers.append(pool_profile)
+                        pool_profiles[pool_name] = pool_profile
 
                     # append pool to endpoint list of fallback rule profile
                     rule_endpoints.append(Endpoint(
@@ -852,8 +939,15 @@ class AzureProvider(BaseProvider):
                         priority=priority,
                     ))
                 else:
-                    # add single-value pool as an external endpoint
+                    # Skip Weighted profile hop for single-value pool
+                    # append its value as an external endpoint to fallback
+                    # rule profile
                     target = pool['values'][0]['value'][:-1]
+                    ep_name = pool_name
+                    if target == default:
+                        # mark default
+                        ep_name += '--default--'
+                        default_seen = True
                     rule_endpoints.append(Endpoint(
                         name=pool_name,
                         target=target,
@@ -863,51 +957,61 @@ class AzureProvider(BaseProvider):
                 priority += 1
                 pool_name = pool.get('fallback')
 
-            # append default profile to the end
+            # append default endpoint unless it is already included in
-            rule_endpoints.append(Endpoint(
+            # last pool of rule profile
-                name='--default--',
+            if not default_seen:
-                target=record.value[:-1],
+                rule_endpoints.append(Endpoint(
-                priority=priority,
+                    name='--default--',
-            ))
+                    target=default,
-            # create rule profile with fallback chain
+                    priority=priority,
-            rule_profile_name = 'rule-{}--{}'.format(rule.data['pool'],
+                ))
-                                                     tm_suffix)
-            rule_profile = profile(rule_profile_name, 'Priority',
-                                   rule_endpoints, record)
-            traffic_managers.append(rule_profile)
 
-            # append rule profile to top-level geo profile
+            if len(rule_endpoints) > 1:
-            rule_geos = rule.data.get('geos', [])
+                # create rule profile with fallback chain
-            geos = []
+                rule_profile_name = 'rule-{}--{}'.format(
-            if len(rule_geos) > 0:
+                    rule.data['pool'], tm_suffix)
-                for geo in rule_geos:
+                rule_profile = profile(rule_profile_name, 'Priority',
-                    if '-' in geo:
+                                       rule_endpoints, record)
-                        # country or state
+                traffic_managers.append(rule_profile)
-                        geos.append(geo.split('-', 1)[-1])
-                    else:
-                        # continent
-                        if geo == 'AS':
-                            # Middle East is part of Asia in octoDNS, but
-                            # Azure treats it as a separate "group", so let's
-                            # add it in the list of geo mappings. We will drop
-                            # it when we later parse the list of regions.
-                            geos.append('GEO-ME')
-                        elif geo == 'OC':
-                            # Azure uses Australia/Pacific (AP) instead of
-                            # Oceania
-                            geo = 'AP'
 
-                        geos.append('GEO-{}'.format(geo))
+                # append rule profile to top-level geo profile
+                geo_endpoints.append(Endpoint(
+                    name='rule-{}'.format(rule.data['pool']),
+                    target_resource_id=rule_profile.id,
+                    geo_mapping=geos,
+                ))
             else:
-                geos.append('WORLD')
+                # Priority profile has only one endpoint; skip the hop and
-            geo_endpoints.append(Endpoint(
+                # append its only endpoint to the top-level profile
-                name='rule-{}'.format(rule.data['pool']),
+                rule_ep = rule_endpoints[0]
-                target_resource_id=rule_profile.id,
+                if rule_ep.target_resource_id:
-                geo_mapping=geos,
+                    # point directly to the Weighted pool profile
-            ))
+                    geo_endpoints.append(Endpoint(
+                        name='rule-{}'.format(rule.data['pool']),
+                        target_resource_id=rule_ep.target_resource_id,
+                        geo_mapping=geos,
+                    ))
+                else:
+                    # just add the value of single-value pool
+                    geo_endpoints.append(Endpoint(
+                        name=rule_ep.name + '--default--',
+                        target=rule_ep.target,
+                        geo_mapping=geos,
+                    ))
 
-        geo_profile = profile(tm_suffix, 'Geographic', geo_endpoints, record)
+        if len(geo_endpoints) == 1 and \
-        traffic_managers.append(geo_profile)
+           geo_endpoints[0].geo_mapping == ['WORLD'] and \
+           geo_endpoints[0].target_resource_id:
+            # Single WORLD rule does not require a Geographic profile, use
+            # the target profile as the root profile
+            target_profile_id = geo_endpoints[0].target_resource_id
+            profile_map = dict((tm.id, tm) for tm in traffic_managers)
+            target_profile = profile_map[target_profile_id]
+            self._update_tm_name(target_profile, tm_suffix)
+        else:
+            geo_profile = profile(tm_suffix, 'Geographic', geo_endpoints,
+                                  record)
+            traffic_managers.append(geo_profile)
 
         return traffic_managers
 

tests/test_octodns_provider_azuredns.py

@@ -453,6 +453,7 @@ class Test_ProfileIsMatch(TestCase):
             name = 'foo-unit-tests',
             ttl = 60,
             method = 'Geographic',
+            dns_name = None,
             monitor_proto = 'HTTPS',
             monitor_port = 4443,
             monitor_path = '/_ping',
@@ -465,7 +466,7 @@ class Test_ProfileIsMatch(TestCase):
             weight = 1,
             priority = 1,
         ):
-            dns = DnsConfig(ttl=ttl)
+            dns = DnsConfig(relative_name=(dns_name or name), ttl=ttl)
             return Profile(
                 name=name, traffic_routing_method=method, dns_config=dns,
                 monitor_config=MonitorConfig(
@@ -488,6 +489,7 @@ class Test_ProfileIsMatch(TestCase):
 
         self.assertFalse(is_match(profile(), profile(name='two')))
         self.assertFalse(is_match(profile(), profile(endpoints=2)))
+        self.assertFalse(is_match(profile(), profile(dns_name='two')))
         self.assertFalse(is_match(profile(), profile(monitor_proto='HTTP')))
         self.assertFalse(is_match(profile(), profile(endpoint_name='a')))
         self.assertFalse(is_match(profile(), profile(endpoint_type='b')))
@@ -596,7 +598,6 @@ class TestAzureDnsProvider(TestCase):
         id_format = base_id + '{}--' + suffix
         name_format = '{}--' + suffix
 
-        dns = DnsConfig(ttl=60)
         header = MonitorConfigCustomHeadersItem(name='Host',
                                                 value='foo.unit.tests')
         monitor = MonitorConfig(protocol='HTTPS', port=4443, path='/_ping',
@@ -604,22 +605,22 @@ class TestAzureDnsProvider(TestCase):
         external = 'Microsoft.Network/trafficManagerProfiles/externalEndpoints'
         nested = 'Microsoft.Network/trafficManagerProfiles/nestedEndpoints'
 
-        return [
+        profiles = [
             Profile(
                 id=id_format.format('pool-two'),
                 name=name_format.format('pool-two'),
                 traffic_routing_method='Weighted',
-                dns_config=dns,
+                dns_config=DnsConfig(ttl=60),
                 monitor_config=monitor,
                 endpoints=[
                     Endpoint(
-                        name='two1.unit.tests',
+                        name='two--two1.unit.tests',
                         type=external,
                         target='two1.unit.tests',
                         weight=3,
                     ),
                     Endpoint(
-                        name='two2.unit.tests',
+                        name='two--two2.unit.tests',
                         type=external,
                         target='two2.unit.tests',
                         weight=4,
@@ -630,7 +631,7 @@ class TestAzureDnsProvider(TestCase):
                 id=id_format.format('rule-one'),
                 name=name_format.format('rule-one'),
                 traffic_routing_method='Priority',
-                dns_config=dns,
+                dns_config=DnsConfig(ttl=60),
                 monitor_config=monitor,
                 endpoints=[
                     Endpoint(
@@ -663,7 +664,7 @@ class TestAzureDnsProvider(TestCase):
                 id=id_format.format('rule-two'),
                 name=name_format.format('rule-two'),
                 traffic_routing_method='Priority',
-                dns_config=dns,
+                dns_config=DnsConfig(ttl=60),
                 monitor_config=monitor,
                 endpoints=[
                     Endpoint(
@@ -690,7 +691,7 @@ class TestAzureDnsProvider(TestCase):
                 id=base_id + suffix,
                 name=suffix,
                 traffic_routing_method='Geographic',
-                dns_config=dns,
+                dns_config=DnsConfig(ttl=60),
                 monitor_config=monitor,
                 endpoints=[
                     Endpoint(
@@ -709,6 +710,11 @@ class TestAzureDnsProvider(TestCase):
             ),
         ]
 
+        for profile in profiles:
+            profile.dns_config.relative_name = profile.name
+
+        return profiles
+
     def _get_dynamic_package(self):
         '''Convenience function to setup a sample dynamic record.
         '''
@@ -841,121 +847,6 @@ class TestAzureDnsProvider(TestCase):
         self.assertEquals(len(zone.records), 17)
         self.assertTrue(exists)
 
-    def test_populate_dynamic(self):
-        # Middle east without Asia raises exception
-        provider, zone, record = self._get_dynamic_package()
-        tm_suffix = _traffic_manager_suffix(record)
-        tm_id = provider._profile_name_to_id
-        tm_list = provider._tm_client.profiles.list_by_resource_group
-        rule_name = 'rule-one--{}'.format(tm_suffix)
-        nested = 'Microsoft.Network/trafficManagerProfiles/nestedEndpoints'
-        tm_list.return_value = [
-            Profile(
-                id=tm_id(tm_suffix),
-                name=tm_suffix,
-                traffic_routing_method='Geographic',
-                endpoints=[
-                    Endpoint(
-                        geo_mapping=['GEO-ME'],
-                    ),
-                ],
-            ),
-        ]
-        azrecord = RecordSet(
-            ttl=60,
-            target_resource=SubResource(id=tm_id(tm_suffix)),
-        )
-        azrecord.name = record.name or '@'
-        azrecord.type = 'Microsoft.Network/dnszones/{}'.format(record._type)
-        with self.assertRaises(AzureException) as ctx:
-            provider._populate_record(zone, azrecord)
-            self.assertTrue(text_type(ctx).startswith(
-                'Middle East (GEO-ME) is not supported'
-            ))
-
-        # empty priority profile raises exception
-        provider, zone, record = self._get_dynamic_package()
-        tm_list = provider._tm_client.profiles.list_by_resource_group
-        rule_name = 'rule-one--{}'.format(tm_suffix)
-        nested = 'Microsoft.Network/trafficManagerProfiles/nestedEndpoints'
-        tm_list.return_value = [
-            Profile(
-                id=tm_id(rule_name),
-                name=rule_name,
-                traffic_routing_method='Priority',
-                endpoints=[],
-            ),
-            Profile(
-                id=tm_id(tm_suffix),
-                name=tm_suffix,
-                traffic_routing_method='Geographic',
-                endpoints=[
-                    Endpoint(
-                        geo_mapping=['WORLD'],
-                        name='rule-one',
-                        type=nested,
-                        target_resource_id=tm_id(rule_name),
-                    ),
-                ],
-            ),
-        ]
-        with self.assertRaises(AzureException) as ctx:
-            provider._populate_record(zone, azrecord)
-            self.assertTrue(text_type(ctx).startswith(
-                'Expected at least 2 endpoints'
-            ))
-
-        # valid set of profiles produce expected dynamic record
-        provider, zone, record = self._get_dynamic_package()
-        root_profile_id = provider._profile_name_to_id(
-            _traffic_manager_suffix(record)
-        )
-        azrecord = RecordSet(
-            ttl=60,
-            target_resource=SubResource(id=root_profile_id),
-        )
-        azrecord.name = record.name or '@'
-        azrecord.type = 'Microsoft.Network/dnszones/{}'.format(record._type)
-
-        record = provider._populate_record(zone, azrecord)
-        self.assertEqual(record.name, 'foo')
-        self.assertEqual(record.ttl, 60)
-        self.assertEqual(record.value, 'default.unit.tests.')
-        self.assertEqual(record.dynamic._data(), {
-            'pools': {
-                'one': {
-                    'values': [
-                        {'value': 'one.unit.tests.', 'weight': 1},
-                    ],
-                    'fallback': 'two',
-                },
-                'two': {
-                    'values': [
-                        {'value': 'two1.unit.tests.', 'weight': 3},
-                        {'value': 'two2.unit.tests.', 'weight': 4},
-                    ],
-                    'fallback': 'three',
-                },
-                'three': {
-                    'values': [
-                        {'value': 'three.unit.tests.', 'weight': 1},
-                    ],
-                    'fallback': None,
-                },
-            },
-            'rules': [
-                {'geos': ['AF', 'EU-DE', 'NA-US-CA', 'OC'], 'pool': 'one'},
-                {'pool': 'two'},
-            ],
-        })
-
-        # valid profiles with Middle East test case
-        geo_profile = provider._get_tm_for_dynamic_record(record)
-        geo_profile.endpoints[0].geo_mapping.extend(['GEO-ME', 'GEO-AS'])
-        record = provider._populate_record(zone, azrecord)
-        self.assertIn('AS', record.dynamic.rules[0].data['geos'])
-        self.assertNotIn('ME', record.dynamic.rules[0].data['geos'])
-
     def test_populate_zone(self):
         provider = self._get_provider()
 
@@ -1108,6 +999,7 @@ class TestAzureDnsProvider(TestCase):
             '/providers/Microsoft.Network/trafficManagerProfiles/' + name
         self.assertEqual(profile.id, expected_id)
         self.assertEqual(profile.name, name)
+        self.assertEqual(profile.name, profile.dns_config.relative_name)
         self.assertEqual(profile.traffic_routing_method, routing)
         self.assertEqual(profile.dns_config.ttl, record.ttl)
         self.assertEqual(len(profile.endpoints), len(endpoints))
@@ -1121,33 +1013,451 @@ class TestAzureDnsProvider(TestCase):
             'Microsoft.Network/trafficManagerProfiles/nestedEndpoints'
         )
 
-    def test_generate_traffic_managers(self):
+    def test_dynamic_record(self):
         provider, zone, record = self._get_dynamic_package()
         profiles = provider._generate_traffic_managers(record)
-        deduped = []
-        seen = set()
-        for profile in profiles:
-            if profile.name not in seen:
-                deduped.append(profile)
-                seen.add(profile.name)
 
         # check that every profile is a match with what we expect
         expected_profiles = self._get_tm_profiles(provider)
-        self.assertEqual(len(expected_profiles), len(deduped))
+        self.assertEqual(len(expected_profiles), len(profiles))
-        for have, expected in zip(deduped, expected_profiles):
+        for have, expected in zip(profiles, expected_profiles):
             self.assertTrue(_profile_is_match(have, expected))
 
+        # check that dynamic record is populated back from profiles
+        azrecord = RecordSet(
+            ttl=60,
+            target_resource=SubResource(id=profiles[-1].id),
+        )
+        azrecord.name = record.name or '@'
+        azrecord.type = 'Microsoft.Network/dnszones/{}'.format(record._type)
+        record2 = provider._populate_record(zone, azrecord)
+        self.assertEqual(record2.dynamic._data(), record.dynamic._data())
+
+    def test_generate_traffic_managers_middle_east(self):
         # check Asia/Middle East test case
+        provider, zone, record = self._get_dynamic_package()
         record.dynamic._data()['rules'][0]['geos'].append('AS')
         profiles = provider._generate_traffic_managers(record)
-        geo_profile_name = _traffic_manager_suffix(record)
+        self.assertIn('GEO-ME', profiles[-1].endpoints[0].geo_mapping)
-        geo_profile = next(
+        self.assertIn('GEO-AS', profiles[-1].endpoints[0].geo_mapping)
-            profile
+
-            for profile in profiles
+    def test_populate_dynamic_middle_east(self):
-            if profile.name == geo_profile_name
+        # Middle east without Asia raises exception
+        provider, zone, record = self._get_dynamic_package()
+        tm_suffix = _traffic_manager_suffix(record)
+        tm_id = provider._profile_name_to_id
+        tm_list = provider._tm_client.profiles.list_by_resource_group
+        tm_list.return_value = [
+            Profile(
+                id=tm_id(tm_suffix),
+                name=tm_suffix,
+                traffic_routing_method='Geographic',
+                endpoints=[
+                    Endpoint(
+                        geo_mapping=['GEO-ME'],
+                    ),
+                ],
+            ),
+        ]
+        azrecord = RecordSet(
+            ttl=60,
+            target_resource=SubResource(id=tm_id(tm_suffix)),
         )
-        self.assertIn('GEO-ME', geo_profile.endpoints[0].geo_mapping)
+        azrecord.name = record.name or '@'
-        self.assertIn('GEO-AS', geo_profile.endpoints[0].geo_mapping)
+        azrecord.type = 'Microsoft.Network/dnszones/{}'.format(record._type)
+        with self.assertRaises(AzureException) as ctx:
+            provider._populate_record(zone, azrecord)
+            self.assertTrue(text_type(ctx).startswith(
+                'Middle East (GEO-ME) is not supported'
+            ))
+
+        # valid profiles with Middle East test case
+        provider, zone, record = self._get_dynamic_package()
+        geo_profile = provider._get_tm_for_dynamic_record(record)
+        geo_profile.endpoints[0].geo_mapping.extend(['GEO-ME', 'GEO-AS'])
+        record = provider._populate_record(zone, azrecord)
+        self.assertIn('AS', record.dynamic.rules[0].data['geos'])
+        self.assertNotIn('ME', record.dynamic.rules[0].data['geos'])
+
+    def test_dynamic_no_geo(self):
+        # test that traffic managers are generated as expected
+        provider, zone, record = self._get_dynamic_package()
+        external = 'Microsoft.Network/trafficManagerProfiles/externalEndpoints'
+
+        record = Record.new(zone, 'foo', data={
+            'type': 'CNAME',
+            'ttl': 60,
+            'value': 'default.unit.tests.',
+            'dynamic': {
+                'pools': {
+                    'one': {
+                        'values': [
+                            {'value': 'one.unit.tests.'},
+                        ],
+                    },
+                },
+                'rules': [
+                    {'pool': 'one'},
+                ],
+            }
+        })
+        profiles = provider._generate_traffic_managers(record)
+
+        self.assertEqual(len(profiles), 1)
+        self.assertTrue(_profile_is_match(profiles[0], Profile(
+            name='foo-unit-tests',
+            traffic_routing_method='Priority',
+            dns_config=DnsConfig(
+                relative_name='foo-unit-tests', ttl=60),
+            monitor_config=_get_monitor(record),
+            endpoints=[
+                Endpoint(
+                    name='one',
+                    type=external,
+                    target='one.unit.tests',
+                    priority=1,
+                ),
+                Endpoint(
+                    name='--default--',
+                    type=external,
+                    target='default.unit.tests',
+                    priority=2,
+                ),
+            ],
+        )))
+
+        # test that same record gets populated back from traffic managers
+        tm_list = provider._tm_client.profiles.list_by_resource_group
+        tm_list.return_value = profiles
+        azrecord = RecordSet(
+            ttl=60,
+            target_resource=SubResource(id=profiles[0].id),
+        )
+        azrecord.name = record.name or '@'
+        azrecord.type = 'Microsoft.Network/dnszones/{}'.format(record._type)
+        record2 = provider._populate_record(zone, azrecord)
+        self.assertEqual(record2.dynamic._data(), record.dynamic._data())
+
+    def test_dynamic_fallback_is_default(self):
+        # test that traffic managers are generated as expected
+        provider, zone, record = self._get_dynamic_package()
+        external = 'Microsoft.Network/trafficManagerProfiles/externalEndpoints'
+
+        record = Record.new(zone, 'foo', data={
+            'type': 'CNAME',
+            'ttl': 60,
+            'value': 'default.unit.tests.',
+            'dynamic': {
+                'pools': {
+                    'def': {
+                        'values': [
+                            {'value': 'default.unit.tests.'},
+                        ],
+                    },
+                },
+                'rules': [
+                    {'geos': ['AF'], 'pool': 'def'},
+                ],
+            }
+        })
+        profiles = provider._generate_traffic_managers(record)
+
+        self.assertEqual(len(profiles), 1)
+        self.assertTrue(_profile_is_match(profiles[0], Profile(
+            name='foo-unit-tests',
+            traffic_routing_method='Geographic',
+            dns_config=DnsConfig(
+                relative_name='foo-unit-tests', ttl=60),
+            monitor_config=_get_monitor(record),
+            endpoints=[
+                Endpoint(
+                    name='def--default--',
+                    type=external,
+                    target='default.unit.tests',
+                    geo_mapping=['GEO-AF'],
+                ),
+            ],
+        )))
+
+        # test that same record gets populated back from traffic managers
+        tm_list = provider._tm_client.profiles.list_by_resource_group
+        tm_list.return_value = profiles
+        azrecord = RecordSet(
+            ttl=60,
+            target_resource=SubResource(id=profiles[0].id),
+        )
+        azrecord.name = record.name or '@'
+        azrecord.type = 'Microsoft.Network/dnszones/{}'.format(record._type)
+        record2 = provider._populate_record(zone, azrecord)
+        self.assertEqual(record2.dynamic._data(), record.dynamic._data())
+
+    def test_dynamic_pool_contains_default(self):
+        # test that traffic managers are generated as expected
+        provider, zone, record = self._get_dynamic_package()
+        tm_id = provider._profile_name_to_id
+        external = 'Microsoft.Network/trafficManagerProfiles/externalEndpoints'
+        nested = 'Microsoft.Network/trafficManagerProfiles/nestedEndpoints'
+
+        record = Record.new(zone, 'foo', data={
+            'type': 'CNAME',
+            'ttl': 60,
+            'value': 'default.unit.tests.',
+            'dynamic': {
+                'pools': {
+                    'rr': {
+                        'values': [
+                            {'value': 'one.unit.tests.'},
+                            {'value': 'two.unit.tests.'},
+                            {'value': 'default.unit.tests.'},
+                            {'value': 'final.unit.tests.'},
+                        ],
+                    },
+                },
+                'rules': [
+                    {'geos': ['AF'], 'pool': 'rr'},
+                ],
+            }
+        })
+        profiles = provider._generate_traffic_managers(record)
+
+        self.assertEqual(len(profiles), 2)
+        self.assertTrue(_profile_is_match(profiles[0], Profile(
+            name='pool-rr--foo-unit-tests',
+            traffic_routing_method='Weighted',
+            dns_config=DnsConfig(
+                relative_name='pool-rr--foo-unit-tests', ttl=60),
+            monitor_config=_get_monitor(record),
+            endpoints=[
+                Endpoint(
+                    name='rr--one.unit.tests',
+                    type=external,
+                    target='one.unit.tests',
+                    weight=1,
+                ),
+                Endpoint(
+                    name='rr--two.unit.tests',
+                    type=external,
+                    target='two.unit.tests',
+                    weight=1,
+                ),
+                Endpoint(
+                    name='rr--default.unit.tests--default--',
+                    type=external,
+                    target='default.unit.tests',
+                    weight=1,
+                ),
+                Endpoint(
+                    name='rr--final.unit.tests',
+                    type=external,
+                    target='final.unit.tests',
+                    weight=1,
+                ),
+            ],
+        )))
+        self.assertTrue(_profile_is_match(profiles[1], Profile(
+            name='foo-unit-tests',
+            traffic_routing_method='Geographic',
+            dns_config=DnsConfig(
+                relative_name='foo-unit-tests', ttl=60),
+            monitor_config=_get_monitor(record),
+            endpoints=[
+                Endpoint(
+                    name='rule-rr',
+                    type=nested,
+                    target_resource_id=tm_id('pool-rr--foo-unit-tests'),
+                    geo_mapping=['GEO-AF'],
+                ),
+            ],
+        )))
+
+        # test that same record gets populated back from traffic managers
+        tm_list = provider._tm_client.profiles.list_by_resource_group
+        tm_list.return_value = profiles
+        azrecord = RecordSet(
+            ttl=60,
+            target_resource=SubResource(id=profiles[1].id),
+        )
+        azrecord.name = record.name or '@'
+        azrecord.type = 'Microsoft.Network/dnszones/{}'.format(record._type)
+        record2 = provider._populate_record(zone, azrecord)
+        self.assertEqual(record2.dynamic._data(), record.dynamic._data())
+
+    def test_dynamic_pool_contains_default_no_geo(self):
+        # test that traffic managers are generated as expected
+        provider, zone, record = self._get_dynamic_package()
+        external = 'Microsoft.Network/trafficManagerProfiles/externalEndpoints'
+
+        record = Record.new(zone, 'foo', data={
+            'type': 'CNAME',
+            'ttl': 60,
+            'value': 'default.unit.tests.',
+            'dynamic': {
+                'pools': {
+                    'rr': {
+                        'values': [
+                            {'value': 'one.unit.tests.'},
+                            {'value': 'two.unit.tests.'},
+                            {'value': 'default.unit.tests.'},
+                            {'value': 'final.unit.tests.'},
+                        ],
+                    },
+                },
+                'rules': [
+                    {'pool': 'rr'},
+                ],
+            }
+        })
+        profiles = provider._generate_traffic_managers(record)
+
+        self.assertEqual(len(profiles), 1)
+        self.assertTrue(_profile_is_match(profiles[0], Profile(
+            name='foo-unit-tests',
+            traffic_routing_method='Weighted',
+            dns_config=DnsConfig(
+                relative_name='foo-unit-tests', ttl=60),
+            monitor_config=_get_monitor(record),
+            endpoints=[
+                Endpoint(
+                    name='rr--one.unit.tests',
+                    type=external,
+                    target='one.unit.tests',
+                    weight=1,
+                ),
+                Endpoint(
+                    name='rr--two.unit.tests',
+                    type=external,
+                    target='two.unit.tests',
+                    weight=1,
+                ),
+                Endpoint(
+                    name='rr--default.unit.tests--default--',
+                    type=external,
+                    target='default.unit.tests',
+                    weight=1,
+                ),
+                Endpoint(
+                    name='rr--final.unit.tests',
+                    type=external,
+                    target='final.unit.tests',
+                    weight=1,
+                ),
+            ],
+        )))
+
+        # test that same record gets populated back from traffic managers
+        tm_list = provider._tm_client.profiles.list_by_resource_group
+        tm_list.return_value = profiles
+        azrecord = RecordSet(
+            ttl=60,
+            target_resource=SubResource(id=profiles[0].id),
+        )
+        azrecord.name = record.name or '@'
+        azrecord.type = 'Microsoft.Network/dnszones/{}'.format(record._type)
+        record2 = provider._populate_record(zone, azrecord)
+        self.assertEqual(record2.dynamic._data(), record.dynamic._data())
+
+    def test_dynamic_last_pool_contains_default_no_geo(self):
+        # test that traffic managers are generated as expected
+        provider, zone, record = self._get_dynamic_package()
+        tm_id = provider._profile_name_to_id
+        external = 'Microsoft.Network/trafficManagerProfiles/externalEndpoints'
+        nested = 'Microsoft.Network/trafficManagerProfiles/nestedEndpoints'
+
+        record = Record.new(zone, 'foo', data={
+            'type': 'CNAME',
+            'ttl': 60,
+            'value': 'default.unit.tests.',
+            'dynamic': {
+                'pools': {
+                    'cloud': {
+                        'values': [
+                            {'value': 'cloud.unit.tests.'},
+                        ],
+                        'fallback': 'rr',
+                    },
+                    'rr': {
+                        'values': [
+                            {'value': 'one.unit.tests.'},
+                            {'value': 'two.unit.tests.'},
+                            {'value': 'default.unit.tests.'},
+                            {'value': 'final.unit.tests.'},
+                        ],
+                    },
+                },
+                'rules': [
+                    {'pool': 'cloud'},
+                ],
+            }
+        })
+        profiles = provider._generate_traffic_managers(record)
+
+        self.assertEqual(len(profiles), 2)
+        self.assertTrue(_profile_is_match(profiles[0], Profile(
+            name='pool-rr--foo-unit-tests',
+            traffic_routing_method='Weighted',
+            dns_config=DnsConfig(
+                relative_name='pool-rr--foo-unit-tests', ttl=60),
+            monitor_config=_get_monitor(record),
+            endpoints=[
+                Endpoint(
+                    name='rr--one.unit.tests',
+                    type=external,
+                    target='one.unit.tests',
+                    weight=1,
+                ),
+                Endpoint(
+                    name='rr--two.unit.tests',
+                    type=external,
+                    target='two.unit.tests',
+                    weight=1,
+                ),
+                Endpoint(
+                    name='rr--default.unit.tests--default--',
+                    type=external,
+                    target='default.unit.tests',
+                    weight=1,
+                ),
+                Endpoint(
+                    name='rr--final.unit.tests',
+                    type=external,
+                    target='final.unit.tests',
+                    weight=1,
+                ),
+            ],
+        )))
+        self.assertTrue(_profile_is_match(profiles[1], Profile(
+            name='foo-unit-tests',
+            traffic_routing_method='Priority',
+            dns_config=DnsConfig(
+                relative_name='foo-unit-tests', ttl=60),
+            monitor_config=_get_monitor(record),
+            endpoints=[
+                Endpoint(
+                    name='cloud',
+                    type=external,
+                    target='cloud.unit.tests',
+                    priority=1,
+                ),
+                Endpoint(
+                    name='rr',
+                    type=nested,
+                    target_resource_id=tm_id('pool-rr--foo-unit-tests'),
+                    priority=2,
+                ),
+            ],
+        )))
+
+        # test that same record gets populated back from traffic managers
+        tm_list = provider._tm_client.profiles.list_by_resource_group
+        tm_list.return_value = profiles
+        azrecord = RecordSet(
+            ttl=60,
+            target_resource=SubResource(id=profiles[1].id),
+        )
+        azrecord.name = record.name or '@'
+        azrecord.type = 'Microsoft.Network/dnszones/{}'.format(record._type)
+        record2 = provider._populate_record(zone, azrecord)
+        self.assertEqual(record2.dynamic._data(), record.dynamic._data())
 
     def test_sync_traffic_managers(self):
         provider, zone, record = self._get_dynamic_package()
@@ -1188,6 +1498,21 @@ class TestAzureDnsProvider(TestCase):
         )
         self.assertEqual(new_profile.endpoints[0].weight, 14)
 
+    @patch(
+        'octodns.provider.azuredns.AzureProvider._generate_traffic_managers')
+    def test_sync_traffic_managers_duplicate(self, mock_gen_tms):
+        provider, zone, record = self._get_dynamic_package()
+        tm_sync = provider._tm_client.profiles.create_or_update
+
+        # change and duplicate profiles
+        profile = self._get_tm_profiles(provider)[0]
+        profile.name = 'changing_this_to_trigger_sync'
+        mock_gen_tms.return_value = [profile, profile]
+        provider._sync_traffic_managers(record)
+
+        # it should only be called once for duplicate profiles
+        tm_sync.assert_called_once()
+
     def test_find_traffic_managers(self):
         provider, zone, record = self._get_dynamic_package()