openstack/nova
Code Issues Proposed changes

only attempt to clean up dangling bdm if cinder is installed

Browse Source 
This change ensure we only try to clean up dangling bdms if
cinder is installed and reachable.

Closes-Bug: #2033752
Change-Id: I0ada59d8901f8620fd1f3dc20d6be303aa7dabca
This commit is contained in:
Sean Mooney 2023-09-01 12:46:21 +01:00 committed by Amit Uniyal
parent 88c73e2931
commit 68b2131d81
2 changed files with 56 additions and 5 deletions
nova
compute
manager.py
tests/unit/compute
test_compute.py

16
nova/compute/manager.py

@ -4185,9 +4185,21 @@ class ComputeManager(manager.Manager):
:param context: The nova request context.
:param instance: instance object.
:param instance: BlockDeviceMappingList list object.
:param bdms: BlockDeviceMappingList list object.
"""
try:
cinder_attachments = self.volume_api.attachment_get_all(
context, instance.uuid)
except (keystone_exception.EndpointNotFound,
cinder_exception.ClientException):
# if cinder is not deployed we never need to check for
# attachments as there cannot be dangling bdms.
# if we cannot connect to cinder we cannot check for dangling
# bdms so we skip the check. Intermittent connection issues
# to cinder should not cause instance reboot to fail.
return
# attachments present in nova DB, ones nova knows about
nova_attachments = []
bdms_to_delete = []
@ -4206,8 +4218,6 @@ class ComputeManager(manager.Manager):
else:
nova_attachments.append(bdm.attachment_id)
cinder_attachments = self.volume_api.attachment_get_all(
context, instance.uuid)
cinder_attachments = [each['id'] for each in cinder_attachments]
if len(set(cinder_attachments) - set(nova_attachments)):

45
nova/tests/unit/compute/test_compute.py

@ -25,7 +25,9 @@ import sys
from unittest import mock
from castellan import key_manager
from cinderclient import exceptions as cinder_exception
import ddt
from keystoneclient import exceptions as keystone_exception
from neutronclient.common import exceptions as neutron_exceptions
from oslo_log import log as logging
import oslo_messaging as messaging
@ -1495,6 +1497,43 @@ class ComputeVolumeTestCase(BaseTestCase):
return mock_destroy, mock_attachment_delete
def _test_delete_dangling_bdms_cinder_error(self):
instance = self._create_fake_instance_obj()
bdms = objects.BlockDeviceMappingList(objects=[
objects.BlockDeviceMapping(
**fake_block_device.AnonFakeDbBlockDeviceDict(
{
'instance_uuid': instance.uuid,
'volume_id': uuids.fake_vol1,
'attachment_id': uuids.fake_attachment_1,
'source_type': 'volume',
'destination_type': 'volume'})),
objects.BlockDeviceMapping(
**fake_block_device.AnonFakeDbBlockDeviceDict(
{
'instance_uuid': instance.uuid,
'volume_id': uuids.fake_vol2,
'attachment_id': uuids.fake_attachment_2,
'source_type': 'image',
'destination_type': 'volume'}))
])
expected = bdms.obj_to_primitive()
self.compute._delete_dangling_bdms(self.context, instance, bdms)
self.assertEqual(expected, bdms.obj_to_primitive())
@mock.patch.object(
cinder.API, 'attachment_get_all',
new=mock.Mock(side_effect=keystone_exception.EndpointNotFound()))
def test_delete_dangling_bdms_cinder_not_found(self):
self._test_delete_dangling_bdms_cinder_error()
@mock.patch.object(
cinder.API, 'attachment_get_all',
new=mock.Mock(side_effect=cinder_exception.ClientException(500)))
def test_delete_dangling_bdms_cinder_client_error(self):
self._test_delete_dangling_bdms_cinder_error()
def test_dangling_bdms_nothing_to_delete(self):
"""no bdm, no attachments"""
instance = self._create_fake_instance_obj()
@ -1588,13 +1627,15 @@ class ComputeVolumeTestCase(BaseTestCase):
cinder_attachments = [
{'id': uuids.fake_attachment_1},
{'id': 2},
{'id': uuids.not_in_nova_bdms},
]
_, mock_attachment_delete = self._test__delete_dangling_bdms(
instance, bdms, cinder_attachments, True)
self.assertTrue(mock_attachment_delete.call_count, 1)
self.assertEqual(mock_attachment_delete.call_args_list[0][0][1], 2)
self.assertEqual(
mock_attachment_delete.call_args_list[0][0][1],
uuids.not_in_nova_bdms)
self.assertEqual(len(bdms), 1)