Clean up manage pools and add additional testing

Change-Id: I6bfe074ec9db9bd95021771b8ffe988f1f23e2be

4 files changed, 128 insertions, 58 deletions

diff --git a/designate/manage/base.py b/designate/manage/base.py
index 0de7b8e9..b275408d 100644
--- a/designate/manage/base.py
+++ b/designate/manage/base.py
@@ -13,7 +13,7 @@
 # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 # License for the specific language governing permissions and limitations
 # under the License.
-from designate.context import DesignateContext
+from designate import context
 
 
 # Decorators for actions
@@ -36,5 +36,6 @@ def name(name):
 
 class Commands(object):
     def __init__(self):
-        self.context = DesignateContext.get_admin_context(
-            request_id='designate-manage')
+        self.context = context.DesignateContext.get_admin_context(
+            request_id='designate-manage'
+        )
diff --git a/designate/manage/pool.py b/designate/manage/pool.py
index 122b2135..1c7344e5 100644
--- a/designate/manage/pool.py
+++ b/designate/manage/pool.py
@@ -37,6 +37,7 @@ CONF = cfg.CONF
 class PoolCommands(base.Commands):
     def __init__(self):
         super(PoolCommands, self).__init__()
+        self.output_msg = ['']
 
     # NOTE(jh): Cannot do this earlier because we are still missing the config
     # at that point, see bug #1651576
@@ -44,13 +45,28 @@ class PoolCommands(base.Commands):
         rpc.init(cfg.CONF)
         self.central_api = central_rpcapi.CentralAPI()
 
+    def _create_pool(self, pool, dry_run):
+        pool = DesignateAdapter.parse('YAML', pool, objects.Pool())
+        for ns_record in pool.ns_records:
+            try:
+                ns_record.validate()
+            except exceptions.InvalidObject as e:
+                LOG.error(e.errors.to_list()[0]['message'])
+                sys.exit(1)
+
+        if dry_run:
+            self.output_msg.append('Create Pool: %s' % pool)
+        else:
+            LOG.info('Creating new pool: %s', pool)
+            self.central_api.create_pool(self.context, pool)
+
     def _update_zones(self, pool):
-        LOG.info("Updating zone masters for pool: {}".format(pool.id))
+        LOG.info('Updating zone masters for pool: %s', pool.id)
 
         def __get_masters_from_pool(pool):
             masters = []
             for target in pool.targets:
-                for master in target.get("masters", []):
+                for master in target.get('masters', []):
                     master = {'host': master['host'], 'port': master['port']}
                     found = False
                     for existing_master in masters:
@@ -69,9 +85,11 @@ class PoolCommands(base.Commands):
 
         for zone in zones:
             zone.masters = objects.ZoneMasterList().from_list(
-                __get_masters_from_pool(pool))
-            self.central_api.update_zone(self.context,
-                                         zone)
+                __get_masters_from_pool(pool)
+            )
+            self.central_api.update_zone(
+                self.context, zone
+            )
 
     @base.args('--file', help='The path to the file the yaml output should be '
                'written to',
@@ -81,8 +99,10 @@ class PoolCommands(base.Commands):
         try:
             pools = self.central_api.find_pools(self.context)
         except messaging.exceptions.MessagingTimeout:
-            LOG.critical("No response received from designate-central. "
-                         "Check it is running, and retry")
+            LOG.critical(
+                'No response received from designate-central. '
+                'Check it is running, and retry'
+            )
             sys.exit(1)
         with open(file, 'w') as stream:
             yaml.dump(
@@ -96,7 +116,7 @@ class PoolCommands(base.Commands):
     def show_config(self, pool_id):
         self._startup()
         try:
-            pool = self.central_api.find_pool(self.context, {"id": pool_id})
+            pool = self.central_api.find_pool(self.context, {'id': pool_id})
 
             print('Pool Configuration:')
             print('-------------------')
@@ -105,8 +125,10 @@ class PoolCommands(base.Commands):
                             default_flow_style=False))
 
         except messaging.exceptions.MessagingTimeout:
