Merge pull request #466 from github/ns1-configure-monitors

Add a way to configure Ns1Provider monitoring regions for records

octodns/provider/ns1.py

@@ -175,6 +175,9 @@ class Ns1Provider(BaseProvider):
     ns1:
         class: octodns.provider.ns1.Ns1Provider
         api_key: env/NS1_API_KEY
+        # Only required if using dynamic records
+        monitor_regions:
+          - lga
     '''
     SUPPORTS_GEO = True
     SUPPORTS_DYNAMIC = True
@@ -224,11 +227,13 @@ class Ns1Provider(BaseProvider):
         'NA': ('US-CENTRAL', 'US-EAST', 'US-WEST'),
     }
 
-    def __init__(self, id, api_key, retry_count=4, *args, **kwargs):
+    def __init__(self, id, api_key, retry_count=4, monitor_regions=None, *args,
+                 **kwargs):
         self.log = getLogger('Ns1Provider[{}]'.format(id))
-        self.log.debug('__init__: id=%s, api_key=***, retry_count=%d', id,
-                       retry_count)
+        self.log.debug('__init__: id=%s, api_key=***, retry_count=%d, '
+                       'monitor_regions=%s', id, retry_count, monitor_regions)
         super(Ns1Provider, self).__init__(id, *args, **kwargs)
+        self.monitor_regions = monitor_regions
 
         self._client = Ns1Client(api_key, retry_count)
 
@@ -679,8 +684,7 @@ class Ns1Provider(BaseProvider):
             'policy': 'quorum',
             'rapid_recheck': False,
             'region_scope': 'fixed',
-            # TODO: what should we do here dal, sjc, lga, sin, ams
-            'regions': ['lga'],
+            'regions': self.monitor_regions,
             'rules': [{
                 'comparison': 'contains',
                 'key': 'output',
@@ -977,12 +981,25 @@ class Ns1Provider(BaseProvider):
         self._client.records_delete(zone, domain, _type)
         self._monitors_gc(existing)
 
+    def _has_dynamic(self, changes):
+        for change in changes:
+            if getattr(change.record, 'dynamic', False):
+                return True
+
+        return False
+
     def _apply(self, plan):
         desired = plan.desired
         changes = plan.changes
         self.log.debug('_apply: zone=%s, len(changes)=%d', desired.name,
                        len(changes))
 
+        # Make sure that if we're going to make any dynamic changes that we
+        # have monitor_regions configured before touching anything so we can
+        # abort early and not half-apply
+        if self._has_dynamic(changes) and self.monitor_regions is None:
+            raise Ns1Exception('Monitored record, but monitor_regions not set')
+
         domain_name = desired.name[:-1]
         try:
             ns1_zone = self._client.zones_retrieve(domain_name)

tests/test_octodns_provider_ns1.py

@@ -14,6 +14,7 @@ from unittest import TestCase
 
 from octodns.record import Delete, Record, Update
 from octodns.provider.ns1 import Ns1Client, Ns1Exception, Ns1Provider
+from octodns.provider.plan import Plan
 from octodns.zone import Zone
 
 
@@ -1198,6 +1199,104 @@ class TestNs1ProviderDynamic(TestCase):
         self.assertFalse(extra)
         monitors_for_mock.assert_not_called()
 
+    DESIRED = Zone('unit.tests.', [])
+
+    SIMPLE = Record.new(DESIRED, 'sim', {
+        'ttl': 33,
+        'type': 'A',
+        'value': '1.2.3.4',
+    })
+
+    # Dynamic record, inspectable
+    DYNAMIC = Record.new(DESIRED, 'dyn', {
+        'dynamic': {
+            'pools': {
+                'iad': {
+                    'values': [{
+                        'value': '1.2.3.4',
+                    }],
+                },
+            },
+            'rules': [{
+                'pool': 'iad',
+            }],
+        },
+        'octodns': {
+            'healthcheck': {
+                'host': 'send.me',
+                'path': '/_ping',
+                'port': 80,
+                'protocol': 'HTTP',
+            }
+        },
+        'ttl': 32,
+        'type': 'A',
+        'value': '1.2.3.4',
+        'meta': {},
+    })
+
+    def test_has_dynamic(self):
+        provider = Ns1Provider('test', 'api-key')
+
+        simple_update = Update(self.SIMPLE, self.SIMPLE)
+        dynamic_update = Update(self.DYNAMIC, self.DYNAMIC)
+
+        self.assertFalse(provider._has_dynamic([simple_update]))
+        self.assertTrue(provider._has_dynamic([dynamic_update]))
+        self.assertTrue(provider._has_dynamic([simple_update, dynamic_update]))
+
+    @patch('octodns.provider.ns1.Ns1Client.zones_retrieve')
+    @patch('octodns.provider.ns1.Ns1Provider._apply_Update')
+    def test_apply_monitor_regions(self, apply_update_mock,
+                                   zones_retrieve_mock):
+        provider = Ns1Provider('test', 'api-key')
+
+        simple_update = Update(self.SIMPLE, self.SIMPLE)
+        simple_plan = Plan(self.DESIRED, self.DESIRED, [simple_update], True)
+        dynamic_update = Update(self.DYNAMIC, self.DYNAMIC)
+        dynamic_update = Update(self.DYNAMIC, self.DYNAMIC)
+        dynamic_plan = Plan(self.DESIRED, self.DESIRED, [dynamic_update],
+                            True)
+        both_plan = Plan(self.DESIRED, self.DESIRED, [simple_update,
+                                                      dynamic_update], True)
+
+        # always return foo, we aren't testing this part here
+        zones_retrieve_mock.side_effect = [
+            'foo',
+            'foo',
+            'foo',
+            'foo',
+        ]
+
+        # Doesn't blow up, and calls apply once
+        apply_update_mock.reset_mock()
+        provider._apply(simple_plan)
+        apply_update_mock.assert_has_calls([call('foo', simple_update)])
+
+        # Blows up and apply not called
+        apply_update_mock.reset_mock()
+        with self.assertRaises(Ns1Exception) as ctx:
+            provider._apply(dynamic_plan)
+        self.assertTrue('monitor_regions not set' in text_type(ctx.exception))
+        apply_update_mock.assert_not_called()
+
+        # Blows up and apply not called even though there's a simple
+        apply_update_mock.reset_mock()
+        with self.assertRaises(Ns1Exception) as ctx:
+            provider._apply(both_plan)
+        self.assertTrue('monitor_regions not set' in text_type(ctx.exception))
+        apply_update_mock.assert_not_called()
+
+        # with monitor_regions set
+        provider.monitor_regions = ['lga']
+
+        apply_update_mock.reset_mock()
+        provider._apply(both_plan)
+        apply_update_mock.assert_has_calls([
+            call('foo', dynamic_update),
+            call('foo', simple_update),
+        ])
+
 
 class TestNs1Client(TestCase):