openstack
/
cinder
Code Issues Proposed changes

Check Volume Status on attahcment create/update 

Currently the new Attachment calls in Cinder aren't checking the status
of the volume when create or update is called.  This isn't good because
it can mask problems with attach processes on the volume.

This patch adds an explicit check of the volume status in both
attachment_create and attachment_update, and if the volume status is in
any error state, the call will result in an InvalidVolume exception.

This is an API change to fix a bug, and is not microversioned.

Closes-Bug: #1805762

Change-Id: I9b32c4db93879dda6490b244643de1a18bddbbf5
This commit is contained in:
j-griffith 2018-11-29 18:07:58 +00:00 committed by John Griffith
parent f423626fee
commit 7d7dc2659d
2 changed files with 51 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
cinder/tests/unit/attachments/test_attachments_api.py
View File

@ -280,3 +280,37 @@ class AttachmentManagerTestCase(test.TestCase):
self.assertEqual('ro', aref.attach_mode)
self.assertEqual(vref.id, aref.volume_id)
self.assertEqual({}, aref.connection_info)
def test_attachment_create_volume_in_error_state(self):
"""Test attachment_create volume in error state."""
volume_params = {'status': 'available'}
vref = tests_utils.create_volume(self.context, **volume_params)
vref.status = "error"
self.assertRaises(exception.InvalidVolume,
self.volume_api.attachment_create,
self.context,
vref,
fake.UUID2)
def test_attachment_update_volume_in_error_state(self):
"""Test attachment_update volumem in error state."""
volume_params = {'status': 'available'}
vref = tests_utils.create_volume(self.context, **volume_params)
aref = self.volume_api.attachment_create(self.context,
vref,
fake.UUID2)
self.assertEqual(fake.UUID2, aref.instance_uuid)
self.assertIsNone(aref.attach_time)
self.assertEqual('reserved', aref.attach_status)
self.assertEqual(vref.id, aref.volume_id)
self.assertEqual({}, aref.connection_info)
vref.status = 'error'
vref.save()
connector = {'fake': 'connector'}
self.assertRaises(exception.InvalidVolume,
self.volume_api.attachment_update,
self.context,
aref,
connector)

17
cinder/volume/api.py
View File

@ -2111,6 +2111,15 @@ class API(base.Base):
"""Create an attachment record for the specified volume."""
ctxt.authorize(attachment_policy.CREATE_POLICY, target_obj=volume_ref)
connection_info = {}
if "error" in volume_ref.status:
msg = ('Volume attachments can not be created if the volume '
'is in an error state. '
'The Volume %(volume_id)s currently has a status of: '
'%(volume_status)s ') % {
'volume_id': volume_ref.id,
'volume_status': volume_ref.status}
LOG.error(msg)
raise exception.InvalidVolume(reason=msg)
attachment_ref = self._attachment_reserve(ctxt,
volume_ref,
instance_uuid)
@ -2155,6 +2164,14 @@ class API(base.Base):
ctxt.authorize(attachment_policy.UPDATE_POLICY,
target_obj=attachment_ref)
volume_ref = objects.Volume.get_by_id(ctxt, attachment_ref.volume_id)
if "error" in volume_ref.status:
msg = ('Volume attachments can not be updated if the volume '
'is in an error state. The Volume %(volume_id)s '
'currently has a status of: %(volume_status)s ') % {
'volume_id': volume_ref.id,
'volume_status': volume_ref.status}
LOG.error(msg)
raise exception.InvalidVolume(reason=msg)
connection_info = (
self.volume_rpcapi.attachment_update(ctxt,
volume_ref,