mgr/snap_schedule: validate fs before execution

Stop command execution if there are more than one filesystem and --fs
argument is missing in command-line, i.e. do not use the first fs in the
fsmap if there are more than one filesystem.
This is to ensure that user doesn't mistakenly run the command against
the first fs by missing to specify the desired fs.

Fixes: https://tracker.ceph.com/issues/62218
Signed-off-by: Milind Changire <mchangir@redhat.com>
(cherry picked from commit c612ce06300fa99f750ce99ef68dbd49ced16c34)

diff --git a/src/pybind/mgr/snap_schedule/module.py b/src/pybind/mgr/snap_schedule/module.py
index 572e26815c00e057c85db9bf2a52b2d84b85cb50..0d565fd92f452c17a4ba840bbc59237b4e1a4d5f 100644 (file)
--- a/src/pybind/mgr/snap_schedule/module.py
+++ b/src/pybind/mgr/snap_schedule/module.py
@@ -38,14 +38,24 @@ class Module(MgrModule):
         self.client = SnapSchedClient(self)
 
     @property
-    def default_fs(self) -> str:
+    def _default_fs(self) -> Tuple[int, str, str]:
         fs_map = self.get('fs_map')
-        if fs_map['filesystems']:
-            return fs_map['filesystems'][0]['mdsmap']['fs_name']
+        if len(fs_map['filesystems']) > 1:
+            return -errno.EINVAL, '', "filesystem argument is required when there is more than one file system"
+        elif len(fs_map['filesystems']) == 1:
+            return 0, fs_map['filesystems'][0]['mdsmap']['fs_name'], "Success"
         else:
             self.log.error('No filesystem instance could be found.')
-            raise CephfsConnectionException(
-                -errno.ENOENT, "no filesystem found")
+            return -errno.ENOENT, "", "no filesystem found"
+
+    def _validate_fs(self, fs: Optional[str]) -> Tuple[int, str, str]:
+        if not fs:
+            rc, fs, err = self._default_fs
+            if rc < 0:
+                return rc, fs, err
+        if not self.has_fs(fs):
+            return -errno.EINVAL, '', f"no such file system: {fs}"
+        return 0, fs, 'Success'
 
     def has_fs(self, fs_name: str) -> bool:
         return fs_name in self.client.get_all_filesystems()
@@ -65,11 +75,11 @@ class Module(MgrModule):
         '''
         List current snapshot schedules
         '''
-        use_fs = fs if fs else self.default_fs
-        if not self.has_fs(use_fs):
-            return -errno.EINVAL, '', f"no such filesystem: {use_fs}"
+        rc, fs, err = self._validate_fs(fs)
+        if rc < 0:
+            return rc, fs, err
         try:
-            ret_scheds = self.client.get_snap_schedules(use_fs, path)
+            ret_scheds = self.client.get_snap_schedules(fs, path)
         except CephfsConnectionException as e:
             return e.to_tuple()
         if format == 'json':
@@ -86,10 +96,10 @@ class Module(MgrModule):
         Get current snapshot schedule for <path>
         '''
         try:
-            use_fs = fs if fs else self.default_fs
-            if not self.has_fs(use_fs):
-                return -errno.EINVAL, '', f"no such filesystem: {use_fs}"
-            scheds = self.client.list_snap_schedules(use_fs, path, recursive)
+            rc, fs, err = self._validate_fs(fs)
+            if rc < 0:
+                return rc, fs, err
+            scheds = self.client.list_snap_schedules(fs, path, recursive)
             self.log.debug(f'recursive is {recursive}')
         except CephfsConnectionException as e:
             return e.to_tuple()
@@ -116,18 +126,18 @@ class Module(MgrModule):
         Set a snapshot schedule for <path>
         '''
         try:
-            use_fs = fs if fs else self.default_fs
-            if not self.has_fs(use_fs):
-                return -errno.EINVAL, '', f"no such filesystem: {use_fs}"
+            rc, fs, err = self._validate_fs(fs)
+            if rc < 0:
+                return rc, fs, err
             abs_path = path
             subvol = None
-            self.client.store_snap_schedule(use_fs,
+            self.client.store_snap_schedule(fs,
                                             abs_path,
                                             (abs_path, snap_schedule,
-                                             use_fs, path, start, subvol))
+                                             fs, path, start, subvol))
             suc_msg = f'Schedule set for path {path}'
         except sqlite3.IntegrityError:
-            existing_scheds = self.client.get_snap_schedules(use_fs, path)
+            existing_scheds = self.client.get_snap_schedules(fs, path)
             report = [s.report() for s in existing_scheds]
             error_msg = f'Found existing schedule {report}'
             self.log.error(error_msg)
