Replace `time.Now()` code with `clock.Clock.Now()`

[PBundyra]

What would you like to be added: There are packages like e.g. pkg/scheduler that use time.Now(). As a result this code is hard to test and prone to flakiness as we are not able to directly control the time during tests. It should be replaced with clock.Clock.Now() as it is in WorkloadController

Why is this needed:

Completion requirements:

This enhancement requires the following artifacts:

The artifacts should be linked in subsequent comments.

[PBundyra]

/cc @mbobrovskyi

[TusharMohapatra07]

can i work on this issue ? @PBundyra @mbobrovskyi

[mbobrovskyi]

can i work on this issue ? @PBundyra @mbobrovskyi

Sure, please take it.

[TusharMohapatra07]

@mbobrovskyi thanks! i'm on it

[mbobrovskyi]

/assign @TusharMohapatra07

[TusharMohapatra07]

@mbobrovskyi am i supposed to change the time.Now() instance everywhere inside the project or just those instances which are present inside pkg folder ??

[mbobrovskyi]

Let's focus on pkg/scheduler. This is an example https://github.com/kubernetes-sigs/kueue/blob/74286b4e96d513de1f3455a5509cdd31b489c253/pkg/controller/core/workload_controller.go#L108.

[mimowo]

I would say the end goal is to use it everywhere, but starting with scheduler might be a good idea to see if there are any obstacles, then feel free to follow up in the next PR.

[TusharMohapatra07]

@mimowo understood

[mbobrovskyi]

/reopen

To fix it in other places.

[k8s-ci-robot]

@mbobrovskyi: Reopened this issue.

In response to [this](https://github.com/kubernetes-sigs/kueue/issues/3380#issuecomment-2460097762): >/reopen > >To fix it in other places. Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.

[TusharMohapatra07]

@mbobrovskyi yes, i'm on it

[TusharMohapatra07]

@mbobrovskyi I don't see any significant place to replace time.Now in pkg/workload. Please confirm !

[mbobrovskyi]

What about this one https://github.com/kubernetes-sigs/kueue/blob/0f54466b991a7d198eeca306dc75f2aee518c482/pkg/workload/admissionchecks.go#L96?

Also here https://github.com/kubernetes-sigs/kueue/blob/0f54466b991a7d198eeca306dc75f2aee518c482/pkg/workload/admissionchecks.go#L110.

Could you please check is it possible to propagate clock.Clock? Or maybe just now time.Time.

This is an example

https://github.com/kubernetes-sigs/kueue/blob/0f54466b991a7d198eeca306dc75f2aee518c482/pkg/workload/admissionchecks.go#L32

[TusharMohapatra07]

@mbobrovskyi Actually, LastTransitionTime takes metav1.Time, so time.Time won't work ig. We could probably pass in the clock.Clock interface inside the function to proceed with this..

[mbobrovskyi]

@mbobrovskyi Actually, LastTransitionTime takes metav1.Time, so time.Time won't work ig. We could probably pass in the clock.Clock interface inside the function to proceed with this..

Why we can't use like this LastTransitionTime: metav1.NewTime(clock.Now()) or LastTransitionTime: metav1.NewTime(now)?

I think just propagation to function is enough for it. Also time.Time is enough.

[TusharMohapatra07]

@mbobrovskyi so, if we propogate it in the function, we have to pass it whenever we're calling the function ? For ex:- https://github.com/kubernetes-sigs/kueue/blob/0f54466b991a7d198eeca306dc75f2aee518c482/pkg/scheduler/preemption/preemption.go#L225

[mbobrovskyi]

Exactly and Preemptor should have clock? If not please add to Preemptor as well.

[TusharMohapatra07]

@mbobrovskyi understood

[TusharMohapatra07]

Hey @mbobrovskyi, sorry for being late but i'm a bit busy with some commitments. I'll restart working on this in a day or two.