[#2383] Change k8s lease expiration calculation #2474
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Arkatufus
commented
Apr 17, 2024
| @@ -41,7 +41,7 @@ public LeaseResource(string? owner, string version, long time) | |||
|
|
|||
| public string? Owner { get; } | |||
| public string Version { get; } | |||
| public long Time { get; } | |||
| public DateTime Time { get; } | |||
There was a problem hiding this comment.
The internal lease resource timeout property now stores the absolute expiration date and time, not the milliseconds offset until lease expiration
| _settings.TimeoutSettings.HeartbeatTimeout.TotalMilliseconds - | ||
| 2 * _settings.TimeoutSettings.HeartbeatInterval.TotalMilliseconds; | ||
| private bool HasLeaseTimedOut(DateTime leaseTime) | ||
| => DateTime.UtcNow > leaseTime + _heartbeatTimeout - _heartbeatOffset; |
There was a problem hiding this comment.
Lease timeout calculation is now based on absolute date and time
| @@ -384,7 +384,7 @@ private LeaseResource ToLeaseResource(object obj) | |||
| return new LeaseResource( | |||
| string.IsNullOrWhiteSpace(lease.Spec.Owner) ? null : lease.Spec.Owner, | |||
| lease.Metadata.ResourceVersion, | |||
| lease.Spec.Time); | |||
| DateTime.SpecifyKind(new DateTime(lease.Spec.Time), DateTimeKind.Utc)); | |||
There was a problem hiding this comment.
Converts the stored ticks to DateTime and set the Kind to UTC
| @@ -39,7 +39,7 @@ public LeaseSpec() | |||
| public LeaseSpec(string? owner, DateTime time) | |||
| { | |||
| Owner = owner; | |||
| Time = (long) time.TimeOfDay.TotalMilliseconds; | |||
| Time = time.Ticks; | |||
There was a problem hiding this comment.
Stores the timeout value to absolute date and time expiration value instead of the millisecond offset until expiration
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #2383
Changes
Change Akka.Coordination.KubernetesApi lease timeout/expiration calculation from
DateTime.TimeOfDay.TotalMillisecondstoDateTime.Ticks.Notes
The wire format is untouched, but the content and its context does.
LeaseSpec.Timenow represents the absoluteDateTimeticks when a lease will expire, instead of the time offset (in milliseconds) when the lease will expire.Warning
This is not backward compatible with any version below 1.5.18, a CVE will need to be issued to warn users about this breaking change.
Checklist
For significant changes, please ensure that the following have been completed (delete if not relevant):