add KEP for pod ready++ #1991

freehan · 2018-03-30T00:10:08Z

KEP for pod ready++

jbeda · 2018-04-01T04:09:10Z

I encourage you to get this PR submitted once @thockin and/or @dchen1107 approve of the scope. Err on the side of smaller targeted PRs that take on a single topic. Once things have settled create a PR to move it from "provisional" to "implementable". That provides a "last call" to make sure that everyone is on the same page. The goal is to avoid 100+ comment PRs that tackle all topics at once.

thockin · 2018-04-03T17:33:32Z

keps/sig-network/0007-pod-ready++.md

+
+### Implementation Details/Notes/Constraints
+
+This proposal mostly involves kubelet changes:


I would detail that the ready condition must continue to be the FINAL signal for controllers to proceed (for compat) which complicates the design overall.

MAYBE also add a short blurb about all-containers-ready (e.g. for endpoints) vs pod-ready? Maybe that belongs in the doc (but sharing docs with community sucks)

thockin · 2018-04-03T17:34:50Z

keps/sig-network/0007-pod-ready++.md

+
+## Proposal
+
+[K8s Proposal: Pod Ready++](https://docs.google.com/document/d/1VFZbc_IqPf_Msd-jul7LKTmGjvQ5qRldYOFV0lGqxf8/edit#)


Is it worth converting that to markdown to be pasted inline here? What is the precedent @calebamiles ?

I'd vote for adding it here at least once the outstanding comments on the doc have been resolved - I think there's value in consistent use of git source control for tracking changes.

+1 for markdown here in git

caseydavenport · 2018-04-04T22:44:36Z

keps/sig-network/0007-pod-ready++.md

+
+## Table of Contents
+
+A table of contents is helpful for quickly jumping to sections of a KEP and for highlighting any additional information provided beyond the standard KEP template.


Probably can drop the "Why we need a table of contents" text here :)

caseydavenport · 2018-04-04T22:50:34Z

keps/sig-network/0007-pod-ready++.md

