Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document canonical way to set priority for static pods #46859

Open
stackbaek opened this issue Jun 17, 2024 · 16 comments · May be fixed by #47055
Open

Document canonical way to set priority for static pods #46859

stackbaek opened this issue Jun 17, 2024 · 16 comments · May be fixed by #47055
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. language/en Issues or PRs related to English language priority/backlog Higher priority than priority/awaiting-more-evidence. sig/docs Categorizes an issue or PR as relevant to SIG Docs. sig/node Categorizes an issue or PR as relevant to SIG Node. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@stackbaek
Copy link

Per doc, static pods does not support priorityClassName field. However, when static pods are created with only priority values set to 2000001000 (that matches with system-node-critical) kubelet prints the following error:

forbidden: the integer value of priority (2000001000) must not be provided in pod spec; priority admission controller computed 0 from the given PriorityClass name" pod="kube-system/...
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Jun 17, 2024
@thisisharrsh
Copy link
Contributor

/language en
/sig docs

@k8s-ci-robot k8s-ci-robot added language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Jun 17, 2024
@thisisharrsh
Copy link
Contributor

@k8s-ci-robot k8s-ci-robot added the sig/node Categorizes an issue or PR as relevant to SIG Node. label Jun 17, 2024
@sftim
Copy link
Contributor

sftim commented Jun 18, 2024

What's the defect @stackbaek? Are you saying that the docs are incorrect, that there is a bug in the kubelet, or that there is a problem but you are not sure where?

@stackbaek
Copy link
Author

stackbaek commented Jun 18, 2024

The instruction to add priority to static pods is unclear from the doc. From the doc, it implies that I should just add spece.priority to the manifest (since priorityClassName was documented to be not supported). In reality, I had to add both priority and priorityClassName.

At the end of the day, I just want to confirm the canonical way to set priority on the static pods to ensure node critical pods are running under node pressure.


NOTE: I have not tried to set just priorityClassName on the static pods.

@sftim
Copy link
Contributor

sftim commented Jun 18, 2024

/retitle Document canonical way to set priority for static pods

@k8s-ci-robot k8s-ci-robot changed the title Documentation states static pod does not support podPriorityClass, but creating static pods with priority fails Document canonical way to set priority for static pods Jun 18, 2024
@sftim
Copy link
Contributor

sftim commented Jun 18, 2024

/kind bug

/priority backlog
/triage accepted

@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. priority/backlog Higher priority than priority/awaiting-more-evidence. triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jun 18, 2024
@thisisharrsh
Copy link
Contributor

The necessary information needs to be added, and the documentation should be corrected. I'd want to hear from more people too.

@ktvargo-ms
Copy link
Contributor

/assign

@ktvargo-ms
Copy link
Contributor

ktvargo-ms commented Jun 27, 2024

@SergeyKanzhelev I was able to create a static pod with priorityClassName and was planning on adding the example to the document. Let me know if anything else is needed or if that isn't expected behavior. I see you have this added for triage with Node bugs.

k get priorityclass
NAME VALUE GLOBAL-DEFAULT AGE
system-cluster-critical 2000000000 false 5d2h
system-node-critical 2000001000 false 5d2h

cat static-web.yaml
apiVersion: v1
kind: Pod
metadata:
name: static-web
labels:
role: myrole
spec:
containers:
- name: web
image: nginx
ports:
- name: web
containerPort: 80
protocol: TCP
priorityClassName: system-cluster-critical

k describe pod static-web-minikube

Name: static-web-minikube
Namespace: default
Priority: 2000000000
Priority Class Name: system-cluster-critical
Node: minikube/192.168.49.2
Start Time: Tue, 25 Jun 2024 15:18:25 -0400
Labels: role=myrole
Annotations: kubernetes.io/config.hash: d13d717ea915d4c4049f7783bb26358a
kubernetes.io/config.mirror: d13d717ea915d4c4049f7783bb26358a
kubernetes.io/config.seen: 2024-06-25T19:18:25.377885764Z
kubernetes.io/config.source: file
Status: Running

@ktvargo-ms
Copy link
Contributor

@stackbaek and @sftim, I was able to confirm that using just priorityClassName on the static pods works (see example above). Since this is a concept page, we do want to add an example or not? Otherwise, it is just a quick update on the docs to use priorityClassName on the static pods as well.

@ktvargo-ms ktvargo-ms linked a pull request Jul 1, 2024 that will close this issue
@stackbaek
Copy link
Author

@ktvargo-ms great! examples always boosts clarity and readability (at least for me).

So, I guess we don't even need the priority number, correct? Otherwise all SGTM!

@sftim
Copy link
Contributor

sftim commented Jul 2, 2024

@stackbaek and @sftim, I was able to confirm that using just priorityClassName on the static pods works (see example above). Since this is a concept page, we do want to add an example or not? Otherwise, it is just a quick update on the docs to use priorityClassName on the static pods as well.

Interesting (and to me surprising); however, let's show that this works for an arbitrary Pod priority and not just a built-in one. We should also show that it'll work OK for a static Pod that starts before the control plane is healthy.

@ktvargo-ms
Copy link
Contributor

The original issue (improper instruction) has been resolved. Would these items be better addressed in the static pod page?

@ktvargo-ms
Copy link
Contributor

@ktvargo-ms great! examples always boosts clarity and readability (at least for me).

So, I guess we don't even need the priority number, correct? Otherwise all SGTM!

Correct. Only the priorityClassName is required. The priority number is displayed when describing the node.

@sftim
Copy link
Contributor

sftim commented Jul 2, 2024

Only the priorityClassName is required. The priority number is displayed when describing the node.

I don't get how this works if the control plane isn't up yet. That's an important consideration.

@ktvargo-ms
Copy link
Contributor

Created 'xyz-priority' and will use that an example

k get priorityclass
NAME VALUE GLOBAL-DEFAULT AGE
system-cluster-critical 2000000000 false 2m2s
system-node-critical 2000001000 false 2m2s
xyz-priority 1000000 false 47s

apiVersion: v1
kind: Pod
metadata:
name: static-web
labels:
role: myrole
spec:
containers:
- name: web
image: nginx
ports:
- name: web
containerPort: 80
protocol: TCP
priorityClassName: xyz-priority

k describe pod static-web-minikube
Name: static-web-minikube
Namespace: default
Priority: 1000000
Priority Class Name: xyz-priority
Node: minikube/192.168.49.2
Start Time: Tue, 02 Jul 2024 15:56:35 -0400
Labels: role=myrole
Annotations: kubernetes.io/config.hash: a8236a64037ee18291f146e8a8013147
kubernetes.io/config.mirror: a8236a64037ee18291f146e8a8013147
kubernetes.io/config.seen: 2024-07-02T19:56:35.763567682Z
kubernetes.io/config.source: file
Status: Running

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. language/en Issues or PRs related to English language priority/backlog Higher priority than priority/awaiting-more-evidence. sig/docs Categorizes an issue or PR as relevant to SIG Docs. sig/node Categorizes an issue or PR as relevant to SIG Node. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
Development

Successfully merging a pull request may close this issue.

5 participants