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 <rraja@redhat.com>
(cherry picked from commit ecf53a3527ed7561ddceeed75ed4aef1daf8230d)

diff --git a/qa/tasks/cephfs/test_volumes.py b/qa/tasks/cephfs/test_volumes.py
index c7dd3997d81c006a100699df979b83ea78cb45df..36d3ac10112cb0e67f1b5e8ab7719946461268cc 100644 (file)
--- 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 75fab056d4dd82211fe3a19f85e1dd6cad53761d..4789bd1338160d3c150a543c6e825dae2866b8da 100644 (file)
--- 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