@@ -148,15 +158,15 @@ class Module(MgrModule):
         Remove a snapshot schedule for <path>
         '''
         try:
-            use_fs = fs if fs else self.default_fs
-            if not self.has_fs(use_fs):
-                return -errno.EINVAL, '', f"no such filesystem: {use_fs}"
+            rc, fs, err = self._validate_fs(fs)
+            if rc < 0:
+                return rc, fs, err
             abs_path = path
-            self.client.rm_snap_schedule(use_fs, abs_path, repeat, start)
-        except CephfsConnectionException as e:
-            return e.to_tuple()
+            self.client.rm_snap_schedule(fs, abs_path, repeat, start)
         except ValueError as e:
             return -errno.ENOENT, '', str(e)
+        except CephfsConnectionException as e:
+            return e.to_tuple()
         return 0, 'Schedule removed for path {}'.format(path), ''
 
     @CLIWriteCommand('fs snap-schedule retention add')
@@ -169,17 +179,17 @@ class Module(MgrModule):
         Set a retention specification for <path>
         '''
         try:
-            use_fs = fs if fs else self.default_fs
-            if not self.has_fs(use_fs):
-                return -errno.EINVAL, '', f"no such filesystem: {use_fs}"
+            rc, fs, err = self._validate_fs(fs)
+            if rc < 0:
+                return rc, fs, err
             abs_path = path
-            self.client.add_retention_spec(use_fs, abs_path,
-                                          retention_spec_or_period,
-                                          retention_count)
-        except CephfsConnectionException as e:
-            return e.to_tuple()
+            self.client.add_retention_spec(fs, abs_path,
+                                           retention_spec_or_period,
+                                           retention_count)
         except ValueError as e:
             return -errno.ENOENT, '', str(e)
+        except CephfsConnectionException as e:
+            return e.to_tuple()
         return 0, 'Retention added to path {}'.format(path), ''
 
     @CLIWriteCommand('fs snap-schedule retention remove')
@@ -192,11 +202,11 @@ class Module(MgrModule):
         Remove a retention specification for <path>
         '''
         try:
-            use_fs = fs if fs else self.default_fs
-            if not self.has_fs(use_fs):
-                return -errno.EINVAL, '', f"no such filesystem: {use_fs}"
+            rc, fs, err = self._validate_fs(fs)
+            if rc < 0:
+                return rc, fs, err
             abs_path = path
-            self.client.rm_retention_spec(use_fs, abs_path,
+            self.client.rm_retention_spec(fs, abs_path,
                                           retention_spec_or_period,
                                           retention_count)
         except CephfsConnectionException as e:
@@ -215,15 +225,15 @@ class Module(MgrModule):
         Activate a snapshot schedule for <path>
         '''
         try:
-            use_fs = fs if fs else self.default_fs
-            if not self.has_fs(use_fs):
-                return -errno.EINVAL, '', f"no such filesystem: {use_fs}"
+            rc, fs, err = self._validate_fs(fs)
+            if rc < 0:
+                return rc, fs, err
             abs_path = path
-            self.client.activate_snap_schedule(use_fs, abs_path, repeat, start)
-        except CephfsConnectionException as e:
-            return e.to_tuple()
+            self.client.activate_snap_schedule(fs, abs_path, repeat, start)
         except ValueError as e:
             return -errno.ENOENT, '', str(e)
+        except CephfsConnectionException as e:
+            return e.to_tuple()
         return 0, 'Schedule activated for path {}'.format(path), ''
 
     @CLIWriteCommand('fs snap-schedule deactivate')
@@ -236,13 +246,13 @@ class Module(MgrModule):
         Deactivate a snapshot schedule for <path>
         '''
         try:
-            use_fs = fs if fs else self.default_fs
-            if not self.has_fs(use_fs):
-                return -errno.EINVAL, '', f"no such filesystem: {use_fs}"
+            rc, fs, err = self._validate_fs(fs)
+            if rc < 0:
+                return rc, fs, err
             abs_path = path
-            self.client.deactivate_snap_schedule(use_fs, abs_path, repeat, start)
-        except CephfsConnectionException as e:
-            return e.to_tuple()
+            self.client.deactivate_snap_schedule(fs, abs_path, repeat, start)
         except ValueError as e:
             return -errno.ENOENT, '', str(e)
+        except CephfsConnectionException as e:
+            return e.to_tuple()
         return 0, 'Schedule deactivated for path {}'.format(path), ''