Rework and clean up Route53Provider's extra_changes check to support dynamic

octodns/provider/route53.py

@@ -1127,8 +1127,83 @@ class Route53Provider(BaseProvider):
         self._gc_health_checks(change.existing, [])
         return self._gen_mods('DELETE', existing_records)
 
+    def _extra_changes_update_needed(self, record, rrset):
+        healthcheck_host = record.healthcheck_host
+        healthcheck_path = record.healthcheck_path
+        healthcheck_protocol = record.healthcheck_protocol
+        healthcheck_port = record.healthcheck_port
+        healthcheck_latency = self._healthcheck_measure_latency(record)
+
+        try:
+            health_check_id = rrset['HealthCheckId']
+            health_check = self.health_checks[health_check_id]
+            caller_ref = health_check['CallerReference']
+            if caller_ref.startswith(self.HEALTH_CHECK_VERSION):
+                if self._health_check_equivilent(healthcheck_host,
+                                                 healthcheck_path,
+                                                 healthcheck_protocol,
+                                                 healthcheck_port,
+                                                 healthcheck_latency,
+                                                 health_check):
+                    # it has the right health check
+                    return False
+        except (IndexError, KeyError):
+            # no health check id or one that isn't the right version
+            pass
+
+        # no good, doesn't have the right health check, needs an update
+        self.log.info('_extra_changes_update_needed: health-check caused '
+                      'update of %s:%s', record.fqdn, record._type)
+        return True
+
+    def _extra_changes_geo_needs_update(self, zone_id, record):
+        # OK this is a record we don't have change for that does have geo
+        # information. We need to look and see if it needs to be updated b/c of
+        # a health check version bump or other mismatch
+        self.log.debug('_extra_changes_geo_needs_update: inspecting=%s, %s',
+                       record.fqdn, record._type)
+
+        fqdn = record.fqdn
+
+        # loop through all the r53 rrsets
+        for rrset in self._load_records(zone_id):
+            if fqdn == rrset['Name'] and record._type == rrset['Type'] and \
+               rrset.get('GeoLocation', {}).get('CountryCode', False) != '*' \
+               and self._extra_changes_update_needed(record, rrset):
+                # no good, doesn't have the right health check, needs an update
+                self.log.info('_extra_changes_geo_needs_update: health-check '
+                              'caused update of %s:%s', record.fqdn,
+                              record._type)
+                return True
+
+        return False
+
+    def _extra_changes_dynamic_needs_update(self, zone_id, record):
+        # OK this is a record we don't have change for that does have dynamic
+        # information. We need to look and see if it needs to be updated b/c of
+        # a health check version bump or other mismatch
+        self.log.debug('_extra_changes_dynamic_needs_update: inspecting=%s, '
+                       '%s', record.fqdn, record._type)
+
+        fqdn = record.fqdn
+
+        # loop through all the r53 rrsets
+        for rrset in self._load_records(zone_id):
+            name = rrset['Name']
+
+            if record._type == rrset['Type'] and name.endswith(fqdn) and \
+               name.startswith('_octodns-') and '-value.' in name and \
+               '-default-' not in name and \
+               self._extra_changes_update_needed(record, rrset):
+                # no good, doesn't have the right health check, needs an update
+                self.log.info('_extra_changes_dynamic_needs_update: '
+                              'health-check caused update of %s:%s',
+                              record.fqdn, record._type)
+                return True
+
+        return False
+
     def _extra_changes(self, desired, changes, **kwargs):
-        # TODO: dynamic records extra changes...
         self.log.debug('_extra_changes: desired=%s', desired.name)
         zone_id = self._get_zone_id(desired.name)
         if not zone_id:
@@ -1138,61 +1213,20 @@ class Route53Provider(BaseProvider):
         changed = set([c.record for c in changes])
         # ok, now it's time for the reason we're here, we need to go over all
         # the desired records
-        extra = []
+        extras = []
         for record in desired.records:
             if record in changed:
                 # already have a change for it, skipping
                 continue
-            if not getattr(record, 'geo', False):
-                # record doesn't support geo, we don't need to inspect it
-                continue
-            # OK this is a record we don't have change for that does have geo
-            # information. We need to look and see if it needs to be updated
-            # b/c of a health check version bump
-            self.log.debug('_extra_changes:   inspecting=%s, %s', record.fqdn,
-                           record._type)
 
