pools used as fallbacks should count as seen

octodns/record/__init__.py

@@ -531,6 +531,7 @@ class _DynamicMixin(object):
 
         pools_exist = set()
         pools_seen = set()
+        pools_seen_as_fallback = set()
         if not isinstance(pools, dict):
             reasons.append('pools must be a dict')
         elif not pools:
@@ -573,7 +574,10 @@ class _DynamicMixin(object):
                                        'value {}'.format(_id, value_num))
 
                 fallback = pool.get('fallback', None)
-                if fallback is not None and fallback not in pools:
+                if fallback is not None:
+                    if fallback in pools:
+                        pools_seen_as_fallback.add(fallback)
+                    else:
                         reasons.append('undefined fallback "{}" for pool "{}"'
                                        .format(fallback, _id))
 
@@ -644,7 +648,7 @@ class _DynamicMixin(object):
                         reasons.extend(GeoCodes.validate(geo, 'rule {} '
                                                          .format(rule_num)))
 
-        unused = pools_exist - pools_seen
+        unused = pools_exist - pools_seen - pools_seen_as_fallback
         if unused:
             unused = '", "'.join(sorted(unused))
             reasons.append('unused pools: "{}"'.format(unused))

tests/config/dynamic.tests.yaml

@@ -109,6 +109,29 @@ cname:
       - pool: iad
   type: CNAME
   value: target.unit.tests.
+pool-only-in-fallback:
+  dynamic:
+    pools:
+      one:
+        fallback: two
+        values:
+          - value: 1.1.1.1
+      three:
+        values:
+          - value: 3.3.3.3
+      two:
+        values:
+          - value: 2.2.2.2
+    rules:
+      - geos:
+        - NA-US
+        pool: one
+      - geos:
+        - AS-SG
+        pool: three
+  ttl: 300
+  type: A
+  values: [4.4.4.4]
 real-ish-a:
   dynamic:
     pools:

tests/test_octodns_provider_yaml.py

@@ -38,7 +38,7 @@ class TestYamlProvider(TestCase):
         self.assertEquals(22, len(zone.records))
 
         source.populate(dynamic_zone)
-        self.assertEquals(5, len(dynamic_zone.records))
+        self.assertEquals(6, len(dynamic_zone.records))
 
         # Assumption here is that a clean round-trip means that everything
         # worked as expected, data that went in came back out and could be
@@ -68,11 +68,11 @@ class TestYamlProvider(TestCase):
 
             # Dynamic plan
             plan = target.plan(dynamic_zone)
-            self.assertEquals(5, len([c for c in plan.changes
+            self.assertEquals(6, len([c for c in plan.changes
                                       if isinstance(c, Create)]))
             self.assertFalse(isfile(dynamic_yaml_file))
             # Apply it
-            self.assertEquals(5, target.apply(plan))
+            self.assertEquals(6, target.apply(plan))
             self.assertTrue(isfile(dynamic_yaml_file))
 
             # There should be no changes after the round trip
@@ -148,6 +148,10 @@ class TestYamlProvider(TestCase):
                 self.assertTrue('value' in dyna)
                 # self.assertTrue('dynamic' in dyna)
 
+                dyna = data.pop('pool-only-in-fallback')
+                self.assertTrue('value' in dyna)
+                # self.assertTrue('dynamic' in dyna)
+
                 # make sure nothing is left
                 self.assertEquals([], list(data.keys()))
 
@@ -397,7 +401,7 @@ class TestOverridingYamlProvider(TestCase):
         # Load the base, should see the 5 records
         base.populate(zone)
         got = {r.name: r for r in zone.records}
-        self.assertEquals(5, len(got))
+        self.assertEquals(6, len(got))
         # We get the "dynamic" A from the base config
         self.assertTrue('dynamic' in got['a'].data)
         # No added
@@ -406,7 +410,7 @@ class TestOverridingYamlProvider(TestCase):
         # Load the overrides, should replace one and add 1
         override.populate(zone)
         got = {r.name: r for r in zone.records}
-        self.assertEquals(6, len(got))
+        self.assertEquals(7, len(got))
         # 'a' was replaced with a generic record
         self.assertEquals({
             'ttl': 3600,