Rework YamlProvider.populate to behave 'normall' and add existing state, rework test to handle that

octodns/provider/yaml.py

@@ -342,11 +342,6 @@ class YamlProvider(BaseProvider):
             lenient,
         )
 
-        if target:
-            # When acting as a target we ignore any existing records so that we
-            # create a completely new copy
-            return False
-
         before = len(zone.records)
 
         sources = []
@@ -363,20 +358,22 @@ class YamlProvider(BaseProvider):
         if self.shared_filename:
             sources.append(join(self.directory, self.shared_filename))
 
-        if not sources:
+        if not sources and not target:
             raise ProviderException(f'no YAMLs found for {zone.decoded_name}')
 
-        # determinstically order our sources
+        # deterministically order our sources
         sources.sort()
 
         for source in sources:
             self._populate_from_file(source, zone, lenient)
 
+        exists = len(sources) > 0
         self.log.info(
-            'populate:   found %s records, exists=False',
+            'populate:   found %s records, exists=%s',
             len(zone.records) - before,
+            exists,
         )
-        return False
+        return exists
 
     def _apply(self, plan):
         # make a copy of existing we can muck with

tests/test_octodns_manager.py

@@ -2,7 +2,7 @@
 #
 #
 
-from os import environ, listdir
+from os import environ, listdir, remove
 from os.path import dirname, isfile, join
 from unittest import TestCase
 from unittest.mock import MagicMock, patch
@@ -39,6 +39,13 @@ def get_config_filename(which):
     return join(config_dir, which)
 
 
+def reset(directory):
+    for filename in listdir(directory):
+        if filename.endswith('.yaml'):
+            filename = join(directory, filename)
+            remove(filename)
+
+
 class TestManager(TestCase):
     def test_missing_provider_class(self):
         with self.assertRaises(ManagerException) as ctx:
@@ -147,40 +154,47 @@ class TestManager(TestCase):
         with TemporaryDirectory() as tmpdir:
             environ['YAML_TMP_DIR'] = tmpdir.dirname
             environ['YAML_TMP_DIR2'] = tmpdir.dirname
+
             tc = Manager(get_config_filename('simple.yaml')).sync(dry_run=False)
             self.assertEqual(28, tc)
 
             # try with just one of the zones
+            reset(tmpdir.dirname)
             tc = Manager(get_config_filename('simple.yaml')).sync(
                 dry_run=False, eligible_zones=['unit.tests.']
             )
             self.assertEqual(22, tc)
 
             # the subzone, with 2 targets
+            reset(tmpdir.dirname)
             tc = Manager(get_config_filename('simple.yaml')).sync(
                 dry_run=False, eligible_zones=['subzone.unit.tests.']
             )
             self.assertEqual(6, tc)
 
             # and finally the empty zone
+            reset(tmpdir.dirname)
             tc = Manager(get_config_filename('simple.yaml')).sync(
                 dry_run=False, eligible_zones=['empty.']
             )
             self.assertEqual(0, tc)
 
             # Again with force
+            reset(tmpdir.dirname)
             tc = Manager(get_config_filename('simple.yaml')).sync(
                 dry_run=False, force=True
             )
             self.assertEqual(28, tc)
 
             # Again with max_workers = 1
+            reset(tmpdir.dirname)
             tc = Manager(
                 get_config_filename('simple.yaml'), max_workers=1
             ).sync(dry_run=False, force=True)
             self.assertEqual(28, tc)
 
             # Include meta
+            reset(tmpdir.dirname)
             tc = Manager(
                 get_config_filename('simple.yaml'),
                 max_workers=1,
@@ -1011,22 +1025,23 @@ class TestManager(TestCase):
         )
 
     def test_auto_arpa(self):
-        manager = Manager(get_config_filename('simple-arpa.yaml'))
+        with TemporaryDirectory() as tmpdir:
+            environ['YAML_TMP_DIR'] = tmpdir.dirname
 
-        # provider config
-        self.assertEqual(
-            True, manager.providers.get("auto-arpa").populate_should_replace
-        )
-        self.assertEqual(1800, manager.providers.get("auto-arpa").ttl)
+            manager = Manager(get_config_filename('simple-arpa.yaml'))
 
-        # processor config
-        self.assertEqual(
-            True, manager.processors.get("auto-arpa").populate_should_replace
-        )
-        self.assertEqual(1800, manager.processors.get("auto-arpa").ttl)
+            # provider config
+            self.assertEqual(
+                True, manager.providers.get("auto-arpa").populate_should_replace
+            )
+            self.assertEqual(1800, manager.providers.get("auto-arpa").ttl)
 
-        with TemporaryDirectory() as tmpdir:
-            environ['YAML_TMP_DIR'] = tmpdir.dirname
+            # processor config
+            self.assertEqual(
+                True,
+                manager.processors.get("auto-arpa").populate_should_replace,
+            )
+            self.assertEqual(1800, manager.processors.get("auto-arpa").ttl)
 
             # we can sync eligible_zones so long as they're not arpa
             tc = manager.sync(dry_run=False, eligible_zones=['unit.tests.'])
@@ -1043,6 +1058,7 @@ class TestManager(TestCase):
             )
 
             # same for eligible_sources
