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 4af4283604f3c967ba9fe0ac6c51e262d20369a3..b66ebad2192a7872d5ff95970d2eec91b06ccacc 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()
         except Exception as e:
@@ -88,10 +98,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()
@@ -120,18 +130,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)
@@ -154,11 +164,11 @@ 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)
+            self.client.rm_snap_schedule(fs, abs_path, repeat, start)
         except ValueError as e:
             return -errno.ENOENT, '', str(e)
         except CephfsConnectionException as e:
@@ -177,13 +187,13 @@ 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)
+            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:
@@ -202,11 +212,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 ValueError as e:
@@ -227,11 +237,11 @@ 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)
+            self.client.activate_snap_schedule(fs, abs_path, repeat, start)
         except ValueError as e:
             return -errno.ENOENT, '', str(e)
         except CephfsConnectionException as e:
@@ -250,11 +260,11 @@ 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)
+            self.client.deactivate_snap_schedule(fs, abs_path, repeat, start)
         except ValueError as e:
             return -errno.ENOENT, '', str(e)
         except CephfsConnectionException as e: