Remove dead server code in storage and add tests

Change-Id: I2dc707f285a0bb42f280a03670875a53db3e5333

designate/backend/base.py

@@ -61,15 +61,6 @@ class Backend(DriverPlugin):
         raise exceptions.NotImplemented(
             'TSIG is not supported by this backend')
 
-    def create_server(self, context, server):
-        """Create a Server"""
-
-    def update_server(self, context, server):
-        """Update a Server"""
-
-    def delete_server(self, context, server):
-        """Delete a Server"""
-
     @abc.abstractmethod
     def create_domain(self, context, domain):
         """Create a DNS domain"""
@@ -312,15 +303,6 @@ class PoolBackend(Backend):
     def delete_tsigkey(self, context, tsigkey):
         pass
 
-    def create_server(self, context, server):
-        pass
-
-    def update_server(self, context, server):
-        pass
-
-    def delete_server(self, context, server):
-        pass
-
     @abc.abstractmethod
     def create_domain(self, context, domain):
         """

designate/backend/impl_multi.py

@@ -128,25 +128,6 @@ class MultiBackend(base.Backend):
                  for record in self.central.find_records(
                      context, {'domain_id': full_domain['id']})]
 
-    def create_server(self, context, server):
-        self.master.create_server(context, server)
-        try:
-            self.slave.create_server(context, server)
-        except Exception:
-            with excutils.save_and_reraise_exception():
-                self.master.delete_server(context, server)
-
-    def update_server(self, context, server):
-        self.master.update_server(context, server)
-
-    def delete_server(self, context, server):
-        self.slave.delete_server(context, server)
-        try:
-            self.master.delete_server(context, server)
-        except Exception:
-            with excutils.save_and_reraise_exception():
-                self.slave.create_server(context, server)
-
     def create_recordset(self, context, domain, recordset):
         self.master.create_recordset(context, domain, recordset)
 

designate/central/rpcapi.py

@@ -46,14 +46,15 @@ class CentralAPI(object):
         4.1 - Add methods for server pools
         4.2 - Add methods for pool manager integration
         4.3 - Added Zone Transfer Methods
+        5.0 - Remove dead server code
     """
-    RPC_API_VERSION = '4.3'
+    RPC_API_VERSION = '5.0'
 
     def __init__(self, topic=None):
         topic = topic if topic else cfg.CONF.central_topic
 
         target = messaging.Target(topic=topic, version=self.RPC_API_VERSION)
-        self.client = rpc.get_client(target, version_cap='4.3')
+        self.client = rpc.get_client(target, version_cap='5.0')
 
     @classmethod
     def get_instance(cls):
@@ -99,35 +100,6 @@ class CentralAPI(object):
 
         return self.client.call(context, 'reset_quotas', tenant_id=tenant_id)
 
-    # Server Methods
-    def create_server(self, context, server):
-        LOG.info(_LI("create_server: Calling central's create_server."))
-
-        return self.client.call(context, 'create_server', server=server)
-
-    def find_servers(self, context, criterion=None, marker=None, limit=None,
-                     sort_key=None, sort_dir=None):
-        LOG.info(_LI("find_servers: Calling central's find_servers."))
-
-        return self.client.call(context, 'find_servers', criterion=criterion,
-                                marker=marker, limit=limit, sort_key=sort_key,
-                                sort_dir=sort_dir)
-
-    def get_server(self, context, server_id):
-        LOG.info(_LI("get_server: Calling central's get_server."))
-
-        return self.client.call(context, 'get_server', server_id=server_id)
-
-    def update_server(self, context, server):
-        LOG.info(_LI("update_server: Calling central's update_server."))
-
-        return self.client.call(context, 'update_server', server=server)
-
-    def delete_server(self, context, server_id):
-        LOG.info(_LI("delete_server: Calling central's delete_server."))
-
-        return self.client.call(context, 'delete_server', server_id=server_id)
-
     # TSIG Key Methods
     def create_tsigkey(self, context, tsigkey):
         LOG.info(_LI("create_tsigkey: Calling central's create_tsigkey."))
