optimize by not creating traffic manager for single-value pools

If single-value pools have a weight defined, it will be lost by this
optimization. Next time octodns-sync is run, it will show an update for
setting the weight on remote. To overcome this, this commit includes a
change to Record object that ignores the weight in single-value pools.

octodns/provider/azuredns.py

@@ -667,14 +667,9 @@ class AzureProvider(BaseProvider):
             for rule_ep in rule_endpoints:
                 pool_name = rule_ep.name
 
-                # third (and last) level weighted RR profile
-                # these should be leaf node profiles with no further nesting
-                pool_profile = rule_ep.target_resource
-
                 # last/default pool
                 if pool_name == '--default--':
-                    for pool_ep in pool_profile.endpoints:
-                        default.add(pool_ep.target)
+                    default.add(rule_ep.target)
                     # this should be the last one, so let's break here
                     break
 
@@ -687,11 +682,20 @@ class AzureProvider(BaseProvider):
                     pool['fallback'] = pool_name
 
                 pool = pools[pool_name]
-                for pool_ep in pool_profile.endpoints:
+                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
+                    endpoints = [rule_ep]
+
+                for pool_ep in endpoints:
                     val = pool_ep.target
                     value_dict = {
                         'value': _check_endswith_dot(val),
-                        'weight': pool_ep.weight,
+                        'weight': pool_ep.weight or 1,
                     }
                     if value_dict not in pool['values']:
                         pool['values'].append(value_dict)
@@ -703,6 +707,7 @@ class AzureProvider(BaseProvider):
                     geo_ep.target_resource.name, len(rule_endpoints)
                 )
                 raise AzureException(msg)
+
             rules.append(rule)
 
         # Order and convert to a list
@@ -800,19 +805,6 @@ class AzureProvider(BaseProvider):
         tm_suffix = _traffic_manager_suffix(record)
         profile = self._generate_tm_profile
 
-        # construct the default pool that will be used at the end of
-        # all rules
-        target = record.value[:-1]
-        default_endpoints = [Endpoint(
-            name=target,
-            target=target,
-            weight=1,
-        )]
-        default_profile_name = 'default--{}'.format(tm_suffix)
-        default_profile = profile(default_profile_name, 'Weighted',
-                                  default_endpoints, record)
-        traffic_managers.append(default_profile)
-
         geo_endpoints = []
 
         for rule in record.dynamic.rules:
@@ -824,26 +816,36 @@ class AzureProvider(BaseProvider):
                 # iterate until we reach end of fallback chain
                 pool = pools[pool_name].data
                 profile_name = 'pool-{}--{}'.format(pool_name, tm_suffix)
