[workload] WaitForPodsReady: Requeue at the back of the queue after timeout #689

trasc · 2023-04-07T12:32:52Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

A new workload condition , Evicted, is added, which gets set when:

The workload gets preempted
The workload hits the PodsReady timeout

In case of PodsReady timeout, the condition transition timestamp will be used in scheduler sorting, therefore the workload will be moved at the end of the queue.

Which issue(s) this PR fixes:

Fixes #599

Special notes for your reviewer:

The preemption based Eviction will be used in solving #510.

netlify · 2023-04-07T12:32:59Z

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name	Link
🔨 Latest commit	`468cc41`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/644a71841bc8490007698835

k8s-ci-robot · 2023-04-07T12:33:01Z

Hi @trasc. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

trasc · 2023-04-11T06:49:14Z

/unhold
/cc @alculquicondor
/cc @mwielgus

kerthcet · 2023-04-11T13:19:58Z

/ok-to-test

alculquicondor

cc @mimowo

pkg/controller/core/workload_controller.go

pkg/queue/cluster_queue_impl.go

pkg/scheduler/preemption/preemption.go

pkg/workload/workload.go

pkg/workload/workload_test.go

alculquicondor · 2023-04-25T20:22:52Z

CHANGELOG/CHANGELOG-0.4.md

+
+- Move the workloads evicted due to pods ready timeout to the end of the queue. #689
+
+## Production Readiness


uhm... this was a mistake... they should all be ###

trasc · 2023-04-26T05:08:01Z

/hold cancel

alculquicondor · 2023-04-26T19:56:58Z

/hold cancel
/lgtm
/label tide/merge-method-squash

I thought we had configuration that prevented merge commits.

alculquicondor · 2023-04-27T12:13:47Z

pkg/controller/core/workload_controller.go

@@ -127,6 +127,17 @@ func (r *WorkloadReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 	ctx = ctrl.LoggerInto(ctx, log)
 	log.V(2).Info("Reconciling Workload")

+	// if a pods ready timeout eviction is ongoing.
+	if evictionCond := apimeta.FindStatusCondition(wl.Status.Conditions, kueue.WorkloadEvicted); evictionCond != nil && evictionCond.Status == metav1.ConditionTrue &&
+		evictionCond.Reason == kueue.WorkloadEvictedByPodsReadyTimeout &&


Maybe I said this before, but I think it shouldn't be conditional on the reason.
But we can standardize with preemption in a follow up, and based on the outcome of #510

Leaving this comment for future reference.

pkg/controller/core/workload_controller.go

The new condition is set when a workload is preempted or it's pod ready timeout expired. In case of pods ready timeout, the condition's transition timestamp will be used in ordering the workloads in the scheduling queues.

alculquicondor · 2023-04-27T14:53:16Z

/lgtm
/approve

k8s-ci-robot · 2023-04-27T14:53:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alculquicondor, trasc

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [alculquicondor]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/feature Categorizes issue or PR as related to a new feature. labels Apr 7, 2023

k8s-ci-robot requested review from alculquicondor and denkensk April 7, 2023 12:32

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 7, 2023

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Apr 7, 2023

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 7, 2023

mcariatm mentioned this pull request Apr 7, 2023

[workload] WaitForPodsReady: Requeue at the back of the queue after timeout #628

Closed

trasc force-pushed the evicted_to_end_of_queue branch 4 times, most recently from 2285aad to 6464e21 Compare April 11, 2023 06:46

k8s-ci-robot requested a review from mwielgus April 11, 2023 06:49

trasc marked this pull request as ready for review April 11, 2023 06:50

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 11, 2023

k8s-ci-robot requested a review from ahg-g April 11, 2023 06:50

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 11, 2023

alculquicondor added this to the v0.4 milestone Apr 11, 2023

alculquicondor reviewed Apr 12, 2023

View reviewed changes

trasc force-pushed the evicted_to_end_of_queue branch 3 times, most recently from 68edd48 to 3e4fa75 Compare April 19, 2023 12:24

This was referenced Apr 19, 2023

REQUEST: New membership for trasc kubernetes/org#4168

Closed

WaitForPodsReady: a mode where jobs don't block the queue head #610

Closed

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 20, 2023

trasc force-pushed the evicted_to_end_of_queue branch from c981389 to cc766a8 Compare April 25, 2023 07:20

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 25, 2023

k8s-ci-robot assigned alculquicondor Apr 25, 2023

k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Apr 25, 2023

alculquicondor reviewed Apr 25, 2023

View reviewed changes

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 25, 2023

trasc force-pushed the evicted_to_end_of_queue branch from cc766a8 to 61eb227 Compare April 26, 2023 05:05

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 26, 2023

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 26, 2023

trasc force-pushed the evicted_to_end_of_queue branch 2 times, most recently from c3d9155 to 749d43c Compare April 26, 2023 05:31

k8s-ci-robot added tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 26, 2023

alculquicondor mentioned this pull request Apr 26, 2023

Prevent merge commits #728

Closed

trasc force-pushed the evicted_to_end_of_queue branch from 6c695a3 to 4f3018e Compare April 26, 2023 20:27

k8s-ci-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 26, 2023

alculquicondor reviewed Apr 27, 2023

View reviewed changes

trasc added 2 commits April 27, 2023 15:58

[workload] Add evicted condition

5d70d71

The new condition is set when a workload is preempted or it's pod ready timeout expired. In case of pods ready timeout, the condition's transition timestamp will be used in ordering the workloads in the scheduling queues.

[workload] Add evicted condition - CHANGELOG

468cc41

trasc force-pushed the evicted_to_end_of_queue branch from 4f3018e to 468cc41 Compare April 27, 2023 12:58

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 27, 2023

k8s-ci-robot merged commit a036cd0 into kubernetes-sigs:main Apr 27, 2023

trasc deleted the evicted_to_end_of_queue branch April 28, 2023 13:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[workload] WaitForPodsReady: Requeue at the back of the queue after timeout #689

[workload] WaitForPodsReady: Requeue at the back of the queue after timeout #689

trasc commented Apr 7, 2023

netlify bot commented Apr 7, 2023 •

edited

Loading

k8s-ci-robot commented Apr 7, 2023

trasc commented Apr 11, 2023

kerthcet commented Apr 11, 2023

alculquicondor left a comment

alculquicondor Apr 25, 2023

alculquicondor Apr 25, 2023

trasc commented Apr 26, 2023

alculquicondor commented Apr 26, 2023

alculquicondor Apr 27, 2023

alculquicondor commented Apr 27, 2023

k8s-ci-robot commented Apr 27, 2023


		- Move the workloads evicted due to pods ready timeout to the end of the queue. #689

		## Production Readiness

[workload] WaitForPodsReady: Requeue at the back of the queue after timeout #689

[workload] WaitForPodsReady: Requeue at the back of the queue after timeout #689

Conversation

trasc commented Apr 7, 2023

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

netlify bot commented Apr 7, 2023 • edited Loading

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

k8s-ci-robot commented Apr 7, 2023

trasc commented Apr 11, 2023

kerthcet commented Apr 11, 2023

alculquicondor left a comment

Choose a reason for hiding this comment

alculquicondor Apr 25, 2023

Choose a reason for hiding this comment

alculquicondor Apr 25, 2023

Choose a reason for hiding this comment

trasc commented Apr 26, 2023

alculquicondor commented Apr 26, 2023

alculquicondor Apr 27, 2023

Choose a reason for hiding this comment

alculquicondor commented Apr 27, 2023

k8s-ci-robot commented Apr 27, 2023

netlify bot commented Apr 7, 2023 •

edited

Loading