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 <mchangir@redhat.com>
(cherry picked from commit 2f6d87d1811b745c4efcac72c03ddb42751ad634)

diff --git a/src/pybind/mgr/snap_schedule/fs/schedule.py b/src/pybind/mgr/snap_schedule/fs/schedule.py
index 95e43b7e0ecd51a70982e5fef5ed259da1ccd7c2..3538fe501ddb2d2d8f95170f6d99462747175b69 100644 (file)
--- 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 28d54639a3e0ae913264ff6e5983a28e4845cd55..ec3d32fb44384a2b87251e2620574e31a6250197 100644 (file)
--- 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