-                endpoints = []
-                for val in pool['values']:
-                    target = val['value']
-                    # strip trailing dot from CNAME value
-                    target = target[:-1]
-                    endpoints.append(Endpoint(
-                        name=target,
+                if len(pool['values']) > 1:
+                    # create Weighted profile for multi-value pool
+                    endpoints = []
+                    for val in pool['values']:
+                        target = val['value']
+                        # strip trailing dot from CNAME value
+                        target = target[:-1]
+                        endpoints.append(Endpoint(
+                            name=target,
+                            target=target,
+                            weight=val.get('weight', 1),
+                        ))
+                    pool_profile = profile(profile_name, 'Weighted', endpoints,
+                                           record)
+                    traffic_managers.append(pool_profile)
+
+                    # append pool to endpoint list of fallback rule profile
+                    rule_endpoints.append(Endpoint(
+                        name=pool_name,
+                        target_resource_id=pool_profile.id,
+                        priority=priority,
+                    ))
+                else:
+                    # add single-value pool as an external endpoint
+                    target = pool['values'][0]['value'][:-1]
+                    rule_endpoints.append(Endpoint(
+                        name=pool_name,
                         target=target,
-                        weight=val.get('weight', 1),
+                        priority=priority,
                     ))
-                pool_profile = profile(profile_name, 'Weighted', endpoints,
-                                       record)
-                traffic_managers.append(pool_profile)
-
-                # append pool to endpoint list of fallback rule profile
-                rule_endpoints.append(Endpoint(
-                    name=pool_name,
-                    target_resource_id=pool_profile.id,
-                    priority=priority,
-                ))
 
                 priority += 1
                 pool_name = pool.get('fallback')
@@ -851,7 +853,7 @@ class AzureProvider(BaseProvider):
             # append default profile to the end
             rule_endpoints.append(Endpoint(
                 name='--default--',
-                target_resource_id=default_profile.id,
+                target=record.value[:-1],
                 priority=priority,
             ))
             # create rule profile with fallback chain

octodns/record/__init__.py

@@ -429,6 +429,10 @@ class _DynamicPool(object):
         ]
         values.sort(key=lambda d: d['value'])
 
+        # normalize weight of a single-value pool
+        if len(values) == 1:
+            values[0]['weight'] = 1
+
         fallback = data.get('fallback', None)
         self.data = {
             'fallback': fallback if fallback != 'default' else None,

tests/test_octodns_provider_azuredns.py

@@ -574,7 +574,7 @@ class TestAzureDnsProvider(TestCase):
                 },
                 'rules': [
                     {'geos': ['AF', 'EU-DE', 'NA-US-CA'], 'pool': 'one'},
-                    {'pool': 'three'},
+                    {'pool': 'two'},
                 ],
             },
             'octodns': {
@@ -605,36 +605,6 @@ class TestAzureDnsProvider(TestCase):
         nested = 'Microsoft.Network/trafficManagerProfiles/nestedEndpoints'
 
         return [
-            Profile(
-                id=id_format.format('default'),
-                name=name_format.format('default'),
-                traffic_routing_method='Weighted',
-                dns_config=dns,
-                monitor_config=monitor,
-                endpoints=[
-                    Endpoint(
-                        name='default.unit.tests',
-                        type=external,
-                        target='default.unit.tests',
-                        weight=1,
-                    ),
-                ],
-            ),
-            Profile(
-                id=id_format.format('pool-one'),
-                name=name_format.format('pool-one'),
-                traffic_routing_method='Weighted',
-                dns_config=dns,
-                monitor_config=monitor,
-                endpoints=[
-                    Endpoint(
-                        name='one.unit.tests',
-                        type=external,
-                        target='one.unit.tests',
-                        weight=11,
-                    ),
-                ],
-            ),
             Profile(
                 id=id_format.format('pool-two'),
                 name=name_format.format('pool-two'),
@@ -656,21 +626,6 @@ class TestAzureDnsProvider(TestCase):
                     ),
                 ],
             ),
-            Profile(
-                id=id_format.format('pool-three'),
-                name=name_format.format('pool-three'),
-                traffic_routing_method='Weighted',
-                dns_config=dns,
-                monitor_config=monitor,
-                endpoints=[
-                    Endpoint(
-                        name='three.unit.tests',
-                        type=external,
-                        target='three.unit.tests',
-                        weight=13,
-                    ),
-                ],
-            ),
             Profile(
                 id=id_format.format('rule-one'),
                 name=name_format.format('rule-one'),
@@ -680,8 +635,8 @@ class TestAzureDnsProvider(TestCase):
                 endpoints=[
                     Endpoint(
                         name='one',
-                        type=nested,
-                        target_resource_id=id_format.format('pool-one'),
+                        type=external,
+                        target='one.unit.tests',
                         priority=1,
                     ),
                     Endpoint(
@@ -692,37 +647,43 @@ class TestAzureDnsProvider(TestCase):
                     ),
                     Endpoint(
                         name='three',
-                        type=nested,
-                        target_resource_id=id_format.format('pool-three'),
+                        type=external,
+                        target='three.unit.tests',
                         priority=3,
                     ),
                     Endpoint(
                         name='--default--',
-                        type=nested,
-                        target_resource_id=id_format.format('default'),
+                        type=external,
+                        target='default.unit.tests',
                         priority=4,
                     ),
                 ],
             ),
             Profile(
-                id=id_format.format('rule-three'),
-                name=name_format.format('rule-three'),
+                id=id_format.format('rule-two'),
+                name=name_format.format('rule-two'),
                 traffic_routing_method='Priority',
                 dns_config=dns,
                 monitor_config=monitor,
                 endpoints=[
                     Endpoint(
-                        name='three',
+                        name='two',
                         type=nested,
-                        target_resource_id=id_format.format('pool-three'),
+                        target_resource_id=id_format.format('pool-two'),
                         priority=1,
                     ),
                     Endpoint(
-                        name='--default--',
-                        type=nested,
-                        target_resource_id=id_format.format('default'),
+                        name='three',
+                        type=external,
+                        target='three.unit.tests',
                         priority=2,
                     ),
+                    Endpoint(
+                        name='--default--',
+                        type=external,
+                        target='default.unit.tests',
+                        priority=3,
+                    ),
                 ],
             ),
             Profile(
@@ -740,9 +701,9 @@ class TestAzureDnsProvider(TestCase):
                     ),
                     Endpoint(
                         geo_mapping=['WORLD'],
-                        name='rule-three',
+                        name='rule-two',
                         type=nested,
-                        target_resource_id=id_format.format('rule-three'),
+                        target_resource_id=id_format.format('rule-two'),
                     ),
                 ],
             ),
