strimzi-kafka-operator
strimzi-kafka-operator copied to clipboard
Moving from a `Date` supplier to a `Clock` supplier
The KafkaAssemblyOperator is currently using a Date supplier to provide a Date instance where it's needed and allowing more easily the corresponding testing.
It's mostly used to check maintenance window time (related to the cron expression) and when stamping the time on CR conditions.
As discussed on #6580 it would be better (for consistency) to move to use a Clock instead of a Date.
Reporting the reason after that discussion.
I see the Date supplier used in two places ...
The first one is when writing the time in the CR conditions by using the StatusUtils.iso8601 method which takes the Date instance and write the time in UTC starting from it.
The second place is about the maintenance window in the Util.isMaintenanceTimeWindowsSatisfied method where it sets the timezone on the cron expression as UTC and then uses the isSatisfiedBy(date). Taking a look at the code inside this method it seems that anyway when we pass our Date instance, it's getting the timezone of the cron expression (that we set to UTC) and compares dates in UTC anyway (even if our Date instance is referring to the local time).
So I was wondering ... because these are the only two places where we use the Date supplier and it's always handled in UTC, why not having "something" (Clock?) already taking the time in UTC by default?
Looking at the code @ppatierno, the dateSupplier method exists to be overridden by tests, but it isn't being overridden.
I definitely agree with replacing any usage of Date with Instant, and either a) making a Clock dependency that is used by Instant.now overridable/injectable instead or b) remove the supplier method altogether, as it's not currently being used in any tests.
Triaged on 18.8.2022: Still seems to make sense.
Reopened: The PR addressed this for the CaReconciler but not for the other reconcilers.