@@ -393,59 +365,50 @@ class CentralAPI(object):
     # Pool Server Methods
     def create_pool(self, context, pool):
         LOG.info(_LI("create_pool: Calling central's create_pool."))
-        cctxt = self.client.prepare(version='4.1')
+        return self.client.call(context, 'create_pool', pool=pool)
-        return cctxt.call(context, 'create_pool', pool=pool)
 
     def find_pools(self, context, criterion=None, marker=None, limit=None,
                    sort_key=None, sort_dir=None):
         LOG.info(_LI("find_pools: Calling central's find_pools."))
-        cctxt = self.client.prepare(version='4.1')
+        return self.client.call(context, 'find_pools', criterion=criterion,
-        return cctxt.call(context, 'find_pools', criterion=criterion,
+                                marker=marker, limit=limit, sort_key=sort_key,
-                          marker=marker, limit=limit, sort_key=sort_key,
+                                sort_dir=sort_dir)
-                          sort_dir=sort_dir)
 
     def find_pool(self, context, criterion=None):
         LOG.info(_LI("find_pool: Calling central's find_pool."))
-        cctxt = self.client.prepare(version='4.1')
+        return self.client.call(context, 'find_pool', criterion=criterion)
-        return cctxt.call(context, 'find_pool', criterion=criterion)
 
     def get_pool(self, context, pool_id):
         LOG.info(_LI("get_pool: Calling central's get_pool."))
-        cctxt = self.client.prepare(version='4.1')
+        return self.client.call(context, 'get_pool', pool_id=pool_id)
-        return cctxt.call(context, 'get_pool', pool_id=pool_id)
 
     def update_pool(self, context, pool):
         LOG.info(_LI("update_pool: Calling central's update_pool."))
-        cctxt = self.client.prepare(version='4.1')
+        return self.client.call(context, 'update_pool', pool=pool)
-        return cctxt.call(context, 'update_pool', pool=pool)
 
     def delete_pool(self, context, pool_id):
         LOG.info(_LI("delete_pool: Calling central's delete_pool."))
-        cctxt = self.client.prepare(version='4.1')
+        return self.client.call(context, 'delete_pool', pool_id=pool_id)
-        return cctxt.call(context, 'delete_pool', pool_id=pool_id)
 
     # Pool Manager Integration Methods
     def update_status(self, context, domain_id, status, serial):
         LOG.info(_LI("update_status: Calling central's update_status."))
-        cctxt = self.client.prepare(version='4.2')
+        return self.client.call(context, 'update_status', domain_id=domain_id,
-        return cctxt.call(context, 'update_status', domain_id=domain_id,
+                                status=status, serial=serial)
-                          status=status, serial=serial)
 
     # Zone Ownership Transfers
     def create_zone_transfer_request(self, context, zone_transfer_request):
         LOG.info(_LI("create_zone_transfer_request: \
                      Calling central's create_zone_transfer_request."))
 
