Skip to content

Deprecate iso8601 public API, and some other cleanups#4097

Draft
nrwahl2 wants to merge 91 commits intoClusterLabs:mainfrom
nrwahl2:nrwahl2-time
Draft

Deprecate iso8601 public API, and some other cleanups#4097
nrwahl2 wants to merge 91 commits intoClusterLabs:mainfrom
nrwahl2:nrwahl2-time

Conversation

@nrwahl2
Copy link
Copy Markdown
Contributor

@nrwahl2 nrwahl2 commented Apr 30, 2026

I've been doing some work in the background on switching over from crm_time_t to GDateTime. It's a pretty big pain, but that's largely because GDateTime only supports dates between 0001-01-01 00:00:00 and 9999-12-31 23:59:59. We're going to have to make behavioral changes in some cases, to consider dates invalid if they can't be represented by four digits, and/or to clip them to the valid range.

One commit in this PR does this for crm_time_period_t. It is odd to change behavior in that function immediately before deprecating it. However, the more places we can guarantee a crm_time_t is in the 0001 to 9999 range, the easier future changes will be.

Another does this for crm_rule.

This PR pulls out the commits I have so far that don't involve GDateTime or GDate directly.


Edit: I realized we can deprecate almost all of the public API at this point, so I went ahead and did that. The PR is now much larger, but most of the commits are trivial. We can split it up if you want.

@nrwahl2 nrwahl2 requested a review from clumens April 30, 2026 08:01
@nrwahl2 nrwahl2 force-pushed the nrwahl2-time branch 2 times, most recently from d9b26ea to f6540ec Compare April 30, 2026 23:41
@nrwahl2 nrwahl2 changed the title Deprecate crm_time_period_t, and some other cleanups Deprecate iso8601 public API, and some other cleanups May 1, 2026
@clumens
Copy link
Copy Markdown
Contributor

clumens commented May 6, 2026

Edit: I realized we can deprecate almost all of the public API at this point, so I went ahead and did that. The PR is now much larger, but most of the commits are trivial. We can split it up if you want.

Whew, yeah that's a lot of commits. Go ahead and split it up (also looks like you'll need to resolve a conflict) so it's not quite so demoralizing to review.

@nrwahl2 nrwahl2 marked this pull request as draft May 6, 2026 17:22
nrwahl2 added 23 commits May 6, 2026 10:22
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To match header. Also rename lpc to i in crm_time_add_months().

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
The caller already skips 'T' characters.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
This makes crm_time_parse_duration() a little bit less convoluted.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
It's clearer to show the original string value than the parsed integer.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
It's incorrect to say that a particular integer is too large or too
small, in the way we were doing. In the lax way we're parsing durations,
multiple elements with the same designator may occur. Also, elements
with different designators can add to the same field of the crm_time_t
object. So multiple elements, none of which are too large or too small
on their own, can sum to a value that overflows the size of an int.

Reduce duplication and simplify the log messages on overflow.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
There's no "right" way to do this, and it should be exceedingly rare
that our users specify years or months as part of a duration. But since
we unfortunately support a ISO 8601 duration-like syntax in a wide
variety of contexts, we should handle it sanely. Hopefully we can
deprecate and drop it sometime.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
The pcmk__time_valid_year() function will be used outside this file
soon.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
The reason for the deprecation warning regardless of installed version,
is that we define GLIB_VERSION_MIN_REQUIRED and GLIB_VERSION_MAX_ALLOWED
as GLIB_VERSION_2_42. So we are strictly bound to the 2.42 API.

However, when we can use version 2.58 or later, we're better off using
g_time_zone_new_offset(). This avoids the need to convert the offset to
a string.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Allocate a new string for readability, even though we know the exact
string length in advance and were previously taking advantage of that in
order to use a static buffer.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
This isn't really in keeping with the function name. However, some
callers care only about the resulting hours and minutes, so this is more
convenient than requiring those callers to pass dummy seconds output
variables.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
crm_time_new(NULL) always returns non-NULL.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Check whether parsing was successful, not whether the value was 0 (which
is technically a valid value: epoch).

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
The spec with year 10000 is valid for now.

Note that years get scanned as uint32_t, so -1 becomes UINT32_MAX. I
expect that number to be platform-independent.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
...for the start and end of a period.

This is a behavioral change but shouldn't affect any reasonable cluster
configurations. Use of the iso8601 CLI tool or of the
crm_time_parse_period() function directly will be affected, for dates
with year >= 10000.

ISO 8601 only guarantees support for years up to four digits. Year 0 is
treated as 1 BCE. However, we consider year 0 invalid. The goal is to
move to using GLib's GDateTime internally. 0001-01-1 to 9999-12-31 is
the supported range for GDateTime.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
nrwahl2 added 28 commits May 6, 2026 10:23
To replace crm_time_get_gregorian().

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to inspect a crm_time_t object. Pacemaker
should not be used for general-purpose date/time manipulation.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to inspect a crm_time_t object. Pacemaker
should not be used for general-purpose date/time manipulation.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace crm_time_get_seconds().

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to inspect a crm_time_t object. Pacemaker
should not be used for general-purpose date/time manipulation.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace crm_time_get_seconds_since_epoch(). The name
"pcmk__time_to_unix()" is chosen in order to align with
g_date_time_to_unix(), which we plan to adopt eventually.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to inspect a crm_time_t object. Pacemaker
should not be used for general-purpose date/time manipulation.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
It's equivalent to free(). There are no dynamically allocated fields
within crm_time_t.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Use free() instead.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace pcmk_copy_time().

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to copy a crm_time_t object. Pacemaker
should not be used for general-purpose date/time manipulation.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace crm_time_add().

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to add to a crm_time_t object. Pacemaker
should not be used for general-purpose date/time manipulation.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace crm_time_subtract().

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to subtract from a crm_time_t object.
Pacemaker should not be used for general-purpose date/time manipulation.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace crm_time_add_seconds().

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to add seconds to a crm_time_t object.
Pacemaker should not be used for general-purpose date/time manipulation.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to add minutes to a crm_time_t object.
Pacemaker should not be used for general-purpose date/time manipulation.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to add hours to a crm_time_t object.
Pacemaker should not be used for general-purpose date/time manipulation.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace crm_time_add_days().

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to add days to a crm_time_t object.
Pacemaker should not be used for general-purpose date/time manipulation.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to add weeks to a crm_time_t object.
Pacemaker should not be used for general-purpose date/time manipulation.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to add months to a crm_time_t object.
Pacemaker should not be used for general-purpose date/time manipulation.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace crm_time_add_years().

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to add years to a crm_time_t object.
Pacemaker should not be used for general-purpose date/time manipulation.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants