Update KEP with new name: PodReadyToStartContainers condition #3778

ddebroy · 2023-01-24T17:02:24Z

One-line PR description: Update KEP with PodReadyToStartContainers condition and updating milestone for Beta

Issue link: Pod conditions around readiness to start containers after completion of pod sandbox creation #3085

Other comments: This KEP updates the name of the condition to PodReadyToStartContainers (from the previous PodHasNetwork that sig-network had concerns with) as previously discussed with sig-node and sig-network leads. The description of the condition and goals are updated accordingly.

Signed-off-by: Deep Debroy <ddebroy@gmail.com>

dchen1107 · 2023-01-24T18:51:52Z

/lgtm
/approve

Not sure the new name is better than the previous one though since it implies that if a pod can start the container purely depends on the network, which is not true. But I am not good at naming, go with the majority here. :-)

ddebroy · 2023-01-24T19:53:14Z

/hold

for sig-network lgtm as well.

aojea · 2023-01-25T00:08:04Z

/lgtm

previous name imply a lot of things for the network that may not be true or may be misleading, no strong opinion about the name, but I'll adventure to throw a suggestion: PodInitialized 😄

mrunalp · 2023-01-25T19:48:13Z

I like PodInitialized :)

ddebroy · 2023-01-31T22:15:50Z

Latest commit updates the PRR section to Beta. No other sections already LGTM ed were altered.

mrunalp · 2023-02-01T21:52:06Z

keps/sig-node/3085-pod-conditions-for-starting-completition-of-sandbox-creation/README.md

+and CNI plugins). Completion of all these phases puts the pod sandbox in a
+state where the containers in a pod can be started. This KEP proposes a
+`PodReadyToStartContainers` condition in pod status to indicate a pod has
+reached a state where it's containers are ready to be started. The


mrunalp · 2023-02-01T22:11:40Z

/lgtm

cici37

Couple comments regarding with PRR.
One more question regarding with Will enabling / using this feature result in increasing time taken by any operations covered by existing SLIs/SLOs?(Ask here since cannot comment on unchanged line), due to Pod startup latency SLI/SLO details, will this feature slightly add up on the latency?

/cc @johnbelamaric

keps/sig-node/3085-pod-conditions-for-starting-completition-of-sandbox-creation/README.md

cici37 · 2023-02-02T11:52:19Z

keps/sig-node/3085-pod-conditions-for-starting-completition-of-sandbox-creation/README.md

+```
+apiserver_request_total{verb="PATCH", resource="pods", subresource="status"}
+```
+for this.


Increase of this metrics could potentially be caused by other issue besides current feature. Is there any other info cluster admin could use to inform a rollback due to PodReadyToStartContainersCondition issue?

There isn't any specific metric right now for distinct Pod Status updates (specific pod conditions or other aspects of pod status like container statuses). We may consider that as an independent enhancement to the Kubelet status manager (covering all pod status fields) as mentioned below.

Do you know if there is a node label on those metrics? If the spike is specifically from nodes with this feature enabled, that could be why. I believe @logicalhan recently added feature gate metrics, so in theory you could construct a query that looked for spikes only on nodes with the feature enabled.

There is no originating node label on the API Server request metrics here: https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics.go#L80

ddebroy · 2023-02-08T00:18:15Z

One more question regarding with Will enabling / using this feature result in increasing time taken by any operations covered by existing SLIs/SLOs?(Ask here since cannot comment on unchanged line), due to Pod startup latency SLI/SLO details, will this feature slightly add up on the latency?

This may result in more API calls from Kubelet Status Manager to API server (under certain circumstances when the Kubelet Status Manager did not batch a bunch of updates for some reason). This aspect is covered under the section ###### Will enabling / using this feature result in any new API calls?

No specific SLIs or metrics are surfaced by Kubelet associated with distinct pod status updates. So that can be considered a separate enhancement to broadly address the overall Kubelet Status manager.

johnbelamaric · 2023-02-08T01:13:38Z

keps/sig-node/3085-pod-conditions-for-starting-completition-of-sandbox-creation/README.md

+```
+apiserver_request_total{verb="PATCH", resource="pods", subresource="status"}
+```
+for this.


Do you know if there is a node label on those metrics? If the spike is specifically from nodes with this feature enabled, that could be why. I believe @logicalhan recently added feature gate metrics, so in theory you could construct a query that looked for spikes only on nodes with the feature enabled.

keps/sig-node/3085-pod-conditions-for-starting-completition-of-sandbox-creation/README.md

Signed-off-by: Deep Debroy <ddebroy@gmail.com>

ddebroy · 2023-02-09T02:24:41Z

@johnbelamaric @cici37 the PRR section has been updated with the suggestions above. Please note that the update (scoped to the last commit) also removed the original LGTM from sig-node reviews.

johnbelamaric

/approve
/lgtm

johnbelamaric · 2023-02-09T03:23:21Z

keps/sig-node/3085-pod-conditions-for-starting-completition-of-sandbox-creation/README.md

+```
+apiserver_request_total{verb="PATCH", resource="pods", subresource="status"}
+```
+for this.


k8s-ci-robot · 2023-02-09T03:24:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dchen1107, ddebroy, johnbelamaric

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:

~~keps/prod-readiness/OWNERS~~ [johnbelamaric]
~~keps/sig-node/OWNERS~~ [dchen1107,johnbelamaric]

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

ddebroy · 2023-02-09T04:24:56Z

