Merge pull request #40 from github/failsafe

use a percentage of change/deleted records rather than an absolute count

octodns/provider/base.py

@@ -7,6 +7,7 @@ from __future__ import absolute_import, division, print_function, \
 
 from ..source.base import BaseSource
 from ..zone import Zone
+from logging import getLogger
 
 
 class UnsafePlan(Exception):
@@ -14,8 +15,11 @@ class UnsafePlan(Exception):
 
 
 class Plan(object):
-    MAX_SAFE_UPDATES = 4
-    MAX_SAFE_DELETES = 4
+    log = getLogger('Plan')
+
+    MAX_SAFE_UPDATE_PCENT = .3
+    MAX_SAFE_DELETE_PCENT = .3
+    MIN_EXISTING_RECORDS = 10
 
     def __init__(self, existing, desired, changes):
         self.existing = existing
@@ -31,12 +35,40 @@ class Plan(object):
             change_counts[change.__class__.__name__] += 1
         self.change_counts = change_counts
 
+        try:
+            existing_n = len(self.existing.records)
+        except AttributeError:
+            existing_n = 0
+
+        self.log.debug('__init__: Creates=%d, Updates=%d, Deletes=%d'
+                       'Existing=%d',
+                       self.change_counts['Create'],
+                       self.change_counts['Update'],
+                       self.change_counts['Delete'], existing_n)
+
     def raise_if_unsafe(self):
         # TODO: what is safe really?
-        if self.change_counts['Update'] > self.MAX_SAFE_UPDATES:
-            raise UnsafePlan('Too many updates')
-        if self.change_counts['Delete'] > self.MAX_SAFE_DELETES:
-            raise UnsafePlan('Too many deletes')
+        if self.existing and \
+           len(self.existing.records) >= self.MIN_EXISTING_RECORDS:
+
+            existing_record_count = len(self.existing.records)
+            update_pcent = self.change_counts['Update'] / existing_record_count
+            delete_pcent = self.change_counts['Delete'] / existing_record_count
+
+            if update_pcent > self.MAX_SAFE_UPDATE_PCENT:
+                raise UnsafePlan('Too many updates, %s is over %s percent'
+                                 '(%s/%s)',
+                                 update_pcent,
+                                 self.MAX_SAFE_UPDATE_PCENT * 100,
+                                 self.change_counts['Update'],
+                                 existing_record_count)
+            if delete_pcent > self.MAX_SAFE_DELETE_PCENT:
+                raise UnsafePlan('Too many deletes, %s is over %s percent'
+                                 '(%s/%s)',
+                                 delete_pcent,
+                                 self.MAX_SAFE_DELETE_PCENT * 100,
+                                 self.change_counts['Delete'],
+                                 existing_record_count)
 
     def __repr__(self):
         return 'Creates={}, Updates={}, Deletes={}, Existing Records={}' \

tests/test_octodns_provider_base.py

@@ -138,33 +138,140 @@ class TestBaseProvider(TestCase):
         # We filtered out the only change
         self.assertFalse(plan)
 
-    def test_safe(self):
-        ignored = Zone('unit.tests.', [])
-
+    def test_safe_none(self):
         # No changes is safe
         Plan(None, None, []).raise_if_unsafe()
 
