mgr/rbd_support: Fix "start-time" arg behavior#66735
Conversation
af6bf53 to
2fcdbb4
Compare
|
TODO
|
b490657 to
fb02d8c
Compare
fb02d8c to
130ae59
Compare
|
Docs lgtm, others will have to approve the code. |
I'm suggesting taking them out completely, especially in the PR title.
Only the PR and commit messages.
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. |
130ae59 to
ecde646
Compare
|
? I don’t have the juice for that. I quoted it inline |
trociny
left a comment
There was a problem hiding this comment.
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.
ecde646 to
bdcf3e2
Compare
bdcf3e2 to
532e1f6
Compare
532e1f6 to
f489edf
Compare
|
No related failures (full run before the test case was amended): https://pulpito.ceph.com/dis-2026-02-03_19:27:10-rbd-wip-dis-testing-distro-default-trial/ |
|
jenkins test docs |
|
The docs job is failing persistently, but this was fixed in #66787: @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>
f489edf to
5882c17
Compare
|
jenkins test make check |
|
jenkins test make check arm64 |
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>
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>
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:
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:
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 lsoutput now displays "start-time" with date andtime 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
xbetween the brackets:[x]. Spaces and capitalization matter when checking off items this way.Checklist
Show available Jenkins commands
jenkins test classic perfJenkins Job | Jenkins Job Definitionjenkins test crimson perfJenkins Job | Jenkins Job Definitionjenkins test signedJenkins Job | Jenkins Job Definitionjenkins test make checkJenkins Job | Jenkins Job Definitionjenkins test make check arm64Jenkins Job | Jenkins Job Definitionjenkins test submodulesJenkins Job | Jenkins Job Definitionjenkins test dashboardJenkins Job | Jenkins Job Definitionjenkins test dashboard cephadmJenkins Job | Jenkins Job Definitionjenkins test apiJenkins Job | Jenkins Job Definitionjenkins test docsReadTheDocs | Github Workflow Definitionjenkins test ceph-volume allJenkins Jobs | Jenkins Jobs Definitionjenkins test windowsJenkins Job | Jenkins Job Definitionjenkins test rook e2eJenkins Job | Jenkins Job DefinitionYou must only issue one Jenkins command per-comment. Jenkins does not understand
comments with more than one command.