Revert "mgr/volumes: handle bad arguments during subvolume create"

PR https://github.com/ceph/ceph/pull/53989 is causing failures in
fs:upgrade. Also, @VallariAg reported an issue with something
similar. I don't think adequate tests were run to qualify the PR
as mergeable. Reverting the change for now.

Signed-off-by: Venky Shankar <vshankar@redhat.com>

diff --git a/doc/_ext/ceph_commands.py b/doc/_ext/ceph_commands.py
index 682abe327e464bcf8c7331036a0310923b3202e2..d96eab08853f42ada63cfcd909bf8c4783eb80d9 100644 (file)
--- a/doc/_ext/ceph_commands.py
+++ b/doc/_ext/ceph_commands.py
@@ -83,7 +83,7 @@ class CmdParam(object):
 
     def __init__(self, type, name,
                  who=None, n=None, req=True, range=None, strings=None,
-                 goodchars=None, positional=True, **kwargs):
+                 goodchars=None, positional=True):
         self.type = type
         self.name = name
         self.who = who
@@ -93,7 +93,6 @@ class CmdParam(object):
         self.strings = strings.split('|') if strings else []
         self.goodchars = goodchars
         self.positional = positional != 'false'
-        self.allowempty = kwargs.pop('allowempty', True) in (True, 'True', 'true')
 
         assert who is None
 
@@ -109,8 +108,6 @@ class CmdParam(object):
             advanced.append('goodchars= ``{}`` '.format(self.goodchars))
         if self.n:
             advanced.append('(can be repeated)')
-        if self.allowempty:
-            advanced.append('(can be empty string)')
 
         advanced = advanced or ["(string)"]
         return ' '.join(advanced)

diff --git a/qa/suites/fs/volumes/tasks/volumes/test/basic.yaml b/qa/suites/fs/volumes/tasks/volumes/test/basic.yaml
index 4e557bfb32531e8a87aa3294e7b86d665576cee6..49783bcc03bf8e83f7e08b09a6d55418dadc6f99 100644 (file)
--- a/qa/suites/fs/volumes/tasks/volumes/test/basic.yaml
+++ b/qa/suites/fs/volumes/tasks/volumes/test/basic.yaml
@@ -6,5 +6,4 @@ tasks:
         - tasks.cephfs.test_volumes.TestVolumeCreate
         - tasks.cephfs.test_volumes.TestSubvolumeGroups
         - tasks.cephfs.test_volumes.TestSubvolumes
-        - tasks.cephfs.test_volumes.TestEmptyStringForCreates
         - tasks.cephfs.test_subvolume

diff --git a/qa/tasks/cephfs/filesystem.py b/qa/tasks/cephfs/filesystem.py
index f628748346e2bf99f658289d1d2bbdcc743ce899..1533df52cdfe19000530f57749e2f8df5baebc3c 100644 (file)
--- a/qa/tasks/cephfs/filesystem.py
+++ b/qa/tasks/cephfs/filesystem.py
@@ -834,7 +834,7 @@ class FilesystemBase(MDSClusterBase):
                 assert(subvols['create'] > 0)
 
                 self.run_ceph_cmd('fs', 'subvolumegroup', 'create', self.name, 'qa')
-                subvol_options = self.fs_config.get('subvol_options', None)
+                subvol_options = self.fs_config.get('subvol_options', '')
 
                 for sv in range(0, subvols['create']):
                     sv_name = f'sv_{sv}'

diff --git a/qa/tasks/cephfs/test_volumes.py b/qa/tasks/cephfs/test_volumes.py
index 108dd34cad00ac144ec36d7027852b39350b766f..59f18e0c3da4fab6f314f0a93d434f7cae967a79 100644 (file)
--- a/qa/tasks/cephfs/test_volumes.py
+++ b/qa/tasks/cephfs/test_volumes.py
@@ -9260,48 +9260,3 @@ class TestPerModuleFinsherThread(TestVolumesHelper):
 
         # verify trash dir is clean
         self._wait_for_trash_empty()
