diff --git a/cinder/tests/unit/attachments/test_attachments_manager.py b/cinder/tests/unit/attachments/test_attachments_manager.py index c99912dc272..578ddb0a778 100644 --- a/cinder/tests/unit/attachments/test_attachments_manager.py +++ b/cinder/tests/unit/attachments/test_attachments_manager.py @@ -17,6 +17,7 @@ from oslo_utils import importutils from cinder import context from cinder import db from cinder import exception +from cinder.objects import fields from cinder import test from cinder.tests.unit import fake_constants as fake from cinder.tests.unit import utils as tests_utils @@ -41,6 +42,31 @@ class AttachmentManagerTestCase(test.TestCase): self.manager.stats = {'allocated_capacity_gb': 100, 'pools': {}} + @mock.patch.object(db, 'volume_admin_metadata_update') + @mock.patch('cinder.message.api.API.create', mock.Mock()) + def test_attachment_update_with_readonly_volume(self, mock_update): + mock_update.return_value = {'readonly': 'True'} + vref = tests_utils.create_volume(self.context, **{'status': + 'available'}) + self.manager.create_volume(self.context, vref) + attachment_ref = db.volume_attach(self.context, + {'volume_id': vref.id, + 'volume_host': vref.host, + 'attach_status': 'reserved', + 'instance_uuid': fake.UUID1}) + + with mock.patch.object(self.manager, + '_notify_about_volume_usage', + return_value=None), mock.patch.object( + self.manager, '_connection_create'): + self.assertRaises(exception.InvalidVolumeAttachMode, + self.manager.attachment_update, + self.context, vref, {}, attachment_ref.id) + attachment = db.volume_attachment_get(self.context, + attachment_ref.id) + self.assertEqual(fields.VolumeAttachStatus.ERROR_ATTACHING, + attachment['attach_status']) + def test_attachment_update(self): """Test attachment_update.""" volume_params = {'status': 'available'} diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index 306e3049060..c4dcbfc281c 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -1039,18 +1039,14 @@ class VolumeManager(manager.CleanableManager, attachment.save() raise exception.InvalidUUID(uuid=instance_uuid) - if volume_metadata.get('readonly') == 'True' and mode != 'ro': - attachment.attach_status = ( - fields.VolumeAttachStatus.ERROR_ATTACHING) - attachment.save() - self.message_api.create( - context, defined_messages.EventIds.ATTACH_READONLY_VOLUME, - context.project_id, resource_type=resource_types.VOLUME, - resource_uuid=volume.id) - raise exception.InvalidVolumeAttachMode(mode=mode, - volume_id=volume.id) - try: + if volume_metadata.get('readonly') == 'True' and mode != 'ro': + self.message_api.create( + context, defined_messages.EventIds.ATTACH_READONLY_VOLUME, + context.project_id, resource_type=resource_types.VOLUME, + resource_uuid=volume.id) + raise exception.InvalidVolumeAttachMode(mode=mode, + volume_id=volume.id) # NOTE(flaper87): Verify the driver is enabled # before going forward. The exception will be caught # and the volume status updated. @@ -3946,16 +3942,14 @@ class VolumeManager(manager.CleanableManager, attachment_ref.volume_id, {'attached_mode': mode}, False) - if volume_metadata.get('readonly') == 'True' and mode != 'ro': - self.db.volume_update(context, vref.id, - {'status': 'error_attaching'}) - self.message_api.create( - context, defined_messages.EventIds.ATTACH_READONLY_VOLUME, - context.project_id, resource_type=resource_types.VOLUME, - resource_uuid=vref.id) - raise exception.InvalidVolumeAttachMode(mode=mode, - volume_id=vref.id) try: + if volume_metadata.get('readonly') == 'True' and mode != 'ro': + self.message_api.create( + context, defined_messages.EventIds.ATTACH_READONLY_VOLUME, + context.project_id, resource_type=resource_types.VOLUME, + resource_uuid=vref.id) + raise exception.InvalidVolumeAttachMode(mode=mode, + volume_id=vref.id) utils.require_driver_initialized(self.driver) self.driver.attach_volume(context, vref, @@ -3966,7 +3960,8 @@ class VolumeManager(manager.CleanableManager, with excutils.save_and_reraise_exception(): self.db.volume_attachment_update( context, attachment_ref.id, - {'attach_status': 'error_attaching'}) + {'attach_status': + fields.VolumeAttachStatus.ERROR_ATTACHING}) self.db.volume_attached(context.elevated(), attachment_ref.id,