From: Milind Changire Date: Wed, 9 Aug 2023 11:20:40 +0000 (+0530) Subject: mgr/snap_schedule: validate fs before execution X-Git-Tag: v19.0.0~588^2~4 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=c612ce06300fa99f750ce99ef68dbd49ced16c34;p=ceph.git 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 --- diff --git a/src/pybind/mgr/snap_schedule/module.py b/src/pybind/mgr/snap_schedule/module.py index 4af4283604f3..b66ebad2192a 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: