Skip to content

mgr/rbd_support: Fix "start-time" arg behavior#66735

Merged
idryomov merged 1 commit intoceph:mainfrom
ajarr:wip-fix-schedule-start-time
Feb 21, 2026
Merged

mgr/rbd_support: Fix "start-time" arg behavior#66735
idryomov merged 1 commit intoceph:mainfrom
ajarr:wip-fix-schedule-start-time

Conversation

@ajarr
Copy link
Copy Markdown
Contributor

@ajarr ajarr commented Dec 24, 2025

The "start-time" argument, optionally passed when adding or removing an
image snapshot schedule or a trash purge schedule, does not behave as
intended. It is meant to schedule an initial operation at a specific
time of day in a given time zone. Instead, it offsets the schedule’s
anchor time. By default, the scheduler uses the UNIX epoch as the anchor
to calculate recurring schedule times, and "start-time" simply shifts
this anchor away from UTC, which can confuse users. For example:

$ # current time
$ date --universal
Wed Dec 10 05:55:21 PM UTC 2025
$ rbd mirror snapshot schedule add -p data --image img1 1h 19:00Z
$ rbd mirror snapshot schedule ls -p data --image img1
every 15m starting at 19:00:00+00:00

A user might assume that the scheduler will run the first snapshot each
day at 19:00 UTC and then run snapshots every 15 minutes. Instead, the
scheduler runs the first snapshot at 18:00 UTC and then continues at the
configured interval:

$ rbd mirror snapshot schedule status -p data --image img1
SCHEDULE TIME        IMAGE
2025-12-10 18:00:00  data/img1

Additionally, the "start-time" argument accepts a full ISO 8601
timestamp but silently ignores everything except hour, minute, and time
zone. Even time zone handling is incorrect: specifying "23:00-01:00"
with an interval of "1d" results in a snapshot taken once per day at
22:00 UTC rather than 00:00 UTC, because only utcoffset.seconds is used
while utcoffset.days is ignored.

Fix:
Similar to the handling of the "start" argument in the FS snap-schedule
manager module, require "start-time" to use an ISO 8601 date-time format
with a mandatory date component. Time and time zone are optional and
default to 00:00 and UTC respectively.

The "start-time" now defines the anchor time used to compute recurring
schedule times. The default anchor remains the UNIX epoch. Existing
on-disk schedules with legacy-format "start-time" values are updated to
include the date Jan 1, 1970.

The snap schedule ls output now displays "start-time" with date and
time in the format "%Y-%m-%d %H:%M:00". The display time is in UTC.

Fixes: https://tracker.ceph.com/issues/74192
Signed-off-by: Ramana Raja rraja@redhat.com

Contribution Guidelines

  • To sign and title your commits, please refer to Submitting Patches to Ceph.

  • If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

  • When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands

You must only issue one Jenkins command per-comment. Jenkins does not understand
comments with more than one command.

@ajarr ajarr requested a review from a team as a code owner December 24, 2025 13:02
@ajarr ajarr marked this pull request as draft December 24, 2025 13:02
@ajarr ajarr force-pushed the wip-fix-schedule-start-time branch 2 times, most recently from af6bf53 to 2fcdbb4 Compare December 25, 2025 17:28
@ajarr
Copy link
Copy Markdown
Contributor Author

ajarr commented Dec 25, 2025

TODO

  • Add negative tests
  • Add more details in the commit message
  • Add more details in the documentation

@ajarr ajarr force-pushed the wip-fix-schedule-start-time branch 2 times, most recently from b490657 to fb02d8c Compare December 28, 2025 04:35
@ajarr ajarr marked this pull request as ready for review December 28, 2025 04:50
@ajarr ajarr requested a review from a team as a code owner December 28, 2025 04:50
Comment thread doc/rbd/rbd-mirroring.rst Outdated
@ajarr ajarr force-pushed the wip-fix-schedule-start-time branch from fb02d8c to 130ae59 Compare December 28, 2025 22:07
@ajarr ajarr requested a review from anthonyeleven December 28, 2025 22:08
@anthonyeleven
Copy link
Copy Markdown
Contributor

Docs lgtm, others will have to approve the code.
I'll request, though, that you take out the single quotes from the PR and commit messages, as those have caused problems with various tools in the past.

@ajarr
Copy link
Copy Markdown
Contributor Author

ajarr commented Dec 29, 2025

can change the single quotes to double quotes in the commit message.

I'm suggesting taking them out completely, especially in the PR title.

And in the code, I've preserved the existing style in the python and shell scripts, e.g., python dict keys in single quotes and log messages in double quotes. Are you referring to the single quotes in this code comment,

Only the PR and commit messages.

Would you know what tools get tripped by single quotes?

I’ve seen issues with tools that compile, measure etc. commits and PRs, eg for ranked contributor lists for major releases, release note change logs etc. Quotes there were seen to mess up formatting. Back when we had shirts, the comma-separated list of contributors on the back sometimes had empty entries and other artifacts.

This is a request not a demand.

@ajarr
Copy link
Copy Markdown
Contributor Author

ajarr commented Dec 29, 2025

can change the single quotes to double quotes in the commit message.

I'm suggesting taking them out completely, especially in the PR title.

And in the code, I've preserved the existing style in the python and shell scripts, e.g., python dict keys in single quotes and log messages in double quotes. Are you referring to the single quotes in this code comment,

Only the PR and commit messages.

Would you know what tools get tripped by single quotes?

I’ve seen issues with tools that compile, measure etc. commits and PRs, eg for ranked contributor lists for major releases, release note change logs etc. Quotes there were seen to mess up formatting. Back when we had shirts, the comma-separated list of contributors on the back sometimes had empty entries and other artifacts.

This is a request not a demand.

@anthonyeleven looks like you've edited my comment instead of replying to it. Thanks for the info.

@ajarr ajarr changed the title mgr/rbd_support: Fix 'start-time' arg behavior mgr/rbd_support: Fix "start-time" arg behavior Dec 29, 2025
@ajarr ajarr force-pushed the wip-fix-schedule-start-time branch from 130ae59 to ecde646 Compare December 29, 2025 13:11
@anthonyeleven
Copy link
Copy Markdown
Contributor

? I don’t have the juice for that. I quoted it inline

Copy link
Copy Markdown
Contributor

@trociny trociny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change looks good to me (though not tested). Actually I am not a fan of always explicitly specifying the date in start time, but as it is the behavior we already have in the cephfs snap scheduler, then it is right thing to do.

I do not like very much the commit log and release note messages though, as to me they do not describe the initial problem quite right. To me the behavior before the change was that the date was not specified in the start time and it implicitly used 1970-01-01 date when making the schedule anchor, and now the date must be explicitly specified by a user in the start time to make it less confusing. Or am I not correct here?

Also I would mention in the commit log and the release note that the change is to have the same behavior we already have in cephfs snap scheduler.

Comment thread qa/workunits/rbd/cli_generic.sh
Comment thread doc/rbd/rbd-mirroring.rst Outdated
Comment thread doc/rbd/rbd-mirroring.rst Outdated
Comment thread doc/rbd/rbd-mirroring.rst Outdated
Comment thread src/pybind/mgr/rbd_support/schedule.py
Comment thread src/pybind/mgr/rbd_support/schedule.py
Comment thread src/pybind/mgr/rbd_support/schedule.py
Comment thread src/pybind/mgr/rbd_support/mirror_snapshot_schedule.py
Comment thread src/pybind/mgr/rbd_support/trash_purge_schedule.py
Comment thread qa/workunits/rbd/cli_generic.sh Outdated
Comment thread qa/workunits/rbd/cli_generic.sh
@ajarr ajarr force-pushed the wip-fix-schedule-start-time branch from ecde646 to bdcf3e2 Compare February 19, 2026 21:35
@ajarr ajarr requested a review from idryomov February 19, 2026 21:35
@ajarr ajarr requested a review from sunilangadi2 February 19, 2026 21:35
Comment thread src/pybind/mgr/rbd_support/schedule.py Outdated
@ajarr ajarr force-pushed the wip-fix-schedule-start-time branch from bdcf3e2 to 532e1f6 Compare February 20, 2026 19:27
@ajarr ajarr requested a review from idryomov February 20, 2026 19:31
Comment thread PendingReleaseNotes
Comment thread PendingReleaseNotes Outdated
@ajarr ajarr force-pushed the wip-fix-schedule-start-time branch from 532e1f6 to f489edf Compare February 20, 2026 22:04
@ajarr ajarr requested a review from idryomov February 20, 2026 22:04
@idryomov
Copy link
Copy Markdown
Contributor

@idryomov
Copy link
Copy Markdown
Contributor

idryomov commented Feb 21, 2026

@idryomov
Copy link
Copy Markdown
Contributor

jenkins test docs

@idryomov
Copy link
Copy Markdown
Contributor

idryomov commented Feb 21, 2026

The docs job is failing persistently, but this was fixed in #66787:

[rtd-command-info] start-time: 2026-02-21T11:42:55.197420Z, end-time: 2026-02-21T11:42:55.859330Z, duration: 0, exit-code: 2
python -m sphinx -T -b dirhtml -d _build/doctrees -D language=en . $READTHEDOCS_OUTPUT/html
Running Sphinx v6.2.1

Traceback (most recent call last):
  File "/home/docs/checkouts/readthedocs.org/user_builds/ceph/envs/66735/lib/python3.9/site-packages/sphinx/registry.py", line 442, in load_extension
    mod = import_module(extname)
  File "/home/docs/.asdf/installs/python/3.9.22/lib/python3.9/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1030, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 986, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 680, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 850, in exec_module
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
  File "/home/docs/checkouts/readthedocs.org/user_builds/ceph/envs/66735/lib/python3.9/site-packages/sphinxcontrib/seqdiag.py", line 19, in <module>
    import pkg_resources
ModuleNotFoundError: No module named 'pkg_resources'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/docs/checkouts/readthedocs.org/user_builds/ceph/envs/66735/lib/python3.9/site-packages/sphinx/cmd/build.py", line 280, in build_main
    app = Sphinx(args.sourcedir, args.confdir, args.outputdir,
  File "/home/docs/checkouts/readthedocs.org/user_builds/ceph/envs/66735/lib/python3.9/site-packages/sphinx/application.py", line 229, in __init__
    self.setup_extension(extension)
  File "/home/docs/checkouts/readthedocs.org/user_builds/ceph/envs/66735/lib/python3.9/site-packages/sphinx/application.py", line 404, in setup_extension
    self.registry.load_extension(self, extname)
  File "/home/docs/checkouts/readthedocs.org/user_builds/ceph/envs/66735/lib/python3.9/site-packages/sphinx/registry.py", line 445, in load_extension
    raise ExtensionError(__('Could not import extension %s') % extname,
sphinx.errors.ExtensionError: Could not import extension sphinxcontrib.seqdiag (exception: No module named 'pkg_resources')

Extension error:
Could not import extension sphinxcontrib.seqdiag (exception: No module named 'pkg_resources')

@ajarr Could you please rebase on the tip of main?

The "start-time" argument, optionally passed when adding or removing an
mirror image snapshot schedule or a trash purge schedule, does not
behave as intended. It is meant to schedule an initial operation at a
specific time of day in a given time zone. Instead, it offsets the
schedule’s anchor time. By default, the scheduler uses the UNIX epoch as
the anchor to calculate recurring schedule times, and "start-time"
simply shifts this anchor away from UTC, which can confuse users. For
example:

```
$ # current time
$ date --universal
Wed Dec 10 05:55:21 PM UTC 2025
$ rbd mirror snapshot schedule add -p data --image img1 1h 19:00Z
$ rbd mirror snapshot schedule ls -p data --image img1
every 15m starting at 19:00:00+00:00
```

A user might assume that the scheduler will run the first snapshot each
day at 19:00 UTC and then run snapshots every 15 minutes. Instead, the
scheduler runs the first snapshot at 18:00 UTC and then continues at the
configured interval:

```
$ rbd mirror snapshot schedule status -p data --image img1
SCHEDULE TIME        IMAGE
2025-12-10 18:00:00  data/img1
```

Additionally, the "start-time" argument accepts a full ISO 8601
timestamp but silently ignores everything except hour, minute, and time
zone. Even time zone handling is incorrect: specifying "23:00-01:00"
with an interval of "1d" results in a snapshot taken once per day at
22:00 UTC rather than 00:00 UTC, because only utcoffset.seconds is used
while utcoffset.days is ignored.

Fix:
Similar to the handling of the "start" argument in the FS snap-schedule
manager module, require "start-time" to use an ISO 8601 date-time format
with a mandatory date component. Time and time zone are optional and
default to 00:00 and UTC respectively.

The "start-time" now defines the anchor time used to compute recurring
schedule times. The default anchor remains the UNIX epoch. Existing
on-disk schedules with legacy-format "start-time" values are updated to
include the date Jan 1, 1970.

The `snap schedule ls` output now displays "start-time" with date and
time in the format "%Y-%m-%d %H:%M:00". The display time is in UTC.

Fixes: https://tracker.ceph.com/issues/74192
Signed-off-by: Ramana Raja <rraja@redhat.com>
@ajarr ajarr force-pushed the wip-fix-schedule-start-time branch from f489edf to 5882c17 Compare February 21, 2026 12:21
@idryomov
Copy link
Copy Markdown
Contributor

jenkins test make check

@idryomov
Copy link
Copy Markdown
Contributor

jenkins test make check arm64

@idryomov idryomov merged commit 0f286bb into ceph:main Feb 21, 2026
13 checks passed
@ajarr ajarr deleted the wip-fix-schedule-start-time branch February 21, 2026 17:07
@phlogistonjohn
Copy link
Copy Markdown
Contributor

anoopcs9 added a commit to anoopcs9/go-ceph that referenced this pull request Feb 27, 2026
The start‑time argument always accepted an ISO 8601 timestamp, but
it wasn’t enforced until it was fixed[1] recently. Therefore, it now
conforms to the requirement in the corresponding tests.

[1] ceph/ceph#66735

Signed-off-by: Anoop C S <anoopcs@cryptolab.net>
mergify Bot pushed a commit to ceph/go-ceph that referenced this pull request Feb 27, 2026
The start‑time argument always accepted an ISO 8601 timestamp, but
it wasn’t enforced until it was fixed[1] recently. Therefore, it now
conforms to the requirement in the corresponding tests.

[1] ceph/ceph#66735

Signed-off-by: Anoop C S <anoopcs@cryptolab.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants