Salt: RFC: Change Salt to create JIDs as UTC time

[whiteinge]

Background

JIDs in Salt are currently generated from the Salt Master's current timezone. This makes it difficult or (practically) impossible for consumers of Salt to reconcile a job timestamp with the user's relative local time -- specifically programmatic consumers like via returners or salt-api.

(Please comment with corrections if I am mistaken with anything above!)

Request for comments

What would the potential fallout be for switching Salt to store JIDs in UTC in the next major Salt release?

Initial thoughts

There would be an increased risk for JID collisions for a window of time after users upgrade to this version (I think just the offset of the user's current time and UTC). That risk is already pretty low, but something to consider anyway.

Alternate ideas

• Leave the JID as-is and add a new field to the job cache with a UTC timestamp.

  Safer but less "single source of truth"-y.

• Expose the Salt Master's timezone via a Runner or a salt-api endpoint and let the end-users do the math.

  Safer for Salt but a PITA for users. Also likely fraught with edge-cases and mistakes.

@saltstack/team-core @saltstack/team-riot @saltstack/team-platform @thatch45 thoughts?

[cro]

:+1: from me

[cro]

Huge benefits for external job caches as well. Also, we already have issues with companies who have masters in different timezones. If they aggregate to a single external job cache then you can't meaningfully compare JIDs to see when jobs that hit different masters occurred.

[cachedout]

I'd want to be sold on the benefit a little more. There are maybe a few billion stored Salt jobs out there in the world. The risk of collisions is higher than I'd like to imagine. ;]

[thatch45]

salt checks for a collision before making the jid though....

[cro]

Aren't the collisions just going to exist for less than 24 hours after the switch to UTC is made, though? And only timezones east of UTC are affected since everyone west of UTC will see their JIDs jump forward.

[cachedout]

Both good points.

If we have a retry mechanism (which I vaguely recall but haven't looked into) then I'm cool.

[thatch45]

right, so it is 12 hours of collisions, which we SHOULD have a check in place for, but we should double check it, since I wrote it 5 years ago....

[whiteinge]

I remember that Tom. His voice was a tad more gravelly but he had way, way fewer gray hairs.

[whiteinge]

This week I noticed our event-bus _stamp fields as well as the success_stamp in the job cache for Runners are in UTC.

• Does Salt perform date comparisons anywhere internally that this change could affect?
• As part of this change, should we convert print the StartTime (runners) and start_time (highstate) as the user's local time or UTC?

And are there any other potential pitfalls along those lines?

[timwhite]

This seems to have missed 2 major releases. External tools using the API have issues due to this bug, so it would be really nice to have it fixed. Any updates on which release this may end up in?

[whiteinge]

@timwhite thanks for the bump.

General consensus is this should be an easy switchover. Main blocker is the unknowns. We talked about this today and will make the switch for Oxygen to give any potential issues time to surface.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[whiteinge]

_glares at stalebot_

[stale[bot]]

Thank you for updating this issue. It is no longer marked as stale.

[sagetherage]

Closing as complete from the two PRs referenced above.