From 823e1b416910c41d45551fe345e12f321bae3ca8 Mon Sep 17 00:00:00 2001 From: Victoria Martinez de la Cruz Date: Mon, 19 Sep 2022 08:31:13 +0000 Subject: [PATCH] Shorten snapshots names in CephFS drivers The name set to snapshots in the CephFS drivers is generated appending the snapshot id with the snapshot instance id. This is a problem because Ceph has a limitation in the name of the subvolumes (shares) being created. What happens then is, when trying to create a snapshot, the snapshot gets created with a truncated name and it cannot be accessed later. This change drops the snapshot instance id from the assigned name in order to avoid this limitation. Closes-Bug: #1967760 Change-Id: Ifab3ec9de2abeb1af8e6499feca0236e1ee22fcf (cherry picked from commit c86523d5909a2d61e03fb29f4c0c71b67cf18205) (cherry picked from commit 4d4d4bdb118968b1cb089fa8b415c8354874e3a7) (cherry picked from commit 3be5df4e1fe5feac43c3c1524eb00c4441eec476) (cherry picked from commit 569a30df0533fe1bd0b53d5111d801353a1dc126) (cherry picked from commit 67187c28fae622d679362adf4c68f968ce33473e) --- manila/share/drivers/cephfs/driver.py | 19 +++++++++--- .../tests/share/drivers/cephfs/test_driver.py | 30 ++++++++++++------- ...napshot-names-cephfs-a220e2b9f7ba5739.yaml | 6 ++++ 3 files changed, 41 insertions(+), 14 deletions(-) create mode 100644 releasenotes/notes/bug-1967760-shorten-snapshot-names-cephfs-a220e2b9f7ba5739.yaml diff --git a/manila/share/drivers/cephfs/driver.py b/manila/share/drivers/cephfs/driver.py index 7375f83af5..4074bbb34f 100644 --- a/manila/share/drivers/cephfs/driver.py +++ b/manila/share/drivers/cephfs/driver.py @@ -580,7 +580,7 @@ class CephFSDriver(driver.ExecuteMixin, driver.GaneshaMixin, argdict = { "vol_name": self.volname, "sub_name": snapshot["share_id"], - "snap_name": "_".join([snapshot["snapshot_id"], snapshot["id"]]), + "snap_name": snapshot["snapshot_id"], } rados_command( @@ -590,13 +590,24 @@ class CephFSDriver(driver.ExecuteMixin, driver.GaneshaMixin, # delete a FS snapshot LOG.debug("[%(be)s]: delete_snapshot: snapshot=%(id)s.", {"be": self.backend_name, "id": snapshot['id']}) - argdict = { + + # FIXME(vkmc) remove this in CC (next tick) release. + legacy_snap_name = "_".join([snapshot["snapshot_id"], snapshot["id"]]) + + argdict_legacy = { "vol_name": self.volname, "sub_name": snapshot["share_id"], - "snap_name": '_'.join([snapshot['snapshot_id'], snapshot['id']]), + "snap_name": legacy_snap_name, "force": True, } + # try removing snapshot using legacy naming + rados_command( + self.rados_client, "fs subvolume snapshot rm", argdict_legacy) + + # in case it's a snapshot with new naming, retry remove with new name + argdict = argdict_legacy.copy() + argdict.update({"snap_name": snapshot["snapshot_id"]}) rados_command(self.rados_client, "fs subvolume snapshot rm", argdict) def create_share_group(self, context, sg_dict, share_server=None): @@ -719,7 +730,7 @@ class CephFSDriver(driver.ExecuteMixin, driver.GaneshaMixin, argdict = { "vol_name": self.volname, "sub_name": parent_share["id"], - "snap_name": '_'.join([snapshot["snapshot_id"], snapshot["id"]]), + "snap_name": snapshot["snapshot_id"], "target_sub_name": share["id"] } if share['share_group_id'] is not None: diff --git a/manila/tests/share/drivers/cephfs/test_driver.py b/manila/tests/share/drivers/cephfs/test_driver.py index f9fcc77513..de8b1b3746 100644 --- a/manila/tests/share/drivers/cephfs/test_driver.py +++ b/manila/tests/share/drivers/cephfs/test_driver.py @@ -352,8 +352,7 @@ class CephFSDriverTestCase(test.TestCase): snapshot_create_dict = { "vol_name": self._driver.volname, "sub_name": self._snapshot["share_id"], - "snap_name": "_".join([ - self._snapshot["snapshot_id"], self._snapshot["id"]]), + "snap_name": self._snapshot["snapshot_id"] } self._driver.create_snapshot(self._context, self._snapshot, None) @@ -363,23 +362,35 @@ class CephFSDriverTestCase(test.TestCase): snapshot_create_prefix, snapshot_create_dict) def test_delete_snapshot(self): + legacy_snap_name = "_".join( + [self._snapshot["snapshot_id"], self._snapshot["id"]]) + snapshot_remove_prefix = "fs subvolume snapshot rm" snapshot_remove_dict = { "vol_name": self._driver.volname, "sub_name": self._snapshot["share_id"], - "snap_name": "_".join([ - self._snapshot["snapshot_id"], self._snapshot["id"]]), - "force": True, + "snap_name": legacy_snap_name, + "force": True } + snapshot_remove_dict_2 = snapshot_remove_dict.copy() + snapshot_remove_dict_2.update( + {"snap_name": self._snapshot["snapshot_id"]}) + self._driver.delete_snapshot(self._context, self._snapshot, None) - driver.rados_command.assert_called_once_with( - self._driver.rados_client, - snapshot_remove_prefix, snapshot_remove_dict) + driver.rados_command.assert_has_calls([ + mock.call(self._driver.rados_client, + snapshot_remove_prefix, + snapshot_remove_dict), + mock.call(self._driver.rados_client, + snapshot_remove_prefix, + snapshot_remove_dict_2)]) + + self.assertEqual(2, driver.rados_command.call_count) def test_create_share_group(self): group_create_prefix = "fs subvolumegroup create" @@ -453,8 +464,7 @@ class CephFSDriverTestCase(test.TestCase): create_share_from_snapshot_dict = { "vol_name": self._driver.volname, "sub_name": parent_share["id"], - "snap_name": "_".join([ - self._snapshot["snapshot_id"], self._snapshot["id"]]), + "snap_name": self._snapshot["snapshot_id"], "target_sub_name": self._share["id"] } diff --git a/releasenotes/notes/bug-1967760-shorten-snapshot-names-cephfs-a220e2b9f7ba5739.yaml b/releasenotes/notes/bug-1967760-shorten-snapshot-names-cephfs-a220e2b9f7ba5739.yaml new file mode 100644 index 0000000000..97287f8b0f --- /dev/null +++ b/releasenotes/notes/bug-1967760-shorten-snapshot-names-cephfs-a220e2b9f7ba5739.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Make snapshot names in CephFS drivers shorter to + avoid limitation in Ceph clusters which truncates + the subvolume name and makes the snapshots inaccesible. \ No newline at end of file