mgr/volumes: Fix permission during subvol creation with mode

The subvolume creation with specific mode leads to
creation of parent directories ('/volumes/_no_group') with
the same mode if it's not already created. Fixed the same.

Similarly, the subvolumegroup creation with specific mode
leads to creation of parent directory ('/volumes') with
same mode if it's not already created. Fixed the same.

Fixes: https://tracker.ceph.com/issues/51870
Signed-off-by: Kotresh HR <khiremat@redhat.com>

diff --git a/qa/tasks/cephfs/test_volumes.py b/qa/tasks/cephfs/test_volumes.py
index 89cec3af35e16282e8f603825ad0a12279072e5e..9052b578906c6b7c89309c758506688d414fae56 100644 (file)
--- a/qa/tasks/cephfs/test_volumes.py
+++ b/qa/tasks/cephfs/test_volumes.py
@@ -610,17 +610,20 @@ class TestSubvolumeGroups(TestVolumesHelper):
         expected_mode2 = "777"
 
         # create group
-        self._fs_cmd("subvolumegroup", "create", self.volname, group1)
         self._fs_cmd("subvolumegroup", "create", self.volname, group2, f"--mode={expected_mode2}")
+        self._fs_cmd("subvolumegroup", "create", self.volname, group1)
 
         group1_path = self._get_subvolume_group_path(self.volname, group1)
         group2_path = self._get_subvolume_group_path(self.volname, group2)
+        volumes_path = os.path.dirname(group1_path)
 
         # check group's mode
         actual_mode1 = self.mount_a.run_shell(['stat', '-c' '%a', group1_path]).stdout.getvalue().strip()
         actual_mode2 = self.mount_a.run_shell(['stat', '-c' '%a', group2_path]).stdout.getvalue().strip()
+        actual_mode3 = self.mount_a.run_shell(['stat', '-c' '%a', volumes_path]).stdout.getvalue().strip()
         self.assertEqual(actual_mode1, expected_mode1)
         self.assertEqual(actual_mode2, expected_mode2)
+        self.assertEqual(actual_mode3, expected_mode1)
 
         self._fs_cmd("subvolumegroup", "rm", self.volname, group1)
         self._fs_cmd("subvolumegroup", "rm", self.volname, group2)
@@ -921,6 +924,36 @@ class TestSubvolumes(TestVolumesHelper):
         # verify trash dir is clean
         self._wait_for_trash_empty()
 
+    def test_subvolume_create_with_desired_mode(self):
+        subvol1 = self._generate_random_subvolume_name()
+
+        # default mode
+        default_mode = "755"
+        # desired mode
+        desired_mode = "777"
+
+        self._fs_cmd("subvolume", "create", self.volname, subvol1,  "--mode", "777")
+
+        subvol1_path = self._get_subvolume_path(self.volname, subvol1)
+
+        # check subvolumegroup's mode
+        subvol_par_path = os.path.dirname(subvol1_path)
+        group_path = os.path.dirname(subvol_par_path)
+        actual_mode1 = self.mount_a.run_shell(['stat', '-c' '%a', group_path]).stdout.getvalue().strip()
+        self.assertEqual(actual_mode1, default_mode)
+        # check /volumes mode
+        volumes_path = os.path.dirname(group_path)
+        actual_mode2 = self.mount_a.run_shell(['stat', '-c' '%a', volumes_path]).stdout.getvalue().strip()
+        self.assertEqual(actual_mode2, default_mode)
+        # check subvolume's  mode
+        actual_mode3 = self.mount_a.run_shell(['stat', '-c' '%a', subvol1_path]).stdout.getvalue().strip()
+        self.assertEqual(actual_mode3, desired_mode)
+
+        self._fs_cmd("subvolume", "rm", self.volname, subvol1)
+
+        # verify trash dir is clean
+        self._wait_for_trash_empty()
+
     def test_subvolume_create_with_desired_mode_in_group(self):
         subvol1, subvol2, subvol3 = self._generate_random_subvolume_name(3)
 

diff --git a/src/pybind/mgr/volumes/fs/fs_util.py b/src/pybind/mgr/volumes/fs/fs_util.py
index 366375021a8d4ec7d1112a194fd6f3c3d08a8de4..3d098ddb0588e8b998b2e36b84dd02a3f0a9f231 100644 (file)
--- a/src/pybind/mgr/volumes/fs/fs_util.py
+++ b/src/pybind/mgr/volumes/fs/fs_util.py
@@ -161,3 +161,15 @@ def get_ancestor_xattr(fs, path, attr):
             raise VolumeException(-e.args[0], e.args[1])
         else:
             return get_ancestor_xattr(fs, os.path.split(path)[0], attr)
+
+def create_base_dir(fs, path, mode):
+    """
+    Create volspec base/group directory if it doesn't exist
+    """
+    try:
+        fs.stat(path)
+    except cephfs.Error as e:
+        if e.args[0] == errno.ENOENT:
+            fs.mkdirs(path, mode)
+        else:
+            raise VolumeException(-e.args[0], e.args[1])

