Route53 status: up means no health check created

octodns/provider/route53.py

@@ -383,10 +383,9 @@ class _Route53DynamicValue(_Route53Record):
                         'ResourceRecordSet': existing,
                     }
 
-        return {
+        ret = {
             'Action': action,
             'ResourceRecordSet': {
-                'HealthCheckId': self.health_check_id,
                 'Name': self.fqdn,
                 'ResourceRecords': [{'Value': self.value}],
                 'SetIdentifier': self.identifer,
@@ -396,6 +395,11 @@ class _Route53DynamicValue(_Route53Record):
             }
         }
 
+        if self.health_check_id:
+            ret['ResourceRecordSet']['HealthCheckId'] = self.health_check_id
+
+        return ret
+
     def __hash__(self):
         return f'{self.fqdn}:{self._type}:{self.identifer}'.__hash__()
 
@@ -923,18 +927,15 @@ class Route53Provider(BaseProvider):
                     health_check_id = rrset.get('HealthCheckId', None)
                     health_check = self.health_checks[health_check_id]
                     health_check_config = health_check['HealthCheckConfig']
-                    if health_check_config['Disabled']:
-                        if health_check_config['Inverted']:
-                            # disabled and inverted means down
-                            status = 'down'
-                        else:
-                            # disabled means always up
-                            status = 'up'
+                    if health_check_config['Disabled'] and \
+                       health_check_config['Inverted']:
+                        # disabled and inverted means down
+                        status = 'down'
                     else:
                         # otherwise obey
                         status = 'obey'
                 except KeyError:
-                    # No healthcheck implies status is always up
+                    # No healthcheck means status is up
                     status = 'up'
                 pools[pool_name]['values'].append({
                     'status': status,
@@ -1152,6 +1153,11 @@ class Route53Provider(BaseProvider):
         self.log.debug('get_health_check_id: fqdn=%s, type=%s, value=%s, '
                        'status=%s', fqdn, record._type, value, status)
 
+        if status == 'up':
+            # status up means no health check
+            self.log.debug('get_health_check_id:   status up, no health check')
+            return None
+
         try:
             ip_address(str(value))
             # We're working with an IP, host is the Host header
@@ -1166,13 +1172,10 @@ class Route53Provider(BaseProvider):
         healthcheck_port = record.healthcheck_port
         healthcheck_latency = self._healthcheck_measure_latency(record)
         healthcheck_interval = self._healthcheck_request_interval(record)
-        if status == 'up':
-            healthcheck_disabled = True
-            healthcheck_inverted = False
-        elif status == 'down':
+        if status == 'down':
             healthcheck_disabled = True
             healthcheck_inverted = True
-        else:
+        else:  # obey
             healthcheck_disabled = False
             healthcheck_inverted = False
 

tests/test_octodns_provider_route53.py

@@ -58,7 +58,6 @@ dynamic_rrsets = [{
     'Type': 'A',
     'Weight': 2
 }, {
-    'HealthCheckId': '09',
     'Name': '_octodns-ap-southeast-1-value.unit.tests.',
     'ResourceRecords': [{'Value': '1.4.1.2'}],
     'SetIdentifier': 'ap-southeast-1-001',
@@ -199,12 +198,6 @@ dynamic_health_checks = {
             'Inverted': False,
         }
     },
-    '09': {
-        'HealthCheckConfig': {
-            'Disabled': True,
-            'Inverted': False,
-        }
-    },
     'ab': {
         'HealthCheckConfig': {
             'Disabled': True,
@@ -1368,7 +1361,7 @@ class TestRoute53Provider(TestCase):
         self.assertEquals('42',
                           provider.get_health_check_id(record, '1.1.1.1',
                                                        'obey', False))
-        self.assertEquals('43',
+        self.assertEquals(None,
                           provider.get_health_check_id(record, '2.2.2.2',
                                                        'up', False))
         self.assertEquals('44',
@@ -3129,7 +3122,7 @@ class TestRoute53Records(TestCase):
 
         # If we don't provide the candidate rrsets we get back exactly what we
         # put in minus the healthcheck
-        rrset['HealthCheckId'] = None
+        del rrset['HealthCheckId']
         mod = geo.mod('DELETE', [])
         self.assertEquals(rrset, mod['ResourceRecordSet'])