+
+## Proposal
+
+[K8s Proposal: Pod Ready++](https://docs.google.com/document/d/1VFZbc_IqPf_Msd-jul7LKTmGjvQ5qRldYOFV0lGqxf8/edit#)


I'd vote for adding it here at least once the outstanding comments on the doc have been resolved - I think there's value in consistent use of git source control for tracking changes.

caseydavenport · 2018-04-04T23:29:20Z

keps/sig-network/0007-pod-ready++.md

+
+### Goals
+
+- Allow extra signals for pod readiness.


Perhaps there are a set of signals that we should implement as part of completing this KEP and as part of proving the API?

I added a user adoption item in the graduation criteria. I would not tied this proposal with specific feature for now.

MikeSpreitzer · 2018-04-05T18:08:35Z

keps/sig-network/0007-pod-ready++.md

+
+## Summary
+
+This proposal aims to add extensibility to pod readiness. Besides container readiness, external feedback can be injected into PodStatus and influence pod readiness. Thus, achieving pod “ready++”. 


I think a better summary is to recognize that what will really happen is to split the concept of "pod ready" into two pieces, for two different sets of consumers. The current text focuses one one set of consumers, the "workload controllers" that want "pod ready ++". The other set of consumers is the load balancers, network policy implementors, and so on that contribute to the "pod ready ++" based on "plain old pod ready". Remember, a load balancer (for example) has to react to a pod before all the load balancers and whatever have reacted to the pod. Following is a suggested rewording.

This proposal splits the concept of pod readiness into two concepts, for two distinct sets of consumers. One set of consumers is the workload controllers and other clients that simply want to know when a pod is fully ready for use. They care about a concept of readiness that we may call "ready++" and is stricter than the concept that is supported today; ready++ also indicates that all the relevant load balancers, traffic filters, and whatever have been adjusted to account for the pod. The other set of consumers is those very load balancers, traffic filters, and whatever. They need to continue to react to today's looser concept of readiness, while contributing to the stricter concept. For example, a load balancer needs to put a new pod into its backend set without waiting for ready++, while also contributing its own bit into ready++.

MikeSpreitzer · 2018-04-05T19:30:28Z

This propsoal should explicitly explain how load balancers, NetworkPolicy implementations, and so on will continue to sense "plain old pod ready".

freehan · 2018-04-05T22:35:53Z

Ported the proposal from google doc into the PR. PTAL

krmayankk · 2018-04-10T07:30:30Z

keps/sig-network/0007-pod-ready++.md

+For the workloads that take pod readiness as a critical signal for its decision making, they will automatically comply with this proposal without any change. Majority, if not all, of the workloads satisfy this condition. 
+
+##### Kubelet
+- Use PATCH instead of PUT to update PodStatus fields that are dictated by kubelet. 


What is the advantage of using Patch instead of Put here ?

Avoid conflicts. Similar to node status.

krmayankk · 2018-04-10T07:41:30Z

keps/sig-network/0007-pod-ready++.md

+  - lastProbeTime: null
+    lastTransitionTime: 2018-01-01T00:00:00Z
+    status: "False"
+    type: www.example.com/feature-1


is this documented somewhere on how to create custom Conditions ?

Patch is the way to go.

krmayankk · 2018-04-10T07:43:44Z

keps/sig-network/0007-pod-ready++.md

+
+Pod readiness indicates whether the pod is ready to serve traffic. Pod readiness is dictated by kubelet with user specified readiness probe. On the other hand, pod readiness determines whether pod address shows up on the address list on related endpoints object. K8s primitives that manage pods, such as Deployment, only takes pod status into account for decision making, such as advancement during rolling update. 
+
+For example, during deployment rolling update, a new pod becomes ready. On the other hand, service, network policy and load-balancer are not yet ready for the new pod due to whatever reason (e.g. slowness in api machinery, endpoints controller, kube-proxy, iptables or infrastructure programming). This may cause service disruption or lost of backend capacity. In extreme cases, if rolling update completes before any new replacement pod actually start serving traffic, this will cause service outage. 


how does this cause loss of backend capacity ?

pod ready does not mean pod is serving traffic. Hence loss of backend capacity.

@freehan I mean how is this part better than without this feature . Overall I agree just want to understand the reasoning here

thockin · 2018-04-10T23:58:27Z

keps/sig-network/0007-pod-ready++.md

+… 
+```
+
+Another pod condition `ContainerReady` will be introduced to capture the old pod `Ready` condition. 


ContainersReady (plural) I think

But there may be on one container in pod :P

thockin · 2018-04-10T23:59:25Z

keps/sig-network/0007-pod-ready++.md

+Custom pod condition can be injected thru PATCH action using KubeClient. Please be noted that “kubectl patch” does not support patching object status. Need to use client-go or other KubeClient implementations. 
+
+Naming Convention:
+The type of custom pod condition must comply with k8s label key format. For example, “www.example.com/feature-1”.


Will we allow "naked" named or do we require the prefix?

thockin · 2018-04-11T00:01:05Z

May be worth documenting why we don't want to allow updates.

thockin · 2018-04-11T00:01:19Z

/lgtm
/approve

k8s-ci-robot · 2018-04-11T00:01:26Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: thockin

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/sig-network/OWNERS~~ [thockin]

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

krmayankk · 2018-04-11T20:20:55Z

keps/sig-network/0007-pod-ready++.md

+
+Another pod condition `ContainerReady` will be introduced to capture the old pod `Ready` condition. 
+```
+ContainerReady is true == containers are ready


Will this replace the old Ready condition or in addition. ? It has to be in addition otherwise this is a breaking change

add KEP for pod ready++

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

k8s-ci-robot requested review from caseydavenport and thockin March 30, 2018 00:10

k8s-github-robot added sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/network Categorizes an issue or PR as relevant to SIG Network. labels Mar 30, 2018

freehan force-pushed the pod-ready branch from 760680a to c4d403e Compare March 30, 2018 21:26

thockin self-assigned this Mar 30, 2018

thockin reviewed Apr 3, 2018

View reviewed changes

caseydavenport reviewed Apr 4, 2018

View reviewed changes

MikeSpreitzer reviewed Apr 5, 2018

View reviewed changes

freehan force-pushed the pod-ready branch from c4d403e to 22418ff Compare April 5, 2018 22:20

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 5, 2018

freehan force-pushed the pod-ready branch from 22418ff to 4c50c17 Compare April 5, 2018 22:26

add KEP for pod ready++

eded88e

freehan force-pushed the pod-ready branch from 4c50c17 to eded88e Compare April 5, 2018 22:35

freehan mentioned this pull request Apr 10, 2018

Use Patch instead of Put to sync pod status kubernetes/kubernetes#62306

Merged

krmayankk reviewed Apr 10, 2018

View reviewed changes

thockin reviewed Apr 10, 2018

View reviewed changes

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

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

k8s-ci-robot merged commit 28549b7 into kubernetes:master Apr 11, 2018

krmayankk reviewed Apr 11, 2018

View reviewed changes

derekwaynecarr mentioned this pull request May 3, 2018

If a node status is unknown, overwrite, don't patch. kubernetes/kubernetes#60827

Closed

calebamiles pushed a commit to calebamiles/community that referenced this pull request Sep 5, 2018

Merge pull request kubernetes#1991 from freehan/pod-ready

d7c19a9

add KEP for pod ready++

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add KEP for pod ready++ #1991

add KEP for pod ready++ #1991

freehan commented Mar 30, 2018

jbeda commented Apr 1, 2018

thockin Apr 3, 2018

thockin Apr 3, 2018

caseydavenport Apr 4, 2018

dcbw Apr 5, 2018

caseydavenport Apr 4, 2018

caseydavenport Apr 4, 2018

caseydavenport Apr 4, 2018

freehan Apr 5, 2018 •

edited

Loading

MikeSpreitzer Apr 5, 2018

MikeSpreitzer commented Apr 5, 2018

freehan commented Apr 5, 2018

krmayankk Apr 10, 2018

freehan Apr 10, 2018

krmayankk Apr 10, 2018

freehan Apr 10, 2018

krmayankk Apr 10, 2018

freehan Apr 10, 2018

krmayankk Apr 11, 2018

thockin Apr 10, 2018

freehan Apr 11, 2018

thockin Apr 10, 2018

thockin commented Apr 11, 2018

thockin commented Apr 11, 2018

k8s-ci-robot commented Apr 11, 2018

krmayankk Apr 11, 2018


		### Implementation Details/Notes/Constraints

		This proposal mostly involves kubelet changes:


		## Proposal

		[K8s Proposal: Pod Ready++](https://docs.google.com/document/d/1VFZbc_IqPf_Msd-jul7LKTmGjvQ5qRldYOFV0lGqxf8/edit#)


		## Table of Contents

		A table of contents is helpful for quickly jumping to sections of a KEP and for highlighting any additional information provided beyond the standard KEP template.


		## Summary

		This proposal aims to add extensibility to pod readiness. Besides container readiness, external feedback can be injected into PodStatus and influence pod readiness. Thus, achieving pod “ready++”.


		Pod readiness indicates whether the pod is ready to serve traffic. Pod readiness is dictated by kubelet with user specified readiness probe. On the other hand, pod readiness determines whether pod address shows up on the address list on related endpoints object. K8s primitives that manage pods, such as Deployment, only takes pod status into account for decision making, such as advancement during rolling update.

		For example, during deployment rolling update, a new pod becomes ready. On the other hand, service, network policy and load-balancer are not yet ready for the new pod due to whatever reason (e.g. slowness in api machinery, endpoints controller, kube-proxy, iptables or infrastructure programming). This may cause service disruption or lost of backend capacity. In extreme cases, if rolling update completes before any new replacement pod actually start serving traffic, this will cause service outage.

add KEP for pod ready++ #1991

add KEP for pod ready++ #1991

Conversation

freehan commented Mar 30, 2018

jbeda commented Apr 1, 2018

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

Choose a reason for hiding this comment

freehan Apr 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MikeSpreitzer commented Apr 5, 2018

freehan commented Apr 5, 2018

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

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

thockin commented Apr 11, 2018

thockin commented Apr 11, 2018

k8s-ci-robot commented Apr 11, 2018

Choose a reason for hiding this comment

freehan Apr 5, 2018 •

edited

Loading