-            LOG.critical("No response received from designate-central. "
-                         "Check it is running, and retry")
+            LOG.critical(
+                'No response received from designate-central. '
+                'Check it is running, and retry'
+            )
             sys.exit(1)
 
     @base.args('--file', help='The path to the yaml file describing the pools',
@@ -115,40 +137,45 @@ class PoolCommands(base.Commands):
         '--delete',
         help='Any Pools not listed in the config file will be deleted. '
              ' WARNING: This will delete any zones left in this pool',
-        action="store_true",
+        action='store_true',
         default=False)
     @base.args(
         '--dry-run',
         help='This will simulate what will happen when you run this command',
-        action="store_true",
+        action='store_true',
         default=False)
     def update(self, file, delete, dry_run):
         self._startup()
         print('Updating Pools Configuration')
         print('****************************')
-        output_msg = ['']
 
         with open(file, 'r') as stream:
             xpools = yaml.safe_load(stream)
 
         if dry_run:
-            output_msg.append("The following changes will occur:")
-            output_msg.append("*********************************")
+            self.output_msg.append('The following changes will occur:')
+            self.output_msg.append('*********************************')
 
         for xpool in xpools:
             try:
                 if 'id' in xpool:
                     try:
                         pool = self.central_api.get_pool(
-                            self.context, xpool['id'])
+                            self.context, xpool['id']
+                        )
                     except Exception as e:
-                        msg = ("Bad ID Supplied for pool. pool_id: "
-                            "%(pool)s message: %(res)s")
-                        LOG.critical(msg, {'pool': xpool['id'], 'res': e})
+                        LOG.critical(
+                            'Bad ID Supplied for pool. pool_id: '
+                            '%(pool)s message: %(res)s',
+                            {
+                                'pool': xpool['id'], 'res': e
+                            }
+                        )
                         continue
                 else:
                     pool = self.central_api.find_pool(
-                        self.context, {"name": xpool['name']})
+                        self.context, {'name': xpool['name']}
+                    )
 
                 LOG.info('Updating existing pool: %s', pool)
 
@@ -167,7 +194,7 @@ class PoolCommands(base.Commands):
                         sys.exit(1)
 
                 if dry_run:
-                    output_msg.append("Update Pool: %s" % pool)
+                    self.output_msg.append('Update Pool: %s' % pool)
                 else:
                     pool = self.central_api.update_pool(self.context, pool)
                     # Bug: Changes in the pool targets should trigger a
@@ -175,21 +202,12 @@ class PoolCommands(base.Commands):
                     self._update_zones(pool)
 
             except exceptions.PoolNotFound:
-                pool = DesignateAdapter.parse('YAML', xpool, objects.Pool())
-                for ns_record in pool.ns_records:
-                    try:
-                        ns_record.validate()
-                    except exceptions.InvalidObject as e:
-                        LOG.error(e.errors.to_list()[0]['message'])
-                        sys.exit(1)
-                if dry_run:
-                    output_msg.append("Create Pool: %s" % pool)
-                else:
-                    LOG.info('Creating new pool: %s', pool)
-                    self.central_api.create_pool(self.context, pool)
+                self._create_pool(xpool, dry_run)
             except messaging.exceptions.MessagingTimeout:
-                LOG.critical("No response received from designate-central. "
-                             "Check it is running, and retry")
+                LOG.critical(
+                    'No response received from designate-central. '
+                    'Check it is running, and retry'
+                )
                 sys.exit(1)
 
         if delete:
@@ -206,7 +224,7 @@ class PoolCommands(base.Commands):
                         criterion={'name': pool})
 
                     if dry_run:
-                        output_msg.append("Delete Pool: %s" % p)
+                        self.output_msg.append('Delete Pool: %s' % p)
 
                     else:
                         LOG.info('Deleting %s', p)
@@ -214,9 +232,10 @@ class PoolCommands(base.Commands):
 
                 except messaging.exceptions.MessagingTimeout:
                     LOG.critical(
-                        "No response received from designate-central. "
-                        "Check it is running, and retry")
+                        'No response received from designate-central. '
+                        'Check it is running, and retry'
+                    )
                     sys.exit(1)
 
-        for line in output_msg:
+        for line in self.output_msg:
             print(line)
diff --git a/designate/objects/adapters/yaml/pool_attribute.py b/designate/objects/adapters/yaml/pool_attribute.py
index 3ca3508f..41bc8b77 100644
--- a/designate/objects/adapters/yaml/pool_attribute.py
+++ b/designate/objects/adapters/yaml/pool_attribute.py
@@ -59,7 +59,6 @@ class PoolAttributeListYAMLAdapter(base.YAMLAdapter):
 
     @classmethod
     def parse_list(cls, values, output_object, *args, **kwargs):
