@jan--f The following changes would be required to run these tests:

diff --git a/src/pybind/mgr/snap_schedule/module.py b/src/pybind/mgr/snap_schedule/module.py
index 79ea24b8c4..e39fd1e398 100644
--- a/src/pybind/mgr/snap_schedule/module.py
+++ b/src/pybind/mgr/snap_schedule/module.py
@@ -3,6 +3,7 @@ Copyright (C) 2019 SUSE
 
 LGPL2.1.  See file COPYING.
 """
+import json
 import errno
 import sqlite3
 from .fs.schedule import SnapSchedClient, Schedule
@@ -75,7 +76,7 @@ class Module(MgrModule):
             return e.to_tuple()
         if not scheds:
             return -1, '', f'SnapSchedule for {path} not found'
-        return 0, str([str(sched) for sched in scheds]), ''
+        return 0, json.dumps([[sched[1], sched[2]] for sched in scheds]), ''

and

diff --git a/src/pybind/mgr/snap_schedule/fs/schedule.py b/src/pybind/mgr/snap_schedule/fs/schedule.py
index 4f528f21cf..8ac4026c1d 100644
--- a/src/pybind/mgr/snap_schedule/fs/schedule.py
+++ b/src/pybind/mgr/snap_schedule/fs/schedule.py
@@ -71,7 +71,7 @@ class Schedule(object):
         self.last_run = None
 
     def __str__(self):
-        return f'''{self.rel_path}: {self.schedule}; {self.retention}'''
+        return f'''{self.path}: {self.schedule}; {self.retention}'''
 
     def report(self):
         import pprint
@@ -223,7 +223,7 @@ class SnapSchedClient(CephfsClient):
             log.info(f'finally branch')
             self.refresh_snap_timers(fs_name)
             log.info(f'calling prune')
-            self.prune_snapshots(fs_name, path, retention)
+            #self.prune_snapshots(fs_name, path, retention)
 
     def prune_snapshots(self, fs_name, path, retention):
         log.debug('Pruning snapshots')

@jan--f Note that in the diffs mentioned previously I had commented out the call to prune_snapshots() -- without this the snaps were getting purged every minute. I suspected the prune-every-minute test code that you had but taking out the purge-every-minute thing in get_prune_set() didn't work -- I think get_prune_set() was returning an empty set thereby deleting a snap that was just created.

Another interesting thing that I saw when running test_snap_schedule -- snippet from the manager log below:

2020-04-14T04:45:22.942-0400 7f5a2e9bc700  0 [snap_schedule DEBUG snap_schedule.fs.schedule] Will snapshot snap_test_dir13 in fs cephfs in 38s
2020-04-14T04:46:01.074-0400 7f5a0d83a700  0 [snap_schedule DEBUG snap_schedule.fs.schedule] Will snapshot snap_test_dir13 in fs cephfs in 119s

The test schedules a snap every 2 minutes, but the first snap is scheduled in 38 odd seconds for some reason (the test expects the snap to show up in about 98 seconds (1m + 38s)). Also, this happens once in a while (I checked if there were any stale snap schedules, but there were none). And similar with multiple snap schedules per path ( test_multi_snap_schedule test) as below:

2020-04-14T04:50:28.320-0400 7fe38b690700  0 [snap_schedule DEBUG snap_schedule.fs.schedule] Will snapshot snap_test_dir14 in fs cephfs in 92s
2020-04-14T04:50:28.808-0400 7fe38b690700  0 [snap_schedule DEBUG snap_schedule.fs.schedule] Will snapshot snap_test_dir14 in fs cephfs in 32s
2020-04-14T04:51:00.923-0400 7fe369ccd700  0 [snap_schedule DEBUG snap_schedule.fs.schedule] Will snapshot snap_test_dir14 in fs cephfs in 60s
2020-04-14T04:52:00.983-0400 7fe3654c4700  0 [snap_schedule DEBUG snap_schedule.fs.schedule] Will snapshot snap_test_dir14 in fs cephfs in 120s

The timings look a bit off to me...

thanks @vshankar I'll look into these issue. I haven't yet paid to close attention to the timings, mostly cause I still need to add proper support for start argument to delay snapshots till a given start time. Timezones are also still an open question. I'll bring this up on our next standup.

The mentioned issues should be fixed in #29489

The mentioned issues should be fixed in #29489

will take a look

@jan--f -- test_multi_snap_schedule still fails with the latest push for PR #29489

The test schedules multiple snaps schedules (2m and 3m) on a directory, but the snaps get scheduled every 2m.

2020-04-23T09:05:44.791-0400 7f205d2ed700  0 [snap_schedule DEBUG snap_schedule.fs.schedule] Will snapshot snap_test_dir1 in fs cephfs in 120s
2020-04-23T09:05:45.259-0400 7f205d2ed700  0 [snap_schedule DEBUG snap_schedule.fs.schedule] Will snapshot snap_test_dir1 in fs cephfs in 119s

Is 'ceph mgr module enable snap_schedule' supported already in master branch? I tried the command, it return -ENOENT

@ukernel not yet, the module PR is #29489

does the snap_schedule module remove old snapshots. If yes, it's better to add a test case

does the snap_schedule module remove old snapshots. If yes, it's better to add a test case

It does indeed and I'll add tests for that. I'm in the final stages for the mgr module and will add more tests once this is stabilized.

@jan--f please use the latest update from this for the tests...

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

Let's rebase. @vshankar please run this test through teuthology when ready and post the results.

ack

What's the motivation for storing the allow_minute_snaps flag on a per schedule basis in the DB?

Looking at the changes, I don't seem to recall with acuracy for reason to store it in the db.

I think we shouldn't store this in the DB, as that would make for a strange UI. Say a user unsets the option for the mgr module but has schedules in the DB with this option. Keeping this as a runtime module option makes more sense to me. We'd need to add some better error handling for the case of minute granularity snapshots in the DB and the option turned off. But I think just skipping those schedules plus logging it should be least surprising?

Probably. However, I won't be able to get to looking at it (and fixing) this week. Sorry for that! I'll try to get to it next week. In case you want to give it a shot, that'll be great (since this blocks the octopus backport).

@vshankar I have my proposed changes up at https://github.com/jan--f/ceph/tree/wip-snap-sched-tests

Maybe we can re-use this PR to keep the discussion here?

@vshankar I have my proposed changes up at https://github.com/jan--f/ceph/tree/wip-snap-sched-tests

I'll take a look. Thx!

Maybe we can re-use this PR to keep the discussion here?

Sure.

@vshankar I have my proposed changes up at https://github.com/jan--f/ceph/tree/wip-snap-sched-tests

Look OK. I'll test out the changes tomorrow.

Look OK. I'll test out the changes tomorrow.

tests run fine. I will pull your changes in this PR and post the test results.

We should be able to get this merged pretty soon.

jenkins test make check

@jan--f PR updated with changes from branch https://github.com/jan--f/ceph/tree/wip-snap-sched-tests

test run: https://pulpito.ceph.com/vshankar-2020-11-04_06:34:51-fs-wip-cephfs-snap-sched-20201103-232717-distro-basic-smithi/

jenkins test make check

@batrick @jan--f make check fixed. please take a look

@vshankar this needs rebased. fs:basic_functional no longer exists.

@vshankar this needs rebased. fs:basic_functional no longer exists.

ack -- I'll push an update.

jenkins test api

@vshankar please run through QA for the snap schedule tests whenever you're ready.

@vshankar please run through QA for the snap schedule tests whenever you're ready.

sure -- will post the link.

@batrick https://pulpito.ceph.com/vshankar-2020-11-18_04:01:41-fs-wip-snap-schedule-20201117-012020-distro-basic-smithi/