From 3d221ec529862d43ab303644e74ee9ad6ce8cd40 Mon Sep 17 00:00:00 2001 From: Abhishek Kekane Date: Fri, 26 Nov 2021 14:58:23 +0000 Subject: [PATCH] [RBD] Clone v2: Image is unusable if deletion fails Recently cinder has utilized Ceph clone v2 support for its RBD backend, since then if you attempt to delete an image from glance that has a dependent volume, all future uses of that image will fail in error state. Despite the fact that image itself is still inside of Ceph/Glance. This issue is reproducible if you are using ceph client version greater than 'luminous' To resolve this issue glance RBD driver now checks whether snapshot of original image has any external references before deleting/removing it's snapshot and returns 409 Conflict response if it has any. NOTE: To check this dependency glance osd needs 'read' access to cinder and nova side RBD pool. Closes-Bug: #1954883 Depends-on: https://review.opendev.org/c/openstack/devstack-plugin-ceph/+/819476 Change-Id: If30b7bd7acac148b6f89ce46abbe128c678c90e7 --- glance_store/_drivers/rbd.py | 16 ++++++++++++++++ glance_store/tests/unit/test_multistore_rbd.py | 15 +++++++++++++++ glance_store/tests/unit/test_rbd_store.py | 15 +++++++++++++++ .../notes/bug-1954883-3666d63a3c0233f1.yaml | 13 +++++++++++++ 4 files changed, 59 insertions(+) create mode 100644 releasenotes/notes/bug-1954883-3666d63a3c0233f1.yaml diff --git a/glance_store/_drivers/rbd.py b/glance_store/_drivers/rbd.py index b5431fc4..9c1d45b4 100644 --- a/glance_store/_drivers/rbd.py +++ b/glance_store/_drivers/rbd.py @@ -434,6 +434,14 @@ class Store(driver.Store): 'snapshot': DEFAULT_SNAPNAME, }, self.conf) + def _snapshot_has_external_reference(self, image, snapshot_name): + """Returns True if snapshot has external reference else False. + """ + image.set_snap(snapshot_name) + has_references = bool(image.list_children()) + image.set_snap(None) + return has_references + def _delete_image(self, target_pool, image_name, snapshot_name=None, context=None): """ @@ -453,6 +461,14 @@ class Store(driver.Store): if snapshot_name is not None: with rbd.Image(ioctx, image_name) as image: try: + # NOTE(abhishekk): Check whether snapshot + # has any external references + if self._snapshot_has_external_reference( + image, snapshot_name): + raise rbd.ImageBusy( + "Image snapshot has external " + "references.") + self._unprotect_snapshot(image, snapshot_name) image.remove_snap(snapshot_name) except rbd.ImageNotFound as exc: diff --git a/glance_store/tests/unit/test_multistore_rbd.py b/glance_store/tests/unit/test_multistore_rbd.py index 384fca09..822c1b5e 100644 --- a/glance_store/tests/unit/test_multistore_rbd.py +++ b/glance_store/tests/unit/test_multistore_rbd.py @@ -113,6 +113,12 @@ class MockRBD(object): def remove_snap(self, *args, **kwargs): pass + def set_snap(self, *args, **kwargs): + pass + + def list_children(self, *args, **kwargs): + pass + def protect_snap(self, *args, **kwargs): pass @@ -417,6 +423,15 @@ class TestMultiStore(base.MultiStoreBaseTest, self.called_commands_expected = ['unprotect_snap'] + def test_delete_image_snap_has_external_references(self): + with mock.patch.object(MockRBD.Image, 'list_children') as mocked: + mocked.return_value = True + + self.assertRaises(exceptions.InUseByStore, + self.store._delete_image, + 'fake_pool', self.location.image, + snapshot_name='snap') + def test_delete_image_w_snap_exc_image_has_snap(self): def _fake_remove(*args, **kwargs): self.called_commands_actual.append('remove') diff --git a/glance_store/tests/unit/test_rbd_store.py b/glance_store/tests/unit/test_rbd_store.py index 2d1cae68..cbd34acd 100644 --- a/glance_store/tests/unit/test_rbd_store.py +++ b/glance_store/tests/unit/test_rbd_store.py @@ -114,6 +114,12 @@ class MockRBD(object): def remove_snap(self, *args, **kwargs): pass + def set_snap(self, *args, **kwargs): + pass + + def list_children(self, *args, **kwargs): + pass + def protect_snap(self, *args, **kwargs): pass @@ -623,6 +629,15 @@ class TestStore(base.StoreBaseTest, self.called_commands_expected = ['unprotect_snap'] + def test_delete_image_snap_has_external_references(self): + with mock.patch.object(MockRBD.Image, 'list_children') as mocked: + mocked.return_value = True + + self.assertRaises(exceptions.InUseByStore, + self.store._delete_image, + 'fake_pool', self.location.image, + snapshot_name='snap') + def test_delete_image_w_snap_exc_image_has_snap(self): def _fake_remove(*args, **kwargs): self.called_commands_actual.append('remove') diff --git a/releasenotes/notes/bug-1954883-3666d63a3c0233f1.yaml b/releasenotes/notes/bug-1954883-3666d63a3c0233f1.yaml new file mode 100644 index 00000000..fc589765 --- /dev/null +++ b/releasenotes/notes/bug-1954883-3666d63a3c0233f1.yaml @@ -0,0 +1,13 @@ +--- +fixes: + - | + * Bug 1954883_: [RBD] Image is unusable if deletion fails + + .. _1954883: https://code.launchpad.net/bugs/1954883 + +upgrade: + - | + Deployments which are using Ceph V2 clone feature (i.e. RBD backend for + glance_store as well as cinder driver is RBD or nova is using RBD driver) + and minimum ceph client version is greater than 'luminous' need to grant + glance osd read access to the cinder and nova RBD pool.