diff --git a/src/pybind/mgr/volumes/fs/operations/group.py b/src/pybind/mgr/volumes/fs/operations/group.py
index a705a1892e94dbe2cae6252b84f35f5a1d0def2c..152383c51d43a3e1ae3ccf4b080d7b4b285deebf 100644 (file)
--- a/src/pybind/mgr/volumes/fs/operations/group.py
+++ b/src/pybind/mgr/volumes/fs/operations/group.py
@@ -8,7 +8,7 @@ import cephfs
 from .snapshot_util import mksnap, rmsnap
 from .pin_util import pin
 from .template import GroupTemplate
-from ..fs_util import listdir, listsnaps, get_ancestor_xattr
+from ..fs_util import listdir, listsnaps, get_ancestor_xattr, create_base_dir
 from ..exception import VolumeException
 
 log = logging.getLogger(__name__)
@@ -104,7 +104,11 @@ def create_group(fs, vol_spec, groupname, pool, mode, uid, gid):
     """
     group = Group(fs, vol_spec, groupname)
     path = group.path
-    fs.mkdirs(path, mode)
+    vol_spec_base_dir = group.vol_spec.base_dir.encode('utf-8')
+
+    # create vol_spec base directory with default mode(0o755) if it doesn't exist
+    create_base_dir(fs, vol_spec_base_dir, vol_spec.DEFAULT_MODE)
+    fs.mkdir(path, mode)
     try:
         if not pool:
             pool = get_ancestor_xattr(fs, path, "ceph.dir.layout.pool")

diff --git a/src/pybind/mgr/volumes/fs/operations/versions/subvolume_v1.py b/src/pybind/mgr/volumes/fs/operations/versions/subvolume_v1.py
index 34f151c454ff700600902344821f3871176b82ee..d62effd995bd9c4909adc853aed445d00f2d2a6b 100644 (file)
--- a/src/pybind/mgr/volumes/fs/operations/versions/subvolume_v1.py
+++ b/src/pybind/mgr/volumes/fs/operations/versions/subvolume_v1.py
@@ -18,7 +18,7 @@ from ..template import SubvolumeTemplate
 from ..snapshot_util import mksnap, rmsnap
 from ..access import allow_access, deny_access
 from ...exception import IndexException, OpSmException, VolumeException, MetadataMgrException, EvictionError
-from ...fs_util import listsnaps, is_inherited_snap
+from ...fs_util import listsnaps, is_inherited_snap, create_base_dir
 from ..template import SubvolumeOpType
 from ..group import Group
 from ..rankevicter import RankEvicter
@@ -93,6 +93,8 @@ class SubvolumeV1(SubvolumeBase, SubvolumeTemplate):
 
         subvol_path = os.path.join(self.base_path, str(uuid.uuid4()).encode('utf-8'))
         try:
+            # create group directory with default mode(0o755) if it doesn't exist.
+            create_base_dir(self.fs, self.group.path, self.vol_spec.DEFAULT_MODE)
             # create directory and set attributes
             self.fs.mkdirs(subvol_path, mode)
             self.mark_subvolume()

diff --git a/src/pybind/mgr/volumes/fs/operations/versions/subvolume_v2.py b/src/pybind/mgr/volumes/fs/operations/versions/subvolume_v2.py
index ca92279b5c98291d3d1151b4714c1f5587325488..3937daba61f660f4159e0bae39e6ef4ff02efbf6 100644 (file)
--- a/src/pybind/mgr/volumes/fs/operations/versions/subvolume_v2.py
+++ b/src/pybind/mgr/volumes/fs/operations/versions/subvolume_v2.py
@@ -12,7 +12,7 @@ from .op_sm import SubvolumeOpSm
 from .subvolume_v1 import SubvolumeV1
 from ..template import SubvolumeTemplate
 from ...exception import OpSmException, VolumeException, MetadataMgrException
-from ...fs_util import listdir
+from ...fs_util import listdir, create_base_dir
 from ..template import SubvolumeOpType
 
 log = logging.getLogger(__name__)
@@ -167,6 +167,8 @@ class SubvolumeV2(SubvolumeV1):
             raise VolumeException(-errno.EAGAIN, "asynchronous purge of subvolume in progress")
         subvol_path = os.path.join(self.base_path, str(uuid.uuid4()).encode('utf-8'))
         try:
+            # create group directory with default mode(0o755) if it doesn't exist.
+            create_base_dir(self.fs, self.group.path, self.vol_spec.DEFAULT_MODE)
             self.fs.mkdirs(subvol_path, mode)
             self.mark_subvolume()
             attrs = {

diff --git a/src/pybind/mgr/volumes/fs/vol_spec.py b/src/pybind/mgr/volumes/fs/vol_spec.py
index e18ab06906279488a7879204aa707cb89faa7753..5ff983115c3c1b81e227e3b922767ee4bfcb2e32 100644 (file)
--- a/src/pybind/mgr/volumes/fs/vol_spec.py
+++ b/src/pybind/mgr/volumes/fs/vol_spec.py
@@ -9,6 +9,8 @@ class VolSpec(object):
     DEFAULT_SUBVOL_PREFIX = "/volumes"
     # and the default namespace
     DEFAULT_NS_PREFIX = "fsvolumens_"
+    # default mode for subvol prefix and group
+    DEFAULT_MODE = 0o755
 
     def __init__(self, snapshot_prefix, subvolume_prefix=None, pool_ns_prefix=None):
         self.snapshot_prefix = snapshot_prefix