libvirt: flatten rbd images when unshelving an instance

Previously attempts to remove the shelved snapshot of an unshelved
instance when using the rbd backends for both Nova and Glance would
fail. This was due to the instance disk being cloned from and still
referencing the shelved snapshot image in Glance, blocking any attempt
to remove this image later in the unshelve process.

After much debate this change attempts to fix this issue by flattening
the instance disk while the instance is being spawned as part of an
unshelve. For the rbd imagebackend this removes any reference to the
shelved snapshot in Glance allowing this image to be removed. For all
other imagebackends the call to flatten the image is currently a no-op.

Co-Authored-By: Lee Yarwood <[email protected]>
Co-Authored-By: Vladyslav Drok <[email protected]>

Closes-Bug: #1653953
Change-Id: If3c9d1de3ce0fe394405bd1e1f0fa08ce2baeda8
(cherry picked from commit d89e7d7)
(cherry picked from commit e802ede)

Author: rsritesh

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

@@ -856,6 +856,7 @@ def _create_test_instance():
         'vcpu_model': None,
         'host': 'fake-host',
         'task_state': None,
+        'vm_state': None,
         'trusted_certs': None
     }
 
@@ -16911,8 +16912,8 @@ def fake_prepare(instance, name, tag):
         prepare.side_effect = fake_prepare
         drvr = libvirt_driver.LibvirtDriver(virtapi, False)
 
-        instance = objects.Instance(vm_state=vm_states.BUILDING,
-                                    **self.test_instance)
+        instance = objects.Instance(**self.test_instance)
+        instance.vm_state = vm_states.BUILDING
         vifs = [{'id': uuids.vif_1, 'active': False},
                 {'id': uuids.vif_2, 'active': False}]
 
@@ -18307,6 +18308,7 @@ def _create_instance(self, params=None):
         inst['system_metadata'] = {}
         inst['metadata'] = {}
         inst['task_state'] = None
+        inst['vm_state'] = None
 
         inst.update(params)
 
@@ -19290,6 +19292,54 @@ def test_is_booted_from_volume(self):
         bdm.append({'boot_index': 0})
         self.assertTrue(func(bdi))
 
+    def test_unshelve_noop_flatten_fetch_image_cache(self):
+        instance = self._create_instance(
+            params={'vm_state': vm_states.SHELVED_OFFLOADED})
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        mock_imagebackend = mock.Mock(spec=imagebackend.Lvm)
+        mock_imagebackend.flatten.side_effect = NotImplementedError()
+
+        # Assert that this doesn't raise NotImplementedError
+        drvr._try_fetch_image_cache(mock_imagebackend, mock.sentinel.fetch,
+                self.context, mock.sentinel.filename, uuids.image_id,
+                instance, mock.sentinel.size)
+
+        # Assert that we cache and then flatten the image when an instance is
+        # still SHELVED_OFFLOADED during _try_fetch_image_cache.
+        mock_imagebackend.cache.assert_called_once_with(
+            fetch_func=mock.sentinel.fetch, context=self.context,
+            filename=mock.sentinel.filename, image_id=uuids.image_id,
+            size=mock.sentinel.size, trusted_certs=instance.trusted_certs)
+        mock_imagebackend.flatten.assert_called_once()
+
+    def test_unshelve_rbd_image_flatten_during_fetch_image_cache(self):
+        instance = self._create_instance(
+            params={'vm_state': vm_states.SHELVED_OFFLOADED})
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        mock_rbd_driver = mock.Mock(spec=rbd_utils.RBDDriver)
+        mock_rbd_imagebackend = mock.Mock(spec=imagebackend.Rbd)
+        mock_rbd_imagebackend.rbd_name = mock.sentinel.rbd_name
+        mock_rbd_imagebackend.pool = mock.sentinel.rbd_pool
+        # This is logged so we can't use a sentinel
+        mock_rbd_imagebackend.path = 'rbd:pool/vol_disk'
+        mock_rbd_imagebackend.driver = mock_rbd_driver
+        mock_rbd_imagebackend.flatten.side_effect = \
+            imagebackend.Rbd.flatten(mock_rbd_imagebackend)
+
+        drvr._try_fetch_image_cache(mock_rbd_imagebackend, mock.sentinel.fetch,
+                self.context, mock.sentinel.filename, uuids.image_id,
+                instance, mock.sentinel.size)
+
+        # Assert that we cache and then flatten the image when an instance is
+        # still SHELVED_OFFLOADED during _try_fetch_image_cache.
+        mock_rbd_imagebackend.cache.assert_called_once_with(
+            fetch_func=mock.sentinel.fetch, context=self.context,
+            filename=mock.sentinel.filename, image_id=uuids.image_id,
+            size=mock.sentinel.size, trusted_certs=instance.trusted_certs)
+        mock_rbd_imagebackend.flatten.assert_called_once()
+        mock_rbd_driver.flatten.assert_called_once_with(
+            mock.sentinel.rbd_name, pool=mock.sentinel.rbd_pool)
+
     @mock.patch('nova.virt.libvirt.driver.imagebackend')
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._inject_data')
     @mock.patch('nova.virt.libvirt.driver.imagecache')

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

