From 1e71bce9072838ce0385704502d76813ec8794c0 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 13:47:58 -0800 Subject: [PATCH 01/15] Add create param to Plan --- octodns/manager.py | 2 +- octodns/provider/base.py | 3 ++- octodns/provider/plan.py | 3 ++- tests/test_octodns_plan.py | 4 ++-- tests/test_octodns_provider_azuredns.py | 9 ++++++--- tests/test_octodns_provider_base.py | 22 ++++++++++++---------- tests/test_octodns_provider_cloudflare.py | 4 ++-- tests/test_octodns_provider_dyn.py | 2 +- tests/test_octodns_provider_googlecloud.py | 6 ++++-- 9 files changed, 32 insertions(+), 23 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index d4debf6..bb2c921 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -361,7 +361,7 @@ class Manager(object): plan = target.plan(zone) if plan is None: - plan = Plan(zone, zone, []) + plan = Plan(zone, zone, [], True) target.apply(plan) def validate_configs(self): diff --git a/octodns/provider/base.py b/octodns/provider/base.py index 2d4680f..ddce7a6 100644 --- a/octodns/provider/base.py +++ b/octodns/provider/base.py @@ -64,8 +64,9 @@ class BaseProvider(BaseSource): .join([str(c) for c in extra])) changes += extra + create = False if changes: - plan = Plan(existing, desired, changes, + plan = Plan(existing, desired, changes, create, self.update_pcent_threshold, self.delete_pcent_threshold) self.log.info('plan: %s', plan) diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index 3e86826..8bb72bd 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -21,12 +21,13 @@ class Plan(object): MAX_SAFE_DELETE_PCENT = .3 MIN_EXISTING_RECORDS = 10 - def __init__(self, existing, desired, changes, + def __init__(self, existing, desired, changes, create, update_pcent_threshold=MAX_SAFE_UPDATE_PCENT, delete_pcent_threshold=MAX_SAFE_DELETE_PCENT): self.existing = existing self.desired = desired self.changes = changes + self.create = create self.update_pcent_threshold = update_pcent_threshold self.delete_pcent_threshold = delete_pcent_threshold diff --git a/tests/test_octodns_plan.py b/tests/test_octodns_plan.py index ea35243..36dee68 100644 --- a/tests/test_octodns_plan.py +++ b/tests/test_octodns_plan.py @@ -52,8 +52,8 @@ update = Update(existing, new) delete = Delete(new) changes = [create, delete, update] plans = [ - (simple, Plan(zone, zone, changes)), - (simple, Plan(zone, zone, changes)), + (simple, Plan(zone, zone, changes, False)), + (simple, Plan(zone, zone, changes, False)), ] diff --git a/tests/test_octodns_provider_azuredns.py b/tests/test_octodns_provider_azuredns.py index 598fe48..f44d368 100644 --- a/tests/test_octodns_provider_azuredns.py +++ b/tests/test_octodns_provider_azuredns.py @@ -338,8 +338,10 @@ class TestAzureDnsProvider(TestCase): changes.append(Create(i)) deletes.append(Delete(i)) - self.assertEquals(13, provider.apply(Plan(None, zone, changes))) - self.assertEquals(13, provider.apply(Plan(zone, zone, deletes))) + self.assertEquals(13, provider.apply(Plan(None, zone, + changes, False))) + self.assertEquals(13, provider.apply(Plan(zone, zone, + deletes, False))) def test_create_zone(self): provider = self._get_provider() @@ -354,7 +356,8 @@ class TestAzureDnsProvider(TestCase): _get = provider._dns_client.zones.get _get.side_effect = CloudError(Mock(status=404), err_msg) - self.assertEquals(13, provider.apply(Plan(None, desired, changes))) + self.assertEquals(13, provider.apply(Plan(None, desired, changes, + False))) def test_check_zone_no_create(self): provider = self._get_provider() diff --git a/tests/test_octodns_provider_base.py b/tests/test_octodns_provider_base.py index 472b008..a0ec74e 100644 --- a/tests/test_octodns_provider_base.py +++ b/tests/test_octodns_provider_base.py @@ -153,7 +153,7 @@ class TestBaseProvider(TestCase): def test_safe_none(self): # No changes is safe - Plan(None, None, []).raise_if_unsafe() + Plan(None, None, [], False).raise_if_unsafe() def test_safe_creates(self): # Creates are safe when existing records is under MIN_EXISTING_RECORDS @@ -164,7 +164,8 @@ class TestBaseProvider(TestCase): 'type': 'A', 'value': '1.2.3.4', }) - Plan(zone, zone, [Create(record) for i in range(10)]).raise_if_unsafe() + Plan(zone, zone, [Create(record) for i in range(10)], False) \ + .raise_if_unsafe() def test_safe_min_existing_creates(self): # Creates are safe when existing records is over MIN_EXISTING_RECORDS @@ -183,7 +184,8 @@ class TestBaseProvider(TestCase): 'value': '2.3.4.5' })) - Plan(zone, zone, [Create(record) for i in range(10)]).raise_if_unsafe() + Plan(zone, zone, [Create(record) for i in range(10)], False) \ + .raise_if_unsafe() def test_safe_no_existing(self): # existing records fewer than MIN_EXISTING_RECORDS is safe @@ -195,7 +197,7 @@ class TestBaseProvider(TestCase): }) updates = [Update(record, record), Update(record, record)] - Plan(zone, zone, updates).raise_if_unsafe() + Plan(zone, zone, updates, False).raise_if_unsafe() def test_safe_updates_min_existing(self): # MAX_SAFE_UPDATE_PCENT+1 fails when more @@ -219,7 +221,7 @@ class TestBaseProvider(TestCase): Plan.MAX_SAFE_UPDATE_PCENT) + 1)] with self.assertRaises(UnsafePlan) as ctx: - Plan(zone, zone, changes).raise_if_unsafe() + Plan(zone, zone, changes, False).raise_if_unsafe() self.assertTrue('Too many updates' in ctx.exception.message) @@ -243,7 +245,7 @@ class TestBaseProvider(TestCase): for i in range(int(Plan.MIN_EXISTING_RECORDS * Plan.MAX_SAFE_UPDATE_PCENT))] - Plan(zone, zone, changes).raise_if_unsafe() + Plan(zone, zone, changes, False).raise_if_unsafe() def test_safe_deletes_min_existing(self): # MAX_SAFE_DELETE_PCENT+1 fails when more @@ -267,7 +269,7 @@ class TestBaseProvider(TestCase): Plan.MAX_SAFE_DELETE_PCENT) + 1)] with self.assertRaises(UnsafePlan) as ctx: - Plan(zone, zone, changes).raise_if_unsafe() + Plan(zone, zone, changes, False).raise_if_unsafe() self.assertTrue('Too many deletes' in ctx.exception.message) @@ -291,7 +293,7 @@ class TestBaseProvider(TestCase): for i in range(int(Plan.MIN_EXISTING_RECORDS * Plan.MAX_SAFE_DELETE_PCENT))] - Plan(zone, zone, changes).raise_if_unsafe() + Plan(zone, zone, changes, False).raise_if_unsafe() def test_safe_updates_min_existing_override(self): safe_pcent = .4 @@ -316,7 +318,7 @@ class TestBaseProvider(TestCase): safe_pcent) + 1)] with self.assertRaises(UnsafePlan) as ctx: - Plan(zone, zone, changes, + Plan(zone, zone, changes, False, update_pcent_threshold=safe_pcent).raise_if_unsafe() self.assertTrue('Too many updates' in ctx.exception.message) @@ -344,7 +346,7 @@ class TestBaseProvider(TestCase): safe_pcent) + 1)] with self.assertRaises(UnsafePlan) as ctx: - Plan(zone, zone, changes, + Plan(zone, zone, changes, False, delete_pcent_threshold=safe_pcent).raise_if_unsafe() self.assertTrue('Too many deletes' in ctx.exception.message) diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index 824af9d..7925b94 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -347,7 +347,7 @@ class TestCloudflareProvider(TestCase): 'values': ['2.2.2.2', '3.3.3.3', '4.4.4.4'], }) change = Update(existing, new) - plan = Plan(zone, zone, [change]) + plan = Plan(zone, zone, [change], False) provider._apply(plan) provider._request.assert_has_calls([ @@ -432,7 +432,7 @@ class TestCloudflareProvider(TestCase): 'value': 'ns2.foo.bar.', }) change = Update(existing, new) - plan = Plan(zone, zone, [change]) + plan = Plan(zone, zone, [change], False) provider._apply(plan) provider._request.assert_has_calls([ diff --git a/tests/test_octodns_provider_dyn.py b/tests/test_octodns_provider_dyn.py index 4415347..9e06a6b 100644 --- a/tests/test_octodns_provider_dyn.py +++ b/tests/test_octodns_provider_dyn.py @@ -913,7 +913,7 @@ class TestDynProviderGeo(TestCase): Delete(geo), Delete(regular), ] - plan = Plan(None, desired, changes) + plan = Plan(None, desired, changes, False) provider._apply(plan) mock.assert_has_calls([ call('/Zone/unit.tests/', 'GET', {}), diff --git a/tests/test_octodns_provider_googlecloud.py b/tests/test_octodns_provider_googlecloud.py index adc2112..7060355 100644 --- a/tests/test_octodns_provider_googlecloud.py +++ b/tests/test_octodns_provider_googlecloud.py @@ -263,7 +263,8 @@ class TestGoogleCloudProvider(TestCase): provider.apply(Plan( existing=[update_existing_r, delete_r], desired=desired, - changes=changes + changes=changes, + create=False )) calls_mock = gcloud_zone_mock.changes.return_value @@ -295,7 +296,8 @@ class TestGoogleCloudProvider(TestCase): provider.apply(Plan( existing=[update_existing_r, delete_r], desired=desired, - changes=changes + changes=changes, + create=False )) unsupported_change = Mock() From 94bfb1e5072beec17f759b15344e3f0892d2955e Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 14:06:20 -0800 Subject: [PATCH 02/15] Switch populate to return exists, cleaner setup --- octodns/manager.py | 2 +- octodns/provider/base.py | 10 +++++++--- octodns/provider/plan.py | 4 ++-- octodns/source/base.py | 5 ++++- tests/test_octodns_plan.py | 4 ++-- tests/test_octodns_provider_azuredns.py | 6 +++--- tests/test_octodns_provider_base.py | 20 ++++++++++---------- tests/test_octodns_provider_cloudflare.py | 4 ++-- tests/test_octodns_provider_dyn.py | 2 +- tests/test_octodns_provider_googlecloud.py | 4 ++-- 10 files changed, 34 insertions(+), 27 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index bb2c921..3f46007 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -361,7 +361,7 @@ class Manager(object): plan = target.plan(zone) if plan is None: - plan = Plan(zone, zone, [], True) + plan = Plan(zone, zone, [], False) target.apply(plan) def validate_configs(self): diff --git a/octodns/provider/base.py b/octodns/provider/base.py index ddce7a6..6bfb16d 100644 --- a/octodns/provider/base.py +++ b/octodns/provider/base.py @@ -45,7 +45,12 @@ class BaseProvider(BaseSource): self.log.info('plan: desired=%s', desired.name) existing = Zone(desired.name, desired.sub_zones) - self.populate(existing, target=True, lenient=True) + exists = self.populate(existing, target=True, lenient=True) + if exists is None: + # If your code gets this warning see Source.populate for more + # information + self.log.warn('Provider %s used in target mode did not return ' + 'exists', self.id) # compute the changes at the zone/record level changes = existing.changes(desired, self) @@ -64,9 +69,8 @@ class BaseProvider(BaseSource): .join([str(c) for c in extra])) changes += extra - create = False if changes: - plan = Plan(existing, desired, changes, create, + plan = Plan(existing, desired, changes, exists, self.update_pcent_threshold, self.delete_pcent_threshold) self.log.info('plan: %s', plan) diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index 8bb72bd..10ab167 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -21,13 +21,13 @@ class Plan(object): MAX_SAFE_DELETE_PCENT = .3 MIN_EXISTING_RECORDS = 10 - def __init__(self, existing, desired, changes, create, + def __init__(self, existing, desired, changes, exists, update_pcent_threshold=MAX_SAFE_UPDATE_PCENT, delete_pcent_threshold=MAX_SAFE_DELETE_PCENT): self.existing = existing self.desired = desired self.changes = changes - self.create = create + self.exists = exists self.update_pcent_threshold = update_pcent_threshold self.delete_pcent_threshold = delete_pcent_threshold diff --git a/octodns/source/base.py b/octodns/source/base.py index 4ace09f..ee33619 100644 --- a/octodns/source/base.py +++ b/octodns/source/base.py @@ -22,7 +22,7 @@ class BaseSource(object): def populate(self, zone, target=False, lenient=False): ''' - Loads all zones the provider knows about + Loads all records the provider knows about for the provided zone When `target` is True the populate call is being made to load the current state of the provider. @@ -31,6 +31,9 @@ class BaseSource(object): do a "best effort" load of data. That will allow through some common, but not best practices stuff that we otherwise would reject. E.g. no trailing . or mising escapes for ;. + + When target is True (loading current state) this method should return + True if the zone exists or False if it does not. ''' raise NotImplementedError('Abstract base class, populate method ' 'missing') diff --git a/tests/test_octodns_plan.py b/tests/test_octodns_plan.py index 36dee68..91dd948 100644 --- a/tests/test_octodns_plan.py +++ b/tests/test_octodns_plan.py @@ -52,8 +52,8 @@ update = Update(existing, new) delete = Delete(new) changes = [create, delete, update] plans = [ - (simple, Plan(zone, zone, changes, False)), - (simple, Plan(zone, zone, changes, False)), + (simple, Plan(zone, zone, changes, True)), + (simple, Plan(zone, zone, changes, True)), ] diff --git a/tests/test_octodns_provider_azuredns.py b/tests/test_octodns_provider_azuredns.py index f44d368..adf394b 100644 --- a/tests/test_octodns_provider_azuredns.py +++ b/tests/test_octodns_provider_azuredns.py @@ -339,9 +339,9 @@ class TestAzureDnsProvider(TestCase): deletes.append(Delete(i)) self.assertEquals(13, provider.apply(Plan(None, zone, - changes, False))) + changes, True))) self.assertEquals(13, provider.apply(Plan(zone, zone, - deletes, False))) + deletes, True))) def test_create_zone(self): provider = self._get_provider() @@ -357,7 +357,7 @@ class TestAzureDnsProvider(TestCase): _get.side_effect = CloudError(Mock(status=404), err_msg) self.assertEquals(13, provider.apply(Plan(None, desired, changes, - False))) + True))) def test_check_zone_no_create(self): provider = self._get_provider() diff --git a/tests/test_octodns_provider_base.py b/tests/test_octodns_provider_base.py index a0ec74e..3ebf250 100644 --- a/tests/test_octodns_provider_base.py +++ b/tests/test_octodns_provider_base.py @@ -153,7 +153,7 @@ class TestBaseProvider(TestCase): def test_safe_none(self): # No changes is safe - Plan(None, None, [], False).raise_if_unsafe() + Plan(None, None, [], True).raise_if_unsafe() def test_safe_creates(self): # Creates are safe when existing records is under MIN_EXISTING_RECORDS @@ -164,7 +164,7 @@ class TestBaseProvider(TestCase): 'type': 'A', 'value': '1.2.3.4', }) - Plan(zone, zone, [Create(record) for i in range(10)], False) \ + Plan(zone, zone, [Create(record) for i in range(10)], True) \ .raise_if_unsafe() def test_safe_min_existing_creates(self): @@ -184,7 +184,7 @@ class TestBaseProvider(TestCase): 'value': '2.3.4.5' })) - Plan(zone, zone, [Create(record) for i in range(10)], False) \ + Plan(zone, zone, [Create(record) for i in range(10)], True) \ .raise_if_unsafe() def test_safe_no_existing(self): @@ -197,7 +197,7 @@ class TestBaseProvider(TestCase): }) updates = [Update(record, record), Update(record, record)] - Plan(zone, zone, updates, False).raise_if_unsafe() + Plan(zone, zone, updates, True).raise_if_unsafe() def test_safe_updates_min_existing(self): # MAX_SAFE_UPDATE_PCENT+1 fails when more @@ -221,7 +221,7 @@ class TestBaseProvider(TestCase): Plan.MAX_SAFE_UPDATE_PCENT) + 1)] with self.assertRaises(UnsafePlan) as ctx: - Plan(zone, zone, changes, False).raise_if_unsafe() + Plan(zone, zone, changes, True).raise_if_unsafe() self.assertTrue('Too many updates' in ctx.exception.message) @@ -245,7 +245,7 @@ class TestBaseProvider(TestCase): for i in range(int(Plan.MIN_EXISTING_RECORDS * Plan.MAX_SAFE_UPDATE_PCENT))] - Plan(zone, zone, changes, False).raise_if_unsafe() + Plan(zone, zone, changes, True).raise_if_unsafe() def test_safe_deletes_min_existing(self): # MAX_SAFE_DELETE_PCENT+1 fails when more @@ -269,7 +269,7 @@ class TestBaseProvider(TestCase): Plan.MAX_SAFE_DELETE_PCENT) + 1)] with self.assertRaises(UnsafePlan) as ctx: - Plan(zone, zone, changes, False).raise_if_unsafe() + Plan(zone, zone, changes, True).raise_if_unsafe() self.assertTrue('Too many deletes' in ctx.exception.message) @@ -293,7 +293,7 @@ class TestBaseProvider(TestCase): for i in range(int(Plan.MIN_EXISTING_RECORDS * Plan.MAX_SAFE_DELETE_PCENT))] - Plan(zone, zone, changes, False).raise_if_unsafe() + Plan(zone, zone, changes, True).raise_if_unsafe() def test_safe_updates_min_existing_override(self): safe_pcent = .4 @@ -318,7 +318,7 @@ class TestBaseProvider(TestCase): safe_pcent) + 1)] with self.assertRaises(UnsafePlan) as ctx: - Plan(zone, zone, changes, False, + Plan(zone, zone, changes, True, update_pcent_threshold=safe_pcent).raise_if_unsafe() self.assertTrue('Too many updates' in ctx.exception.message) @@ -346,7 +346,7 @@ class TestBaseProvider(TestCase): safe_pcent) + 1)] with self.assertRaises(UnsafePlan) as ctx: - Plan(zone, zone, changes, False, + Plan(zone, zone, changes, True, delete_pcent_threshold=safe_pcent).raise_if_unsafe() self.assertTrue('Too many deletes' in ctx.exception.message) diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index 7925b94..2977d26 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -347,7 +347,7 @@ class TestCloudflareProvider(TestCase): 'values': ['2.2.2.2', '3.3.3.3', '4.4.4.4'], }) change = Update(existing, new) - plan = Plan(zone, zone, [change], False) + plan = Plan(zone, zone, [change], True) provider._apply(plan) provider._request.assert_has_calls([ @@ -432,7 +432,7 @@ class TestCloudflareProvider(TestCase): 'value': 'ns2.foo.bar.', }) change = Update(existing, new) - plan = Plan(zone, zone, [change], False) + plan = Plan(zone, zone, [change], True) provider._apply(plan) provider._request.assert_has_calls([ diff --git a/tests/test_octodns_provider_dyn.py b/tests/test_octodns_provider_dyn.py index 9e06a6b..0956477 100644 --- a/tests/test_octodns_provider_dyn.py +++ b/tests/test_octodns_provider_dyn.py @@ -913,7 +913,7 @@ class TestDynProviderGeo(TestCase): Delete(geo), Delete(regular), ] - plan = Plan(None, desired, changes, False) + plan = Plan(None, desired, changes, True) provider._apply(plan) mock.assert_has_calls([ call('/Zone/unit.tests/', 'GET', {}), diff --git a/tests/test_octodns_provider_googlecloud.py b/tests/test_octodns_provider_googlecloud.py index 7060355..dacc369 100644 --- a/tests/test_octodns_provider_googlecloud.py +++ b/tests/test_octodns_provider_googlecloud.py @@ -264,7 +264,7 @@ class TestGoogleCloudProvider(TestCase): existing=[update_existing_r, delete_r], desired=desired, changes=changes, - create=False + exists=True )) calls_mock = gcloud_zone_mock.changes.return_value @@ -297,7 +297,7 @@ class TestGoogleCloudProvider(TestCase): existing=[update_existing_r, delete_r], desired=desired, changes=changes, - create=False + exists=True )) unsupported_change = Mock() From 73c002f94c180e97a9b9330caa6b56f2fbb4baab Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 14:26:51 -0800 Subject: [PATCH 03/15] Implement populate exists for Route53Provider --- octodns/provider/plan.py | 22 ++++++++++++++++++++-- octodns/provider/route53.py | 7 +++++-- tests/test_octodns_provider_route53.py | 2 ++ 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index 10ab167..47a4157 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -124,6 +124,12 @@ class PlanLogger(_PlanOutput): buf.write(' (') buf.write(target) buf.write(')\n* ') + + if plan.exists is False: + buf.write('Create ') + buf.write(str(plan.desired)) + buf.write('\n* ') + for change in plan.changes: buf.write(change.__repr__(leader='* ')) buf.write('\n* ') @@ -169,6 +175,11 @@ class PlanMarkdown(_PlanOutput): fh.write('| Operation | Name | Type | TTL | Value | Source |\n' '|--|--|--|--|--|--|\n') + if plan.exists is False: + fh.write('| Create | ') + fh.write(str(plan.desired)) + fh.write(' | | | | |\n') + for change in plan.changes: existing = change.existing new = change.new @@ -194,7 +205,8 @@ class PlanMarkdown(_PlanOutput): fh.write(' | ') fh.write(_value_stringifier(new, '; ')) fh.write(' | ') - fh.write(new.source.id) + if new.source: + fh.write(new.source.id) fh.write(' |\n') fh.write('\nSummary: ') @@ -230,6 +242,11 @@ class PlanHtml(_PlanOutput): ''') + if plan.exists is False: + fh.write(' \n Create\n ') + fh.write(str(plan.desired)) + fh.write('\n \n') + for change in plan.changes: existing = change.existing new = change.new @@ -257,7 +274,8 @@ class PlanHtml(_PlanOutput): fh.write('\n ') fh.write(_value_stringifier(new, '
')) fh.write('\n ') - fh.write(new.source.id) + if new.source: + fh.write(new.source.id) fh.write('\n \n') fh.write(' \n Summary: ') diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 5bda074..9de1b0c 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -451,9 +451,11 @@ class Route53Provider(BaseProvider): target, lenient) before = len(zone.records) + exists = False zone_id = self._get_zone_id(zone.name) if zone_id: + exists = True records = defaultdict(lambda: defaultdict(list)) for rrset in self._load_records(zone_id): record_name = zone.hostname_from_fqdn(rrset['Name']) @@ -483,8 +485,9 @@ class Route53Provider(BaseProvider): lenient=lenient) zone.add_record(record) - self.log.info('populate: found %s records', - len(zone.records) - before) + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _gen_mods(self, action, records): ''' diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index 1cd4548..8da7f2e 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -361,6 +361,7 @@ class TestRoute53Provider(TestCase): plan = provider.plan(self.expected) self.assertEquals(9, len(plan.changes)) + self.assertTrue(plan.exists) for change in plan.changes: self.assertIsInstance(change, Create) stubber.assert_no_pending_responses() @@ -593,6 +594,7 @@ class TestRoute53Provider(TestCase): plan = provider.plan(self.expected) self.assertEquals(9, len(plan.changes)) + self.assertFalse(plan.exists) for change in plan.changes: self.assertIsInstance(change, Create) stubber.assert_no_pending_responses() From d03e07c01c25698d0e4522d8e088cd165b98233b Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 14:27:08 -0800 Subject: [PATCH 04/15] Implement populate exists for PowerDnsProvider --- octodns/provider/powerdns.py | 7 +++++-- tests/test_octodns_provider_powerdns.py | 2 ++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/octodns/provider/powerdns.py b/octodns/provider/powerdns.py index 4527f8e..3ad0e55 100644 --- a/octodns/provider/powerdns.py +++ b/octodns/provider/powerdns.py @@ -187,8 +187,10 @@ class PowerDnsBaseProvider(BaseProvider): raise before = len(zone.records) + exists = False if resp: + exists = True for rrset in resp.json()['rrsets']: _type = rrset['type'] if _type == 'SOA': @@ -199,8 +201,9 @@ class PowerDnsBaseProvider(BaseProvider): source=self, lenient=lenient) zone.add_record(record) - self.log.info('populate: found %s records', - len(zone.records) - before) + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _records_for_multiple(self, record): return [{'content': v, 'disabled': False} diff --git a/tests/test_octodns_provider_powerdns.py b/tests/test_octodns_provider_powerdns.py index 22ccdd6..b0d5dcf 100644 --- a/tests/test_octodns_provider_powerdns.py +++ b/tests/test_octodns_provider_powerdns.py @@ -106,6 +106,7 @@ class TestPowerDnsProvider(TestCase): plan = provider.plan(expected) self.assertEquals(expected_n, len(plan.changes)) self.assertEquals(expected_n, provider.apply(plan)) + self.assertTrue(plan.exists) # Non-existent zone -> creates for every record in expected # OMG this is fucking ugly, probably better to ditch requests_mocks and @@ -124,6 +125,7 @@ class TestPowerDnsProvider(TestCase): plan = provider.plan(expected) self.assertEquals(expected_n, len(plan.changes)) self.assertEquals(expected_n, provider.apply(plan)) + self.assertFalse(plan.exists) with requests_mock() as mock: # get 422's, unknown zone From 3ef91326e8d62f45be7bb869faf0acc8f0e965fc Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 14:35:32 -0800 Subject: [PATCH 05/15] Implement populate exists for Ns1Provider --- octodns/provider/ns1.py | 7 +++++-- tests/test_octodns_provider_ns1.py | 4 +++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 5ea68b6..4911082 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -190,11 +190,13 @@ class Ns1Provider(BaseProvider): nsone_zone = self._client.loadZone(zone.name[:-1]) records = nsone_zone.data['records'] geo_records = nsone_zone.search(has_geo=True) + exists = True except ResourceException as e: if e.message != self.ZONE_NOT_FOUND_MESSAGE: raise records = [] geo_records = [] + exists = False before = len(zone.records) # geo information isn't returned from the main endpoint, so we need @@ -208,8 +210,9 @@ class Ns1Provider(BaseProvider): source=self, lenient=lenient) zone_hash[(_type, name)] = record [zone.add_record(r) for r in zone_hash.values()] - self.log.info('populate: found %s records', - len(zone.records) - before) + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _params_for_A(self, record): params = {'answers': record.values, 'ttl': record.ttl} diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index 0e3a286..fa6cf2d 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -196,9 +196,10 @@ class TestNs1Provider(TestCase): load_mock.side_effect = \ ResourceException('server error: zone not found') zone = Zone('unit.tests.', []) - provider.populate(zone) + exists = provider.populate(zone) self.assertEquals(set(), zone.records) self.assertEquals(('unit.tests',), load_mock.call_args[0]) + self.assertFalse(exists) # Existing zone w/o records load_mock.reset_mock() @@ -269,6 +270,7 @@ class TestNs1Provider(TestCase): # everything except the root NS expected_n = len(self.expected) - 1 self.assertEquals(expected_n, len(plan.changes)) + self.assertTrue(plan.exists) # Fails, general error load_mock.reset_mock() From b54630878f465c79aa1281b12ef34b73d0339995 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 14:37:31 -0800 Subject: [PATCH 06/15] Implement populate exists for DynProvider --- octodns/provider/dyn.py | 8 ++++++-- tests/test_octodns_provider_dyn.py | 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/octodns/provider/dyn.py b/octodns/provider/dyn.py index 010fd31..beb84d5 100644 --- a/octodns/provider/dyn.py +++ b/octodns/provider/dyn.py @@ -353,6 +353,7 @@ class DynProvider(BaseProvider): self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, target, lenient) + exists = False before = len(zone.records) self._check_dyn_sess() @@ -360,10 +361,12 @@ class DynProvider(BaseProvider): td_records = set() if self.traffic_directors_enabled: td_records = self._populate_traffic_directors(zone) + exists = True dyn_zone = _CachingDynZone.get(zone.name[:-1]) if dyn_zone: + exists = True values = defaultdict(lambda: defaultdict(list)) for _type, records in dyn_zone.get_all_records().items(): if _type == 'soa_records': @@ -382,8 +385,9 @@ class DynProvider(BaseProvider): if record not in td_records: zone.add_record(record) - self.log.info('populate: found %s records', - len(zone.records) - before) + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _kwargs_for_A(self, record): return [{ diff --git a/tests/test_octodns_provider_dyn.py b/tests/test_octodns_provider_dyn.py index 0956477..3f95980 100644 --- a/tests/test_octodns_provider_dyn.py +++ b/tests/test_octodns_provider_dyn.py @@ -430,6 +430,7 @@ class TestDynProvider(TestCase): update_mock.assert_not_called() provider.apply(plan) update_mock.assert_called() + self.assertFalse(plan.exists) add_mock.assert_called() # Once for each dyn record (8 Records, 2 of which have dual values) self.assertEquals(15, len(add_mock.call_args_list)) @@ -474,6 +475,7 @@ class TestDynProvider(TestCase): plan = provider.plan(new) provider.apply(plan) update_mock.assert_called() + self.assertTrue(plan.exists) # we expect 4 deletes, 2 from actual deletes and 2 from # updates which delete and recreate self.assertEquals(4, len(delete_mock.call_args_list)) From 1f40b98889a51c88f7dd3e030617f887399bfd76 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 14:40:07 -0800 Subject: [PATCH 07/15] Implement populate exists for CloudflareProvider --- octodns/provider/cloudflare.py | 7 +++++-- tests/test_octodns_provider_cloudflare.py | 2 ++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index 9dfef6d..b1dee2b 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -173,9 +173,11 @@ class CloudflareProvider(BaseProvider): self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, target, lenient) + exists = False before = len(zone.records) records = self.zone_records(zone) if records: + exists = True values = defaultdict(lambda: defaultdict(list)) for record in records: name = zone.hostname_from_fqdn(record['name']) @@ -196,8 +198,9 @@ class CloudflareProvider(BaseProvider): lenient=lenient) zone.add_record(record) - self.log.info('populate: found %s records', - len(zone.records) - before) + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _include_change(self, change): if isinstance(change, Update): diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index 2977d26..328bd6f 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -147,6 +147,7 @@ class TestCloudflareProvider(TestCase): plan = provider.plan(self.expected) self.assertEquals(11, len(plan.changes)) self.assertEquals(11, provider.apply(plan)) + self.assertFalse(plan.exists) provider._request.assert_has_calls([ # created the domain @@ -266,6 +267,7 @@ class TestCloudflareProvider(TestCase): # only see the delete & ttl update, below min-ttl is filtered out self.assertEquals(2, len(plan.changes)) self.assertEquals(2, provider.apply(plan)) + self.assertTrue(plan.exists) # recreate for update, and deletes for the 2 parts of the other provider._request.assert_has_calls([ call('PUT', '/zones/ff12ab34cd5611334422ab3322997650/dns_records/' From 4d180ed991ce956254b612190f3c21cc1eb3dd95 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 14:41:54 -0800 Subject: [PATCH 08/15] Implement populate exists for YamlProvider --- octodns/provider/yaml.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 752e793..0241d50 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -52,7 +52,7 @@ class YamlProvider(BaseProvider): if target: # When acting as a target we ignore any existing records so that we # create a completely new copy - return + return False before = len(zone.records) filename = join(self.directory, '{}yaml'.format(zone.name)) @@ -69,8 +69,9 @@ class YamlProvider(BaseProvider): lenient=lenient) zone.add_record(record) - self.log.info('populate: found %s records', + self.log.info('populate: found %s records, exists=False', len(zone.records) - before) + return False def _apply(self, plan): desired = plan.desired From d35fcd319aa5278cad2f0df2656f2bb0c89684a9 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 14:44:36 -0800 Subject: [PATCH 09/15] Implement populate exists for RackspaceProvider --- octodns/provider/rackspace.py | 5 +++-- tests/test_octodns_provider_rackspace.py | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/octodns/provider/rackspace.py b/octodns/provider/rackspace.py index 12b2c54..02c833d 100644 --- a/octodns/provider/rackspace.py +++ b/octodns/provider/rackspace.py @@ -208,7 +208,7 @@ class RackspaceProvider(BaseProvider): raise Exception('Rackspace request unauthorized') elif e.response.status_code == 404: # Zone not found leaves the zone empty instead of failing. - return + return False raise before = len(zone.records) @@ -225,8 +225,9 @@ class RackspaceProvider(BaseProvider): source=self) zone.add_record(record) - self.log.info('populate: found %s records', + self.log.info('populate: found %s records, exists=True', len(zone.records) - before) + return True def _group_records(self, all_records): records = defaultdict(lambda: defaultdict(list)) diff --git a/tests/test_octodns_provider_rackspace.py b/tests/test_octodns_provider_rackspace.py index 274d63e..c467dec 100644 --- a/tests/test_octodns_provider_rackspace.py +++ b/tests/test_octodns_provider_rackspace.py @@ -73,9 +73,10 @@ class TestRackspaceProvider(TestCase): json={'error': "Could not find domain 'unit.tests.'"}) zone = Zone('unit.tests.', []) - self.provider.populate(zone) + exists = self.provider.populate(zone) self.assertEquals(set(), zone.records) self.assertTrue(mock.called_once) + self.assertFalse(exists) def test_multipage_populate(self): with requests_mock() as mock: @@ -109,6 +110,7 @@ class TestRackspaceProvider(TestCase): plan = self.provider.plan(expected) self.assertTrue(mock.called) + self.assertTrue(plan.exists) # OctoDNS does not propagate top-level NS records. self.assertEquals(1, len(plan.changes)) From d693d2e99e0cec859ed53fe37ef7bc4731c0bbf1 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 14:46:49 -0800 Subject: [PATCH 10/15] Implement populate exists for GoogleCloudProvider --- octodns/provider/googlecloud.py | 7 ++++++- tests/test_octodns_provider_googlecloud.py | 6 ++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/octodns/provider/googlecloud.py b/octodns/provider/googlecloud.py index 6ca0794..1dc5393 100644 --- a/octodns/provider/googlecloud.py +++ b/octodns/provider/googlecloud.py @@ -202,11 +202,14 @@ class GoogleCloudProvider(BaseProvider): self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, target, lenient) + + exists = False before = len(zone.records) gcloud_zone = self.gcloud_zones.get(zone.name) if gcloud_zone: + exists = True for gcloud_record in self._get_gcloud_records(gcloud_zone): if gcloud_record.record_type.upper() not in self.SUPPORTS: continue @@ -227,7 +230,9 @@ class GoogleCloudProvider(BaseProvider): record = Record.new(zone, record_name, data, source=self) zone.add_record(record) - self.log.info('populate: found %s records', len(zone.records) - before) + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _data_for_A(self, gcloud_record): return { diff --git a/tests/test_octodns_provider_googlecloud.py b/tests/test_octodns_provider_googlecloud.py index dacc369..fc7fb03 100644 --- a/tests/test_octodns_provider_googlecloud.py +++ b/tests/test_octodns_provider_googlecloud.py @@ -359,7 +359,8 @@ class TestGoogleCloudProvider(TestCase): "unit.tests.") test_zone = Zone('unit.tests.', []) - provider.populate(test_zone) + exists = provider.populate(test_zone) + self.assertTrue(exists) # test_zone gets fed the same records as zone does, except it's in # the format returned by google API, so after populate they should look @@ -367,7 +368,8 @@ class TestGoogleCloudProvider(TestCase): self.assertEqual(test_zone.records, zone.records) test_zone2 = Zone('nonexistant.zone.', []) - provider.populate(test_zone2, False, False) + exists = provider.populate(test_zone2, False, False) + self.assertFalse(exists) self.assertEqual(len(test_zone2.records), 0, msg="Zone should not get records from wrong domain") From 720e8eb4349d4ee598c452f1345b6b6e46a2d051 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 14:49:20 -0800 Subject: [PATCH 11/15] Implement populate exists for AzureProvider --- octodns/provider/azuredns.py | 7 ++++++- tests/test_octodns_provider_azuredns.py | 6 ++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index 1757274..af23ef7 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -322,6 +322,8 @@ class AzureProvider(BaseProvider): :type return: void ''' self.log.debug('populate: name=%s', zone.name) + + exists = False before = len(zone.records) zone_name = zone.name[:len(zone.name) - 1] @@ -331,6 +333,7 @@ class AzureProvider(BaseProvider): _records = set() records = self._dns_client.record_sets.list_by_dns_zone if self._check_zone(zone_name): + exists = True for azrecord in records(self._resource_group, zone_name): if _parse_azure_type(azrecord.type) in self.SUPPORTS: _records.add(azrecord) @@ -344,7 +347,9 @@ class AzureProvider(BaseProvider): record = Record.new(zone, record_name, data, source=self) zone.add_record(record) - self.log.info('populate: found %s records', len(zone.records) - before) + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _data_for_A(self, azrecord): return {'values': [ar.ipv4_address for ar in azrecord.arecords]} diff --git a/tests/test_octodns_provider_azuredns.py b/tests/test_octodns_provider_azuredns.py index adf394b..9784945 100644 --- a/tests/test_octodns_provider_azuredns.py +++ b/tests/test_octodns_provider_azuredns.py @@ -302,7 +302,8 @@ class TestAzureDnsProvider(TestCase): record_list = provider._dns_client.record_sets.list_by_dns_zone record_list.return_value = rs - provider.populate(zone) + exists = provider.populate(zone) + self.assertTrue(exists) self.assertEquals(len(zone.records), 16) @@ -377,6 +378,7 @@ class TestAzureDnsProvider(TestCase): _get = provider._dns_client.zones.get _get.side_effect = CloudError(Mock(status=404), err_msg) - provider.populate(Zone('unit3.test.', [])) + exists = provider.populate(Zone('unit3.test.', [])) + self.assertFalse(exists) self.assertEquals(len(zone.records), 0) From 88ff1729ab42b38c97ea1d81371b8ccb413527cf Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 14:55:53 -0800 Subject: [PATCH 12/15] Implement populate exists for DigitalOceanProvider --- octodns/provider/digitalocean.py | 6 ++++-- tests/test_octodns_provider_digitalocean.py | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/octodns/provider/digitalocean.py b/octodns/provider/digitalocean.py index c68e7d6..b1df187 100644 --- a/octodns/provider/digitalocean.py +++ b/octodns/provider/digitalocean.py @@ -232,8 +232,10 @@ class DigitalOceanProvider(BaseProvider): source=self, lenient=lenient) zone.add_record(record) - self.log.info('populate: found %s records', - len(zone.records) - before) + exists = zone.name in self._zone_records + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _params_for_multiple(self, record): for value in record.values: diff --git a/tests/test_octodns_provider_digitalocean.py b/tests/test_octodns_provider_digitalocean.py index 5fb47c5..ddc6bc2 100644 --- a/tests/test_octodns_provider_digitalocean.py +++ b/tests/test_octodns_provider_digitalocean.py @@ -165,6 +165,7 @@ class TestDigitalOceanProvider(TestCase): n = len(self.expected.records) - 7 self.assertEquals(n, len(plan.changes)) self.assertEquals(n, provider.apply(plan)) + self.assertFalse(plan.exists) provider._client._request.assert_has_calls([ # created the domain @@ -225,6 +226,7 @@ class TestDigitalOceanProvider(TestCase): })) plan = provider.plan(wanted) + self.assertTrue(plan.exists) self.assertEquals(2, len(plan.changes)) self.assertEquals(2, provider.apply(plan)) # recreate for update, and delete for the 2 parts of the other From 7566250f96a2eb1dc30b5e6a48ab72b7c664dd98 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 14:58:33 -0800 Subject: [PATCH 13/15] Implement populate exists for DnsimpleProvider --- octodns/provider/dnsimple.py | 6 ++++-- tests/test_octodns_provider_dnsimple.py | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/octodns/provider/dnsimple.py b/octodns/provider/dnsimple.py index 43b5b9b..304780d 100644 --- a/octodns/provider/dnsimple.py +++ b/octodns/provider/dnsimple.py @@ -272,8 +272,10 @@ class DnsimpleProvider(BaseProvider): source=self, lenient=lenient) zone.add_record(record) - self.log.info('populate: found %s records', - len(zone.records) - before) + exists = zone.name in self._zone_records + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _params_for_multiple(self, record): for value in record.values: diff --git a/tests/test_octodns_provider_dnsimple.py b/tests/test_octodns_provider_dnsimple.py index 60dacf1..896425e 100644 --- a/tests/test_octodns_provider_dnsimple.py +++ b/tests/test_octodns_provider_dnsimple.py @@ -133,6 +133,7 @@ class TestDnsimpleProvider(TestCase): n = len(self.expected.records) - 3 self.assertEquals(n, len(plan.changes)) self.assertEquals(n, provider.apply(plan)) + self.assertFalse(plan.exists) provider._client._request.assert_has_calls([ # created the domain @@ -186,6 +187,7 @@ class TestDnsimpleProvider(TestCase): })) plan = provider.plan(wanted) + self.assertTrue(plan.exists) self.assertEquals(2, len(plan.changes)) self.assertEquals(2, provider.apply(plan)) # recreate for update, and deletes for the 2 parts of the other From ff305ca1bbc60018333a71321de14c98d7adb299 Mon Sep 17 00:00:00 2001 From: Eric Vergne Date: Mon, 22 Jan 2018 17:12:06 +0100 Subject: [PATCH 14/15] Implement populate exists for OvhProvider --- octodns/provider/ovh.py | 16 +++++++++++++--- tests/test_octodns_provider_ovh.py | 20 ++++++++++++++++---- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/octodns/provider/ovh.py b/octodns/provider/ovh.py index 5c2fe0d..93e7594 100644 --- a/octodns/provider/ovh.py +++ b/octodns/provider/ovh.py @@ -11,6 +11,7 @@ import logging from collections import defaultdict import ovh +from ovh import ResourceNotFoundError from octodns.record import Record from .base import BaseProvider @@ -33,6 +34,7 @@ class OvhProvider(BaseProvider): """ SUPPORTS_GEO = False + ZONE_NOT_FOUND_MESSAGE = 'This service does not exist' # This variable is also used in populate method to filter which OVH record # types are supported by octodns @@ -57,7 +59,14 @@ class OvhProvider(BaseProvider): self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, target, lenient) zone_name = zone.name[:-1] - records = self.get_records(zone_name=zone_name) + try: + records = self.get_records(zone_name=zone_name) + exists = True + except ResourceNotFoundError as e: + if e.message != self.ZONE_NOT_FOUND_MESSAGE: + raise + exists = False + records = [] values = defaultdict(lambda: defaultdict(list)) for record in records: @@ -75,8 +84,9 @@ class OvhProvider(BaseProvider): source=self, lenient=lenient) zone.add_record(record) - self.log.info('populate: found %s records', - len(zone.records) - before) + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _apply(self, plan): desired = plan.desired diff --git a/tests/test_octodns_provider_ovh.py b/tests/test_octodns_provider_ovh.py index 6a44e25..472f03d 100644 --- a/tests/test_octodns_provider_ovh.py +++ b/tests/test_octodns_provider_ovh.py @@ -8,7 +8,7 @@ from __future__ import absolute_import, division, print_function, \ from unittest import TestCase from mock import patch, call -from ovh import APIError +from ovh import APIError, ResourceNotFoundError, InvalidCredential from octodns.provider.ovh import OvhProvider from octodns.record import Record @@ -307,18 +307,30 @@ class TestOvhProvider(TestCase): with patch.object(provider._client, 'get') as get_mock: zone = Zone('unit.tests.', []) - get_mock.side_effect = APIError('boom') + get_mock.side_effect = ResourceNotFoundError('boom') with self.assertRaises(APIError) as ctx: provider.populate(zone) self.assertEquals(get_mock.side_effect, ctx.exception) - with patch.object(provider._client, 'get') as get_mock: + get_mock.side_effect = InvalidCredential('boom') + with self.assertRaises(APIError) as ctx: + provider.populate(zone) + self.assertEquals(get_mock.side_effect, ctx.exception) + + zone = Zone('unit.tests.', []) + get_mock.side_effect = ResourceNotFoundError('This service does ' + 'not exist') + exists = provider.populate(zone) + self.assertEquals(set(), zone.records) + self.assertFalse(exists) + zone = Zone('unit.tests.', []) get_returns = [[record['id'] for record in self.api_record]] get_returns += self.api_record get_mock.side_effect = get_returns - provider.populate(zone) + exists = provider.populate(zone) self.assertEquals(self.expected, zone.records) + self.assertTrue(exists) @patch('ovh.Client') def test_is_valid_dkim(self, client_mock): From 876c09dcc07efb721f3ac08262d23bb2d9861a4e Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 3 Mar 2018 10:12:34 -0800 Subject: [PATCH 15/15] Flesh out UT for new Plan.exists messaging --- tests/test_octodns_plan.py | 45 ++++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/tests/test_octodns_plan.py b/tests/test_octodns_plan.py index 91dd948..7d849be 100644 --- a/tests/test_octodns_plan.py +++ b/tests/test_octodns_plan.py @@ -16,15 +16,6 @@ from octodns.zone import Zone from helpers import SimpleProvider -class TestPlanLogger(TestCase): - - def test_invalid_level(self): - with self.assertRaises(Exception) as ctx: - PlanLogger('invalid', 'not-a-level') - self.assertEquals('Unsupported level: not-a-level', - ctx.exception.message) - - simple = SimpleProvider() zone = Zone('unit.tests.', []) existing = Record.new(zone, 'a', { @@ -48,15 +39,45 @@ create = Create(Record.new(zone, 'b', { 'type': 'CNAME', 'value': 'foo.unit.tests.' }, simple)) +create2 = Create(Record.new(zone, 'c', { + 'ttl': 60, + 'type': 'CNAME', + 'value': 'foo.unit.tests.' +})) update = Update(existing, new) delete = Delete(new) -changes = [create, delete, update] +changes = [create, create2, delete, update] plans = [ (simple, Plan(zone, zone, changes, True)), - (simple, Plan(zone, zone, changes, True)), + (simple, Plan(zone, zone, changes, False)), ] +class TestPlanLogger(TestCase): + + def test_invalid_level(self): + with self.assertRaises(Exception) as ctx: + PlanLogger('invalid', 'not-a-level') + self.assertEquals('Unsupported level: not-a-level', + ctx.exception.message) + + def test_create(self): + + class MockLogger(object): + + def __init__(self): + self.out = StringIO() + + def log(self, level, msg): + self.out.write(msg) + + log = MockLogger() + PlanLogger('logger').run(log, plans) + out = log.out.getvalue() + self.assertTrue('Summary: Creates=2, Updates=1, ' + 'Deletes=1, Existing Records=0' in out) + + class TestPlanHtml(TestCase): log = getLogger('TestPlanHtml') @@ -69,7 +90,7 @@ class TestPlanHtml(TestCase): out = StringIO() PlanHtml('html').run(plans, fh=out) out = out.getvalue() - self.assertTrue(' Summary: Creates=1, Updates=1, ' + self.assertTrue(' Summary: Creates=2, Updates=1, ' 'Deletes=1, Existing Records=0' in out)