Fix _attachment_reserve to not allow attaching an invalid status volume
It is currently possible to create a volume attachment for a server when the volume is in error status because of the override logic in the _attachment_reserve method. What results is that the volume attach operation fails in nova-compute which rolls back and deletes the volume attachment, which puts the volume into 'available' status because it no longer has any attachments, which in fact it should have never allowed the attachment create/reserve in the first place. This updates the override logic such that a volume without any attachments which is in an invalid status will result in an error being raised. Change-Id: Id9cf2f510684cd296ffbcaf53d11889cfe8973b9 Closes-Bug: #1785050
This commit is contained in:
parent
9bc9a528ef
commit
9c0123eb70
|
@ -1859,6 +1859,24 @@ class VolumeTestCase(base.BaseVolumeTestCase):
|
||||||
self.assertIn("status must be available or downloading",
|
self.assertIn("status must be available or downloading",
|
||||||
six.text_type(ex))
|
six.text_type(ex))
|
||||||
|
|
||||||
|
def test_attachment_reserve_with_instance_uuid_error_volume(self):
|
||||||
|
# Tests that trying to create an attachment (with an instance_uuid
|
||||||
|
# provided) on a volume that's not 'available' or 'downloading' status
|
||||||
|
# will fail if the volume does not have any attachments, similar to how
|
||||||
|
# the volume reserve action works.
|
||||||
|
volume = tests_utils.create_volume(self.context, status='error')
|
||||||
|
# Assert that we're not dealing with a multiattach volume and that
|
||||||
|
# it does not have any existing attachments.
|
||||||
|
self.assertFalse(volume.multiattach)
|
||||||
|
self.assertEqual(0, len(volume.volume_attachment))
|
||||||
|
# Try attaching an instance to the volume which should fail based on
|
||||||
|
# the volume status.
|
||||||
|
ex = self.assertRaises(exception.InvalidVolume,
|
||||||
|
self.volume_api._attachment_reserve,
|
||||||
|
self.context, volume, fake.UUID1)
|
||||||
|
self.assertIn("status must be available or downloading",
|
||||||
|
six.text_type(ex))
|
||||||
|
|
||||||
def test_unreserve_volume_success_in_use(self):
|
def test_unreserve_volume_success_in_use(self):
|
||||||
UUID = six.text_type(uuid.uuid4())
|
UUID = six.text_type(uuid.uuid4())
|
||||||
volume = tests_utils.create_volume(self.context, status='attaching')
|
volume = tests_utils.create_volume(self.context, status='attaching')
|
||||||
|
|
|
@ -2086,18 +2086,19 @@ class API(base.Base):
|
||||||
result = vref.conditional_update({'status': 'reserved'}, expected)
|
result = vref.conditional_update({'status': 'reserved'}, expected)
|
||||||
|
|
||||||
if not result:
|
if not result:
|
||||||
# Make sure we're not going to the same instance, in which case
|
|
||||||
# it could be a live-migrate or similar scenario (LP BUG: 1694530)
|
|
||||||
override = False
|
override = False
|
||||||
if instance_uuid:
|
if instance_uuid:
|
||||||
override = True
|
|
||||||
# Refresh the volume reference in case multiple instances were
|
# Refresh the volume reference in case multiple instances were
|
||||||
# being concurrently attached to the same non-multiattach
|
# being concurrently attached to the same non-multiattach
|
||||||
# volume.
|
# volume.
|
||||||
vref = objects.Volume.get_by_id(ctxt, vref.id)
|
vref = objects.Volume.get_by_id(ctxt, vref.id)
|
||||||
for attachment in vref.volume_attachment:
|
for attachment in vref.volume_attachment:
|
||||||
if attachment.instance_uuid != instance_uuid:
|
# If we're attaching the same volume to the same instance,
|
||||||
override = False
|
# we could be migrating the instance to another host in
|
||||||
|
# which case we want to allow the reservation.
|
||||||
|
# (LP BUG: 1694530)
|
||||||
|
if attachment.instance_uuid == instance_uuid:
|
||||||
|
override = True
|
||||||
break
|
break
|
||||||
|
|
||||||
if not override:
|
if not override:
|
||||||
|
|
Loading…
Reference in New Issue