fixtures: Track volume attachments within CinderFixtureNewAttachFlow
Previously volume attachments ids were not tracked at all within the
fixture with only the instance_uuid and volume_id stashed. This change
should allow future functional tests to exercise bugs where attachments
are deleted but not recreated such as bug #1784353.
Change-Id: Ib30144596fe6a8d8ffbb4ebd695ebcf38ef828a4
Co-authored-by: Matthew Booth <mbooth@redhat.com>
Related-Bug: #1784353
(cherry picked from commit bfdc6a0d52
)
This commit is contained in:
parent
a65ded469d
commit
cb9996b45f
|
@ -1398,6 +1398,9 @@ class CinderFixture(fixtures.Fixture):
|
||||||
# This map gets updated on attach/detach operations.
|
# This map gets updated on attach/detach operations.
|
||||||
self.attachments = collections.defaultdict(list)
|
self.attachments = collections.defaultdict(list)
|
||||||
|
|
||||||
|
def volume_ids_for_instance(self, instance_uuid):
|
||||||
|
return self.attachments.get(instance_uuid)
|
||||||
|
|
||||||
def setUp(self):
|
def setUp(self):
|
||||||
super(CinderFixture, self).setUp()
|
super(CinderFixture, self).setUp()
|
||||||
|
|
||||||
|
@ -1587,15 +1590,28 @@ class CinderFixtureNewAttachFlow(fixtures.Fixture):
|
||||||
self.swap_volume_instance_uuid = None
|
self.swap_volume_instance_uuid = None
|
||||||
self.swap_volume_instance_error_uuid = None
|
self.swap_volume_instance_error_uuid = None
|
||||||
self.attachment_error_id = None
|
self.attachment_error_id = None
|
||||||
# This is a map of instance UUIDs mapped to a list of volume IDs.
|
# A map of volumes to a list of (attachment_id, instance_uuid).
|
||||||
# This map gets updated on attach/detach operations.
|
# Note that a volume can have multiple attachments even without
|
||||||
self.attachments = collections.defaultdict(list)
|
# multi-attach, as some flows create a blank 'reservation' attachment
|
||||||
|
# before deleting another attachment.
|
||||||
|
self.volume_to_attachment = collections.defaultdict(list)
|
||||||
|
|
||||||
|
def volume_ids_for_instance(self, instance_uuid):
|
||||||
|
for volume_id, attachments in self.volume_to_attachment.items():
|
||||||
|
for _, _instance_uuid in attachments:
|
||||||
|
if _instance_uuid == instance_uuid:
|
||||||
|
# we might have multiple volumes attached to this instance
|
||||||
|
# so yield rather than return
|
||||||
|
yield volume_id
|
||||||
|
break
|
||||||
|
|
||||||
def setUp(self):
|
def setUp(self):
|
||||||
super(CinderFixtureNewAttachFlow, self).setUp()
|
super(CinderFixtureNewAttachFlow, self).setUp()
|
||||||
|
|
||||||
def fake_get(self_api, context, volume_id, microversion=None):
|
def fake_get(self_api, context, volume_id, microversion=None):
|
||||||
# Check for the special swap volumes.
|
# Check for the special swap volumes.
|
||||||
|
attachments = self.volume_to_attachment[volume_id]
|
||||||
|
|
||||||
if volume_id in (CinderFixture.SWAP_OLD_VOL,
|
if volume_id in (CinderFixture.SWAP_OLD_VOL,
|
||||||
CinderFixture.SWAP_ERR_OLD_VOL):
|
CinderFixture.SWAP_ERR_OLD_VOL):
|
||||||
volume = {
|
volume = {
|
||||||
|
@ -1614,12 +1630,15 @@ class CinderFixtureNewAttachFlow(fixtures.Fixture):
|
||||||
if volume_id == CinderFixture.SWAP_OLD_VOL
|
if volume_id == CinderFixture.SWAP_OLD_VOL
|
||||||
else self.swap_volume_instance_error_uuid)
|
else self.swap_volume_instance_error_uuid)
|
||||||
|
|
||||||
|
if attachments:
|
||||||
|
attachment_id, instance_uuid = attachments[0]
|
||||||
|
|
||||||
volume.update({
|
volume.update({
|
||||||
'status': 'in-use',
|
'status': 'in-use',
|
||||||
'attachments': {
|
'attachments': {
|
||||||
instance_uuid: {
|
instance_uuid: {
|
||||||
'mountpoint': '/dev/vdb',
|
'mountpoint': '/dev/vdb',
|
||||||
'attachment_id': volume_id
|
'attachment_id': attachment_id
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
'attach_status': 'attached'
|
'attach_status': 'attached'
|
||||||
|
@ -1627,9 +1646,9 @@ class CinderFixtureNewAttachFlow(fixtures.Fixture):
|
||||||
return volume
|
return volume
|
||||||
|
|
||||||
# Check to see if the volume is attached.
|
# Check to see if the volume is attached.
|
||||||
for instance_uuid, volumes in self.attachments.items():
|
if attachments:
|
||||||
if volume_id in volumes:
|
|
||||||
# The volume is attached.
|
# The volume is attached.
|
||||||
|
attachment_id, instance_uuid = attachments[0]
|
||||||
volume = {
|
volume = {
|
||||||
'status': 'in-use',
|
'status': 'in-use',
|
||||||
'display_name': volume_id,
|
'display_name': volume_id,
|
||||||
|
@ -1639,12 +1658,11 @@ class CinderFixtureNewAttachFlow(fixtures.Fixture):
|
||||||
'size': 1,
|
'size': 1,
|
||||||
'attachments': {
|
'attachments': {
|
||||||
instance_uuid: {
|
instance_uuid: {
|
||||||
'attachment_id': volume_id,
|
'attachment_id': attachment_id,
|
||||||
'mountpoint': '/dev/vdb'
|
'mountpoint': '/dev/vdb'
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
break
|
|
||||||
else:
|
else:
|
||||||
# This is a test that does not care about the actual details.
|
# This is a test that does not care about the actual details.
|
||||||
volume = {
|
volume = {
|
||||||
|
@ -1680,26 +1698,45 @@ class CinderFixtureNewAttachFlow(fixtures.Fixture):
|
||||||
new_volume_id, error):
|
new_volume_id, error):
|
||||||
return {'save_volume_id': new_volume_id}
|
return {'save_volume_id': new_volume_id}
|
||||||
|
|
||||||
|
def _find_attachment(attachment_id):
|
||||||
|
"""Find attachment corresponding to ``attachment_id``.
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
A tuple of the volume ID, an attachment-instance mapping tuple
|
||||||
|
for the given attachment ID, and a list of attachment-instance
|
||||||
|
mapping tuples for the volume.
|
||||||
|
"""
|
||||||
|
for volume_id, attachments in self.volume_to_attachment.items():
|
||||||
|
for attachment in attachments:
|
||||||
|
_attachment_id, instance_uuid = attachment
|
||||||
|
if attachment_id == _attachment_id:
|
||||||
|
return volume_id, attachment, attachments
|
||||||
|
raise exception.VolumeAttachmentNotFound(
|
||||||
|
attachment_id=attachment_id)
|
||||||
|
|
||||||
def fake_attachment_create(_self, context, volume_id, instance_uuid,
|
def fake_attachment_create(_self, context, volume_id, instance_uuid,
|
||||||
connector=None, mountpoint=None):
|
connector=None, mountpoint=None):
|
||||||
attachment_id = uuidutils.generate_uuid()
|
attachment_id = uuidutils.generate_uuid()
|
||||||
if self.attachment_error_id is not None:
|
if self.attachment_error_id is not None:
|
||||||
attachment_id = self.attachment_error_id
|
attachment_id = self.attachment_error_id
|
||||||
attachment = {'id': attachment_id, 'connection_info': {'data': {}}}
|
attachment = {'id': attachment_id, 'connection_info': {'data': {}}}
|
||||||
self.attachments['instance_uuid'].append(instance_uuid)
|
self.volume_to_attachment[volume_id].append(
|
||||||
self.attachments[instance_uuid].append(volume_id)
|
(attachment_id, instance_uuid))
|
||||||
|
|
||||||
return attachment
|
return attachment
|
||||||
|
|
||||||
def fake_attachment_delete(_self, context, attachment_id):
|
def fake_attachment_delete(_self, context, attachment_id):
|
||||||
instance_uuid = self.attachments['instance_uuid'][0]
|
# 'attachment' is a tuple defining a attachment-instance mapping
|
||||||
del self.attachments[instance_uuid][0]
|
_, attachment, attachments = _find_attachment(attachment_id)
|
||||||
del self.attachments['instance_uuid'][0]
|
attachments.remove(attachment)
|
||||||
|
|
||||||
if attachment_id == CinderFixtureNewAttachFlow.SWAP_ERR_ATTACH_ID:
|
if attachment_id == CinderFixtureNewAttachFlow.SWAP_ERR_ATTACH_ID:
|
||||||
self.swap_error = True
|
self.swap_error = True
|
||||||
|
|
||||||
def fake_attachment_update(_self, context, attachment_id, connector,
|
def fake_attachment_update(_self, context, attachment_id, connector,
|
||||||
mountpoint=None):
|
mountpoint=None):
|
||||||
|
# Ensure the attachment exists
|
||||||
|
_find_attachment(attachment_id)
|
||||||
attachment_ref = {'driver_volume_type': 'fake_type',
|
attachment_ref = {'driver_volume_type': 'fake_type',
|
||||||
'id': attachment_id,
|
'id': attachment_id,
|
||||||
'connection_info': {'data':
|
'connection_info': {'data':
|
||||||
|
@ -1710,6 +1747,8 @@ class CinderFixtureNewAttachFlow(fixtures.Fixture):
|
||||||
return attachment_ref
|
return attachment_ref
|
||||||
|
|
||||||
def fake_attachment_get(_self, context, attachment_id):
|
def fake_attachment_get(_self, context, attachment_id):
|
||||||
|
# Ensure the attachment exists
|
||||||
|
_find_attachment(attachment_id)
|
||||||
attachment_ref = {'driver_volume_type': 'fake_type',
|
attachment_ref = {'driver_volume_type': 'fake_type',
|
||||||
'id': attachment_id,
|
'id': attachment_id,
|
||||||
'connection_info': {'data':
|
'connection_info': {'data':
|
||||||
|
|
|
@ -96,7 +96,8 @@ class DeleteWithReservedVolumes(integrated_helpers._IntegratedTestBase,
|
||||||
|
|
||||||
# There should now exist an attachment to the volume as it was created
|
# There should now exist an attachment to the volume as it was created
|
||||||
# by Nova.
|
# by Nova.
|
||||||
self.assertIn(volume_id, self.cinder.attachments[server_id])
|
self.assertIn(volume_id,
|
||||||
|
self.cinder.volume_ids_for_instance(server_id))
|
||||||
|
|
||||||
# Delete this server, which should delete BDMs and remove the
|
# Delete this server, which should delete BDMs and remove the
|
||||||
# reservation on the instances.
|
# reservation on the instances.
|
||||||
|
@ -104,4 +105,5 @@ class DeleteWithReservedVolumes(integrated_helpers._IntegratedTestBase,
|
||||||
|
|
||||||
# The volume should no longer have any attachments as instance delete
|
# The volume should no longer have any attachments as instance delete
|
||||||
# should have removed them.
|
# should have removed them.
|
||||||
self.assertNotIn(volume_id, self.cinder.attachments[server_id])
|
self.assertNotIn(volume_id,
|
||||||
|
self.cinder.volume_ids_for_instance(server_id))
|
||||||
|
|
|
@ -160,7 +160,8 @@ class TestLocalDeleteAttachedVolumes(test.TestCase):
|
||||||
self._wait_for_volume_attach(server_id, volume_id)
|
self._wait_for_volume_attach(server_id, volume_id)
|
||||||
# Check to see that the fixture is tracking the server and volume
|
# Check to see that the fixture is tracking the server and volume
|
||||||
# attachment.
|
# attachment.
|
||||||
self.assertIn(volume_id, self.cinder.attachments[server_id])
|
self.assertIn(volume_id,
|
||||||
|
self.cinder.volume_ids_for_instance(server_id))
|
||||||
|
|
||||||
# At this point the instance.host is no longer set, so deleting
|
# At this point the instance.host is no longer set, so deleting
|
||||||
# the server will take the local delete path in the API.
|
# the server will take the local delete path in the API.
|
||||||
|
@ -172,7 +173,8 @@ class TestLocalDeleteAttachedVolumes(test.TestCase):
|
||||||
LOG.info('Validating that volume %s was detached from server %s.',
|
LOG.info('Validating that volume %s was detached from server %s.',
|
||||||
volume_id, server_id)
|
volume_id, server_id)
|
||||||
# Now that the bug is fixed, assert the volume was detached.
|
# Now that the bug is fixed, assert the volume was detached.
|
||||||
self.assertNotIn(volume_id, self.cinder.attachments[server_id])
|
self.assertNotIn(volume_id,
|
||||||
|
self.cinder.volume_ids_for_instance(server_id))
|
||||||
|
|
||||||
|
|
||||||
@mock.patch('nova.objects.Service.get_minimum_version',
|
@mock.patch('nova.objects.Service.get_minimum_version',
|
||||||
|
|
|
@ -308,7 +308,8 @@ class ServersPreSchedulingTestCase(test.TestCase,
|
||||||
|
|
||||||
# Since _IntegratedTestBase uses the CastAsCall fixture, when we
|
# Since _IntegratedTestBase uses the CastAsCall fixture, when we
|
||||||
# get the server back we know all of the volume stuff should be done.
|
# get the server back we know all of the volume stuff should be done.
|
||||||
self.assertIn(volume_id, cinder.attachments[server['id']])
|
self.assertIn(volume_id,
|
||||||
|
cinder.volume_ids_for_instance(server['id']))
|
||||||
|
|
||||||
# Now delete the server, which should go through the "local delete"
|
# Now delete the server, which should go through the "local delete"
|
||||||
# code in the API, find the build request and delete it along with
|
# code in the API, find the build request and delete it along with
|
||||||
|
@ -317,7 +318,8 @@ class ServersPreSchedulingTestCase(test.TestCase,
|
||||||
|
|
||||||
# The volume should no longer have any attachments as instance delete
|
# The volume should no longer have any attachments as instance delete
|
||||||
# should have removed them.
|
# should have removed them.
|
||||||
self.assertNotIn(volume_id, cinder.attachments[server['id']])
|
self.assertNotIn(volume_id,
|
||||||
|
cinder.volume_ids_for_instance(server['id']))
|
||||||
|
|
||||||
def test_instance_list_build_request_marker_ip_filter(self):
|
def test_instance_list_build_request_marker_ip_filter(self):
|
||||||
"""Tests listing instances with a marker that is in the build_requests
|
"""Tests listing instances with a marker that is in the build_requests
|
||||||
|
|
Loading…
Reference in New Issue