Merge "compute: Don't detach volumes when RescheduledException raised without retry" into stable/victoria

Author: Zuul

nova/compute/manager.py

@@ -2230,8 +2230,6 @@ def _do_build_and_run_instance(self, context, instance, image,
                     instance=instance)
                 self._cleanup_allocated_networks(context, instance,
                     requested_networks)
-                self._cleanup_volumes(context, instance,
-                    block_device_mapping, raise_exc=False)
                 compute_utils.add_instance_fault_from_exc(context,
                         instance, e, sys.exc_info(),
                         fault_message=e.kwargs['reason'])

nova/tests/functional/regressions/test_bug_1899649.py

@@ -92,19 +92,9 @@ def test_failure_to_schedule_with_host(self):
         self._assert_failure_and_volume_attachments(server)
 
     def test_failure_to_build_with_az_and_host(self):
-        # Assert that a volume attachments does not remain after a failure to
+        # Assert that a volume attachments remain after a failure to
         # build and reschedule by providing an availability_zone *and* host,
         # skipping the scheduler. This is bug #1899649.
         self.server['availability_zone'] = 'nova:compute1'
         server = self.admin_api.post_server({'server': self.server})
-
-        # Assert the server ends up in an ERROR state
-        self._wait_for_state_change(server, 'ERROR')
-
-        # FIXME(lyarwood): A single volume attachment should be present for the
-        # instance at this stage as the volume *can* otherwise be marked as
-        # available within Cinder if it isn't multi-attached.
-        attachments = self.cinder.volume_to_attachment.get(self.volume_id)
-        self.assertEqual(0, len(attachments))
-        self.assertNotIn(
-            self.volume_id, self.cinder.volume_ids_for_instance(server['id']))
+        self._assert_failure_and_volume_attachments(server)

nova/tests/unit/compute/test_compute_mgr.py

@@ -6876,14 +6876,13 @@ def test_rescheduled_exception_with_sriov_network_allocated(self,
     @mock.patch.object(objects.Instance, 'save')
     @mock.patch.object(manager.ComputeManager,
                        '_nil_out_instance_obj_host_and_node')
-    @mock.patch.object(manager.ComputeManager, '_cleanup_volumes')
     @mock.patch.object(manager.ComputeManager, '_cleanup_allocated_networks')
     @mock.patch.object(manager.ComputeManager, '_set_instance_obj_error_state')
     @mock.patch.object(compute_utils, 'add_instance_fault_from_exc')
     @mock.patch.object(manager.ComputeManager, '_build_and_run_instance')
     def test_rescheduled_exception_without_retry(self,
-            mock_build_run, mock_add, mock_set, mock_clean_net, mock_clean_vol,
-            mock_nil, mock_save, mock_start, mock_finish):
+            mock_build_run, mock_add, mock_set, mock_clean_net, mock_nil,
+            mock_save, mock_start, mock_finish):
         self._do_build_instance_update(mock_save)
         mock_build_run.side_effect = exception.RescheduledException(reason='',
                 instance_uuid=self.instance.uuid)
@@ -6919,9 +6918,6 @@ def _wrapped_do_build_and_run_instance(*args, **kwargs):
                 self.accel_uuids)
         mock_clean_net.assert_called_once_with(self.context, self.instance,
                 self.requested_networks)
-        mock_clean_vol.assert_called_once_with(self.context,
-                self.instance, self.block_device_mapping,
-                raise_exc=False)
         mock_add.assert_called_once_with(self.context, self.instance,
                 mock.ANY, mock.ANY, fault_message=mock.ANY)
         mock_nil.assert_called_once_with(self.instance)