/unhold

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory labels Jan 24, 2023

k8s-ci-robot requested a review from dchen1107 January 24, 2023 17:02

k8s-ci-robot added the sig/node Categorizes an issue or PR as relevant to SIG Node. label Jan 24, 2023

k8s-ci-robot requested a review from derekwaynecarr January 24, 2023 17:02

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

Update KEP with PodReadyToStartContainers condition for Beta

d052b7b

Signed-off-by: Deep Debroy <ddebroy@gmail.com>

ddebroy force-pushed the sandbox2 branch from 4994c9f to d052b7b Compare January 24, 2023 17:10

ddebroy mentioned this pull request Jan 24, 2023

Pod conditions around readiness to start containers after completion of pod sandbox creation #3085

Closed

10 tasks

dchen1107 added this to the v1.27 milestone Jan 24, 2023

k8s-ci-robot assigned dchen1107 Jan 24, 2023

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

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

k8s-ci-robot assigned aojea Jan 25, 2023

kannon92 mentioned this pull request Jan 25, 2023

Enhancement: Marking a pending pod as failed after a certain timeout kubernetes/kubernetes#113211

Open

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 31, 2023

mrunalp reviewed Feb 1, 2023

View reviewed changes

k8s-ci-robot assigned mrunalp Feb 1, 2023

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

cici37 reviewed Feb 2, 2023

View reviewed changes

k8s-ci-robot requested a review from johnbelamaric February 2, 2023 12:01

kannon92 mentioned this pull request Feb 3, 2023

KEP-3815 : Add Condition for Pending Pods that are stuck due to configuration errors #3816

Closed

johnbelamaric reviewed Feb 8, 2023

View reviewed changes

Update PRR for Beta

78574a3

Signed-off-by: Deep Debroy <ddebroy@gmail.com>

ddebroy force-pushed the sandbox2 branch from f0eeb64 to 78574a3 Compare February 9, 2023 02:23

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

johnbelamaric approved these changes Feb 9, 2023

View reviewed changes

k8s-ci-robot assigned johnbelamaric Feb 9, 2023

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

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 9, 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 Feb 9, 2023

k8s-ci-robot merged commit c80f656 into kubernetes:master Feb 9, 2023

dims mentioned this pull request Jun 12, 2023

feat: rename PodHasNetwork to PodReadyToStartContainers kubernetes/kubernetes#117702

Merged

ddebroy changed the title ~~Update KEP with PodReadyToStartContainers condition for Beta~~ Update KEP with new name: PodReadyToStartContainers condition Jun 15, 2023

kannon92 mentioned this pull request Jul 26, 2023

[KEP-3085] Add condition for sandbox creation (xposted from original issue) #4138

Open

20 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update KEP with new name: PodReadyToStartContainers condition #3778

Update KEP with new name: PodReadyToStartContainers condition #3778

ddebroy commented Jan 24, 2023

dchen1107 commented Jan 24, 2023 •

edited

Loading

ddebroy commented Jan 24, 2023

aojea commented Jan 25, 2023

mrunalp commented Jan 25, 2023

ddebroy commented Jan 31, 2023

mrunalp Feb 1, 2023

mrunalp commented Feb 1, 2023

cici37 left a comment

cici37 Feb 2, 2023

ddebroy Feb 8, 2023 •

edited

Loading

johnbelamaric Feb 8, 2023

ddebroy Feb 8, 2023 •

edited

Loading

johnbelamaric Feb 9, 2023

ddebroy commented Feb 8, 2023 •

edited

Loading

johnbelamaric Feb 8, 2023

ddebroy commented Feb 9, 2023

johnbelamaric left a comment

johnbelamaric Feb 9, 2023

k8s-ci-robot commented Feb 9, 2023

ddebroy commented Feb 9, 2023

Update KEP with new name: PodReadyToStartContainers condition #3778

Update KEP with new name: PodReadyToStartContainers condition #3778

Conversation

ddebroy commented Jan 24, 2023

dchen1107 commented Jan 24, 2023 • edited Loading

ddebroy commented Jan 24, 2023

aojea commented Jan 25, 2023

mrunalp commented Jan 25, 2023

ddebroy commented Jan 31, 2023

mrunalp Feb 1, 2023

Choose a reason for hiding this comment

mrunalp commented Feb 1, 2023

cici37 left a comment

Choose a reason for hiding this comment

cici37 Feb 2, 2023

Choose a reason for hiding this comment

ddebroy Feb 8, 2023 • edited Loading

Choose a reason for hiding this comment

johnbelamaric Feb 8, 2023

Choose a reason for hiding this comment

ddebroy Feb 8, 2023 • edited Loading

Choose a reason for hiding this comment

johnbelamaric Feb 9, 2023

Choose a reason for hiding this comment

ddebroy commented Feb 8, 2023 • edited Loading

johnbelamaric Feb 8, 2023

Choose a reason for hiding this comment

ddebroy commented Feb 9, 2023

johnbelamaric left a comment

Choose a reason for hiding this comment

johnbelamaric Feb 9, 2023

Choose a reason for hiding this comment

k8s-ci-robot commented Feb 9, 2023

ddebroy commented Feb 9, 2023

dchen1107 commented Jan 24, 2023 •

edited

Loading

ddebroy Feb 8, 2023 •

edited

Loading

ddebroy Feb 8, 2023 •

edited

Loading

ddebroy commented Feb 8, 2023 •

edited

Loading