@@ -964,7 +925,7 @@ class TestAzureDnsProvider(TestCase):
             'pools': {
                 'one': {
                     'values': [
-                        {'value': 'one.unit.tests.', 'weight': 11},
+                        {'value': 'one.unit.tests.', 'weight': 1},
                     ],
                     'fallback': 'two',
                 },
@@ -977,14 +938,14 @@ class TestAzureDnsProvider(TestCase):
                 },
                 'three': {
                     'values': [
-                        {'value': 'three.unit.tests.', 'weight': 13},
+                        {'value': 'three.unit.tests.', 'weight': 1},
                     ],
                     'fallback': None,
                 },
             },
             'rules': [
                 {'geos': ['AF', 'EU-DE', 'NA-US-CA'], 'pool': 'one'},
-                {'pool': 'three'},
+                {'pool': 'two'},
             ],
         })
 
@@ -1196,10 +1157,8 @@ class TestAzureDnsProvider(TestCase):
 
         suffix = 'foo-unit-tests'
         expected_seen = {
-            suffix, 'default--{}'.format(suffix),
-            'rule-one--{}'.format(suffix), 'rule-three--{}'.format(suffix),
-            'pool-one--{}'.format(suffix), 'pool-two--{}'.format(suffix),
-            'pool-three--{}'.format(suffix),
+            suffix, 'pool-two--{}'.format(suffix),
+            'rule-one--{}'.format(suffix), 'rule-two--{}'.format(suffix),
         }
 
         # test no change
@@ -1209,7 +1168,7 @@ class TestAzureDnsProvider(TestCase):
 
         # test that changing weight causes update API call
         dynamic = record.dynamic._data()
-        dynamic['pools']['one']['values'][0]['weight'] = 14
+        dynamic['pools']['two']['values'][0]['weight'] = 14
         data = {
             'type': 'CNAME',
             'ttl': record.ttl,
@@ -1225,7 +1184,7 @@ class TestAzureDnsProvider(TestCase):
 
         # test that new profile was successfully inserted in cache
         new_profile = provider._get_tm_profile_by_name(
-            'pool-one--{}'.format(suffix)
+            'pool-two--{}'.format(suffix)
         )
         self.assertEqual(new_profile.endpoints[0].weight, 14)
 
@@ -1257,10 +1216,8 @@ class TestAzureDnsProvider(TestCase):
         # implicitly asserts that non-matching profile is not included
         suffix = _traffic_manager_suffix(record)
         self.assertEqual(provider._find_traffic_managers(record), {
-            suffix, 'default--{}'.format(suffix),
-            'rule-one--{}'.format(suffix), 'rule-three--{}'.format(suffix),
-            'pool-one--{}'.format(suffix), 'pool-two--{}'.format(suffix),
-            'pool-three--{}'.format(suffix),
+            suffix, 'pool-two--{}'.format(suffix),
+            'rule-one--{}'.format(suffix), 'rule-two--{}'.format(suffix),
         })
 
     def test_traffic_manager_gc(self):

tests/test_octodns_record.py

@@ -3013,6 +3013,7 @@ class TestDynamicRecords(TestCase):
                 'pools': {
                     'one': {
                         'values': [{
+                            'weight': 10,
                             'value': '3.3.3.3',
                         }],
                     },