Allow mutating priority classes #4129

wackxu · 2023-07-20T02:19:11Z

take over #2851 and Push this feature forward

One-line PR description: Allow mutating priority classes

Issue link: Allow priority class values to be updated #4133

Other comments:
xref: Allow priority class values to be updated. kubernetes#99205

wackxu · 2023-07-20T02:20:33Z

You are the author of this proposal, could you help review or give some suggestions?
/assign @ravisantoshgudimetla

wackxu · 2023-07-20T04:16:38Z

/assign @alculquicondor

alculquicondor · 2023-07-20T12:19:51Z

you should start a new KEP

k8s-ci-robot · 2023-09-16T09:45:50Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: wackxu
Once this PR has been reviewed and has the lgtm label, please ask for approval from alculquicondor and additionally assign wojtek-t for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

keps/prod-readiness/OWNERS
keps/sig-scheduling/OWNERS

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

Signed-off-by: wackxu <xushiwei5@huawei.com>

wackxu · 2023-09-16T10:08:54Z

you should start a new KEP

Done， /assign @Huang-Wei @kerthcet

wackxu · 2023-09-23T03:15:44Z

Hi @alculquicondor @Huang-Wei @ahg-g Could you take a look at this? Since It is a small change to loose the validation of priority class

Huang-Wei · 2023-10-03T18:28:58Z

/assign

alculquicondor · 2023-10-03T18:36:30Z

/unassign

Huang-Wei · 2023-10-04T00:51:17Z

keps/sig-scheduling/4133-priority-class-value-mutate/README.md

+At present, we cannot update the priorityClass value directly. This enhancement proposes
+to relax update validation on priorityClass and make priorityClass value to be mutable.


Suggested change

At present, we cannot update the priorityClass value directly. This enhancement proposes

to relax update validation on priorityClass and make priorityClass value to be mutable.

This enhancement proposes to make PriorityClass's `value` field mutable.

Huang-Wei · 2023-10-04T00:52:47Z

keps/sig-scheduling/4133-priority-class-value-mutate/README.md

+
+## Motivation
+
+We'd like priority classes value to be mutable going from release 1.29 because of we're technically achieving the


Avoid mentioning release info here. That's part of the metadata file (kep.yaml).

Huang-Wei · 2023-10-04T01:04:32Z

keps/sig-scheduling/4133-priority-class-value-mutate/README.md

