Cleanup old resize instances dir before resize

If there is a failed resize that also failed the cleanup
process performed by _cleanup_remote_migration() the retry
of the resize will fail because it cannot rename the current
instances directory to _resize.

This renames the _cleanup_failed_migration() that does the
same logic we want to _cleanup_failed_instance_base() and
uses it for both migration and resize cleanup of directory.

It then simply calls _cleanup_failed_instances_base() with
the resize dir path before trying a resize.

Closes-Bug: 1960230
Change-Id: I7412b16be310632da59a6139df9f0913281b5d77

Author: tobias-urdin

nova/tests/unit/virt/libvirt/test_driver.py

@@ -21544,6 +21544,8 @@ def test_migrate_disk_and_power_off_exception(
                           context.get_admin_context(), ins_ref, '10.0.0.2',
                           flavor_obj, None)
 
+    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
+                '_cleanup_failed_instance_base')
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.unplug_vifs')
     @mock.patch('nova.virt.libvirt.utils.save_and_migrate_vtpm_dir')
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
@@ -21560,7 +21562,7 @@ def _test_migrate_disk_and_power_off(
             self, ctxt, flavor_obj, mock_execute, mock_exists, mock_rename,
             mock_is_shared, mock_get_host_ip, mock_destroy,
             mock_get_disk_info, mock_vtpm, mock_unplug_vifs,
-            block_device_info=None, params_for_instance=None):
+            mock_cleanup, block_device_info=None, params_for_instance=None):
         """Test for nova.virt.libvirt.driver.LivirtConnection
         .migrate_disk_and_power_off.
         """
@@ -21575,6 +21577,8 @@ def _test_migrate_disk_and_power_off(
                ctxt, instance, '10.0.0.2', flavor_obj, None,
                block_device_info=block_device_info)
 
+        mock_cleanup.assert_called_once()
+        mock_cleanup.reset_mock()
         self.assertEqual(out, disk_info_text)
         mock_vtpm.assert_called_with(
             instance.uuid, mock.ANY, mock.ANY, '10.0.0.2', mock.ANY, mock.ANY)
@@ -21585,6 +21589,7 @@ def _test_migrate_disk_and_power_off(
                ctxt, instance, '10.0.0.1', flavor_obj, None,
                block_device_info=block_device_info)
 
+        mock_cleanup.assert_called_once()
         self.assertEqual(out, disk_info_text)
         mock_vtpm.assert_called_with(
             instance.uuid, mock.ANY, mock.ANY, '10.0.0.1', mock.ANY, mock.ANY)
@@ -22514,8 +22519,8 @@ def test_finish_revert_migration_snap_backend_image_does_not_exist(self):
             self.assertFalse(drvr.image_backend.remove_snap.called)
 
     @mock.patch.object(shutil, 'rmtree')
-    def test_cleanup_failed_migration(self, mock_rmtree):
-        self.drvr._cleanup_failed_migration('/fake/inst')
+    def test_cleanup_failed_instance_base(self, mock_rmtree):
+        self.drvr._cleanup_failed_instance_base('/fake/inst')
         mock_rmtree.assert_called_once_with('/fake/inst')
 
     @mock.patch.object(libvirt_driver.LibvirtDriver, '_cleanup_resize')

nova/virt/libvirt/driver.py

@@ -10973,6 +10973,9 @@ def migrate_disk_and_power_off(self, context, instance, dest,
         disk_info = self._get_instance_disk_info(instance, block_device_info)
 
         try:
+            # If cleanup failed in previous resize attempts we try to remedy
+            # that before a resize is tried again
+            self._cleanup_failed_instance_base(inst_base_resize)
             os.rename(inst_base, inst_base_resize)
             # if we are migrating the instance with shared instance path then
             # create the directory.  If it is a remote node the directory
@@ -11196,9 +11199,9 @@ def finish_migration(
 
         LOG.debug("finish_migration finished successfully.", instance=instance)
 
-    def _cleanup_failed_migration(self, inst_base):
-        """Make sure that a failed migrate doesn't prevent us from rolling
-        back in a revert.
+    def _cleanup_failed_instance_base(self, inst_base):
+        """Make sure that a failed migrate or resize doesn't prevent us from
+        rolling back in a revert or retrying a resize.
         """
         try:
             shutil.rmtree(inst_base)
@@ -11254,7 +11257,7 @@ def finish_revert_migration(
         # that would conflict. Also, don't fail on the rename if the
         # failure happened early.
         if os.path.exists(inst_base_resize):
-            self._cleanup_failed_migration(inst_base)
+            self._cleanup_failed_instance_base(inst_base)
             os.rename(inst_base_resize, inst_base)
 
         root_disk = self.image_backend.by_name(instance, 'disk')

releasenotes/notes/bug-1960230-cleanup-instances-dir-resize-56282e1b436a4908.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Fixed bug `1960230 <https://bugs.launchpad.net/nova/+bug/1960230>`_ that
+    prevented resize of instances that had previously failed and not been
+    cleaned up.