@@ -1550,6 +1550,12 @@ def get_mon_addrs():
             ["server1:1899", "server2:1920"]),
                          model)
 
+    @mock.patch.object(rbd_utils.RBDDriver, 'flatten')
+    def test_flatten(self, mock_flatten):
+        image = self.image_class(self.INSTANCE, self.NAME)
+        image.flatten()
+        mock_flatten.assert_called_once_with(image.rbd_name, pool=self.POOL)
+
     def test_import_file(self):
         image = self.image_class(self.INSTANCE, self.NAME)
 

nova/virt/libvirt/driver.py

@@ -7821,6 +7821,26 @@ def copy_from_host(target):
             image.cache(fetch_func=copy_from_host, size=size,
                         filename=filename)
 
+        # NOTE(lyarwood): If the instance vm_state is shelved offloaded then we
+        # must be unshelving for _try_fetch_image_cache to be called.
+        if instance.vm_state == vm_states.SHELVED_OFFLOADED:
+            # NOTE(lyarwood): When using the rbd imagebackend the call to cache
+            # above will attempt to clone from the shelved snapshot in Glance
+            # if available from this compute. We then need to flatten the
+            # resulting image to avoid it still referencing and ultimately
+            # blocking the removal of the shelved snapshot at the end of the
+            # unshelve. This is a no-op for all but the rbd imagebackend.
+            try:
+                image.flatten()
+                LOG.debug('Image %s flattened successfully while unshelving '
+                          'instance.', image.path, instance=instance)
+            except NotImplementedError:
+                # NOTE(lyarwood): There's an argument to be made for logging
+                # our inability to call flatten here, however given this isn't
+                # implemented for most of the backends it may do more harm than
+                # good, concerning operators etc so for now just pass.
+                pass
+
     def _create_images_and_backing(self, context, instance, instance_dir,
                                    disk_info, fallback_from_host=None):
         """:param context: security context

nova/virt/libvirt/imagebackend.py

@@ -431,6 +431,14 @@ def clone(self, context, image_id_or_uri):
         raise exception.ImageUnacceptable(image_id=image_id_or_uri,
                                           reason=reason)
 
+    def flatten(self):
+        """Flatten an image.
+
+        The implementation of this method is optional and therefore is
+        not an abstractmethod.
+        """
+        raise NotImplementedError('flatten() is not implemented')
+
     def direct_snapshot(self, context, snapshot_name, image_format, image_id,
                         base_image_id):
         """Prepare a snapshot for direct reference from glance.
@@ -961,6 +969,9 @@ def clone(self, context, image_id_or_uri):
         raise exception.ImageUnacceptable(image_id=image_id_or_uri,
                                           reason=reason)
 
+    def flatten(self):
+        self.driver.flatten(self.rbd_name, pool=self.pool)
+
     def get_model(self, connection):
         secret = None
         if CONF.libvirt.rbd_secret_uuid: