From 7a755d15be842f4910cb0ead7d7edc2f1e358b24 Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Sat, 24 Mar 2018 10:28:09 -0700
Subject: [PATCH] Make sure to insert new rulesets at the "end"

Dyn will still match them, even while they're empty before we've had a
chance to add the respons pools to them which is BAD and caused a
medium-size outage (thankfully not as bad as it could have been.) Ideally
we'd use publish=False and stage things, but that seems broken in the client
lib, there's no way to send publish=N. I also tried sending the
response_pool_ids as part of the create calls and response pool config if
one didn't exist, but neither of those routes worked :-(
---
 octodns/provider/dyn.py            | 16 ++++++++++++++--
 tests/test_octodns_provider_dyn.py | 10 +++++-----
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/octodns/provider/dyn.py b/octodns/provider/dyn.py
index 2799368..ef989bd 100644
--- a/octodns/provider/dyn.py
+++ b/octodns/provider/dyn.py
@@ -547,10 +547,22 @@ class DynProvider(BaseProvider):
 
         # Rulesets
 
+        # We need to make sure and insert the new rules after any existing
+        # rules so they won't take effect before we've had a chance to add
+        # response pools to them. I've tried both publish=False (which is
+        # completely broken in the client) and creating the rulesets with
+        # response_pool_ids neither of which appear to work from the client
+        # library. If there are no existing rulesets fallback to 0
+        insert_at = max([
+            int(r._ordering)
+            for r in existing_rulesets
+        ] + [-1]) + 1
+        self.log.debug('_mod_rulesets: insert_at=%d', insert_at)
+
         # add the default
         label = 'default:{}'.format(uuid4().hex)
         ruleset = DSFRuleset(label, 'always', [])
-        ruleset.create(td, index=0)
+        ruleset.create(td, index=insert_at)
         pool = self._find_or_create_pool(td, pools, 'default', new._type,
                                          new.values)
         # There's no way in the client lib to create a ruleset with an existing
@@ -583,7 +595,7 @@ class DynProvider(BaseProvider):
                 'geoip': criteria
             })
             # Something you have to call create others the constructor does it
-            ruleset.create(td, index=0)
+            ruleset.create(td, index=insert_at)
 
             first = geo.values[0]
             pool = self._find_or_create_pool(td, pools, first, new._type,
diff --git a/tests/test_octodns_provider_dyn.py b/tests/test_octodns_provider_dyn.py
index 85de59c..edb604a 100644
--- a/tests/test_octodns_provider_dyn.py
+++ b/tests/test_octodns_provider_dyn.py
@@ -1151,11 +1151,11 @@ class TestDynProviderGeo(TestCase):
         change = Create(self.geo_record)
         provider._mod_rulesets(td_mock, change)
         ruleset_create_mock.assert_has_calls((
-            call(td_mock, index=0),
-            call(td_mock, index=0),
-            call(td_mock, index=0),
-            call(td_mock, index=0),
-            call(td_mock, index=0),
+            call(td_mock, index=2),
+            call(td_mock, index=2),
+            call(td_mock, index=2),
+            call(td_mock, index=2),
+            call(td_mock, index=2),
         ))
         add_response_pool_mock.assert_has_calls((
             # default