-        # Creates are safe
-        record = Record.new(ignored, 'a', {
+    def test_safe_creates(self):
+        # Creates are safe when existing records is under MIN_EXISTING_RECORDS
+        zone = Zone('unit.tests.', [])
+
+        record = Record.new(zone, 'a', {
+            'ttl': 30,
+            'type': 'A',
+            'value': '1.2.3.4',
+        })
+        Plan(zone, zone, [Create(record) for i in range(10)]).raise_if_unsafe()
+
+    def test_safe_min_existing_creates(self):
+        # Creates are safe when existing records is over MIN_EXISTING_RECORDS
+        zone = Zone('unit.tests.', [])
+
+        record = Record.new(zone, 'a', {
+            'ttl': 30,
+            'type': 'A',
+            'value': '1.2.3.4',
+        })
+
+        for i in range(int(Plan.MIN_EXISTING_RECORDS)):
+            zone.add_record(Record.new(zone, str(i), {
+                            'ttl': 60,
+                            'type': 'A',
+                            'value': '2.3.4.5'
+                            }))
+
+        Plan(zone, zone, [Create(record) for i in range(10)]).raise_if_unsafe()
+
+    def test_safe_no_existing(self):
+        # existing records fewer than MIN_EXISTING_RECORDS is safe
+        zone = Zone('unit.tests.', [])
+        record = Record.new(zone, 'a', {
+            'ttl': 30,
+            'type': 'A',
+            'value': '1.2.3.4',
+        })
+
+        updates = [Update(record, record), Update(record, record)]
+        Plan(zone, zone, updates).raise_if_unsafe()
+
+    def test_safe_updates_min_existing(self):
+        # MAX_SAFE_UPDATE_PCENT+1 fails when more
+        # than MIN_EXISTING_RECORDS exist
+        zone = Zone('unit.tests.', [])
+        record = Record.new(zone, 'a', {
             'ttl': 30,
             'type': 'A',
             'value': '1.2.3.4',
         })
-        Plan(None, None, [Create(record) for i in range(10)]).raise_if_unsafe()
 
-        # max Updates is safe
+        for i in range(int(Plan.MIN_EXISTING_RECORDS)):
+            zone.add_record(Record.new(zone, str(i), {
+                            'ttl': 60,
+                            'type': 'A',
+                            'value': '2.3.4.5'
+                            }))
+
         changes = [Update(record, record)
-                   for i in range(Plan.MAX_SAFE_UPDATES)]
-        Plan(None, None, changes).raise_if_unsafe()
-        # but max + 1 isn't
+                   for i in range(int(Plan.MIN_EXISTING_RECORDS *
+                                      Plan.MAX_SAFE_UPDATE_PCENT) + 1)]
+
         with self.assertRaises(UnsafePlan):
-            changes.append(Update(record, record))
-            Plan(None, None, changes).raise_if_unsafe()
+            Plan(zone, zone, changes).raise_if_unsafe()
+
+    def test_safe_updates_min_existing_pcent(self):
+        # MAX_SAFE_UPDATE_PCENT is safe when more
+        # than MIN_EXISTING_RECORDS exist
+        zone = Zone('unit.tests.', [])
+        record = Record.new(zone, 'a', {
+            'ttl': 30,
+            'type': 'A',
+            'value': '1.2.3.4',
+        })
+
+        for i in range(int(Plan.MIN_EXISTING_RECORDS)):
+            zone.add_record(Record.new(zone, str(i), {
+                            'ttl': 60,
+                            'type': 'A',
+                            'value': '2.3.4.5'
+                            }))
+        changes = [Update(record, record)
+                   for i in range(int(Plan.MIN_EXISTING_RECORDS *
+                                      Plan.MAX_SAFE_UPDATE_PCENT))]
+
+        Plan(zone, zone, changes).raise_if_unsafe()
+
+    def test_safe_deletes_min_existing(self):
+        # MAX_SAFE_DELETE_PCENT+1 fails when more
+        # than MIN_EXISTING_RECORDS exist
+        zone = Zone('unit.tests.', [])
+        record = Record.new(zone, 'a', {
+            'ttl': 30,
+            'type': 'A',
+            'value': '1.2.3.4',
+        })
+
+        for i in range(int(Plan.MIN_EXISTING_RECORDS)):
+            zone.add_record(Record.new(zone, str(i), {
+                            'ttl': 60,
+                            'type': 'A',
+                            'value': '2.3.4.5'
+                            }))
+
+        changes = [Delete(record)
+                   for i in range(int(Plan.MIN_EXISTING_RECORDS *
+                                      Plan.MAX_SAFE_DELETE_PCENT) + 1)]
 
-        # max Deletes is safe
-        changes = [Delete(record) for i in range(Plan.MAX_SAFE_DELETES)]
-        Plan(None, None, changes).raise_if_unsafe()
-        # but max + 1 isn't
         with self.assertRaises(UnsafePlan):
-            changes.append(Delete(record))
-            Plan(None, None, changes).raise_if_unsafe()
+            Plan(zone, zone, changes).raise_if_unsafe()
+
+    def test_safe_deletes_min_existing_pcent(self):
+        # MAX_SAFE_DELETE_PCENT is safe when more
+        # than MIN_EXISTING_RECORDS exist
+        zone = Zone('unit.tests.', [])
+        record = Record.new(zone, 'a', {
+            'ttl': 30,
+            'type': 'A',
+            'value': '1.2.3.4',
+        })
+
+        for i in range(int(Plan.MIN_EXISTING_RECORDS)):
+            zone.add_record(Record.new(zone, str(i), {
+                            'ttl': 60,
+                            'type': 'A',
+                            'value': '2.3.4.5'
+                            }))
+        changes = [Delete(record)
+                   for i in range(int(Plan.MIN_EXISTING_RECORDS *
+                                      Plan.MAX_SAFE_DELETE_PCENT))]
+
+        Plan(zone, zone, changes).raise_if_unsafe()