Deprecate critical pod annotation #10733

ravisantoshgudimetla · 2018-10-25T17:37:03Z

As part of this PR.

Removed references to rescheduler which we removed in 1.13.
Added section for critical pod annotation deprecation because of pod priorities going to beta in 1.11.

/cc @bsalamat
/sig scheduling

k8sio-netlify-preview-bot · 2018-10-25T17:42:12Z

Deploy preview for kubernetes-io-master-staging ready!

Built with commit c5349fe

https://deploy-preview-10733--kubernetes-io-master-staging.netlify.com

k8sio-netlify-preview-bot · 2018-10-25T17:42:56Z

Deploy preview for kubernetes-io-master-staging ready!

Built with commit 6e48de3

https://deploy-preview-10733--kubernetes-io-master-staging.netlify.com

bsalamat · 2018-10-25T17:56:01Z

content/en/docs/tasks/administer-cluster/guaranteed-scheduling-critical-addon-pods.md

-*Warning:* currently there is no guarantee which node is chosen and which pods are being killed
-in order to schedule critical pods, so if rescheduler is enabled your pods might be occasionally
-killed for this purpose. Please ensure that rescheduler is not enabled along with priorities & preemptions in default-scheduler as rescheduler is oblivious to priorities and it may evict high priority pods, instead of low priority ones.
+before upgrading to Kubernetes 1.11 or higher.**


This message is a bit misleading. Priority is enabled by default in 1.11+. We should change the message and say that users should configure appropriate pod priorities for their critical pods.

Sure. I will add it.

bsalamat · 2018-10-25T17:58:35Z

content/en/docs/tasks/administer-cluster/guaranteed-scheduling-critical-addon-pods.md

-* have the PodSpec's `tolerations` field set to `[{"key":"CriticalAddonsOnly", "operator":"Exists"}]`.
-
-The first one marks a pod a critical. The second one is required by Rescheduler algorithm.
+* have the `scheduler.alpha.kubernetes.io/critical-pod` annotation set to empty string.


We should also change this and say that the pod must have its priorityClassName set to system-node-critical or system-cluster-critical and link to the pod priority doc.

The subsequent section talks about using priority.

I see. I think we should remove this paragraph given that we no longer want users to use this annotation.

Shouldn't we remove this when we are deleting the critical pod annotation completely.

Given that these docs are versioned and we are changing this for 1.13, I think it makes sense to delete this paragraph. We don't want people to use this deprecated feature in 1.13.

Ok, done. Thanks

bsalamat

Thanks, Ravi. Almost there. You can squash commits.

bsalamat · 2018-10-26T21:31:31Z

content/en/docs/tasks/administer-cluster/guaranteed-scheduling-critical-addon-pods.md

-* have the PodSpec's `tolerations` field set to `[{"key":"CriticalAddonsOnly", "operator":"Exists"}]`.
-
-The first one marks a pod a critical. The second one is required by Rescheduler algorithm.
+* Have the priorityClass set as "system-cluster-critical" or "system-node-critical", the latter being the highest for entire cluster and `scheduler.alpha.kubernetes.io/critical-pod` annotation set to empty string(This will be deprecated too).


s/priorityClass/priorityClassName/

What I understand from this sentence is that both priorityClassName and the annotation are needed for a pod to be considered critical. So, I would write it as:

Have the priorityClassName set as "system-cluster-critical" or "system-node-critical", the latter being the highest for entire cluster. Alternatively, you could add an annotation with scheduler.alpha.kubernetes.io/critical-pod as key and empty string as value to your pod, but this annotation is deprecated as of version 1.13 and will be removed in 1.14.

Ohh you are right, while reading it again, I got what you were telling. I have modified it.

bsalamat · 2018-10-26T21:32:46Z

content/en/docs/tasks/administer-cluster/guaranteed-scheduling-critical-addon-pods.md

-* have the PodSpec's `tolerations` field set to `[{"key":"CriticalAddonsOnly", "operator":"Exists"}]`.
-
-The first one marks a pod a critical. The second one is required by Rescheduler algorithm.
+* Have the priorityClass set as "system-cluster-critical" or "system-node-critical", the latter being the highest for entire cluster and `scheduler.alpha.kubernetes.io/critical-pod` annotation set to empty string(This will be deprecated too).

 A pod could also be considered critical, if its priority is greater than or equal to system-critical-priority.


This line seems to be redundant.

bsalamat · 2018-10-26T21:34:03Z

content/en/docs/tasks/administer-cluster/guaranteed-scheduling-critical-addon-pods.md

-Rescheduler doesn't have any user facing configuration (component config) or API.
-
-### Marking pod as critical when using Rescheduler.
+### Marking pod as critical.


Please remove the period from the end.

ravisantoshgudimetla · 2018-10-28T14:37:48Z

Thanks for review @bsalamat - I have modified the content as per your suggestions.

bsalamat

/lgtm
/approve

bsalamat · 2018-10-30T00:11:41Z

Thanks, @ravisantoshgudimetla!

steveperry-53 · 2018-10-30T17:16:26Z

/approve

k8s-ci-robot · 2018-10-30T17:16:40Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bsalamat, steveperry-53

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:

