From: Ramana Raja Date: Wed, 21 Aug 2019 09:41:47 +0000 (+0530) Subject: mgr/volumes: cleanup FS subvolume or subvolume group path X-Git-Tag: v14.2.5~258^2~4 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=d894ac96ce825e756171c054d38f24531fa31ecc;p=ceph.git mgr/volumes: cleanup FS subvolume or subvolume group path ... on best effort basis when FS subvolume or subvolumegroup create command fails. Fixes: https://tracker.ceph.com/issues/41371 Signed-off-by: Ramana Raja (cherry picked from commit ecf53a3527ed7561ddceeed75ed4aef1daf8230d) --- diff --git a/qa/tasks/cephfs/test_volumes.py b/qa/tasks/cephfs/test_volumes.py index c7dd3997d81c..36d3ac10112c 100644 --- a/qa/tasks/cephfs/test_volumes.py +++ b/qa/tasks/cephfs/test_volumes.py @@ -146,6 +146,22 @@ class TestVolumes(CephFSTestCase): # clean up self._fs_cmd("subvolume", "rm", self.volname, subvolume, "--force") + def test_subvolume_create_with_auto_cleanup_on_fail(self): + subvolume = self._generate_random_subvolume_name() + data_pool = "invalid_pool" + # create subvolume with invalid data pool layout fails + with self.assertRaises(CommandFailedError): + self._fs_cmd("subvolume", "create", self.volname, subvolume, "--pool_layout", data_pool) + + # check whether subvol path is cleaned up + try: + self._fs_cmd("subvolume", "getpath", self.volname, subvolume) + except CommandFailedError as ce: + if ce.exitstatus != errno.ENOENT: + raise + else: + raise + def test_nonexistent_subvolume_rm(self): # remove non-existing subvolume subvolume = "non_existent_subvolume" @@ -249,6 +265,22 @@ class TestVolumes(CephFSTestCase): # clean up self._fs_cmd("subvolumegroup", "rm", self.volname, group, "--force") + def test_subvolume_group_create_with_auto_cleanup_on_fail(self): + group = self._generate_random_group_name() + data_pool = "invalid_pool" + # create group with invalid data pool layout + with self.assertRaises(CommandFailedError): + self._fs_cmd("subvolumegroup", "create", self.volname, group, "--pool_layout", data_pool) + + # check whether group path is cleaned up + try: + self._fs_cmd("subvolumegroup", "getpath", self.volname, group) + except CommandFailedError as ce: + if ce.exitstatus != errno.ENOENT: + raise + else: + raise + def test_subvolume_create_with_desired_data_pool_layout_in_group(self): subvol1 = self._generate_random_subvolume_name() subvol2 = self._generate_random_subvolume_name() diff --git a/src/pybind/mgr/volumes/fs/subvolume.py b/src/pybind/mgr/volumes/fs/subvolume.py index 75fab056d4dd..4789bd133816 100644 --- a/src/pybind/mgr/volumes/fs/subvolume.py +++ b/src/pybind/mgr/volumes/fs/subvolume.py @@ -75,29 +75,40 @@ class SubVolume(object): self.fs.mkdirs(subvolpath, mode) - if size is not None: - self.fs.setxattr(subvolpath, 'ceph.quota.max_bytes', str(size).encode('utf-8'), 0) - - if pool: + try: + if size is not None: + self.fs.setxattr(subvolpath, 'ceph.quota.max_bytes', str(size).encode('utf-8'), 0) + + if pool: + try: + self.fs.setxattr(subvolpath, 'ceph.dir.layout.pool', pool.encode('utf-8'), 0) + except cephfs.InvalidValue: + raise VolumeException(-errno.EINVAL, + "Invalid pool layout '{0}'. It must be a valid data pool".format(pool)) + + xattr_key = xattr_val = None + if namespace_isolated: + # enforce security isolation, use separate namespace for this subvolume + xattr_key = 'ceph.dir.layout.pool_namespace' + xattr_val = spec.fs_namespace + elif not pool: + # If subvolume's namespace layout is not set, then the subvolume's pool + # layout remains unset and will undesirably change with ancestor's + # pool layout changes. + xattr_key = 'ceph.dir.layout.pool' + xattr_val = self._get_ancestor_xattr(subvolpath, "ceph.dir.layout.pool") + # TODO: handle error... + self.fs.setxattr(subvolpath, xattr_key, xattr_val.encode('utf-8'), 0) + except Exception as e: try: - self.fs.setxattr(subvolpath, 'ceph.dir.layout.pool', pool.encode('utf-8'), 0) - except cephfs.InvalidValue: - raise VolumeException(-errno.EINVAL, - "Invalid pool layout '{0}'. It must be a valid data pool".format(pool)) - - xattr_key = xattr_val = None - if namespace_isolated: - # enforce security isolation, use separate namespace for this subvolume - xattr_key = 'ceph.dir.layout.pool_namespace' - xattr_val = spec.fs_namespace - elif not pool: - # If subvolume's namespace layout is not set, then the subvolume's pool - # layout remains unset and will undesirably change with ancestor's - # pool layout changes. - xattr_key = 'ceph.dir.layout.pool' - xattr_val = self._get_ancestor_xattr(subvolpath, "ceph.dir.layout.pool") - # TODO: handle error... - self.fs.setxattr(subvolpath, xattr_key, xattr_val.encode('utf-8'), 0) + # cleanup subvol path on best effort basis + log.debug("cleaning up subvolume with path: {0}".format(subvolpath)) + self.fs.rmdir(subvolpath) + except Exception: + log.debug("failed to clean up subvolume with path: {0}".format(subvolpath)) + pass + finally: + raise e def remove_subvolume(self, spec, force): """ @@ -181,13 +192,24 @@ class SubVolume(object): def create_group(self, spec, mode=0o755, pool=None): path = spec.group_path self.fs.mkdirs(path, mode) - if not pool: - pool = self._get_ancestor_xattr(path, "ceph.dir.layout.pool") try: - self.fs.setxattr(path, 'ceph.dir.layout.pool', pool.encode('utf-8'), 0) - except cephfs.InvalidValue: - raise VolumeException(-errno.EINVAL, - "Invalid pool layout '{0}'. It must be a valid data pool".format(pool)) + if not pool: + pool = self._get_ancestor_xattr(path, "ceph.dir.layout.pool") + try: + self.fs.setxattr(path, 'ceph.dir.layout.pool', pool.encode('utf-8'), 0) + except cephfs.InvalidValue: + raise VolumeException(-errno.EINVAL, + "Invalid pool layout '{0}'. It must be a valid data pool".format(pool)) + except Exception as e: + try: + # cleanup group path on best effort basis + log.debug("cleaning up subvolumegroup with path: {0}".format(path)) + self.fs.rmdir(path) + except Exception: + log.debug("failed to clean up subvolumegroup with path: {0}".format(path)) + pass + finally: + raise e def remove_group(self, spec, force): path = spec.group_path