diff options
Diffstat (limited to 'cinder/tests/unit')
-rw-r--r-- | cinder/tests/unit/api/contrib/test_admin_actions.py | 32 | ||||
-rw-r--r-- | cinder/tests/unit/api/v3/test_attachments.py | 2 | ||||
-rw-r--r-- | cinder/tests/unit/attachments/test_attachments_api.py | 191 | ||||
-rw-r--r-- | cinder/tests/unit/policies/test_attachments.py | 3 | ||||
-rw-r--r-- | cinder/tests/unit/policies/test_volume_actions.py | 7 | ||||
-rw-r--r-- | cinder/tests/unit/volume/test_connection.py | 5 |
6 files changed, 235 insertions, 5 deletions
diff --git a/cinder/tests/unit/api/contrib/test_admin_actions.py b/cinder/tests/unit/api/contrib/test_admin_actions.py index e56b112d8..22246868f 100644 --- a/cinder/tests/unit/api/contrib/test_admin_actions.py +++ b/cinder/tests/unit/api/contrib/test_admin_actions.py @@ -1037,6 +1037,8 @@ class AdminActionsAttachDetachTest(BaseAdminTest): super(AdminActionsAttachDetachTest, self).setUp() # start service to handle rpc messages for attach requests self.svc = self.start_service('volume', host='test') + self.mock_deletion_allowed = self.mock_object( + volume_api.API, 'attachment_deletion_allowed', return_value=None) def tearDown(self): self.svc.stop() @@ -1092,6 +1094,16 @@ class AdminActionsAttachDetachTest(BaseAdminTest): admin_metadata = volume.admin_metadata self.assertEqual(1, len(admin_metadata)) self.assertEqual('False', admin_metadata['readonly']) + # One call is for the terminate_connection and the other is for the + # detach + self.assertEqual(2, self.mock_deletion_allowed.call_count) + self.mock_deletion_allowed.assert_has_calls( + [mock.call(self.ctx, None, mock.ANY), + mock.call(self.ctx, attachment['id'], mock.ANY)]) + for i in (0, 1): + self.assertIsInstance( + self.mock_deletion_allowed.call_args_list[i][0][2], + objects.Volume) def test_force_detach_host_attached_volume(self): # current status is available @@ -1143,6 +1155,16 @@ class AdminActionsAttachDetachTest(BaseAdminTest): admin_metadata = volume['admin_metadata'] self.assertEqual(1, len(admin_metadata)) self.assertEqual('False', admin_metadata['readonly']) + # One call is for the terminate_connection and the other is for the + # detach + self.assertEqual(2, self.mock_deletion_allowed.call_count) + self.mock_deletion_allowed.assert_has_calls( + [mock.call(self.ctx, None, mock.ANY), + mock.call(self.ctx, attachment['id'], mock.ANY)]) + for i in (0, 1): + self.assertIsInstance( + self.mock_deletion_allowed.call_args_list[i][0][2], + objects.Volume) def test_volume_force_detach_raises_remote_error(self): # current status is available @@ -1186,6 +1208,10 @@ class AdminActionsAttachDetachTest(BaseAdminTest): resp = req.get_response(app()) self.assertEqual(HTTPStatus.BAD_REQUEST, resp.status_int) + self.mock_deletion_allowed.assert_called_once_with( + self.ctx, None, volume) + self.mock_deletion_allowed.reset_mock() + # test for VolumeBackendAPIException volume_remote_error = ( messaging.RemoteError(exc_type='VolumeBackendAPIException')) @@ -1205,6 +1231,8 @@ class AdminActionsAttachDetachTest(BaseAdminTest): self.assertRaises(messaging.RemoteError, req.get_response, app()) + self.mock_deletion_allowed.assert_called_once_with( + self.ctx, None, volume) def test_volume_force_detach_raises_db_error(self): # In case of DB error 500 error code is returned to user @@ -1250,6 +1278,8 @@ class AdminActionsAttachDetachTest(BaseAdminTest): self.assertRaises(messaging.RemoteError, req.get_response, app()) + self.mock_deletion_allowed.assert_called_once_with( + self.ctx, None, volume) def test_volume_force_detach_missing_connector(self): # current status is available @@ -1290,6 +1320,8 @@ class AdminActionsAttachDetachTest(BaseAdminTest): # make request resp = req.get_response(app()) self.assertEqual(HTTPStatus.ACCEPTED, resp.status_int) + self.mock_deletion_allowed.assert_called_once_with( + self.ctx, None, volume) def test_attach_in_used_volume_by_instance(self): """Test that attaching to an in-use volume fails.""" diff --git a/cinder/tests/unit/api/v3/test_attachments.py b/cinder/tests/unit/api/v3/test_attachments.py index 6adf5a37c..35251357a 100644 --- a/cinder/tests/unit/api/v3/test_attachments.py +++ b/cinder/tests/unit/api/v3/test_attachments.py @@ -159,6 +159,8 @@ class AttachmentsAPITestCase(test.TestCase): @ddt.data('reserved', 'attached') @mock.patch.object(volume_rpcapi.VolumeAPI, 'attachment_delete') def test_delete_attachment(self, status, mock_delete): + self.patch('cinder.volume.api.API.attachment_deletion_allowed', + return_value=None) volume1 = self._create_volume(display_name='fake_volume_1', project_id=fake.PROJECT_ID) attachment = self._create_attachment( diff --git a/cinder/tests/unit/attachments/test_attachments_api.py b/cinder/tests/unit/attachments/test_attachments_api.py index 8f663f39e..b9564160d 100644 --- a/cinder/tests/unit/attachments/test_attachments_api.py +++ b/cinder/tests/unit/attachments/test_attachments_api.py @@ -12,12 +12,14 @@ from unittest import mock +from cinder.compute import nova from cinder import context from cinder import db from cinder import exception from cinder import objects from cinder.tests.unit.api.v2 import fakes as v2_fakes from cinder.tests.unit import fake_constants as fake +from cinder.tests.unit import fake_volume from cinder.tests.unit import test from cinder.tests.unit import utils as tests_utils from cinder.volume import api as volume_api @@ -78,10 +80,13 @@ class AttachmentManagerTestCase(test.TestCase): attachment.id) self.assertEqual(connection_info, new_attachment.connection_info) + @mock.patch.object(volume_api.API, 'attachment_deletion_allowed') @mock.patch('cinder.volume.rpcapi.VolumeAPI.attachment_delete') def test_attachment_delete_reserved(self, - mock_rpc_attachment_delete): + mock_rpc_attachment_delete, + mock_allowed): """Test attachment_delete with reserved.""" + mock_allowed.return_value = None volume_params = {'status': 'available'} vref = tests_utils.create_volume(self.context, **volume_params) @@ -94,18 +99,22 @@ class AttachmentManagerTestCase(test.TestCase): self.assertEqual(vref.id, aref.volume_id) self.volume_api.attachment_delete(self.context, aobj) + mock_allowed.assert_called_once_with(self.context, aobj) # Since it's just reserved and never finalized, we should never make an # rpc call mock_rpc_attachment_delete.assert_not_called() + @mock.patch.object(volume_api.API, 'attachment_deletion_allowed') @mock.patch('cinder.volume.rpcapi.VolumeAPI.attachment_delete') @mock.patch('cinder.volume.rpcapi.VolumeAPI.attachment_update') def test_attachment_create_update_and_delete( self, mock_rpc_attachment_update, - mock_rpc_attachment_delete): + mock_rpc_attachment_delete, + mock_allowed): """Test attachment_delete.""" + mock_allowed.return_value = None volume_params = {'status': 'available'} connection_info = {'fake_key': 'fake_value', 'fake_key2': ['fake_value1', 'fake_value2']} @@ -142,6 +151,7 @@ class AttachmentManagerTestCase(test.TestCase): self.volume_api.attachment_delete(self.context, aref) + mock_allowed.assert_called_once_with(self.context, aref) mock_rpc_attachment_delete.assert_called_once_with(self.context, aref.id, mock.ANY) @@ -173,10 +183,13 @@ class AttachmentManagerTestCase(test.TestCase): vref.id) self.assertEqual(2, len(vref.volume_attachment)) + @mock.patch.object(volume_api.API, 'attachment_deletion_allowed') @mock.patch('cinder.volume.rpcapi.VolumeAPI.attachment_update') def test_attachment_create_reserve_delete( self, - mock_rpc_attachment_update): + mock_rpc_attachment_update, + mock_allowed): + mock_allowed.return_value = None volume_params = {'status': 'available'} connector = { "initiator": "iqn.1993-08.org.debian:01:cad181614cec", @@ -211,12 +224,15 @@ class AttachmentManagerTestCase(test.TestCase): # attachments reserve self.volume_api.attachment_delete(self.context, aref) + mock_allowed.assert_called_once_with(self.context, aref) vref = objects.Volume.get_by_id(self.context, vref.id) self.assertEqual('reserved', vref.status) - def test_reserve_reserve_delete(self): + @mock.patch.object(volume_api.API, 'attachment_deletion_allowed') + def test_reserve_reserve_delete(self, mock_allowed): """Test that we keep reserved status across multiple reserves.""" + mock_allowed.return_value = None volume_params = {'status': 'available'} vref = tests_utils.create_volume(self.context, **volume_params) @@ -235,6 +251,7 @@ class AttachmentManagerTestCase(test.TestCase): self.assertEqual('reserved', vref.status) self.volume_api.attachment_delete(self.context, aref) + mock_allowed.assert_called_once_with(self.context, aref) vref = objects.Volume.get_by_id(self.context, vref.id) self.assertEqual('reserved', vref.status) @@ -344,3 +361,169 @@ class AttachmentManagerTestCase(test.TestCase): self.context, vref, fake.UUID1) + + def _get_attachment(self, with_instance_id=True): + volume = fake_volume.fake_volume_obj(self.context, id=fake.VOLUME_ID) + volume.volume_attachment = objects.VolumeAttachmentList() + attachment = fake_volume.volume_attachment_ovo( + self.context, + volume_id=fake.VOLUME_ID, + instance_uuid=fake.INSTANCE_ID if with_instance_id else None, + connection_info='{"a": 1}') + attachment.volume = volume + return attachment + + @mock.patch('cinder.compute.nova.API.get_server_volume') + def test_attachment_deletion_allowed_service_call(self, mock_get_server): + """Service calls are never redirected.""" + self.context.service_roles = ['reader', 'service'] + attachment = self._get_attachment() + self.volume_api.attachment_deletion_allowed(self.context, attachment) + mock_get_server.assert_not_called() + + @mock.patch('cinder.compute.nova.API.get_server_volume') + def test_attachment_deletion_allowed_service_call_different_service_name( + self, mock_get_server): + """Service calls are never redirected and role can be different. + + In this test we support 2 different service roles, the standard service + and a custom one called captain_awesome, and passing the custom one + works as expected. + """ + self.override_config('service_token_roles', + ['service', 'captain_awesome'], + group='keystone_authtoken') + + self.context.service_roles = ['reader', 'captain_awesome'] + attachment = self._get_attachment() + self.volume_api.attachment_deletion_allowed(self.context, attachment) + mock_get_server.assert_not_called() + + @mock.patch('cinder.compute.nova.API.get_server_volume') + def test_attachment_deletion_allowed_no_instance(self, mock_get_server): + """Attachments with no instance id are never redirected.""" + attachment = self._get_attachment(with_instance_id=False) + self.volume_api.attachment_deletion_allowed(self.context, attachment) + mock_get_server.assert_not_called() + + @mock.patch('cinder.compute.nova.API.get_server_volume') + def test_attachment_deletion_allowed_no_conn_info(self, mock_get_server): + """Attachments with no connection information are never redirected.""" + attachment = self._get_attachment(with_instance_id=False) + attachment.connection_info = None + self.volume_api.attachment_deletion_allowed(self.context, attachment) + + mock_get_server.assert_not_called() + + def test_attachment_deletion_allowed_no_attachment(self): + """For users don't allow operation with no attachment reference.""" + self.assertRaises(exception.ConflictNovaUsingAttachment, + self.volume_api.attachment_deletion_allowed, + self.context, None) + + @mock.patch('cinder.objects.VolumeAttachment.get_by_id', + side_effect=exception.VolumeAttachmentNotFound()) + def test_attachment_deletion_allowed_attachment_id_not_found(self, + mock_get): + """For users don't allow if attachment cannot be found.""" + attachment = self._get_attachment(with_instance_id=False) + attachment.connection_info = None + self.assertRaises(exception.ConflictNovaUsingAttachment, + self.volume_api.attachment_deletion_allowed, + self.context, fake.ATTACHMENT_ID) + mock_get.assert_called_once_with(self.context, fake.ATTACHMENT_ID) + + def test_attachment_deletion_allowed_volume_no_attachments(self): + """For users allow if volume has no attachments.""" + volume = tests_utils.create_volume(self.context) + self.volume_api.attachment_deletion_allowed(self.context, None, volume) + + def test_attachment_deletion_allowed_multiple_attachment(self): + """For users don't allow if volume has multiple attachments.""" + attachment = self._get_attachment() + volume = attachment.volume + volume.volume_attachment = objects.VolumeAttachmentList( + objects=[attachment, attachment]) + self.assertRaises(exception.ConflictNovaUsingAttachment, + self.volume_api.attachment_deletion_allowed, + self.context, None, volume) + + @mock.patch('cinder.compute.nova.API.get_server_volume') + def test_attachment_deletion_allowed_vm_not_found(self, mock_get_server): + """Don't reject if instance doesn't exist""" + mock_get_server.side_effect = nova.API.NotFound(404) + attachment = self._get_attachment() + self.volume_api.attachment_deletion_allowed(self.context, attachment) + + mock_get_server.assert_called_once_with(self.context, fake.INSTANCE_ID, + fake.VOLUME_ID) + + @mock.patch('cinder.compute.nova.API.get_server_volume') + def test_attachment_deletion_allowed_attachment_from_volume( + self, mock_get_server): + """Don't reject if instance doesn't exist""" + mock_get_server.side_effect = nova.API.NotFound(404) + attachment = self._get_attachment() + volume = attachment.volume + volume.volume_attachment = objects.VolumeAttachmentList( + objects=[attachment]) + self.volume_api.attachment_deletion_allowed(self.context, None, volume) + + mock_get_server.assert_called_once_with(self.context, fake.INSTANCE_ID, + volume.id) + + @mock.patch('cinder.objects.VolumeAttachment.get_by_id') + def test_attachment_deletion_allowed_mismatched_volume_and_attach_id( + self, mock_get_attatchment): + """Reject if volume and attachment don't match.""" + attachment = self._get_attachment() + volume = attachment.volume + volume.volume_attachment = objects.VolumeAttachmentList( + objects=[attachment]) + attachment2 = self._get_attachment() + attachment2.volume_id = attachment.volume.id = fake.VOLUME2_ID + self.assertRaises(exception.InvalidInput, + self.volume_api.attachment_deletion_allowed, + self.context, attachment2.id, volume) + mock_get_attatchment.assert_called_once_with(self.context, + attachment2.id) + + @mock.patch('cinder.objects.VolumeAttachment.get_by_id') + @mock.patch('cinder.compute.nova.API.get_server_volume') + def test_attachment_deletion_allowed_not_found_attachment_id( + self, mock_get_server, mock_get_attachment): + """Don't reject if instance doesn't exist""" + mock_get_server.side_effect = nova.API.NotFound(404) + mock_get_attachment.return_value = self._get_attachment() + + self.volume_api.attachment_deletion_allowed(self.context, + fake.ATTACHMENT_ID) + + mock_get_attachment.assert_called_once_with(self.context, + fake.ATTACHMENT_ID) + + mock_get_server.assert_called_once_with(self.context, fake.INSTANCE_ID, + fake.VOLUME_ID) + + @mock.patch('cinder.compute.nova.API.get_server_volume') + def test_attachment_deletion_allowed_mismatch_id(self, mock_get_server): + """Don't reject if attachment id on nova doesn't match""" + mock_get_server.return_value.attachment_id = fake.ATTACHMENT2_ID + attachment = self._get_attachment() + self.volume_api.attachment_deletion_allowed(self.context, attachment) + + mock_get_server.assert_called_once_with(self.context, fake.INSTANCE_ID, + fake.VOLUME_ID) + + @mock.patch('cinder.compute.nova.API.get_server_volume') + def test_attachment_deletion_allowed_user_call_fails(self, + mock_get_server): + """Fail user calls""" + attachment = self._get_attachment() + mock_get_server.return_value.attachment_id = attachment.id + self.assertRaises(exception.ConflictNovaUsingAttachment, + self.volume_api.attachment_deletion_allowed, + self.context, attachment) + + mock_get_server.assert_called_once_with(self.context, fake.INSTANCE_ID, + fake.VOLUME_ID) diff --git a/cinder/tests/unit/policies/test_attachments.py b/cinder/tests/unit/policies/test_attachments.py index 7307a83ca..8d3040b2f 100644 --- a/cinder/tests/unit/policies/test_attachments.py +++ b/cinder/tests/unit/policies/test_attachments.py @@ -62,6 +62,9 @@ class AttachmentsPolicyTest(base.BasePolicyTest): self.api_path = '/v3/%s/attachments' % (self.project_id) self.api_version = mv.NEW_ATTACH + self.mock_is_service = self.patch( + 'cinder.volume.api.API.is_service_request', + return_value=True) def _initialize_connection(self, volume, connector): return {'data': connector} diff --git a/cinder/tests/unit/policies/test_volume_actions.py b/cinder/tests/unit/policies/test_volume_actions.py index d9b2edc8e..17dacd5ef 100644 --- a/cinder/tests/unit/policies/test_volume_actions.py +++ b/cinder/tests/unit/policies/test_volume_actions.py @@ -89,6 +89,9 @@ class VolumeActionsPolicyTest(base.BasePolicyTest): self._initialize_connection) self.api_path = '/v3/%s/volumes' % (self.project_id) self.api_version = mv.BASE_VERSION + self.mock_is_service = self.patch( + 'cinder.volume.api.API.is_service_request', + return_value=True) def _initialize_connection(self, volume, connector): return {'data': connector} @@ -946,6 +949,7 @@ class VolumeProtectionTests(test_base.CinderPolicyTests): self.assertEqual(HTTPStatus.ACCEPTED, response.status_int) body = {"os-detach": {}} + # Detach for user call succeeds because the volume has no attachments response = self._get_request_response(admin_context, path, 'POST', body=body) self.assertEqual(HTTPStatus.ACCEPTED, response.status_int) @@ -966,6 +970,7 @@ class VolumeProtectionTests(test_base.CinderPolicyTests): body=body) self.assertEqual(HTTPStatus.ACCEPTED, response.status_int) + # Succeeds for a user call because there are no attachments body = {"os-detach": {}} response = self._get_request_response(user_context, path, 'POST', body=body) @@ -1062,6 +1067,7 @@ class VolumeProtectionTests(test_base.CinderPolicyTests): 'terminate_connection') def test_admin_can_initialize_terminate_conn(self, mock_t, mock_i): admin_context = self.admin_context + admin_context.service_roles = ['service'] volume = self._create_fake_volume(admin_context) path = '/v3/%(project_id)s/volumes/%(volume_id)s/action' % { @@ -1084,6 +1090,7 @@ class VolumeProtectionTests(test_base.CinderPolicyTests): 'terminate_connection') def test_owner_can_initialize_terminate_conn(self, mock_t, mock_i): user_context = self.user_context + user_context.service_roles = ['service'] volume = self._create_fake_volume(user_context) path = '/v3/%(project_id)s/volumes/%(volume_id)s/action' % { diff --git a/cinder/tests/unit/volume/test_connection.py b/cinder/tests/unit/volume/test_connection.py index 0d472d622..6bd9b89b6 100644 --- a/cinder/tests/unit/volume/test_connection.py +++ b/cinder/tests/unit/volume/test_connection.py @@ -1334,7 +1334,8 @@ class VolumeAttachDetachTestCase(base.BaseVolumeTestCase): self.context, volume, None, None, None, None) - def test_volume_detach_in_maintenance(self): + @mock.patch('cinder.volume.api.API.attachment_deletion_allowed') + def test_volume_detach_in_maintenance(self, mock_attachment_deletion): """Test detach the volume in maintenance.""" test_meta1 = {'fake_key1': 'fake_value1', 'fake_key2': 'fake_value2'} volume = tests_utils.create_volume(self.context, metadata=test_meta1, @@ -1345,3 +1346,5 @@ class VolumeAttachDetachTestCase(base.BaseVolumeTestCase): volume_api.detach, self.context, volume, None) + mock_attachment_deletion.assert_called_once_with(self.context, + None, volume) |