~~content/en/OWNERS~~ [steveperry-53]

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

seh · 2019-01-01T14:29:11Z

content/en/docs/tasks/administer-cluster/guaranteed-scheduling-critical-addon-pods.md

-### Marking pod as critical when priorites are enabled.
-
-To be considered critical, the pod has to run in the `kube-system` namespace (configurable via flag) and
+* Have the priorityClassName set as "system-cluster-critical" or "system-node-critical", the latter being the highest for entire cluster. Alternatively, you could add an annotation `scheduler.alpha.kubernetes.io/critical-pod` as key and empty string as value to your pod, but this annotation is deprecated as of version 1.13 and will be removed in 1.14.


Should that be "former" instead of "latter?" It sounds like system-cluster-critical would be the highest priority, above system-node-critical.

@seh The text of the PR is correct. system-node-critical is the highest priority. It means that the pod must run on the node. system-cluster-critical means that the pod must run in the cluster. So, it is ok to evict it from the current node and schedule it somewhere else in the cluster.

Ah, that makes sense now. Thank you for clarifying.

Having seen a similar sentence elsewhere in the code—mentioning "former" instead of "latter"—made me think I had found an inconsistent case here, but now I see those other sentences had the two classes reversed.

The CriticalAddonsOnly taint had been deprecated [1]. Critical add-on pods should use higher priority class now [2]. [1]: kubernetes/website#10733 [2]: https://kubernetes.io/docs/tasks/administer-cluster/guaranteed-scheduling-critical-addon-pods/ Signed-off-by: Toshikuni Fukaya <toshikuni-fukaya@cybozu.co.jp>

k8s-ci-robot requested a review from bsalamat October 25, 2018 17:37

k8s-ci-robot added the sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. label Oct 25, 2018

kubernetes-docs-i18n-bot added the language/en Issues or PRs related to English language label Oct 25, 2018

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 25, 2018

bsalamat reviewed Oct 25, 2018

View reviewed changes

bsalamat reviewed Oct 26, 2018

View reviewed changes

Deprecate critical pod annotation

6e48de3

ravisantoshgudimetla force-pushed the patch-3 branch from cb4f984 to 6e48de3 Compare October 28, 2018 14:36

k8s-ci-robot assigned bsalamat Oct 30, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 30, 2018

bsalamat approved these changes Oct 30, 2018

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 30, 2018

k8s-ci-robot merged commit 315c716 into kubernetes:master Oct 30, 2018

seh reviewed Jan 1, 2019

View reviewed changes

toshipp mentioned this pull request May 14, 2024

Update a way to mark pods as critical add-on topolvm/topolvm#912

Closed

4 tasks

toshipp mentioned this pull request May 14, 2024

Change a way to mark pods as critical add-on topolvm/topolvm#913

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecate critical pod annotation #10733

Deprecate critical pod annotation #10733

ravisantoshgudimetla commented Oct 25, 2018 •

edited

Loading

k8sio-netlify-preview-bot commented Oct 25, 2018

k8sio-netlify-preview-bot commented Oct 25, 2018 •

edited

Loading

bsalamat Oct 25, 2018

ravisantoshgudimetla Oct 25, 2018

ravisantoshgudimetla Oct 26, 2018

bsalamat Oct 25, 2018

ravisantoshgudimetla Oct 25, 2018 •

edited

Loading

bsalamat Oct 25, 2018

ravisantoshgudimetla Oct 25, 2018

bsalamat Oct 26, 2018

ravisantoshgudimetla Oct 26, 2018

bsalamat left a comment

bsalamat Oct 26, 2018

ravisantoshgudimetla Oct 28, 2018

bsalamat Oct 26, 2018

bsalamat Oct 26, 2018

ravisantoshgudimetla Oct 28, 2018

ravisantoshgudimetla commented Oct 28, 2018 •

edited

Loading

bsalamat left a comment

bsalamat commented Oct 30, 2018

steveperry-53 commented Oct 30, 2018

k8s-ci-robot commented Oct 30, 2018

seh Jan 1, 2019

bsalamat Jan 3, 2019

seh Jan 3, 2019

Deprecate critical pod annotation #10733

Deprecate critical pod annotation #10733

Conversation

ravisantoshgudimetla commented Oct 25, 2018 • edited Loading

k8sio-netlify-preview-bot commented Oct 25, 2018

k8sio-netlify-preview-bot commented Oct 25, 2018 • edited Loading

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

ravisantoshgudimetla Oct 25, 2018 • edited Loading

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

bsalamat left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ravisantoshgudimetla commented Oct 28, 2018 • edited Loading

bsalamat left a comment

Choose a reason for hiding this comment

bsalamat commented Oct 30, 2018

steveperry-53 commented Oct 30, 2018

k8s-ci-robot commented Oct 30, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ravisantoshgudimetla commented Oct 25, 2018 •

edited

Loading

k8sio-netlify-preview-bot commented Oct 25, 2018 •

edited

Loading

ravisantoshgudimetla Oct 25, 2018 •

edited

Loading

ravisantoshgudimetla commented Oct 28, 2018 •

edited

Loading