-            healthcheck_host = record.healthcheck_host
-            healthcheck_path = record.healthcheck_path
-            healthcheck_protocol = record.healthcheck_protocol
-            healthcheck_port = record.healthcheck_port
-            healthcheck_latency = self._healthcheck_measure_latency(record)
-            fqdn = record.fqdn
-
-            # loop through all the r53 rrsets
-            for rrset in self._load_records(zone_id):
-                if fqdn != rrset['Name'] or record._type != rrset['Type']:
-                    # not a name and type match
-                    continue
-                if rrset.get('GeoLocation', {}) \
-                   .get('CountryCode', False) == '*':
-                    # it's a default record
-                    continue
-                # we expect a healthcheck now
-                try:
-                    health_check_id = rrset['HealthCheckId']
-                    health_check = self.health_checks[health_check_id]
-                    caller_ref = health_check['CallerReference']
-                    if caller_ref.startswith(self.HEALTH_CHECK_VERSION):
-                        if self._health_check_equivilent(healthcheck_host,
-                                                         healthcheck_path,
-                                                         healthcheck_protocol,
-                                                         healthcheck_port,
-                                                         healthcheck_latency,
-                                                         health_check):
-                            # it has the right health check
-                            continue
-                except (IndexError, KeyError):
-                    # no health check id or one that isn't the right version
-                    pass
-                # no good, doesn't have the right health check, needs an update
-                self.log.info('_extra_changes:     health-check caused '
-                              'update of %s:%s', record.fqdn, record._type)
-                extra.append(Update(record, record))
-                # We don't need to process this record any longer
-                break
+            if getattr(record, 'geo', False):
+                if self._extra_changes_geo_needs_update(zone_id, record):
+                    extras.append(Update(record, record))
+            elif getattr(record, 'dynamic', False):
+                if self._extra_changes_dynamic_needs_update(zone_id, record):
+                    extras.append(Update(record, record))
 
-        return extra
+        return extras
 
     def _apply(self, plan):
         desired = plan.desired

tests/test_octodns_provider_route53.py

@@ -1616,6 +1616,119 @@ class TestRoute53Provider(TestCase):
         self.assertEquals(1, len(extra))
         stubber.assert_no_pending_responses()
 
+    def test_extra_change_dynamic_has_health_check(self):
+        provider, stubber = self._get_stubbed_provider()
+
+        list_hosted_zones_resp = {
+            'HostedZones': [{
+                'Name': 'unit.tests.',
+                'Id': 'z42',
+                'CallerReference': 'abc',
+            }],
+            'Marker': 'm',
+            'IsTruncated': False,
+            'MaxItems': '100',
+        }
+        stubber.add_response('list_hosted_zones', list_hosted_zones_resp, {})
+
+        # record with geo and no health check returns change
+        desired = Zone('unit.tests.', [])
+        record = Record.new(desired, 'a', {
+            'ttl': 30,
+            'type': 'A',
+            'value': '1.2.3.4',
+            'dynamic': {
+                'pools': {
+                    'one': {
+                        'values': [{
+                            'value': '2.2.3.4',
+                        }],
+                    },
+                },
+                'rules': [{
+                    'pool': 'one',
+                }],
+            },
+        })
+        desired.add_record(record)
+        list_resource_record_sets_resp = {
+            'ResourceRecordSets': [{
+                # other name
+                'Name': 'unit.tests.',
+                'Type': 'A',
+                'GeoLocation': {
+                    'CountryCode': '*',
+                },
+                'ResourceRecords': [{
+                    'Value': '1.2.3.4',
+                }],
+                'TTL': 61,
+            }, {
+                # matching name, other type
+                'Name': 'a.unit.tests.',
+                'Type': 'AAAA',
+                'ResourceRecords': [{
+                    'Value': '2001:0db8:3c4d:0015:0000:0000:1a2f:1a4b'
+                }],
+                'TTL': 61,
+            }, {
+                # default value pool
+                'Name': '_octodns-default-pool.a.unit.tests.',
+                'Type': 'A',
+                'GeoLocation': {
+                    'CountryCode': '*',
+                },
+                'ResourceRecords': [{
+                    'Value': '1.2.3.4',
+                }],
+                'TTL': 61,
+            }, {
+                # match
+                'Name': '_octodns-one-value.a.unit.tests.',
+                'Type': 'A',
+                'ResourceRecords': [{
+                    'Value': '2.2.3.4',
+                }],
+                'TTL': 61,
+                'HealthCheckId': '42',
+            }],
+            'IsTruncated': False,
+            'MaxItems': '100',
+        }
+        stubber.add_response('list_resource_record_sets',
+                             list_resource_record_sets_resp,
+                             {'HostedZoneId': 'z42'})
+        stubber.add_response('list_health_checks', {
+            'HealthChecks': [{
+                'Id': '42',
+                'CallerReference': self.caller_ref,
+                'HealthCheckConfig': {
+                    'Type': 'HTTPS',
+                    'FullyQualifiedDomainName': 'a.unit.tests',
+                    'IPAddress': '2.2.3.4',
+                    'ResourcePath': '/_dns',
+                    'Type': 'HTTPS',
+                    'Port': 443,
+                    'MeasureLatency': True
+                },
+                'HealthCheckVersion': 2,
+            }],
+            'IsTruncated': False,
+            'MaxItems': '100',
+            'Marker': '',
+        })
+        extra = provider._extra_changes(desired=desired, changes=[])
+        self.assertEquals(0, len(extra))
+        stubber.assert_no_pending_responses()
+
+        # change b/c of healthcheck path
+        record._octodns['healthcheck'] = {
+            'path': '/_ready'
+        }
+        extra = provider._extra_changes(desired=desired, changes=[])
+        self.assertEquals(1, len(extra))
+        stubber.assert_no_pending_responses()
+
         # change b/c of healthcheck host
         record._octodns['healthcheck'] = {
             'host': 'foo.bar.io'