-
         for key, value in values.items():
             # Add the object to the list
             output_object.append(
diff --git a/designate/tests/test_manage/test_update_pool.py b/designate/tests/test_manage/test_update_pool.py
index f759d178..70087977 100644
--- a/designate/tests/test_manage/test_update_pool.py
+++ b/designate/tests/test_manage/test_update_pool.py
@@ -13,6 +13,7 @@ from unittest import mock
 
 from oslo_log import log as logging
 
+from designate import context
 from designate.manage.pool import PoolCommands
 from designate import objects
 from designate.tests import fixtures
@@ -27,6 +28,10 @@ class UpdatePoolTestCase(DesignateManageTestCase):
         self.stdlog = fixtures.StandardLogging()
         self.useFixture(self.stdlog)
 
+        self.context = context.DesignateContext.get_admin_context(
+            request_id='designate-manage'
+        )
+
     def hydrate_pool_targets(self, target_masters):
         pool_targets = objects.PoolTargetList()
         masters = objects.PoolTargetMasterList()
@@ -49,19 +54,21 @@ class UpdatePoolTestCase(DesignateManageTestCase):
 
         # Ensure the correct NS Records are in place
         pool = self.central_service.get_pool(
-            self.admin_context, zone.pool_id)
+            self.admin_context, zone.pool_id
+        )
 
         pool.targets = self.hydrate_pool_targets([objects.PoolTargetMaster(
-                  pool_target_id=pool.id,
-                  host="127.0.0.1",
-                  port="53")])
+            pool_target_id=pool.id,
+            host='192.0.2.2',
+            port='53')]
+        )
 
         command = PoolCommands()
-        command.context = self.admin_context
+        command.context = self.context
         command.central_api = self.central_service
 
-        with mock.patch.object(self.central_service,
-                               "update_zone") as mock_update_zone:
+        with mock.patch.object(
+                self.central_service, 'update_zone') as mock_update_zone:
             command._update_zones(pool)
             mock_update_zone.assert_called_once()
 
@@ -77,25 +84,69 @@ class UpdatePoolTestCase(DesignateManageTestCase):
 
         # Ensure the correct NS Records are in place
         pool = self.central_service.get_pool(
-            self.admin_context, zone.pool_id)
+            self.admin_context, zone.pool_id
+        )
 
         targets1 = self.hydrate_pool_targets([
             objects.PoolTargetMaster(
-                  pool_target_id=pool.id,
-                  host="127.0.0.1",
-                  port="53")
+                pool_target_id=pool.id,
+                host='192.0.2.3',
+                port='53')
         ])
         targets2 = self.hydrate_pool_targets([
             objects.PoolTargetMaster(
-                  pool_target_id=pool.id,
-                  host="127.0.0.1",
-                  port="53")
+                pool_target_id=pool.id,
+                host='192.0.2.4',
+                port='53')
         ])
         pool.targets = objects.PoolTargetList()
         pool.targets.extend(targets1.objects + targets2.objects)
 
         command = PoolCommands()
-        command.context = self.admin_context
+        command.context = self.context
         command.central_api = self.central_service
 
         command._update_zones(pool)
+
+    def test_create_new_pool(self):
+        pool = {
+            'name': 'new_pool',
+            'description': 'New PowerDNS Pool',
+            'attributes': {},
+            'ns_records': [
+                {'hostname': 'ns1-1.example.org.', 'priority': 1},
+                {'hostname': 'ns1-2.example.org.', 'priority': 2}
+            ],
+            'nameservers': [
+                {'host': '192.0.2.2', 'port': 53}
+            ],
+            'targets': [
+                {
+                    'type': 'powerdns',
+                    'description': 'PowerDNS Database Cluster',
+                    'masters': [
+                        {'host': '192.0.2.1', 'port': 5354}
+                    ],
+                    'options': {
+                        'host': '192.0.2.2', 'port': 53,
+                        'connection': 'connection'
+                    }
+                }
+            ],
+            'also_notifies': [
+                {'host': '192.0.2.4', 'port': 53}
+            ]
+        }
+
+        command = PoolCommands()
+        command.context = self.context
+        command.central_api = self.central_service
+
+        command._create_pool(pool, dry_run=False)
+
+        pool = self.central_service.find_pool(
+            self.admin_context, {'name': 'new_pool'}
+        )
+
+        self.assertEqual('new_pool', pool.name)
+        self.assertEqual('New PowerDNS Pool', pool.description)