From bec51645551c74e808f609affaba45d7f6af5e08 Mon Sep 17 00:00:00 2001 From: Milind Changire Date: Wed, 9 Aug 2023 16:50:40 +0530 Subject: [PATCH] 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 (cherry picked from commit c612ce06300fa99f750ce99ef68dbd49ced16c34) --- src/pybind/mgr/snap_schedule/module.py | 92 ++++++++++++++------------ 1 file changed, 51 insertions(+), 41 deletions(-) diff --git a/src/pybind/mgr/snap_schedule/module.py b/src/pybind/mgr/snap_schedule/module.py index 4af4283604f3c..b66ebad2192a7 100644 --- 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 ''' 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 ''' 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 ''' 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 ''' 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 ''' 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 ''' 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 ''' 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: -- 2.39.5