+same effect with re-creation. Reasons for making priority classes immutable are mentioned in this
+[proposal](https://github.com/kubernetes/design-proposals-archive/blob/main/scheduling/pod-priority-api.md#drawbacks-of-changing-priority-classes).
+But we noticed that people are anyways deleting/creating priority classes despite the reasons mentioned above.
+While the deletion and re-creation seems like lesser of both the evils, our experience says that the deletion


In a motivation section, I think you should elaborate why recreating PriorityClass is necessary here, like what's radical problems end-users stomped on and they have to recreate.

Only by clarifying the fundamental use cases, we can know whether making PC's value field mutable is the right approach. Maybe PC is not the problem realm.

Huang-Wei · 2023-10-04T01:07:33Z

keps/sig-scheduling/4133-priority-class-value-mutate/README.md

+But we noticed that people are anyways deleting/creating priority classes despite the reasons mentioned above.
+While the deletion and re-creation seems like lesser of both the evils, our experience says that the deletion
+and re-creation of priority classes is more dangerous and error prone, considering people might delete another
+priority class, instead of doing just an update where an update can be rejected if there is a name mismatch or


I'm not sure I fully understand "re-creation of priority classes is more dangerous and error prone". Are you trying to say PC might be deleted by manual mistake like typo, while update is safer upon typo-ed PC name? If so, I don't think it's a strong motivation.

Huang-Wei · 2023-10-04T01:21:27Z

keps/sig-scheduling/4133-priority-class-value-mutate/README.md

+For some integrators (not default scheduler), they may rely on PriorityClass deletion to
+trigger some customized behavior (like recreating/updating pending pods). This proposal may get them impacted.
+
+## Design Details


I think some aspects need to illustrated:

will PC's admission plugin's logic change? do pods carry pre-updated PC's value get populated after PC's update? if so, does it impact unschedulable pods only?

is system/cluster reserved PC mutable? (although you gave a snippet, but it's always better illustrated it explicitly)

clear document that other fields like preemptionPolicy is still immutable

kerthcet

I think we need more work on this especially the use cases and motivations? 🤔

kerthcet · 2023-10-06T10:04:51Z

keps/sig-scheduling/4133-priority-class-value-mutate/README.md

+### Non-Goals
+
+- Allow mutating priorityClass other immutable filed like `PreemptionPolicy`
+- Change the priority of running or pending pods when changing the priority value of a priority class


Can you also sort out the risks to running pods with the changed priority. Maybe at the ### Risks and Mitigations

kerthcet · 2023-10-06T10:18:45Z

keps/sig-scheduling/4133-priority-class-value-mutate/README.md

+
+#### Story 1
+
+As a cluster administrator, I wish I could directly adjust the priority class value instead of re-creation.


I'm not quite convinced by this use case because I hope we can have a real case here rather than just emphasizing that I want it. You can explain in which case you want to adjust the priority class directly and what we can benefit from it.

More stories is also welcomed.

k8s-triage-robot · 2024-01-22T02:21:54Z

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as fresh with /remove-lifecycle stale
Close this PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

sftim · 2024-02-01T14:29:48Z

keps/sig-scheduling/4133-priority-class-value-mutate/README.md

+
+## Summary
+
+At present, we cannot update the priorityClass value directly. This enhancement proposes


Suggested change

At present, we cannot update the priorityClass value directly. This enhancement proposes

At present, we cannot update the `value` of a PriorityClass directly. This enhancement proposes

k8s-triage-robot · 2024-03-02T15:03:19Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as fresh with /remove-lifecycle rotten
Close this PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-triage-robot · 2024-04-01T15:21:05Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Reopen this PR with /reopen
Mark this PR as fresh with /remove-lifecycle rotten
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

k8s-ci-robot · 2024-04-01T15:21:10Z

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied

After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied

After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Reopen this PR with /reopen

Mark this PR as fresh with /remove-lifecycle rotten

Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

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.

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

k8s-ci-robot requested review from ahg-g and alculquicondor July 20, 2023 02:19

k8s-ci-robot added kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 20, 2023

k8s-ci-robot assigned ravisantoshgudimetla Jul 20, 2023

k8s-ci-robot assigned alculquicondor Jul 20, 2023

wackxu force-pushed the allowupdatepriorityclass branch from c5873ef to 81ed9ad Compare July 22, 2023 10:56

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 Jul 22, 2023

wackxu force-pushed the allowupdatepriorityclass branch 2 times, most recently from 53aefa8 to da8c5e0 Compare July 22, 2023 11:00

alculquicondor mentioned this pull request Aug 17, 2023

KEP: workload priority kubernetes-sigs/kueue#1041

Merged

alculquicondor mentioned this pull request Sep 6, 2023

KEP 973 workload-Priority kubernetes-sigs/kueue#1081

Merged

wackxu force-pushed the allowupdatepriorityclass branch from da8c5e0 to 1185c19 Compare September 16, 2023 09:45

wackxu force-pushed the allowupdatepriorityclass branch from 1185c19 to 4857d68 Compare September 16, 2023 09:53

Allow mutating priority classes

c2a5194

Signed-off-by: wackxu <xushiwei5@huawei.com>

wackxu force-pushed the allowupdatepriorityclass branch from 4857d68 to c2a5194 Compare September 16, 2023 10:01

k8s-ci-robot assigned Huang-Wei Oct 3, 2023

k8s-ci-robot unassigned alculquicondor Oct 3, 2023

Huang-Wei reviewed Oct 4, 2023

View reviewed changes

kerthcet reviewed Oct 6, 2023

View reviewed changes

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 22, 2024

sftim reviewed Feb 1, 2024

View reviewed changes

wojtek-t self-assigned this Feb 1, 2024

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Mar 2, 2024

k8s-ci-robot closed this Apr 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow mutating priority classes #4129

Allow mutating priority classes #4129

wackxu commented Jul 20, 2023 •

edited

Loading

wackxu commented Jul 20, 2023

wackxu commented Jul 20, 2023 •

edited

Loading

alculquicondor commented Jul 20, 2023

k8s-ci-robot commented Sep 16, 2023

wackxu commented Sep 16, 2023

wackxu commented Sep 23, 2023

Huang-Wei commented Oct 3, 2023

alculquicondor commented Oct 3, 2023

Huang-Wei Oct 4, 2023

Huang-Wei Oct 4, 2023

Huang-Wei Oct 4, 2023

Huang-Wei Oct 4, 2023

Huang-Wei Oct 4, 2023

kerthcet left a comment

kerthcet Oct 6, 2023

kerthcet Oct 6, 2023

k8s-triage-robot commented Jan 22, 2024

sftim Feb 1, 2024

k8s-triage-robot commented Mar 2, 2024

k8s-triage-robot commented Apr 1, 2024

k8s-ci-robot commented Apr 1, 2024

		At present, we cannot update the priorityClass value directly. This enhancement proposes
		to relax update validation on priorityClass and make priorityClass value to be mutable.

	At present, we cannot update the priorityClass value directly. This enhancement proposes
	to relax update validation on priorityClass and make priorityClass value to be mutable.
	This enhancement proposes to make PriorityClass's `value` field mutable.


		## Motivation

		We'd like priority classes value to be mutable going from release 1.29 because of we're technically achieving the


		#### Story 1

		As a cluster administrator, I wish I could directly adjust the priority class value instead of re-creation.


		## Summary

		At present, we cannot update the priorityClass value directly. This enhancement proposes

	At present, we cannot update the priorityClass value directly. This enhancement proposes
	At present, we cannot update the `value` of a PriorityClass directly. This enhancement proposes

Allow mutating priority classes #4129

Allow mutating priority classes #4129

Conversation

wackxu commented Jul 20, 2023 • edited Loading

wackxu commented Jul 20, 2023

wackxu commented Jul 20, 2023 • edited Loading

alculquicondor commented Jul 20, 2023

k8s-ci-robot commented Sep 16, 2023

wackxu commented Sep 16, 2023

wackxu commented Sep 23, 2023

Huang-Wei commented Oct 3, 2023

alculquicondor commented Oct 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kerthcet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-triage-robot commented Jan 22, 2024

Choose a reason for hiding this comment

k8s-triage-robot commented Mar 2, 2024

k8s-triage-robot commented Apr 1, 2024

k8s-ci-robot commented Apr 1, 2024

wackxu commented Jul 20, 2023 •

edited

Loading

wackxu commented Jul 20, 2023 •

edited

Loading