From 74c4d5f9b89fdfaf1fcf540acfbe445a4fb33db3 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 | 108 ++++++++++++++----------- 1 file changed, 59 insertions(+), 49 deletions(-) diff --git a/src/pybind/mgr/snap_schedule/module.py b/src/pybind/mgr/snap_schedule/module.py index 572e26815c00..0d565fd92f45 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() if format == 'json': @@ -86,10 +96,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() @@ -116,18 +126,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) @@ -148,15 +158,15 @@ 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) - 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 ''' 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 ''' 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 ''' 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 ''' 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), '' -- 2.47.3