From 2f6d87d1811b745c4efcac72c03ddb42751ad634 Mon Sep 17 00:00:00 2001 From: Milind Changire Date: Thu, 24 Aug 2023 07:47:46 +0530 Subject: [PATCH] mgr/snap_schedule: change minute duration specifier from M to m Problem: As per the issue tracker, the period spec specifier 'M' is not consistent with what is used elsewhere, like the period specifiers displayed in the 'ceph status' command output. The 'M' period specifier is used as a 'minute' level period specifier by the cephfs team. The issue reporter suggests to use 'M' as a 'month' period specifier. Solution: Since the 'minute' level period specifer, 'M', is used internally by the development team, it is failrly easy to swap the 'minute' ('M') level and 'month' ('m') level period specifers to finally mean that 'm' implies 'minute' level period and 'M' implies 'month' level period. Also, since this is the first time that somebody has ever reported that neither the 'M' nor the 'm' level specifiers work in production, it is a good idea to fix them once and for all. Fixes: https://tracker.ceph.com/issues/62494 Signed-off-by: Milind Changire --- src/pybind/mgr/snap_schedule/fs/schedule.py | 29 ++++++++++++++++--- .../mgr/snap_schedule/fs/schedule_client.py | 13 +++++---- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/src/pybind/mgr/snap_schedule/fs/schedule.py b/src/pybind/mgr/snap_schedule/fs/schedule.py index 95e43b7e0ecd5..3538fe501ddb2 100644 --- a/src/pybind/mgr/snap_schedule/fs/schedule.py +++ b/src/pybind/mgr/snap_schedule/fs/schedule.py @@ -65,7 +65,7 @@ def parse_retention(retention: str) -> Dict[str, int]: return ret -RETENTION_MULTIPLIERS = ['n', 'M', 'h', 'd', 'w', 'm', 'y'] +RETENTION_MULTIPLIERS = ['n', 'm', 'h', 'd', 'w', 'M', 'Y'] TableRowT = Dict[str, Union[int, str]] @@ -103,6 +103,11 @@ class Schedule(object): self.path = path self.rel_path = rel_path self.schedule = schedule + # test to see if period and spec are valid + # this test will throw a ValueError exception if + # period is negative or zero + # spec is empty or other than n,m,h,d,w,M,Y + rep = self.repeat self.retention = json.loads(retention_policy) if start is None: now = datetime.now(timezone.utc) @@ -396,8 +401,19 @@ class Schedule(object): @property def repeat(self) -> int: - period, mult = self.parse_schedule(self.schedule) - if mult == 'M': + period = -1 + mult = "" + try: + period, mult = self.parse_schedule(self.schedule) + except ValueError: + raise ValueError('invalid schedule specified - period should be ' + 'non-zero positive value and multiplier should ' + 'be one of h,d,w,M,Y e.g. 1h or 4d etc.') + if period <= 0: + raise ValueError('invalid schedule specified - period must be a ' + 'non-zero positive value e.g. 1h or 4d etc.') + # 'm' is only for developer testing of minute level snapshots + if mult == 'm': return period * 60 elif mult == 'h': return period * 60 * 60 @@ -405,8 +421,13 @@ class Schedule(object): return period * 60 * 60 * 24 elif mult == 'w': return period * 60 * 60 * 24 * 7 + elif mult == 'M': + return period * 60 * 60 * 24 * 30 + elif mult == 'Y': + return period * 60 * 60 * 24 * 365 else: - raise ValueError(f'schedule multiplier "{mult}" not recognized') + raise ValueError('invalid schedule specified - multiplier should ' + 'be one of h,d,w,M,Y') UPDATE_LAST = '''UPDATE schedules_meta SET diff --git a/src/pybind/mgr/snap_schedule/fs/schedule_client.py b/src/pybind/mgr/snap_schedule/fs/schedule_client.py index 28d54639a3e0a..ec3d32fb44384 100644 --- a/src/pybind/mgr/snap_schedule/fs/schedule_client.py +++ b/src/pybind/mgr/snap_schedule/fs/schedule_client.py @@ -79,12 +79,12 @@ def get_prune_set(candidates: Set[Tuple[cephfs.DirEntry, datetime]], # NOTE: prune set has tz suffix stripped out. ("n", SNAPSHOT_TS_FORMAT), # TODO remove M for release - ("M", '%Y-%m-%d-%H_%M'), + ("m", '%Y-%m-%d-%H_%M'), ("h", '%Y-%m-%d-%H'), ("d", '%Y-%m-%d'), ("w", '%G-%V'), - ("m", '%Y-%m'), - ("y", '%Y'), + ("M", '%Y-%m'), + ("Y", '%Y'), ]) keep = [] if not retention: @@ -212,7 +212,7 @@ class SnapSchedClient(CephfsClient): return DBConnectionManager(dbinfo) def _is_allowed_repeat(self, exec_row: Dict[str, str], path: str) -> bool: - if Schedule.parse_schedule(exec_row['schedule'])[1] == 'M': + if Schedule.parse_schedule(exec_row['schedule'])[1] == 'm': if self.allow_minute_snaps: log.debug(('Minute repeats allowed, ' f'scheduling snapshot on path {path}')) @@ -373,9 +373,10 @@ class SnapSchedClient(CephfsClient): Optional[str], Optional[str]]) -> None: sched = Schedule(*args) log.debug(f'repeat is {sched.repeat}') - if sched.parse_schedule(sched.schedule)[1] == 'M' and not self.allow_minute_snaps: + if sched.parse_schedule(sched.schedule)[1] == 'm' and not self.allow_minute_snaps: log.error('not allowed') - raise ValueError('no minute snaps allowed') + raise ValueError('invalid schedule specified - multiplier should ' + 'be one of h,d,w,M,Y') log.debug(f'attempting to add schedule {sched}') with self.get_schedule_db(fs) as conn_mgr: db = conn_mgr.dbinfo.db -- 2.39.5