From a012e923f61547316d3e241d9e38b22a19ee1704 Mon Sep 17 00:00:00 2001 From: Joe Williams Date: Tue, 10 Oct 2017 13:54:52 -0700 Subject: [PATCH 1/4] add ability to configure update/delete thresholds --- octodns/provider/base.py | 25 ++++++++++++++++++++----- tests/test_octodns_provider_base.py | 2 ++ 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/octodns/provider/base.py b/octodns/provider/base.py index d2561ba..86175e7 100644 --- a/octodns/provider/base.py +++ b/octodns/provider/base.py @@ -21,10 +21,14 @@ class Plan(object): MAX_SAFE_DELETE_PCENT = .3 MIN_EXISTING_RECORDS = 10 - def __init__(self, existing, desired, changes): + def __init__(self, existing, desired, changes, + update_pcent_threshold=MAX_SAFE_UPDATE_PCENT, + delete_pcent_threshold=MAX_SAFE_DELETE_PCENT): self.existing = existing self.desired = desired self.changes = changes + self.update_pcent_threshold = update_pcent_threshold + self.delete_pcent_threshold = delete_pcent_threshold change_counts = { 'Create': 0, @@ -55,14 +59,19 @@ class Plan(object): 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: + self.log.debug('raise_if_unsafe: update_pcent_threshold=%d, ' + 'delete_pcent_threshold=%d', + self.update_pcent_threshold, + self.delete_pcent_threshold) + + if update_pcent > self.update_pcent_threshold: raise UnsafePlan('Too many updates, {} is over {} percent' '({}/{})'.format( update_pcent, self.MAX_SAFE_UPDATE_PCENT * 100, self.change_counts['Update'], existing_record_count)) - if delete_pcent > self.MAX_SAFE_DELETE_PCENT: + if delete_pcent > self.delete_pcent_threshold: raise UnsafePlan('Too many deletes, {} is over {} percent' '({}/{})'.format( delete_pcent, @@ -79,11 +88,15 @@ class Plan(object): class BaseProvider(BaseSource): - def __init__(self, id, apply_disabled=False): + def __init__(self, id, apply_disabled=False, + update_pcent_threshold=Plan.MAX_SAFE_UPDATE_PCENT, + delete_pcent_threshold=Plan.MAX_SAFE_DELETE_PCENT): super(BaseProvider, self).__init__(id) self.log.debug('__init__: id=%s, apply_disabled=%s', id, apply_disabled) self.apply_disabled = apply_disabled + self.update_pcent_threshold = update_pcent_threshold + self.delete_pcent_threshold = delete_pcent_threshold def _include_change(self, change): ''' @@ -124,7 +137,9 @@ class BaseProvider(BaseSource): changes += extra if changes: - plan = Plan(existing, desired, changes) + plan = Plan(existing, desired, changes, + self.update_pcent_threshold, + self.delete_pcent_threshold) self.log.info('plan: %s', plan) return plan self.log.info('plan: No changes') diff --git a/tests/test_octodns_provider_base.py b/tests/test_octodns_provider_base.py index e44adc0..f761405 100644 --- a/tests/test_octodns_provider_base.py +++ b/tests/test_octodns_provider_base.py @@ -23,6 +23,8 @@ class HelperProvider(BaseProvider): self.__extra_changes = extra_changes self.apply_disabled = apply_disabled self.include_change_callback = include_change_callback + self.update_pcent_threshold = Plan.MAX_SAFE_UPDATE_PCENT + self.delete_pcent_threshold = Plan.MAX_SAFE_DELETE_PCENT def populate(self, zone, target=False, lenient=False): pass From 50ac2f794cf01da8ac0cc48e5e462b019592da9a Mon Sep 17 00:00:00 2001 From: Joe Williams Date: Tue, 10 Oct 2017 14:39:25 -0700 Subject: [PATCH 2/4] add tests --- tests/test_octodns_provider_base.py | 56 +++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/test_octodns_provider_base.py b/tests/test_octodns_provider_base.py index f761405..fde1396 100644 --- a/tests/test_octodns_provider_base.py +++ b/tests/test_octodns_provider_base.py @@ -290,3 +290,59 @@ class TestBaseProvider(TestCase): Plan.MAX_SAFE_DELETE_PCENT))] Plan(zone, zone, changes).raise_if_unsafe() + + def test_safe_updates_min_existing_override(self): + safe_pcent = .4 + # 40% + 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 = [Update(record, record) + for i in range(int(Plan.MIN_EXISTING_RECORDS * + safe_pcent) + 1)] + + with self.assertRaises(UnsafePlan) as ctx: + Plan(zone, zone, changes, + update_pcent_threshold=safe_pcent).raise_if_unsafe() + + self.assertTrue('Too many updates' in ctx.exception.message) + + def test_safe_deletes_min_existing_override(self): + safe_pcent = .4 + # 40% + 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 * + safe_pcent) + 1)] + + with self.assertRaises(UnsafePlan) as ctx: + Plan(zone, zone, changes, + delete_pcent_threshold=safe_pcent).raise_if_unsafe() + + self.assertTrue('Too many deletes' in ctx.exception.message) From 3562b0dd4cf3abc5bc84d5a7abf38ac2c7766985 Mon Sep 17 00:00:00 2001 From: Joe Williams Date: Tue, 10 Oct 2017 15:05:58 -0700 Subject: [PATCH 3/4] log this in init --- octodns/provider/base.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/octodns/provider/base.py b/octodns/provider/base.py index 86175e7..f6ff1b7 100644 --- a/octodns/provider/base.py +++ b/octodns/provider/base.py @@ -59,11 +59,6 @@ class Plan(object): update_pcent = self.change_counts['Update'] / existing_record_count delete_pcent = self.change_counts['Delete'] / existing_record_count - self.log.debug('raise_if_unsafe: update_pcent_threshold=%d, ' - 'delete_pcent_threshold=%d', - self.update_pcent_threshold, - self.delete_pcent_threshold) - if update_pcent > self.update_pcent_threshold: raise UnsafePlan('Too many updates, {} is over {} percent' '({}/{})'.format( @@ -92,8 +87,12 @@ class BaseProvider(BaseSource): update_pcent_threshold=Plan.MAX_SAFE_UPDATE_PCENT, delete_pcent_threshold=Plan.MAX_SAFE_DELETE_PCENT): super(BaseProvider, self).__init__(id) - self.log.debug('__init__: id=%s, apply_disabled=%s', id, - apply_disabled) + self.log.debug('__init__: id=%s, apply_disabled=%s, ' + 'update_pcent_threshold=%d, delete_pcent_threshold=%d', + id, + apply_disabled, + update_pcent_threshold, + delete_pcent_threshold) self.apply_disabled = apply_disabled self.update_pcent_threshold = update_pcent_threshold self.delete_pcent_threshold = delete_pcent_threshold From ffeceb39b19ab85789e83cb2b72ea2cdba3f1425 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 13 Oct 2017 13:15:24 -0700 Subject: [PATCH 4/4] Handle Manager.dump with an empty Zone --- octodns/manager.py | 4 +++- tests/test_octodns_manager.py | 12 ++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/octodns/manager.py b/octodns/manager.py index 8439eb6..36a3592 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -11,7 +11,7 @@ from importlib import import_module from os import environ import logging -from .provider.base import BaseProvider +from .provider.base import BaseProvider, Plan from .provider.yaml import YamlProvider from .record import Record from .yaml import safe_load @@ -362,6 +362,8 @@ class Manager(object): source.populate(zone, lenient=lenient) plan = target.plan(zone) + if plan is None: + plan = Plan(zone, zone, []) target.apply(plan) def validate_configs(self): diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 45a3b55..a5f2022 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -11,6 +11,7 @@ from unittest import TestCase from octodns.record import Record from octodns.manager import _AggregateTarget, MainThreadExecutor, Manager +from octodns.yaml import safe_load from octodns.zone import Zone from helpers import GeoProvider, NoSshFpProvider, SimpleProvider, \ @@ -211,6 +212,17 @@ class TestManager(TestCase): with self.assertRaises(IOError): manager.dump('unknown.zone.', tmpdir.dirname, False, 'in') + def test_dump_empty(self): + with TemporaryDirectory() as tmpdir: + environ['YAML_TMP_DIR'] = tmpdir.dirname + manager = Manager(get_config_filename('simple.yaml')) + + manager.dump('empty.', tmpdir.dirname, False, 'in') + + with open(join(tmpdir.dirname, 'empty.yaml')) as fh: + data = safe_load(fh, False) + self.assertFalse(data) + def test_validate_configs(self): Manager(get_config_filename('simple-validate.yaml')).validate_configs()