-        cctxt = self.client.prepare(version='4.3')
+        return self.client.call(
-        return cctxt.call(
             context, 'create_zone_transfer_request',
             zone_transfer_request=zone_transfer_request)
 
     def get_zone_transfer_request(self, context, zone_transfer_request_id):
         LOG.info(_LI("get_zone_transfer_request: \
                      Calling central's get_zone_transfer_request."))
-        cctxt = self.client.prepare(version='4.3')
+        return self.client.call(
-        return cctxt.call(
             context,
             'get_zone_transfer_request',
             zone_transfer_request_id=zone_transfer_request_id)
@@ -455,32 +418,28 @@ class CentralAPI(object):
         LOG.info(_LI("find_zone_transfer_requests: \
                      Calling central's find_zone_transfer_requests."))
 
-        cctxt = self.client.prepare(version='4.3')
+        return self.client.call(
-        return cctxt.call(
             context, 'find_zone_transfer_requests', criterion=criterion,
             marker=marker, limit=limit, sort_key=sort_key, sort_dir=sort_dir)
 
     def find_zone_transfer_request(self, context, zone_transfer_request):
         LOG.info(_LI("find_zone_transfer_request: \
                      Calling central's find_zone_transfer_request."))
-        cctxt = self.client.prepare(version='4.3')
+        return self.client.call(
-        return cctxt.call(
             context, 'find_zone_transfer_request',
             zone_transfer_request=zone_transfer_request)
 
     def update_zone_transfer_request(self, context, zone_transfer_request):
         LOG.info(_LI("update_zone_transfer_request: \
                      Calling central's update_zone_transfer_request."))
-        cctxt = self.client.prepare(version='4.3')
+        return self.client.call(
-        return cctxt.call(
             context, 'update_zone_transfer_request',
             zone_transfer_request=zone_transfer_request)
 
     def delete_zone_transfer_request(self, context, zone_transfer_request_id):
         LOG.info(_LI("delete_zone_transfer_request: \
                      Calling central's delete_zone_transfer_request."))
-        cctxt = self.client.prepare(version='4.3')
+        return self.client.call(
-        return cctxt.call(
             context,
             'delete_zone_transfer_request',
             zone_transfer_request_id=zone_transfer_request_id)
@@ -488,16 +447,14 @@ class CentralAPI(object):
     def create_zone_transfer_accept(self, context, zone_transfer_accept):
         LOG.info(_LI("create_zone_transfer_accept: \
                      Calling central's create_zone_transfer_accept."))
-        cctxt = self.client.prepare(version='4.3')
+        return self.client.call(
-        return cctxt.call(
             context, 'create_zone_transfer_accept',
             zone_transfer_accept=zone_transfer_accept)
 
     def get_zone_transfer_accept(self, context, zone_transfer_accept_id):
         LOG.info(_LI("get_zone_transfer_accept: \
                      Calling central's get_zone_transfer_accept."))
-        cctxt = self.client.prepare(version='4.3')
+        return self.client.call(
-        return cctxt.call(
             context,
             'get_zone_transfer_accept',
             zone_transfer_accept_id=zone_transfer_accept_id)
@@ -506,32 +463,28 @@ class CentralAPI(object):
                                    limit=None, sort_key=None, sort_dir=None):
         LOG.info(_LI("find_zone_transfer_accepts: \
                      Calling central's find_zone_transfer_accepts."))
-        cctxt = self.client.prepare(version='4.3')
+        return self.client.call(
-        return cctxt.call(
             context, 'find_zone_transfer_accepts', criterion=criterion,
             marker=marker, limit=limit, sort_key=sort_key, sort_dir=sort_dir)
 
     def find_zone_transfer_accept(self, context, zone_transfer_accept):
         LOG.info(_LI("find_zone_transfer_accept: \
                      Calling central's find_zone_transfer_accept."))
-        cctxt = self.client.prepare(version='4.3')
+        return self.client.call(
-        return cctxt.call(
             context, 'find_zone_transfer_accept',
             zone_transfer_accept=zone_transfer_accept)
 
     def update_zone_transfer_accept(self, context, zone_transfer_accept):
         LOG.info(_LI("update_zone_transfer_accept: \
                      Calling central's update_zone_transfer_accept."))
-        cctxt = self.client.prepare(version='4.3')
+        return self.client.call(
-        return cctxt.call(
             context, 'update_zone_transfer_accept',
             zone_transfer_accept=zone_transfer_accept)
 
     def delete_zone_transfer_accept(self, context, zone_transfer_accept_id):
         LOG.info(_LI("delete_zone_transfer_accept: \
                      Calling central's delete_zone_transfer_accept."))
-        cctxt = self.client.prepare(version='4.3')
+        return self.client.call(
-        return cctxt.call(
             context,
             'delete_zone_transfer_accept',
             zone_transfer_accept_id=zone_transfer_accept_id)

designate/central/service.py

@@ -185,7 +185,7 @@ def notification(notification_type):
 
 
 class Service(service.RPCService):
-    RPC_API_VERSION = '4.3'
+    RPC_API_VERSION = '5.0'
 
     target = messaging.Target(version=RPC_API_VERSION)
 

designate/storage/impl_sqlalchemy/__init__.py

@@ -838,16 +838,6 @@ class SQLAlchemyStorage(sqlalchemy_base.SQLAlchemy, storage_base.Storage):
                 exceptions.ZoneTransferAcceptNotFound, criterion,
                 one, marker, limit, sort_key, sort_dir)
 
-    def _get_domain_name(self, context, domain_id, all_tenants=False):
-
-        if all_tenants:
-            ctxt = context.elevated()
-            ctxt.all_tenants = True
-        else:
-            ctxt = context
-
-        return self.get_domain(ctxt, domain_id).name
-
     def create_zone_transfer_accept(self, context, zone_transfer_accept):
 
         return self._create(

designate/tests/test_central/_test_service_ipa.py

@@ -163,7 +163,7 @@ class CentralServiceTestIPA(designate.tests.test_central.
         values['ttl'] = -100
 
         # Create a server
-        self.create_server()
+        self.create_nameserver()
 
         # Create domain with negative TTL
         with testtools.ExpectedException(impl_ipa.IPAInvalidData):

designate/tests/test_storage/__init__.py

@@ -16,6 +16,7 @@
 import uuid
 import math
 
+import mock
 import testtools
 from oslo.config import cfg
 from oslo_log import log as logging
@@ -483,6 +484,14 @@ class StorageTestCase(object):
         tenants = self.storage.count_tenants(context)
         self.assertEqual(tenants, 2)
 
+    def test_count_tenants_none_result(self):
+        rp = mock.Mock()
+        rp.fetchone.return_value = None
+        with mock.patch.object(self.storage.session, 'execute',
+                               return_value=rp):
+            tenants = self.storage.count_tenants(self.admin_context)
+            self.assertEqual(tenants, 0)
+
     # Domain Tests
     def test_create_domain(self):
         pool_id = cfg.CONF['service:central'].default_pool_id
@@ -713,6 +722,14 @@ class StorageTestCase(object):
         # well, did we get 1?
         self.assertEqual(domains, 1)
 
+    def test_count_domains_none_result(self):
+        rp = mock.Mock()
+        rp.fetchone.return_value = None
+        with mock.patch.object(self.storage.session, 'execute',
+                               return_value=rp):
+            domains = self.storage.count_domains(self.admin_context)
+            self.assertEqual(domains, 0)
+
     def test_create_recordset(self):
         domain = self.create_domain()
 
@@ -969,6 +986,9 @@ class StorageTestCase(object):
         # Update the Object
         recordset.ttl = 1800
 
+        # Change records as well
+        recordset.records.append(objects.Record(data="10.0.0.1"))
+
         # Perform the update
         recordset = self.storage.update_recordset(self.admin_context,
                                                   recordset)
@@ -1116,6 +1136,14 @@ class StorageTestCase(object):
         recordsets = self.storage.count_recordsets(self.admin_context)
         self.assertEqual(recordsets, 3)
 
+    def test_count_recordsets_none_result(self):
+        rp = mock.Mock()
+        rp.fetchone.return_value = None
+        with mock.patch.object(self.storage.session, 'execute',
+                               return_value=rp):
+            recordsets = self.storage.count_recordsets(self.admin_context)
+            self.assertEqual(recordsets, 0)
+
     def test_create_record(self):
         domain = self.create_domain()
         recordset = self.create_recordset(domain, type='A')
@@ -1393,12 +1421,27 @@ class StorageTestCase(object):
         records = self.storage.count_records(self.admin_context)
         self.assertEqual(records, 3)
 
+    def test_count_records_none_result(self):
+        rp = mock.Mock()
+        rp.fetchone.return_value = None
+        with mock.patch.object(self.storage.session, 'execute',
+                               return_value=rp):
+            records = self.storage.count_records(self.admin_context)
+            self.assertEqual(records, 0)
+
     def test_ping(self):
         pong = self.storage.ping(self.admin_context)
 
         self.assertEqual(pong['status'], True)
         self.assertIsNotNone(pong['rtt'])
 
+    def test_ping_fail(self):
+        with mock.patch.object(self.storage.engine, "execute",
+                               side_effect=Exception):
+            result = self.storage.ping(self.admin_context)
+            self.assertEqual(False, result['status'])
+            self.assertIsNotNone(result['rtt'])
+
     # TLD Tests
     def test_create_tld(self):
         values = {
@@ -1914,6 +1957,22 @@ class StorageTestCase(object):
             self.storage.get_zone_transfer_request(
                 tenant_3_context, result.id)
 
+    def test_find_zone_transfer_requests(self):
+        domain = self.create_domain()
+
+        values = {
+            'tenant_id': self.admin_context.tenant,
+            'domain_id': domain.id,
+            'key': 'qwertyuiop'
+        }
+
+        self.storage.create_zone_transfer_request(
+            self.admin_context, objects.ZoneTransferRequest(**values))
+
+        requests = self.storage.find_zone_transfer_requests(
+            self.admin_context, {"tenant_id": self.admin_context.tenant})
+        self.assertEqual(len(requests), 1)
+
     def test_delete_zone_transfer_request(self):
         domain = self.create_domain()
         zt_request = self.create_zone_transfer_request(domain)
@@ -1964,6 +2023,40 @@ class StorageTestCase(object):
         self.assertEqual(result['tenant_id'], self.admin_context.tenant)
         self.assertIn('status', result)
 
+    def test_find_zone_transfer_accepts(self):
+        domain = self.create_domain()
+        zt_request = self.create_zone_transfer_request(domain)
+        values = {
+            'tenant_id': self.admin_context.tenant,
+            'zone_transfer_request_id': zt_request.id,
+            'domain_id': domain.id,
+            'key': zt_request.key
+        }
+
+        self.storage.create_zone_transfer_accept(
+            self.admin_context, objects.ZoneTransferAccept(**values))
+
+        accepts = self.storage.find_zone_transfer_accepts(
+            self.admin_context, {"tenant_id": self.admin_context.tenant})
+        self.assertEqual(len(accepts), 1)
+
+    def test_find_zone_transfer_accept(self):
+        domain = self.create_domain()
+        zt_request = self.create_zone_transfer_request(domain)
+        values = {
+            'tenant_id': self.admin_context.tenant,
+            'zone_transfer_request_id': zt_request.id,
+            'domain_id': domain.id,
+            'key': zt_request.key
+        }
+
+        result = self.storage.create_zone_transfer_accept(
+            self.admin_context, objects.ZoneTransferAccept(**values))
+
+        accept = self.storage.find_zone_transfer_accept(
+            self.admin_context, {"id": result.id})
+        self.assertEqual(accept.id, result.id)
+
     def test_transfer_zone_ownership(self):
         tenant_1_context = self.get_context(tenant='1')
         tenant_2_context = self.get_context(tenant='2')

designate/tests/test_storage/test_sqlalchemy.py

@@ -14,6 +14,7 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 from oslo_log import log as logging
+import mock
 
 from designate import storage
 from designate.tests import TestCase
@@ -27,3 +28,11 @@ class SqlalchemyStorageTest(StorageTestCase, TestCase):
         super(SqlalchemyStorageTest, self).setUp()
 
         self.storage = storage.get_storage('sqlalchemy')
+
+    def test_ping_negative(self):
+        with mock.patch.object(self.storage.engine, 'execute',
+                               return_value=0):
+            pong = self.storage.ping(self.admin_context)
+
+            self.assertEqual(pong['status'], False)
+            self.assertIsNotNone(pong['rtt'])

etc/designate/policy.json

@@ -18,12 +18,6 @@
     "set_quota": "rule:admin",
     "reset_quotas": "rule:admin",
 
-    "create_server": "rule:admin",
-    "find_servers": "rule:admin",
-    "get_server": "rule:admin",
-    "update_server": "rule:admin",
-    "delete_server": "rule:admin",
-
     "create_tld": "rule:admin",
     "find_tlds": "rule:admin",
     "get_tld": "rule:admin",