-
-class TestEmptyStringForCreates(CephFSTestCase):
-    CLIENTS_REQUIRED = 1
-    MDSS_REQUIRED = 1
-
-    def setUp(self):
-        super().setUp()
-        result = json.loads(self.get_ceph_cmd_stdout("fs", "volume", "ls"))
-        self.assertTrue(len(result) > 0)
-        self.volname = result[0]['name']
-
-    def tearDown(self):
-        # clean up
-        super().tearDown()
-
-    def test_empty_name_string_for_subvolumegroup_name(self):
-        """
-        To test that an empty string is unacceptable for a subvolumegroup name
-        """
-        with self.assertRaises(CommandFailedError):
-            self.run_ceph_cmd("fs", "subvolumegroup", "create", self.volname, "")
-
-        with self.assertRaises(CommandFailedError):
-            self.run_ceph_cmd("fs", "subvolumegroup", "create", self.volname, "''")
-
-    def test_empty_name_string_for_subvolume_name(self):
-        """
-        To test that an empty string is unacceptable for a subvolume name
-        """
-        with self.assertRaises(CommandFailedError):
-            self.run_ceph_cmd("fs", "subvolume", "create", "")
-
-        with self.assertRaises(CommandFailedError):
-            self.run_ceph_cmd("fs", "subvolume", "create", "''")
-
-    def test_empty_name_string_for_subvolumegroup_name_argument(self):
-        """
-        To test that an empty string is unacceptable for a subvolumegroup name
-        argument when creating a subvolume
-        """
-        with self.assertRaises(CommandFailedError):
-            self.run_ceph_cmd("fs", "subvolume", "create", self.volname, "sv1", "--group_name")
-
-        with self.assertRaises(CommandFailedError):
-            self.run_ceph_cmd("fs", "subvolume", "create", self.volname, "sv1", "--group_name", "''")

diff --git a/src/pybind/ceph_argparse.py b/src/pybind/ceph_argparse.py
index 6ea47b3c6b4e81a14e24429bbf6855dc1cad052d..a2792c12e9523c2ad407a05a2bd5eecf7ea96b19 100644 (file)
--- a/src/pybind/ceph_argparse.py
+++ b/src/pybind/ceph_argparse.py
@@ -327,7 +327,7 @@ class CephString(CephArgtype):
     """
     String; pretty generic.  goodchars is a RE char class of valid chars
     """
-    def __init__(self, goodchars='', allowempty=True):
+    def __init__(self, goodchars=''):
         from string import printable
         try:
             re.compile(goodchars)
@@ -338,12 +338,8 @@ class CephString(CephArgtype):
         self.goodset = frozenset(
             [c for c in printable if re.match(goodchars, c)]
         )
-        self.allowempty = allowempty in (True, 'True', 'true')
 
     def valid(self, s: str, partial: bool = False) -> None:
-        if not self.allowempty and s == "":
-            raise ArgumentFormat("argument can't be an empty string")
-
         sset = set(s)
         if self.goodset and not sset <= self.goodset:
             raise ArgumentFormat("invalid chars {0} in {1}".
@@ -354,7 +350,6 @@ class CephString(CephArgtype):
         b = ''
         if self.goodchars:
             b += '(goodchars {0})'.format(self.goodchars)
-        b += f'(allowempty {self.allowempty})'
         return '<string{0}>'.format(b)
 
     def complete(self, s) -> List[str]:
@@ -366,7 +361,6 @@ class CephString(CephArgtype):
     def argdesc(self, attrs):
         if self.goodchars:
             attrs['goodchars'] = self.goodchars
-        attrs['allowempty'] = repr(self.allowempty)
         return super().argdesc(attrs)
 
 

diff --git a/src/pybind/mgr/volumes/module.py b/src/pybind/mgr/volumes/module.py
index 67c7f8804bdeee601af5c4c441ed0ed5c18cac90..e91c6bb504ce50f60b6d90fed0aa49ba07354860 100644 (file)
--- a/src/pybind/mgr/volumes/module.py
+++ b/src/pybind/mgr/volumes/module.py
@@ -86,7 +86,7 @@ class Module(orchestrator.OrchestratorClientMixin, MgrModule):
         {
             'cmd': 'fs subvolumegroup create '
                    'name=vol_name,type=CephString '
-                   f'name=group_name,type=CephString,goodchars={goodchars},allowempty=false '
+                   f'name=group_name,type=CephString,goodchars={goodchars} '
                    'name=size,type=CephInt,req=false '
                    'name=pool_layout,type=CephString,req=false '
                    'name=uid,type=CephInt,req=false '
@@ -136,9 +136,9 @@ class Module(orchestrator.OrchestratorClientMixin, MgrModule):
         {
             'cmd': 'fs subvolume create '
                    'name=vol_name,type=CephString '
-                   f'name=sub_name,type=CephString,goodchars={goodchars},allowempty=false '
+                   f'name=sub_name,type=CephString,goodchars={goodchars} '
                    'name=size,type=CephInt,req=false '
-                   f'name=group_name,type=CephString,req=false,goodchars={goodchars},allowempty=false '
+                   'name=group_name,type=CephString,req=false '
                    'name=pool_layout,type=CephString,req=false '
                    'name=uid,type=CephInt,req=false '
                    'name=gid,type=CephInt,req=false '