From 75acc514897e555cc64981413eb5bf2955ba656d Mon Sep 17 00:00:00 2001 From: Douglas Viroel Date: Tue, 23 Mar 2021 09:24:54 -0300 Subject: [PATCH] Remove unused model properties and increase unit test coverage This is a follow up patch for [1] which removes some unused model properties and add more unit tests to increase coverage. [1] https://review.opendev.org/c/openstack/manila/+/774728 Change-Id: I10af132203cb07cea62839014925c7e4c99499e4 Signed-off-by: Douglas Viroel --- manila/db/sqlalchemy/models.py | 14 +- manila/share/manager.py | 3 - manila/tests/api/v2/test_share_networks.py | 170 +++++++++++++++- manila/tests/api/views/test_share_networks.py | 31 +++ manila/tests/cmd/test_manage.py | 42 ++++ manila/tests/db/sqlalchemy/test_api.py | 164 +++++++++++++++- manila/tests/share/test_access.py | 12 ++ manila/tests/share/test_api.py | 184 ++++++++++++++++++ manila/tests/share/test_manager.py | 1 - 9 files changed, 588 insertions(+), 33 deletions(-) diff --git a/manila/db/sqlalchemy/models.py b/manila/db/sqlalchemy/models.py index 5d045f968e..81a388d149 100644 --- a/manila/db/sqlalchemy/models.py +++ b/manila/db/sqlalchemy/models.py @@ -188,8 +188,7 @@ class Share(BASE, ManilaBase): __tablename__ = 'shares' _extra_keys = ['name', 'export_location', 'export_locations', 'status', 'host', 'share_server_id', 'share_network_id', - 'availability_zone', 'access_rules_status', 'share_type_id', - 'share_network_status'] + 'availability_zone', 'access_rules_status', 'share_type_id'] @property def name(self): @@ -228,8 +227,7 @@ class Share(BASE, ManilaBase): def __getattr__(self, item): proxified_properties = ('status', 'host', 'share_server_id', 'share_network_id', 'availability_zone', - 'share_type_id', 'share_type', - 'share_network_status') + 'share_type_id', 'share_type') if item in proxified_properties: return getattr(self.instance, item, None) @@ -1016,10 +1014,6 @@ class ShareNetworkSubnet(BASE, ManilaBase): def share_network_name(self): return self.share_network['name'] - @property - def share_network_status(self): - return self.share_network['status'] - class ShareServer(BASE, ManilaBase): """Represents share server used by share.""" @@ -1078,10 +1072,6 @@ class ShareServer(BASE, ManilaBase): return {model['key']: model['value'] for model in self._backend_details} - @property - def share_network_status(self): - return self.share_network_subnet['share_network']['status'] - _extra_keys = ['backend_details'] diff --git a/manila/share/manager.py b/manila/share/manager.py index a5db38d7ab..917b688623 100644 --- a/manila/share/manager.py +++ b/manila/share/manager.py @@ -4665,8 +4665,6 @@ class ShareManager(manager.SchedulerDependentManager): 'identifier': share_server.get('identifier', None), 'network_allocations': share_server.get('network_allocations', None), - 'share_network_status': share_server.get( - 'share_network_status', None) } return share_server_ref @@ -4717,7 +4715,6 @@ class ShareManager(manager.SchedulerDependentManager): 'source_share_group_snapshot_member_id': share_instance.get( 'source_share_group_snapshot_member_id'), 'availability_zone': share_instance.get('availability_zone'), - 'share_network_status': share_instance.get('share_network_status') } if share_instance_ref['share_server']: share_instance_ref['share_server'] = self._get_share_server_dict( diff --git a/manila/tests/api/v2/test_share_networks.py b/manila/tests/api/v2/test_share_networks.py index fba42e20f1..c72fcd3f4b 100644 --- a/manila/tests/api/v2/test_share_networks.py +++ b/manila/tests/api/v2/test_share_networks.py @@ -1203,6 +1203,91 @@ class ShareNetworkAPITest(test.TestCase): share_networks.RESOURCE_NAME, 'remove_security_service', target_obj=share_network) + @mock.patch.object(share_networks.policy, 'check_policy', mock.Mock()) + def test_action_remove_security_service_share_no_share_servers(self): + share_network = fake_share_network.copy() + subnet = fake_share_network_subnet.copy() + share_network['share_network_subnets'] = [subnet] + mock_share_net_get = self.mock_object( + db_api, 'share_network_get', mock.Mock(return_value=share_network)) + mock_contains_servers = self.mock_object( + self.controller, '_share_network_subnets_contain_share_servers', + mock.Mock(return_value=False)) + mock_api_remove = self.mock_object( + db_api, 'share_network_remove_security_service', + mock.Mock(return_value=share_network)) + mock_view = self.mock_object( + self.controller._view_builder, 'build_share_network', + mock.Mock(return_value='fake_view')) + body = { + 'remove_security_service': { + 'security_service_id': 'fake_ss_id', + }, + } + + view = self.controller.remove_security_service( + self.req, share_network['id'], body) + + self.assertEqual('fake_view', view) + mock_share_net_get.assert_called_once_with( + self.req.environ['manila.context'], share_network['id']) + share_networks.policy.check_policy.assert_called_once_with( + self.req.environ['manila.context'], + share_networks.RESOURCE_NAME, + 'remove_security_service', target_obj=share_network) + mock_contains_servers.assert_called_once_with(share_network) + mock_api_remove.assert_called_once_with( + self.req.environ['manila.context'], share_network['id'], + 'fake_ss_id') + mock_view.assert_called_once_with(self.req, share_network) + + @ddt.data( + (KeyError, webob_exc.HTTPBadRequest), + (exception.NotFound(message='fake'), webob_exc.HTTPNotFound), + (exception.ShareNetworkSecurityServiceDissociationError( + message='fake'), webob_exc.HTTPBadRequest)) + @ddt.unpack + def test_action_remove_security_service_share_api_exception(self, api_exp, + expect_exp): + share_network = fake_share_network.copy() + subnet = fake_share_network_subnet.copy() + share_network['share_network_subnets'] = [subnet] + mock_policy = self.mock_object( + share_networks.policy, 'check_policy', mock.Mock()) + mock_share_net_get = self.mock_object( + db_api, 'share_network_get', + mock.Mock(return_value=share_network)) + mock_contains_servers = self.mock_object( + self.controller, '_share_network_subnets_contain_share_servers', + mock.Mock(return_value=False)) + mock_api_remove = self.mock_object( + db_api, 'share_network_remove_security_service', + mock.Mock(side_effect=api_exp)) + body = { + 'remove_security_service': { + 'security_service_id': 'fake_ss_id', + }, + } + + self.assertRaises(expect_exp, + self.controller.remove_security_service, + self.req, + share_network['id'], + body) + + mock_share_net_get.assert_called_once_with( + self.req.environ['manila.context'], share_network['id']) + db_api.share_network_get.assert_called_once_with( + self.req.environ['manila.context'], share_network['id']) + mock_policy.assert_called_once_with( + self.req.environ['manila.context'], + share_networks.RESOURCE_NAME, + 'remove_security_service', target_obj=share_network) + mock_contains_servers.assert_called_once_with(share_network) + mock_api_remove.assert_called_once_with( + self.req.environ['manila.context'], share_network['id'], + 'fake_ss_id') + @ddt.data('add_security_service', 'remove_security_service') def test_action_security_service_contains_share_servers(self, action): share_network = fake_share_network.copy() @@ -1237,24 +1322,28 @@ class ShareNetworkAPITest(test.TestCase): self.req.environ['manila.context'], share_networks.RESOURCE_NAME, action, target_obj=share_network) - def _setup_data_for_check_update_tests(self): + def _setup_data_for_update_tests(self, is_check=False): security_services = [ db_utils.create_security_service() for i in range(2)] share_network = db_utils.create_share_network() + action = ('update_security_service_check' if is_check + else 'update_security_service') body = { - 'update_security_service_check': { - 'reset_operation': False, + action: { 'current_service_id': security_services[0]['id'], 'new_service_id': security_services[1]['id'], } } + if is_check: + body[action]['reset_operation'] = False request = fakes.HTTPRequest.blank( - '/share-networks', use_admin_context=True, version='2.63') + '/v2/fake/share-networks/%s/action' % share_network['id'], + use_admin_context=True, version='2.63') return security_services, share_network, body, request def test_check_update_security_service_not_found(self): security_services, share_network, body, request = ( - self._setup_data_for_check_update_tests()) + self._setup_data_for_update_tests(is_check=True)) context = request.environ['manila.context'] @@ -1279,7 +1368,7 @@ class ShareNetworkAPITest(test.TestCase): def test_check_update_security_service(self): security_services, share_network, body, request = ( - self._setup_data_for_check_update_tests()) + self._setup_data_for_update_tests(is_check=True)) context = request.environ['manila.context'] share_api_return = {'fake_key': 'fake_value'} @@ -1315,12 +1404,14 @@ class ShareNetworkAPITest(test.TestCase): @ddt.data( (exception.ServiceIsDown(message='fake'), webob_exc.HTTPConflict), (exception.InvalidShareNetwork(message='fake'), - webob_exc.HTTPBadRequest)) + webob_exc.HTTPBadRequest), + (exception.InvalidSecurityService(message='fake'), + webob_exc.HTTPConflict)) @ddt.unpack def test_check_update_security_service_share_api_failed( self, captured_exception, exception_to_be_raised): security_services, share_network, body, request = ( - self._setup_data_for_check_update_tests()) + self._setup_data_for_update_tests(is_check=True)) context = request.environ['manila.context'] self.mock_object(share_networks.policy, 'check_policy') @@ -1426,7 +1517,9 @@ class ShareNetworkAPITest(test.TestCase): @ddt.data( (exception.ServiceIsDown(message='fake'), webob_exc.HTTPConflict), (exception.InvalidShareNetwork(message='fake'), - webob_exc.HTTPBadRequest)) + webob_exc.HTTPBadRequest), + (exception.InvalidSecurityService(message='fake'), + webob_exc.HTTPConflict)) @ddt.unpack def test_check_add_security_service_share_api_failed( self, captured_exception, exception_to_be_raised): @@ -1460,3 +1553,62 @@ class ShareNetworkAPITest(test.TestCase): assert_called_once_with( context, share_network, security_service, reset_operation=False)) + + @ddt.data( + (exception.ServiceIsDown(message='fake'), webob_exc.HTTPConflict), + (exception.InvalidShareNetwork(message='fake'), + webob_exc.HTTPBadRequest), + (exception.InvalidSecurityService(message='fake'), + webob_exc.HTTPConflict)) + @ddt.unpack + def test_update_security_service_share_api_failed( + self, captured_exception, exception_to_be_raised): + security_services, share_network, body, request = ( + self._setup_data_for_update_tests()) + context = request.environ['manila.context'] + + self.mock_object(share_networks.policy, 'check_policy') + self.mock_object(db_api, 'share_network_get', + mock.Mock(return_value=share_network)) + self.mock_object( + db_api, 'security_service_get', + mock.Mock( + side_effect=[security_services[0], security_services[1]])) + self.mock_object( + self.controller.share_api, + 'update_share_network_security_service', + mock.Mock(side_effect=captured_exception)) + + self.assertRaises(exception_to_be_raised, + self.controller.update_security_service, + request, + share_network['id'], + body) + + db_api.share_network_get.assert_called_once_with( + context, share_network['id']) + db_api.security_service_get.assert_has_calls( + [mock.call(context, security_services[0]['id']), + mock.call(context, security_services[1]['id'])]) + (self.controller.share_api.update_share_network_security_service. + assert_called_once_with( + context, share_network, security_services[1], + current_security_service=security_services[0])) + + def test_reset_status(self): + share_network = db_utils.create_share_network() + request = fakes.HTTPRequest.blank( + '/v2/fake/share-networks/%s/action' % share_network['id'], + use_admin_context=True, version='2.63') + self.mock_object(db_api, 'share_network_update') + + status = {'status': 'active'} + body = {'reset_status': status} + + response = self.controller.reset_status(request, + share_network['id'], body) + + self.assertEqual(202, response.status_int) + db_api.share_network_update.assert_called_once_with( + request.environ['manila.context'], + share_network['id'], {'status': 'active'}) diff --git a/manila/tests/api/views/test_share_networks.py b/manila/tests/api/views/test_share_networks.py index 4095bc8017..44c2397042 100644 --- a/manila/tests/api/views/test_share_networks.py +++ b/manila/tests/api/views/test_share_networks.py @@ -217,3 +217,34 @@ class ViewBuilderTestCase(test.TestCase): is_detail=False) self.assertEqual(expected, result) + + @ddt.data(('update_security_service', True), + ('add_security_service', False)) + @ddt.unpack + def test_build_security_service_update_check(self, operation, is_admin): + req = fakes.HTTPRequest.blank('/share-networks', + use_admin_context=is_admin) + params = {'new_service_id': 'new_id'} + if operation == 'update_security_service': + params['current_service_id'] = 'current_id' + + hosts_result = { + 'compatible': True, + 'hosts_check_result': {'hostA': True} + } + expected = { + 'compatible': True, + 'requested_operation': { + 'operation': operation, + 'current_security_service': params.get('current_service_id'), + 'new_security_service': params.get('new_service_id'), + }, + } + if is_admin: + expected['hosts_check_result'] = hosts_result['hosts_check_result'] + + result = self.builder.build_security_service_update_check(req, + params, + hosts_result) + + self.assertEqual(expected, result) diff --git a/manila/tests/cmd/test_manage.py b/manila/tests/cmd/test_manage.py index afd2afc8a6..ed6e43f715 100644 --- a/manila/tests/cmd/test_manage.py +++ b/manila/tests/cmd/test_manage.py @@ -47,6 +47,7 @@ class ManilaCmdManageTestCase(test.TestCase): self.get_log_cmds = manila_manage.GetLogCommands() self.service_cmds = manila_manage.ServiceCommands() self.share_cmds = manila_manage.ShareCommands() + self.server_cmds = manila_manage.ShareServerCommands() @mock.patch.object(manila_manage.ShellCommands, 'run', mock.Mock()) def test_shell_commands_bpython(self): @@ -437,3 +438,44 @@ class ManilaCmdManageTestCase(test.TestCase): self.assertEqual(expected_op, intercepted_op.getvalue().strip()) db.share_resources_host_update.assert_called_once_with( 'admin_ctxt', current_host, new_host) + + def test_share_server_update_capability(self): + self.mock_object(context, 'get_admin_context', + mock.Mock(return_value='admin_ctxt')) + self.mock_object(db, 'share_servers_update') + share_servers = 'server_id_a,server_id_b' + share_server_list = [server.strip() + for server in share_servers.split(",")] + capability = 'security_service_update_support' + values_to_update = { + capability: True + } + + with mock.patch('sys.stdout', new=io.StringIO()) as output: + self.server_cmds.update_share_server_capabilities( + share_servers, capability, True) + + expected_op = ("The capability(ies) %(cap)s of the following share " + "server(s) %(servers)s was(were) updated to " + "%(value)s.") % { + 'cap': [capability], + 'servers': share_server_list, + 'value': True, + } + + self.assertEqual(expected_op, output.getvalue().strip()) + db.share_servers_update.assert_called_once_with( + 'admin_ctxt', share_server_list, values_to_update) + + def test_share_server_update_capability_not_supported(self): + share_servers = 'server_id_a' + capabilities = 'invalid_capability' + + exit = self.assertRaises( + SystemExit, + self.server_cmds.update_share_server_capabilities, + share_servers, + capabilities, + True) + + self.assertEqual(1, exit.code) diff --git a/manila/tests/db/sqlalchemy/test_api.py b/manila/tests/db/sqlalchemy/test_api.py index 3efbd0e037..39fb3cd74b 100644 --- a/manila/tests/db/sqlalchemy/test_api.py +++ b/manila/tests/db/sqlalchemy/test_api.py @@ -3199,6 +3199,22 @@ class ShareServerDatabaseAPITestCase(test.TestCase): for share_server in share_servers: self.assertEqual(host, share_server['host']) + def test_share_servers_update(self): + servers = [db_utils.create_share_server() + for __ in range(1, 3)] + server_ids = [server['id'] for server in servers] + values = {'status': constants.STATUS_NETWORK_CHANGE} + + db_api.share_servers_update( + self.ctxt, server_ids, values) + + share_servers = [ + db_api.share_server_get(self.ctxt, server_id) + for server_id in server_ids] + + for ss in share_servers: + self.assertEqual(constants.STATUS_NETWORK_CHANGE, ss['status']) + class ServiceDatabaseAPITestCase(test.TestCase): @@ -4277,8 +4293,10 @@ class ShareResourcesAPITestCase(test.TestCase): def test_share_instances_status_update(self): for i in range(1, 3): - instances = [db_utils.create_share_instance( - status=constants.STATUS_SERVER_MIGRATING, share_id='fake')] + instances = [ + db_utils.create_share_instance( + status=constants.STATUS_SERVER_MIGRATING, share_id='fake') + for __ in range(1, 3)] share_instance_ids = [instance['id'] for instance in instances] values = {'status': constants.STATUS_AVAILABLE} @@ -4295,10 +4313,11 @@ class ShareResourcesAPITestCase(test.TestCase): def test_share_snapshot_instances_status_update(self): share_instance = db_utils.create_share_instance( status=constants.STATUS_AVAILABLE, share_id='fake') - for i in range(1, 3): - instances = [db_utils.create_snapshot_instance( + instances = [ + db_utils.create_snapshot_instance( 'fake_snapshot_id_1', status=constants.STATUS_CREATING, - share_instance_id=share_instance['id'])] + share_instance_id=share_instance['id']) + for __ in range(1, 3)] snapshot_instance_ids = [instance['id'] for instance in instances] values = {'status': constants.STATUS_AVAILABLE} @@ -4318,10 +4337,11 @@ class ShareResourcesAPITestCase(test.TestCase): status=constants.STATUS_AVAILABLE, share_id='fake') share_instance_ids = [share_instance['id']] fake_session = db_api.get_session() - for i in range(1, 3): - snap_instances = [db_utils.create_snapshot_instance( + snap_instances = [ + db_utils.create_snapshot_instance( 'fake_snapshot_id_1', status=constants.STATUS_CREATING, - share_instance_id=share_instance['id'])] + share_instance_id=share_instance['id']) + for __ in range(1, 3)] snapshot_instance_ids = [instance['id'] for instance in snap_instances] values = {'status': constants.STATUS_AVAILABLE} @@ -4398,3 +4418,131 @@ class ShareResourcesAPITestCase(test.TestCase): mock_snap_instances_get_all.assert_called_once_with( self.context, {'instance_ids': snap_instance_ids}, session=fake_session) + + +@ddt.ddt +class AsyncOperationDatabaseAPITestCase(test.TestCase): + + def setUp(self): + """Run before each test.""" + super(AsyncOperationDatabaseAPITestCase, self).setUp() + self.user_id = uuidutils.generate_uuid() + self.project_id = uuidutils.generate_uuid() + self.ctxt = context.RequestContext( + user_id=self.user_id, project_id=self.project_id, is_admin=False) + + def _get_async_operation_test_data(self): + return uuidutils.generate_uuid() + + @ddt.data({"details": {"foo": "bar", "tee": "too"}, + "valid": {"foo": "bar", "tee": "too"}}, + {"details": {"foo": "bar", "tee": ["test"]}, + "valid": {"foo": "bar", "tee": str(["test"])}}) + @ddt.unpack + def test_update(self, details, valid): + entity_id = self._get_async_operation_test_data() + + initial_data = db_api.async_operation_data_get(self.ctxt, entity_id) + db_api.async_operation_data_update(self.ctxt, entity_id, details) + actual_data = db_api.async_operation_data_get(self.ctxt, entity_id) + + self.assertEqual({}, initial_data) + self.assertEqual(valid, actual_data) + + @ddt.data({'with_deleted': True, 'append': False}, + {'with_deleted': True, 'append': True}, + {'with_deleted': False, 'append': False}, + {'with_deleted': False, 'append': True}) + @ddt.unpack + def test_update_with_more_values(self, with_deleted, append): + entity_id = self._get_async_operation_test_data() + details = {"tee": "too"} + more_details = {"foo": "bar"} + result = {"tee": "too", "foo": "bar"} + + db_api.async_operation_data_update(self.ctxt, entity_id, details) + if with_deleted: + db_api.async_operation_data_delete(self.ctxt, entity_id) + if append: + more_details.update(details) + if with_deleted and not append: + result.pop("tee") + db_api.async_operation_data_update(self.ctxt, entity_id, more_details) + + actual_result = db_api.async_operation_data_get(self.ctxt, entity_id) + + self.assertEqual(result, actual_result) + + @ddt.data(True, False) + def test_update_with_duplicate(self, with_deleted): + entity_id = self._get_async_operation_test_data() + details = {"tee": "too"} + + db_api.async_operation_data_update(self.ctxt, entity_id, details) + if with_deleted: + db_api.async_operation_data_delete(self.ctxt, entity_id) + db_api.async_operation_data_update(self.ctxt, entity_id, details) + + actual_result = db_api.async_operation_data_get(self.ctxt, + entity_id) + + self.assertEqual(details, actual_result) + + def test_update_with_delete_existing(self): + resource_id = self._get_async_operation_test_data() + details = {"key1": "val1", "key2": "val2", "key3": "val3"} + details_update = {"key1": "val1_upd", "key4": "new_val"} + + # Create new details + db_api.async_operation_data_update(self.ctxt, resource_id, details) + db_api.async_operation_data_update(self.ctxt, resource_id, + details_update, + delete_existing=True) + + actual_result = db_api.async_operation_data_get(self.ctxt, resource_id) + + self.assertEqual(details_update, actual_result) + + def test_get(self): + resource_id = self._get_async_operation_test_data() + test_key = "foo" + test_keys = [test_key, "tee"] + details = {test_keys[0]: "val", test_keys[1]: "val", "mee": "foo"} + db_api.async_operation_data_update(self.ctxt, resource_id, details) + + actual_result_all = db_api.async_operation_data_get( + self.ctxt, resource_id) + actual_result_single_key = db_api.async_operation_data_get( + self.ctxt, resource_id, test_key) + actual_result_list = db_api.async_operation_data_get( + self.ctxt, resource_id, test_keys) + + self.assertEqual(details, actual_result_all) + self.assertEqual(details[test_key], actual_result_single_key) + self.assertEqual(dict.fromkeys(test_keys, "val"), actual_result_list) + + def test_delete_single(self): + test_id = self._get_async_operation_test_data() + test_key = "foo" + details = {test_key: "bar", "tee": "too"} + valid_result = {"tee": "too"} + db_api.async_operation_data_update(self.ctxt, test_id, details) + + db_api.async_operation_data_delete(self.ctxt, test_id, test_key) + + actual_result = db_api.async_operation_data_get( + self.ctxt, test_id) + + self.assertEqual(valid_result, actual_result) + + def test_delete_all(self): + test_id = self._get_async_operation_test_data() + details = {"foo": "bar", "tee": "too"} + db_api.async_operation_data_update(self.ctxt, test_id, details) + + db_api.async_operation_data_delete(self.ctxt, test_id) + + actual_result = db_api.async_operation_data_get( + self.ctxt, test_id) + + self.assertEqual({}, actual_result) diff --git a/manila/tests/share/test_access.py b/manila/tests/share/test_access.py index 64fc6184e2..75789b5b5a 100644 --- a/manila/tests/share/test_access.py +++ b/manila/tests/share/test_access.py @@ -796,3 +796,15 @@ class ShareInstanceAccessTestCase(test.TestCase): }, ] return pass_rules, fail_rules + + def test_update_share_instances_access_rules_status(self): + mock_db_instances_update = self.mock_object( + db, 'share_instances_status_update') + share_instances = ['fake_instance_id', 'fake_instance_id_2'] + + self.access_helper.update_share_instances_access_rules_status( + self.context, 'fake_status', share_instances) + + mock_db_instances_update.assert_called_once_with( + self.context, share_instances, + {'access_rules_status': 'fake_status'}) diff --git a/manila/tests/share/test_api.py b/manila/tests/share/test_api.py index e05fb8fbc0..94bffc8416 100644 --- a/manila/tests/share/test_api.py +++ b/manila/tests/share/test_api.py @@ -16,6 +16,7 @@ import copy import datetime +import json from unittest import mock @@ -5830,6 +5831,82 @@ class ShareAPITestCase(test.TestCase): self.context, share_network, new_sec_service, ) + def test_update_share_network_security_service_no_share_servers(self): + mock_initial_checks = self.mock_object( + self.api, '_share_network_update_initial_checks', + mock.Mock(return_value=([], []))) + mock_get_key = self.mock_object( + self.api, 'get_security_service_update_key') + fake_share_network = {'id': 'fake_share_net_id'} + fake_sec_service = {'id': 'fake_sec_serv_id'} + + self.api.update_share_network_security_service( + self.context, fake_share_network, fake_sec_service, + current_security_service=None) + + mock_initial_checks.assert_called_once_with( + self.context, fake_share_network, fake_sec_service, + current_security_service=None) + mock_get_key.assert_not_called() + + def test_update_share_network_security_service_without_check(self): + mock_initial_checks = self.mock_object( + self.api, '_share_network_update_initial_checks', + mock.Mock(return_value=(['fake_server'], ['fake_host']))) + mock_get_key = self.mock_object( + self.api, 'get_security_service_update_key', + mock.Mock(return_value=None)) + fake_share_network = {'id': 'fake_share_net_id'} + fake_sec_service = {'id': 'fake_sec_serv_id'} + + self.assertRaises(exception.InvalidShareNetwork, + self.api.update_share_network_security_service, + self.context, fake_share_network, fake_sec_service) + + mock_initial_checks.assert_called_once_with( + self.context, fake_share_network, fake_sec_service, + current_security_service=None) + mock_get_key.assert_called_once_with( + 'hosts_check', fake_sec_service['id'], + current_security_service_id=None) + + def test_update_share_network_security_service_update_hosts_failure(self): + mock_initial_checks = self.mock_object( + self.api, '_share_network_update_initial_checks', + mock.Mock(return_value=(['fake_server'], ['fake_host']))) + mock_get_key = self.mock_object( + self.api, 'get_security_service_update_key', + mock.Mock(return_value='fake_key')) + mock_async_db_get = self.mock_object( + db_api, 'async_operation_data_get', + mock.Mock(return_value='fake_value')) + mock_validate_host = self.mock_object( + self.api, '_security_service_update_validate_hosts', + mock.Mock(side_effect=Exception)) + mock_async_db_delete = self.mock_object( + db_api, 'async_operation_data_delete') + fake_share_network = {'id': 'fake_share_net_id'} + fake_sec_service = {'id': 'fake_sec_serv_id'} + + self.assertRaises(exception.InvalidShareNetwork, + self.api.update_share_network_security_service, + self.context, fake_share_network, fake_sec_service) + + mock_initial_checks.assert_called_once_with( + self.context, fake_share_network, fake_sec_service, + current_security_service=None) + mock_get_key.assert_called_once_with( + 'hosts_check', fake_sec_service['id'], + current_security_service_id=None) + mock_async_db_get.assert_called_once_with( + self.context, fake_share_network['id'], 'fake_key') + mock_validate_host.assert_called_once_with( + self.context, fake_share_network, ['fake_host'], ['fake_server'], + new_security_service_id=fake_sec_service['id'], + current_security_service_id=None) + mock_async_db_delete.assert_called_once_with( + self.context, fake_share_network['id'], 'fake_key') + def test_update_share_network_security_service_backend_host_failure(self): share_network = db_utils.create_share_network() security_service = db_utils.create_security_service() @@ -5924,6 +6001,113 @@ class ShareAPITestCase(test.TestCase): mock_db_async_op_del.assert_called_once_with( self.context, share_network['id'], fake_update_key) + def test__security_service_update_validate_hosts_new_check(self): + curr_sec_service_id = "fake_curr_sec_serv_id" + new_sec_service_id = "fake_new_sec_serv_id" + fake_key = (curr_sec_service_id + '_' + new_sec_service_id + + '_' + 'hosts_check') + fake_share_network = {'id': 'fake_network_id'} + backend_hosts = {'hostA', 'hostB'} + hosts_to_validate = {} + for bh in backend_hosts: + hosts_to_validate[bh] = None + mock_get_key = self.mock_object( + self.api, 'get_security_service_update_key', + mock.Mock(return_value=fake_key)) + mock_async_data_get = self.mock_object( + db_api, 'async_operation_data_get', mock.Mock(return_value=None)) + mock_async_data_update = self.mock_object( + db_api, 'async_operation_data_update') + mock_shareapi_check = self.mock_object( + self.share_rpcapi, 'check_update_share_network_security_service') + + compatible, hosts_info = ( + self.api._security_service_update_validate_hosts( + self.context, fake_share_network, backend_hosts, None, + new_security_service_id=new_sec_service_id, + current_security_service_id=curr_sec_service_id)) + + self.assertIsNone(compatible) + self.assertEqual(hosts_to_validate, hosts_info) + + mock_get_key.assert_called_once_with( + 'hosts_check', new_sec_service_id, + current_security_service_id=curr_sec_service_id) + mock_async_data_get.assert_called_once_with( + self.context, fake_share_network['id'], fake_key) + mock_async_data_update.assert_called_once_with( + self.context, fake_share_network['id'], + {fake_key: json.dumps(hosts_to_validate)}) + mock_shareapi_check_calls = [] + for host in backend_hosts: + mock_shareapi_check_calls.append( + mock.call(self.context, host, fake_share_network['id'], + new_sec_service_id, + current_security_service_id=curr_sec_service_id)) + mock_shareapi_check.assert_has_calls(mock_shareapi_check_calls) + + @ddt.data( + {'new_host': None, 'host_support': None, 'exp_result': None}, + {'new_host': None, 'host_support': False, 'exp_result': False}, + {'new_host': None, 'host_support': True, 'exp_result': True}, + {'new_host': 'hostC', 'host_support': None, 'exp_result': None}, + {'new_host': 'hostC', 'host_support': False, 'exp_result': False}, + {'new_host': 'hostC', 'host_support': True, 'exp_result': None}) + @ddt.unpack + def test__security_service_update_validate_hosts(self, new_host, + host_support, + exp_result): + curr_sec_service_id = "fake_curr_sec_serv_id" + new_sec_service_id = "fake_new_sec_serv_id" + fake_key = (curr_sec_service_id + '_' + new_sec_service_id + + '_' + 'hosts_check') + fake_share_network = {'id': 'fake_network_id'} + backend_hosts = ['hostA', 'hostB'] + hosts_to_validate = {} + for bh in backend_hosts: + hosts_to_validate[bh] = host_support + json_orig_hosts = json.dumps(hosts_to_validate) + + if new_host: + backend_hosts.append(new_host) + hosts_to_validate[new_host] = None + + mock_get_key = self.mock_object( + self.api, 'get_security_service_update_key', + mock.Mock(return_value=fake_key)) + mock_async_data_get = self.mock_object( + db_api, 'async_operation_data_get', + mock.Mock(return_value=json_orig_hosts)) + mock_async_data_update = self.mock_object( + db_api, 'async_operation_data_update') + mock_shareapi_check = self.mock_object( + self.share_rpcapi, 'check_update_share_network_security_service') + + result, hosts_info = ( + self.api._security_service_update_validate_hosts( + self.context, fake_share_network, backend_hosts, None, + new_security_service_id=new_sec_service_id, + current_security_service_id=curr_sec_service_id)) + + self.assertEqual(exp_result, result) + self.assertEqual(hosts_to_validate, hosts_info) + + mock_get_key.assert_called_once_with( + 'hosts_check', new_sec_service_id, + current_security_service_id=curr_sec_service_id) + mock_async_data_get.assert_called_once_with( + self.context, fake_share_network['id'], fake_key) + + # we fail earlier if one one the host answer False + if new_host and host_support is not False: + mock_async_data_update.assert_called_once_with( + self.context, fake_share_network['id'], + {fake_key: json.dumps(hosts_to_validate)}) + mock_shareapi_check.assert_called_once_with( + self.context, new_host, fake_share_network['id'], + new_sec_service_id, + current_security_service_id=curr_sec_service_id) + class OtherTenantsShareActionsTestCase(test.TestCase): def setUp(self): diff --git a/manila/tests/share/test_manager.py b/manila/tests/share/test_manager.py index c5331f12b6..df59a0cb27 100644 --- a/manila/tests/share/test_manager.py +++ b/manila/tests/share/test_manager.py @@ -579,7 +579,6 @@ class ShareManagerTestCase(test.TestCase): 'source_share_group_snapshot_member_id'), 'availability_zone': share_instance.get('availability_zone'), 'export_locations': share_instance.get('export_locations') or [], - 'share_network_status': share_instance.get('share_network_status') } return share_instance_ref