only attempt to clean up dangling bdm if cinder is installed

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

Closes-Bug: #2033752
Change-Id: I0ada59d8901f8620fd1f3dc20d6be303aa7dabca

Author: SeanMooney

nova/compute/manager.py

@@ -4185,9 +4185,21 @@ def _delete_dangling_bdms(self, context, instance, bdms):
 
         :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 @@ def _delete_dangling_bdms(self, context, instance, bdms):
                 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)):

nova/tests/unit/compute/test_compute.py

@@ -25,7 +25,9 @@
 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 @@ def _test__delete_dangling_bdms(
 
             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 @@ def test_dangling_bdms_delete_cinder_attachments(self):
 
         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)