Extract preemption executor into Framework by Argh4k · Pull Request #137562 · kubernetes/kubernetes

Argh4k · 2026-03-09T08:24:59Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR is a prework for two other features: Workload Aware Preeemption from KEP-5710 and delayed preemption from KEP-4671. With that we make preemption a first class citizen in the scheduler framework, which will allow us to call the executor outside of the default preemption plugin.

Which issue(s) this PR is related to:

Special notes for your reviewer:

For now, the Preempt from preemption.go still actuates the preemptions if the Evaluator has the Executor set. In the future we should get rid of that completely and rely only on external Executor to actuate preemptions but I wanted to minimize the changes. This should result in no user-facing changes.

Does this PR introduce a user-facing change?

Preemption Executor is extracted out of preemption Evaluator. Authors of out of tree custom preemption plugins can pass Executor to Evaluator to keep the current behavior, or preferably call the Executor to actuate preemptions after Evaluator calculates them.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot · 2026-03-09T08:25:10Z

This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Details

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-sigs/prow repository.

Argh4k · 2026-03-09T08:26:17Z

/area workload-aware

Argh4k · 2026-03-09T08:43:04Z

/hold

k8s-ci-robot · 2026-03-09T11:37:13Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Argh4k
Once this PR has been reviewed and has the lgtm label, please assign dom4ha for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

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

tosi3k · 2026-03-11T08:47:12Z

+	for _, fwk := range profiles {
+		fwk.SetPreemptionExecutor(preemption.NewExecutor(fwk, plfeature.NewSchedulerFeaturesFromGates(feature.DefaultFeatureGate)))
+	}


Ack. If so, could we open an issue for cleaning this up and mention it here with a TODO?

tosi3k · 2026-03-11T09:15:06Z

 		return nil
 	}
