Implement Route53Provider mod ordering via a sort

This will ensure that deletes come before creates which are before upserts and
that records that uses aliases always come after their target (though implicitly
based on sorting types and not explicitly by looking at them.)

octodns/provider/route53.py

@@ -235,6 +235,35 @@ class _Route53GeoRecord(_Route53Record):
                                                           self.values)
 
 
+_mod_keyer_action_order = {
+    'DELETE': '0',  # Delete things first
+    'CREATE': '1',  # Then Create things
+    'UPSERT': '2',  # Upsert things last
+}
+
+
+def _mod_keyer(mod):
+    rrset = mod['ResourceRecordSet']
+    action_order = _mod_keyer_action_order[mod['Action']]
+
+    # We're sorting by 3 "columns", the action, the rrset type, and finally the
+    # name/id of the rrset
+
+    if rrset.get('GeoLocation', False):
+        return '{}3{}'.format(action_order, rrset['SetIdentifier'])
+    elif rrset.get('AliasTarget', False):
+        # We use an alias
+        if rrset.get('Failover', False) == 'SECONDARY':
+            # We're a secondary we'll ref primaries
+            return '{}2{}'.format(action_order, rrset['Name'])
+        else:
+            # We're a primary we'll ref values
+            return '{}1{}'.format(action_order, rrset['Name'])
+
+    # We're just a plain value, these come first
+    return '{}0{}'.format(action_order, rrset['Name'])
+
+
 class Route53Provider(BaseProvider):
     '''
     AWS Route53 Provider
@@ -827,6 +856,11 @@ class Route53Provider(BaseProvider):
             # Generate the mods for this change
             mod_type = getattr(self, '_mod_{}'.format(c.__class__.__name__))
             mods = mod_type(c, zone_id)
+
+            # Order our mods to make sure targets exist before alises point to
+            # them and we CRUD in the desired order
+            mods.sort(key=_mod_keyer)
+
             mods_rs_count = sum(
                 [len(m['ResourceRecordSet']['ResourceRecords']) for m in mods]
             )

tests/test_octodns_provider_route53.py

@@ -12,7 +12,7 @@ from mock import patch
 
 from octodns.record import Create, Delete, Record, Update
 from octodns.provider.route53 import Route53Provider, _Route53GeoDefault, \
-    _Route53GeoRecord, _Route53Record, _octal_replace
+    _Route53GeoRecord, _Route53Record, _mod_keyer, _octal_replace
 from octodns.zone import Zone
 
 from helpers import GeoProvider
@@ -529,23 +529,23 @@ class TestRoute53Provider(TestCase):
                 }, {
                     'Action': 'UPSERT',
                     'ResourceRecordSet': {
-                        'GeoLocation': {'CountryCode': '*'},
+                        'GeoLocation': {'CountryCode': 'US'},
+                        'HealthCheckId': u'43',
                         'Name': 'unit.tests.',
-                        'ResourceRecords': [{'Value': '2.2.3.4'},
-                                            {'Value': '3.2.3.4'}],
-                        'SetIdentifier': 'default',
+                        'ResourceRecords': [{'Value': '5.2.3.4'},
+                                            {'Value': '6.2.3.4'}],
+                        'SetIdentifier': 'NA-US',
                         'TTL': 61,
                         'Type': 'A'
                     }
                 }, {
                     'Action': 'UPSERT',
                     'ResourceRecordSet': {
-                        'GeoLocation': {'CountryCode': 'US'},
-                        'HealthCheckId': u'43',
+                        'GeoLocation': {'CountryCode': '*'},
                         'Name': 'unit.tests.',
-                        'ResourceRecords': [{'Value': '5.2.3.4'},
-                                            {'Value': '6.2.3.4'}],
-                        'SetIdentifier': 'NA-US',
+                        'ResourceRecords': [{'Value': '2.2.3.4'},
+                                            {'Value': '3.2.3.4'}],
+                        'SetIdentifier': 'default',
                         'TTL': 61,
                         'Type': 'A'
                     }
@@ -588,21 +588,21 @@ class TestRoute53Provider(TestCase):
                 'Changes': [{
                     'Action': 'DELETE',
                     'ResourceRecordSet': {
-                        'GeoLocation': {'CountryCode': '*'},
+                        'GeoLocation': {'ContinentCode': 'OC'},
                         'Name': 'simple.unit.tests.',
-                        'ResourceRecords': [{'Value': '1.2.3.4'},
-                                            {'Value': '2.2.3.4'}],
-                        'SetIdentifier': 'default',
+                        'ResourceRecords': [{'Value': '3.2.3.4'},
+                                            {'Value': '4.2.3.4'}],
+                        'SetIdentifier': 'OC',
                         'TTL': 61,
                         'Type': 'A'}
                 }, {
                     'Action': 'DELETE',
                     'ResourceRecordSet': {
-                        'GeoLocation': {'ContinentCode': 'OC'},
+                        'GeoLocation': {'CountryCode': '*'},
                         'Name': 'simple.unit.tests.',
-                        'ResourceRecords': [{'Value': '3.2.3.4'},
-                                            {'Value': '4.2.3.4'}],
-                        'SetIdentifier': 'OC',
+                        'ResourceRecords': [{'Value': '1.2.3.4'},
+                                            {'Value': '2.2.3.4'}],
+                        'SetIdentifier': 'default',
                         'TTL': 61,
                         'Type': 'A'}
                 }, {
@@ -1623,3 +1623,67 @@ class TestRoute53Records(TestCase):
         a.__repr__()
         e.__repr__()
         f.__repr__()
+
+
+class TestModKeyer(TestCase):
+
+    def test_mod_keyer(self):
+
+        # First "column"
+
+        # Deletes come first
+        self.assertEquals('00something', _mod_keyer({
+            'Action': 'DELETE',
+            'ResourceRecordSet': {
+                'Name': 'something',
+            }
+        }))
+
+        # Creates come next
+        self.assertEquals('10another', _mod_keyer({
+            'Action': 'CREATE',
+            'ResourceRecordSet': {
+                'Name': 'another',
+            }
+        }))
+
+        # Then upserts
+        self.assertEquals('20last', _mod_keyer({
+            'Action': 'UPSERT',
+            'ResourceRecordSet': {
+                'Name': 'last',
+            }
+        }))
+
+        # Second "column" value records tested above
+
+        # AliasTarget primary second (to value)
+        self.assertEquals('01thing', _mod_keyer({
+            'Action': 'DELETE',
+            'ResourceRecordSet': {
+                'AliasTarget': 'some-target',
+                'Failover': 'PRIMARY',
+                'Name': 'thing',
+            }
+        }))
+
+        # AliasTarget secondary third
+        self.assertEquals('02thing', _mod_keyer({
+            'Action': 'DELETE',
+            'ResourceRecordSet': {
+                'AliasTarget': 'some-target',
+                'Failover': 'SECONDARY',
+                'Name': 'thing',
+            }
+        }))
+
+        # GeoLocation fourth
+        self.assertEquals('03some-id', _mod_keyer({
+            'Action': 'DELETE',
+            'ResourceRecordSet': {
+                'GeoLocation': 'some-target',
+                'SetIdentifier': 'some-id',
+            }
+        }))
+
+        # The third "column" has already been tested above, Name/SetIdentifier