From cd2f0fb87444f9957702e863af76f88563ff9cad Mon Sep 17 00:00:00 2001 From: Endre Karlson Date: Thu, 15 Jan 2015 23:35:38 +0100 Subject: [PATCH] Remove dead server code in storage and add tests Change-Id: I2dc707f285a0bb42f280a03670875a53db3e5333 --- designate/backend/base.py | 18 ---- designate/backend/impl_multi.py | 19 ---- designate/central/rpcapi.py | 97 +++++-------------- designate/central/service.py | 2 +- designate/storage/impl_sqlalchemy/__init__.py | 10 -- .../tests/test_central/_test_service_ipa.py | 2 +- designate/tests/test_storage/__init__.py | 93 ++++++++++++++++++ .../tests/test_storage/test_sqlalchemy.py | 9 ++ etc/designate/policy.json | 6 -- 9 files changed, 129 insertions(+), 127 deletions(-) diff --git a/designate/backend/base.py b/designate/backend/base.py index e81b30bda..c6d4e7f66 100644 --- a/designate/backend/base.py +++ b/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): """ diff --git a/designate/backend/impl_multi.py b/designate/backend/impl_multi.py index de936013f..38654a014 100644 --- a/designate/backend/impl_multi.py +++ b/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) diff --git a/designate/central/rpcapi.py b/designate/central/rpcapi.py index 58b1dd780..dca2a0d50 100644 --- a/designate/central/rpcapi.py +++ b/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 cctxt.call(context, 'create_pool', pool=pool) + return self.client.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 cctxt.call(context, 'find_pools', criterion=criterion, - marker=marker, limit=limit, sort_key=sort_key, - sort_dir=sort_dir) + return self.client.call(context, 'find_pools', criterion=criterion, + marker=marker, limit=limit, sort_key=sort_key, + 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 cctxt.call(context, 'find_pool', criterion=criterion) + return self.client.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 cctxt.call(context, 'get_pool', pool_id=pool_id) + return self.client.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 cctxt.call(context, 'update_pool', pool=pool) + return self.client.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 cctxt.call(context, 'delete_pool', pool_id=pool_id) + return self.client.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 cctxt.call(context, 'update_status', domain_id=domain_id, - status=status, serial=serial) + return self.client.call(context, 'update_status', domain_id=domain_id, + 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 cctxt.call( + return self.client.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 cctxt.call( + return self.client.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 cctxt.call( + return self.client.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 cctxt.call( + return self.client.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 cctxt.call( + return self.client.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 cctxt.call( + return self.client.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 cctxt.call( + return self.client.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 cctxt.call( + return self.client.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 cctxt.call( + return self.client.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 cctxt.call( + return self.client.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 cctxt.call( + return self.client.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 cctxt.call( + return self.client.call( context, 'delete_zone_transfer_accept', zone_transfer_accept_id=zone_transfer_accept_id) diff --git a/designate/central/service.py b/designate/central/service.py index cae270019..3b7c6d690 100644 --- a/designate/central/service.py +++ b/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) diff --git a/designate/storage/impl_sqlalchemy/__init__.py b/designate/storage/impl_sqlalchemy/__init__.py index bd9a3bb62..f23685883 100644 --- a/designate/storage/impl_sqlalchemy/__init__.py +++ b/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( diff --git a/designate/tests/test_central/_test_service_ipa.py b/designate/tests/test_central/_test_service_ipa.py index cfc39ff7a..f2d9beaca 100644 --- a/designate/tests/test_central/_test_service_ipa.py +++ b/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): diff --git a/designate/tests/test_storage/__init__.py b/designate/tests/test_storage/__init__.py index bfb6dc040..2dac0e532 100644 --- a/designate/tests/test_storage/__init__.py +++ b/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') diff --git a/designate/tests/test_storage/test_sqlalchemy.py b/designate/tests/test_storage/test_sqlalchemy.py index 3b8622dab..a36fe0d1d 100644 --- a/designate/tests/test_storage/test_sqlalchemy.py +++ b/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']) diff --git a/etc/designate/policy.json b/etc/designate/policy.json index cbf2d94fa..20c4fe053 100644 --- a/etc/designate/policy.json +++ b/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",