-	if pl.Evaluator.IsPodRunningPreemption(p.GetUID()) {
+	executor := pl.fh.PreemptionExecutor()


Why not make this a top-level field of DefaultPreemption struct?

Same as with podLister, the executor is initialized after the plugins in the current scenario.

tosi3k · 2026-03-11T09:17:59Z

+func NewExecutor(fh fwk.Handle, fts feature.Features) *Executor {
 	e := &Executor{
 		fh:                           fh,
-		podLister:                    fh.SharedInformerFactory().Core().V1().Pods().Lister(),


Ok - in that case, leaving the decision what to do here to you - I don't have any strong opinion here :)

Argh4k · 2026-03-12T08:08:32Z

/retest

macsko · 2026-03-12T15:31:39Z

 	}()

+	executor := pl.fh.PreemptionExecutor()
+	if executor == nil {


Should we check the nil? Isn't it always set?

With that I managed to quickly get some tests that were doing some manual initialization of scheduler, where I had to add also the initialization of preemption executor. So I would leave it as it is, as this check should not be expensive but it can allow for quicker debugging.

So, can you add a comment that it can happen only in tests?

macsko · 2026-03-13T10:07:39Z

+		return nil, fwk.NewStatus(fwk.Error, "preemption executor is nil")
+	}
+
 	result, status := pl.Evaluator.Preempt(ctx, state, pod, m)


The name Preempt becomes misleading now, because it only evaluates potential preemption without preempting anything

I wonder whether we can rename it directly. Given that I want to split Evaluator and Executor completely, I can:

add a note that use of executor in evaluator is deprecated and eventually will be removed

once the executor is removed from an evaluator rename the method.

WDYT?

sanposhiho

I don't understand why we need this effort decoupling executor from the preemption given we already dropped the delayed preemption.

I know we might need such if we really end up implementing the delayed preemption, but can we delay implementing it later? if this work is not particularly necessary for WAP implementation. As I commented in the other PR, I feel like it stays simpler if Preempt actually preempts pods.

Argh4k · 2026-03-17T10:20:40Z

I don't understand why we need this effort decoupling executor from the preemption given we already dropped the delayed preemption.

I know we might need such if we really end up implementing the delayed preemption, but can we delay implementing it later? if this work is not particularly necessary for WAP implementation. As I commented in the other PR, I feel like it stays simpler if Preempt actually preempts pods.

We still need to have Executor as a part of the framework to easily pass it to the pod group preemption evaluator in the pod group cycle. However the effort to call ActuatePreemption outside of the Evaluator.Preempt() can be removed. I will do so

Argh4k · 2026-03-17T11:11:59Z

@sanposhiho @macsko @tosi3k, so I removed the move of victim actuation outside of the Evalautor, but I kept the Executor as a part of the framework handle. I need that so in the main PR adding workload preemption, the same executor can be shared between pod group and pod evaluator, so we can check whether we run pod group preemption for given pod in the IsPodRunningPreemption method.

Argh4k · 2026-03-17T12:44:15Z

/retest

Argh4k · 2026-03-17T13:17:01Z

Also, given the limited time before 1.36 I lean towards closing this PR and just merging this as a part of the #137606.

The workload aware preemption is rebased on top of this one, including changes that resolved comments. @sanposhiho @tosi3k @macsko any objections?

macsko · 2026-03-17T13:18:02Z

Also, given the limited time before 1.36 I lean towards closing this PR and just merging this as a part of the #137606.

The workload aware preemption is rebased on top of this one, including changes that resolved comments.

Sounds good

Argh4k · 2026-03-17T14:38:47Z

Closing in favor of #137606

github-project-automation Bot added this to SIG Scheduling Mar 9, 2026

github-project-automation Bot moved this to Needs Triage in SIG Scheduling Mar 9, 2026

k8s-ci-robot requested review from denkensk and macsko March 9, 2026 08:25

k8s-ci-robot added the area/workload-aware Categorizes an issue or PR as relevant to Workload-aware and Topology-aware scheduling subprojects. label Mar 9, 2026

github-project-automation Bot added this to Workload-aware & Topology-aware Workstream Mar 9, 2026

github-project-automation Bot moved this to Backlog in Workload-aware & Topology-aware Workstream Mar 9, 2026

Argh4k changed the title ~~Extract preemption executor from preemption Evaluator in default preemption~~ Extract preemption executor into Framework Mar 9, 2026

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

Argh4k changed the title ~~Extract preemption executor into Framework~~ [WiP] Extract preemption executor into Framework Mar 9, 2026

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 9, 2026

Argh4k force-pushed the extract-executor-clean branch from 6404ffb to d476361 Compare March 9, 2026 11:36

k8s-ci-robot added area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Mar 9, 2026

Argh4k force-pushed the extract-executor-clean branch 2 times, most recently from 290a31c to dc03e8e Compare March 9, 2026 15:09

Argh4k changed the title ~~[WiP] Extract preemption executor into Framework~~ Extract preemption executor into Framework Mar 9, 2026

whtssub mentioned this pull request Mar 11, 2026

Workload-aware preemption kubernetes/enhancements#5710

Open

7 tasks

tosi3k reviewed Mar 11, 2026

View reviewed changes

tosi3k mentioned this pull request Mar 11, 2026

Add tosi3k to the SIG Scheduling reviewers #137630

Merged

Argh4k force-pushed the extract-executor-clean branch from 363804c to 061a224 Compare March 11, 2026 10:12

macsko reviewed Mar 13, 2026

View reviewed changes

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

helayoty moved this from Backlog to In Progress in Workload-aware & Topology-aware Workstream Mar 16, 2026

helayoty moved this from In Progress to In Review in Workload-aware & Topology-aware Workstream Mar 16, 2026

Argh4k force-pushed the extract-executor-clean branch from 061a224 to 768ce00 Compare March 16, 2026 14:45

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

sanposhiho reviewed Mar 17, 2026

View reviewed changes

Argh4k force-pushed the extract-executor-clean branch 2 times, most recently from 881160d to 484bf54 Compare March 17, 2026 08:34

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 17, 2026

Argh4k force-pushed the extract-executor-clean branch from 484bf54 to 0f497ca Compare March 17, 2026 11:01

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 17, 2026

Extract executor from preemption Evaluator

33af82a

Argh4k force-pushed the extract-executor-clean branch from 0f497ca to 33af82a Compare March 17, 2026 11:07

Argh4k closed this Mar 17, 2026

github-project-automation Bot moved this from 1st Reviewer In Progress to Closed in SIG Scheduling Mar 17, 2026

github-project-automation Bot moved this from In Review to Closed in Workload-aware & Topology-aware Workstream Mar 17, 2026

Conversation

Argh4k commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR is related to:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

Uh oh!

k8s-ci-robot commented Mar 9, 2026

Uh oh!

Argh4k commented Mar 9, 2026

Uh oh!

Argh4k commented Mar 9, 2026

Uh oh!

k8s-ci-robot commented Mar 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Argh4k commented Mar 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sanposhiho left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Argh4k commented Mar 17, 2026

Uh oh!

Argh4k commented Mar 17, 2026

Uh oh!

Argh4k commented Mar 17, 2026

Uh oh!

Argh4k commented Mar 17, 2026

Uh oh!

macsko commented Mar 17, 2026

Uh oh!

Argh4k commented Mar 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Argh4k commented Mar 9, 2026 •

edited

Loading

sanposhiho left a comment •

edited

Loading