+            reset(tmpdir.dirname)
             tc = manager.sync(
                 dry_run=False,
                 eligible_zones=['unit.tests.'],
@@ -1058,6 +1074,7 @@ class TestManager(TestCase):
             )
 
             # same for eligible_targets
+            reset(tmpdir.dirname)
             tc = manager.sync(
                 dry_run=False,
                 eligible_zones=['unit.tests.'],
@@ -1073,10 +1090,11 @@ class TestManager(TestCase):
             )
 
             # full sync with arpa is fine, 2 extra records from it
+            reset(tmpdir.dirname)
             tc = manager.sync(dry_run=False)
             self.assertEqual(26, tc)
 
-    def test_dynamic_config(self):
+    def test_dynamic_config_targeted(self):
         with TemporaryDirectory() as tmpdir:
             environ['YAML_TMP_DIR'] = tmpdir.dirname
 
@@ -1100,6 +1118,12 @@ class TestManager(TestCase):
                 ),
             )
 
+    def test_dynamic_config_all(self):
+        with TemporaryDirectory() as tmpdir:
+            environ['YAML_TMP_DIR'] = tmpdir.dirname
+
+            manager = Manager(get_config_filename('dynamic-config.yaml'))
+
             # should sync everything across all zones, total of 32 records
             self.assertEqual(32, manager.sync(dry_run=False))
 

tests/test_octodns_provider_yaml.py

@@ -29,11 +29,12 @@ class TestYamlProvider(TestCase):
         zone = Zone('unit.tests.', [])
         dynamic_zone = Zone('dynamic.tests.', [])
 
-        # With target we don't add anything
+        # With target we see everything
         source.populate(zone, target=source)
-        self.assertEqual(0, len(zone.records))
+        self.assertEqual(25, len(zone.records))
 
         # without it we see everything
+        zone = Zone('unit.tests.', [])
         source.populate(zone)
         self.assertEqual(25, len(zone.records))
 
@@ -95,11 +96,9 @@ class TestYamlProvider(TestCase):
 
             self.assertFalse(zone.changes(reloaded, target=source))
 
-            # A 2nd sync should still create everything
+            # A 2nd sync should result in no changes, thus no plan
             plan = target.plan(zone)
-            self.assertEqual(
-                22, len([c for c in plan.changes if isinstance(c, Create)])
-            )
+            self.assertFalse(plan)
 
             with open(yaml_file) as fh:
                 data = safe_load(fh.read())
@@ -177,7 +176,8 @@ class TestYamlProvider(TestCase):
             zone = Zone(idna_encode(name), [])
 
             # create a idna named file
-            with open(join(td.dirname, idna_encode(filename)), 'w') as fh:
+            idna_filename = join(td.dirname, idna_encode(filename))
+            with open(idna_filename, 'w') as fh:
                 fh.write(
                     '''---
 '':
@@ -204,7 +204,14 @@ xn--dj-kia8a:
             self.assertEqual(['2.3.4.5'], d['xn--dj-kia8a'].values)
             self.assertEqual(['3.4.5.6'], d['xn--28jm5b5a8k5k8cra'].values)
 
-            # create a utf8 named file (provider always writes utf-8 filenames
+            # if we plan there'll be nothing to do
+            plan = provider.plan(zone)
+            self.assertFalse(plan)
+
+            # get rid of the idna file
+            remove(idna_filename)
+            # create a utf8 named file (provider always writes utf-8 filenames,
+            # no file should have a plan now
             plan = provider.plan(zone)
             provider.apply(plan)
 
@@ -214,6 +221,9 @@ xn--dj-kia8a:
                 self.assertTrue('déjà:' in content)
                 self.assertTrue('これはテストです:' in content)
 
+            # recreate the idna version of the file
+            with open(idna_filename, 'w') as fh:
+                fh.write('')
             # does not allow both idna and utf8 named files
             with self.assertRaises(ProviderException) as ctx:
                 provider.populate(zone)
@@ -495,11 +505,12 @@ class TestSplitYamlProvider(TestCase):
         zone = Zone('unit.tests.', [])
         dynamic_zone = Zone('dynamic.tests.', [])
 
-        # With target we don't add anything
+        # With target we still see whatever is in the file
         source.populate(zone, target=source)
-        self.assertEqual(0, len(zone.records))
+        self.assertEqual(20, len(zone.records))
 
-        # without it we see everything
+        # without it we see everything too, doesn't make a difference
+        zone = Zone('unit.tests.', [])
         source.populate(zone)
         self.assertEqual(20, len(zone.records))
         self.assertFalse([r for r in zone.records if r.name.startswith('only')])
@@ -586,11 +597,9 @@ class TestSplitYamlProvider(TestCase):
 
             self.assertFalse(zone.changes(reloaded, target=source))
 
-            # A 2nd sync should still create everything
+            # A 2nd sync should have nothing to do
             plan = target.plan(zone)
-            self.assertEqual(
-                17, len([c for c in plan.changes if isinstance(c, Create)])
-            )
+            self.assertFalse(plan)
 
             yaml_file = join(zone_dir, '$unit.tests.yaml')
             self.assertTrue(isfile(yaml_file))