diff --git a/nova/compute/api.py b/nova/compute/api.py index 28368d910fde..dc332b992f2c 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -4803,10 +4803,24 @@ class API: This method is separated to make it possible for cells version to override it. """ - volume_bdm = self._create_volume_bdm( - context, instance, device, volume, disk_bus=disk_bus, - device_type=device_type, tag=tag, - delete_on_termination=delete_on_termination) + try: + volume_bdm = self._create_volume_bdm( + context, instance, device, volume, disk_bus=disk_bus, + device_type=device_type, tag=tag, + delete_on_termination=delete_on_termination) + except oslo_exceptions.MessagingTimeout: + # The compute node might have already created the attachment but + # we never received the answer. In this case it is safe to delete + # the attachment as nobody will ever pick it up again. + with excutils.save_and_reraise_exception(): + try: + objects.BlockDeviceMapping.get_by_volume_and_instance( + context, volume['id'], instance.uuid).destroy() + LOG.debug("Delete BDM after compute did not respond to " + f"attachment request for volume {volume['id']}") + except exception.VolumeBDMNotFound: + LOG.debug("BDM not found, ignoring removal. " + f"Error attaching volume {volume['id']}") try: self._check_attach_and_reserve_volume(context, volume, instance, volume_bdm, diff --git a/nova/tests/unit/compute/test_api.py b/nova/tests/unit/compute/test_api.py index 64064cf63674..81eed9988da4 100644 --- a/nova/tests/unit/compute/test_api.py +++ b/nova/tests/unit/compute/test_api.py @@ -520,6 +520,36 @@ class _ComputeAPIUnitTestMixIn(object): mock_attach.assert_called_once_with(self.context, instance, fake_bdm) + @mock.patch.object(compute_rpcapi.ComputeAPI, 'reserve_block_device_name') + @mock.patch.object( + objects.BlockDeviceMapping, 'get_by_volume_and_instance') + @mock.patch.object(objects.BlockDeviceMappingList, 'get_by_volume') + def test_attach_volume_reserve_bdm_timeout( + self, mock_get_by_volume, mock_get_by_volume_and_instance, + mock_reserve): + mock_get_by_volume.side_effect = exception.VolumeBDMNotFound( + volume_id='fake-volume-id') + + fake_bdm = mock.MagicMock(spec=objects.BlockDeviceMapping) + mock_get_by_volume_and_instance.return_value = fake_bdm + instance = self._create_instance_obj() + volume = fake_volume.fake_volume(1, 'test-vol', 'test-vol', + None, None, None, None, None) + + mock_reserve.side_effect = oslo_exceptions.MessagingTimeout() + + mock_volume_api = mock.patch.object(self.compute_api, 'volume_api', + mock.MagicMock(spec=cinder.API)) + + with mock_volume_api as mock_v_api: + mock_v_api.get.return_value = volume + self.assertRaises(oslo_exceptions.MessagingTimeout, + self.compute_api.attach_volume, + self.context, instance, volume['id']) + mock_get_by_volume_and_instance.assert_called_once_with( + self.context, volume['id'], instance.uuid) + fake_bdm.destroy.assert_called_once_with() + @mock.patch.object(compute_rpcapi.ComputeAPI, 'reserve_block_device_name') @mock.patch.object(objects.BlockDeviceMappingList, 'get_by_volume') @mock.patch.object(compute_rpcapi.ComputeAPI, 'attach_volume') diff --git a/releasenotes/notes/bug-1960401-504eb255253d966a.yaml b/releasenotes/notes/bug-1960401-504eb255253d966a.yaml new file mode 100644 index 000000000000..ef5582543a2a --- /dev/null +++ b/releasenotes/notes/bug-1960401-504eb255253d966a.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + The `bug 1960401`_ is fixed which can cause invalid `BlockDeviceMappings` + to accumulate in the database. This prevented the respective volumes from + being attached again to the instance. + + .. _bug 1960401: https://bugs.launchpad.net/nova/+bug/1960401