Add ContainersReady condition into Pod Status #64646

freehan · 2018-06-02T01:36:07Z

Last 3 commits are new

Follow up PR of: #64057 and #64344

Have a single PR for adding ContainersReady per #64344 (comment)

Introduce ContainersReady condition in Pod Status

/assign yujuhong for review
/assign thockin for the tiny API change

dixudx

Mostly lgtm. Some nits.

Moreover, lastTransitionTime is missing for new PodConditionType ContainersReady. You have to
add updateLastTransitionTime(&status, &oldStatus, v1.ContainersReady) in method updateStatusInternal (pkg/kubelet/status/status_manager.go).

dixudx · 2018-06-02T06:09:45Z

pkg/api/v1/pod/util.go

+	if conditions == nil {
+		return -1, nil
+	}
+	for i := range conditions {


combine L266 with L269? So we can have one return -1, nil.

dixudx · 2018-06-02T07:10:29Z

staging/src/k8s.io/api/core/v1/generated.proto

+  // all conditions specified in the readiness gates have status equal to "True"
+  // More info: https://github.com/kubernetes/community/blob/master/keps/sig-network/0007-pod-ready%2B%2B.md
+  // +optional
+  repeated PodReadinessGate readinessGates = 28;


change to 27?

dixudx · 2018-06-02T07:11:04Z

staging/src/k8s.io/api/core/v1/types.go

+	// all conditions specified in the readiness gates have status equal to "True"
+	// More info: https://github.com/kubernetes/community/blob/master/keps/sig-network/0007-pod-ready%2B%2B.md
+	// +optional
+	ReadinessGates []PodReadinessGate `json:"readinessGates,omitempty" protobuf:"bytes,28,opt,name=readinessGates"`


change 28 to 27?

27 is already taken

dixudx · 2018-06-02T07:42:53Z

pkg/kubelet/kubelet_pods.go

@@ -1407,6 +1408,11 @@ func (kl *Kubelet) convertStatusToAPIStatus(pod *v1.Pod, podStatus *kubecontaine
 		true,
 	)

+	for _, c := range pod.Status.Conditions {


Better add a comment for this.

Like, "preserve non-system condition types". WDYT?

yujuhong

@freehan could you remove the readiness gate commits from this PR?

yujuhong · 2018-06-04T17:22:58Z

pkg/api/v1/pod/util.go

+	return GetPodConditionFromList(status.Conditions, conditionType)
+}
+
+// GetPodConditionFromList extracts the provided condition from the given list of condition and returns that.


s/and returns that./and returns the index of the condition and the condition.

yujuhong · 2018-06-04T17:25:07Z

pkg/api/v1/pod/util.go

+}
+
+// GetPodConditionFromList extracts the provided condition from the given list of condition and returns that.
+// Returns nil and -1 if the condition is not present, and the index of the located condition.


s/Returns nil and -1/Returns -1 and nil
Remove , and the index of the located condition.

yujuhong · 2018-06-04T17:25:52Z

pkg/apis/core/types.go

@@ -2068,6 +2068,8 @@ const (
 	// PodReasonUnschedulable reason in PodScheduled PodCondition means that the scheduler
 	// can't schedule the pod right now, for example due to insufficient resources in the cluster.
 	PodReasonUnschedulable = "Unschedulable"
+	// ContainersReady indicates whether all containers in the pod are ready


nit: end the comment with a period (.)

yujuhong · 2018-06-04T17:27:18Z

pkg/apis/core/types.go

@@ -2409,6 +2411,12 @@ const (
 	TolerationOpEqual  TolerationOperator = "Equal"
 )

+// PodReadinessGate contains the reference to a pod condition


Not sure why the add ReadinessGates in pod spec commit is in this PR...

Because it was not merged at the time...

freehan · 2018-06-04T21:11:20Z

/assign thockin for approval

k8s-ci-robot · 2018-06-04T21:11:21Z

@freehan: GitHub didn't allow me to assign the following users: for, approval.

Note that only kubernetes members and repo collaborators can be assigned.

In response to this:

/assign thockin for approval

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.

yujuhong

Will review after the dependency is merged. Adding this to prevent the PR from merging.

thockin · 2018-06-04T22:14:02Z

Thanks @freehan and @yujuhong

jberkus · 2018-06-05T00:22:07Z

So, is this PR required if the other PodReady PRs merge?

You need some more labels here:

/sig network
/priority important-soon

freehan · 2018-06-05T18:25:30Z

So, is this PR required if the other PodReady PRs merge?

Yes. This PR completes the proposal.
@jberkus

colemickens · 2018-06-05T20:54:03Z

Hey folks, from the comments, it sounds like this is desired for 1.11, but it's not labeled correctly for that to happen, and there are additional labels needed if it doesn't merge today before the beginning of code freeze. Please ensure the labels for the milestone and urgency are set correctly.

jberkus · 2018-06-05T21:10:06Z

Going to bump to critical-urgent based on @freehan comment

/priority critical-urgent

yujuhong

/lgtm

thockin · 2018-06-05T21:51:18Z

/approve

k8s-ci-robot · 2018-06-05T21:51:24Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: freehan, thockin, yujuhong

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:

~~pkg/apis/core/OWNERS~~ [thockin]
~~pkg/kubelet/OWNERS~~ [thockin,yujuhong]
~~staging/src/k8s.io/api/OWNERS~~ [thockin]

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

k8s-github-robot · 2018-06-06T00:50:12Z

[MILESTONENOTIFIER] Milestone Pull Request: Up-to-date for process

@freehan @thockin @yujuhong

Pull Request Labels

sig/network: Pull Request will be escalated to these SIGs if needed.
priority/critical-urgent: Never automatically move pull request out of a release milestone; continually escalate to contributor and SIG through all available channels.
kind/feature: New functionality.

Help

k8s-github-robot · 2018-06-06T07:47:58Z

/test all [submit-queue is verifying that this PR is safe to merge]

k8s-github-robot · 2018-06-06T08:24:13Z

Automatic merge from submit-queue (batch tested with PRs 63717, 64646, 64792, 64784, 64800). If you want to cherry-pick this change to another branch, please follow the instructions here.

k8s-ci-robot · 2018-06-06T08:33:19Z

@freehan: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-kubernetes-e2e-kops-aws	`370268f`	link	`/test pull-kubernetes-e2e-kops-aws`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

zparnold · 2018-06-13T15:01:39Z

Hello there! @freehan I'm Zach Arnold working on Docs for the 1.11 release. This PR was identified as one needing some documentation in the https://github.com/kubernetes/website repo around your contributions (thanks by the way!) When you have some time, could you please modify/add/remove the relevant content that needs changing in our documentation repo? Thanks! Please let me or my colleague Misty know (@zparnold/@misty on K8s Slack) if you need any assistance with the documentation.

lucperkins · 2018-06-22T00:28:29Z

@freehan Please note that I've created a PR for this in the website repo: kubernetes/website#9197.

Please let me know if there'is any usage-related info that I should be aware of. Thus far I've added it to the list of possible Pod statuses but would love to add more than that.

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here: https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md. add missing LastTransitionTime of ContainerReady condition **What this PR does / why we need it**: add missing LastTransitionTime of ContainerReady condition **Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*: xref #64646 **Special notes for your reviewer**: /cc freehan yujuhong **Release note**: ```release-note add missing LastTransitionTime of ContainerReady condition ```

timchenxiaoyu · 2018-11-22T02:56:56Z

we hava pod container ready condition，i don't know， why we need ContainersReady？

freehan added this to the v1.11 milestone Jun 2, 2018

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. release-note Denotes a PR that will be considered when it comes time to generate release notes. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 2, 2018

k8s-ci-robot requested review from caesarxuchao and gmarek June 2, 2018 01:36

k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API milestone/incomplete-labels labels Jun 2, 2018

dixudx reviewed Jun 2, 2018

View reviewed changes

yujuhong reviewed Jun 4, 2018

View reviewed changes

yujuhong mentioned this pull request Jun 4, 2018

Teach Kubelet about Pod Ready++ #64344

Merged

freehan force-pushed the pod-ready-plus2-new branch from c4158c0 to 2d8695b Compare June 4, 2018 19:19

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 4, 2018

freehan assigned yujuhong Jun 4, 2018

freehan force-pushed the pod-ready-plus2-new branch from 2d8695b to 3d868b8 Compare June 4, 2018 20:50

k8s-ci-robot assigned thockin Jun 4, 2018

yujuhong added the status/approved-for-milestone label Jun 4, 2018

yujuhong suggested changes Jun 4, 2018

View reviewed changes

k8s-ci-robot added sig/network Categorizes an issue or PR as relevant to SIG Network. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Jun 5, 2018

k8s-github-robot added milestone/removed and removed milestone/incomplete-labels labels Jun 5, 2018

k8s-github-robot removed this from the v1.11 milestone Jun 5, 2018

freehan added 2 commits June 5, 2018 11:10

add ContainersReady condition

6b08ef5

Generate ContainersReady condition

176f34e

Inject ContainersReady

370268f

freehan force-pushed the pod-ready-plus2-new branch from 3d868b8 to 370268f Compare June 5, 2018 18:11

k8s-ci-robot added priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Jun 5, 2018

yujuhong approved these changes Jun 5, 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 Jun 5, 2018

freehan added this to the v1.11 milestone Jun 6, 2018

k8s-github-robot added milestone/incomplete-labels and removed milestone/removed labels Jun 6, 2018

freehan added the kind/feature Categorizes issue or PR as related to a new feature. label Jun 6, 2018

yujuhong removed the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Jun 6, 2018

k8s-github-robot removed the milestone/incomplete-labels label Jun 6, 2018

k8s-github-robot merged commit 0b8394a into kubernetes:master Jun 6, 2018

dixudx mentioned this pull request Jun 7, 2018

add missing LastTransitionTime of ContainerReady condition #64867

Merged

tengqm mentioned this pull request Jun 16, 2018

Document the ContainersReady condition in Pod Status kubernetes/website#9099

Closed

1 task

mimowo mentioned this pull request Jul 28, 2022

Append new pod conditions when deleting pods to indicate the reason for pod deletion #110959

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ContainersReady condition into Pod Status #64646

Add ContainersReady condition into Pod Status #64646

freehan commented Jun 2, 2018

dixudx left a comment

dixudx Jun 2, 2018

dixudx Jun 2, 2018

dixudx Jun 2, 2018

freehan Jun 4, 2018

dixudx Jun 2, 2018

freehan Jun 4, 2018

yujuhong left a comment

yujuhong Jun 4, 2018

yujuhong Jun 4, 2018

yujuhong Jun 4, 2018

yujuhong Jun 4, 2018

freehan Jun 4, 2018

freehan commented Jun 4, 2018

k8s-ci-robot commented Jun 4, 2018

yujuhong left a comment

thockin commented Jun 4, 2018

jberkus commented Jun 5, 2018

freehan commented Jun 5, 2018

colemickens commented Jun 5, 2018

jberkus commented Jun 5, 2018

yujuhong left a comment

thockin commented Jun 5, 2018

k8s-ci-robot commented Jun 5, 2018

k8s-github-robot commented Jun 6, 2018

k8s-github-robot commented Jun 6, 2018

k8s-github-robot commented Jun 6, 2018

k8s-ci-robot commented Jun 6, 2018 •

edited

Loading

zparnold commented Jun 13, 2018

lucperkins commented Jun 22, 2018

timchenxiaoyu commented Nov 22, 2018

Add ContainersReady condition into Pod Status #64646

Add ContainersReady condition into Pod Status #64646

Conversation

freehan commented Jun 2, 2018

dixudx 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

Choose a reason for hiding this comment

yujuhong 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

freehan commented Jun 4, 2018

k8s-ci-robot commented Jun 4, 2018

yujuhong left a comment

Choose a reason for hiding this comment

thockin commented Jun 4, 2018

jberkus commented Jun 5, 2018

freehan commented Jun 5, 2018

colemickens commented Jun 5, 2018

jberkus commented Jun 5, 2018

yujuhong left a comment

Choose a reason for hiding this comment

thockin commented Jun 5, 2018

k8s-ci-robot commented Jun 5, 2018

k8s-github-robot commented Jun 6, 2018

k8s-github-robot commented Jun 6, 2018

k8s-github-robot commented Jun 6, 2018

k8s-ci-robot commented Jun 6, 2018 • edited Loading

zparnold commented Jun 13, 2018

lucperkins commented Jun 22, 2018

timchenxiaoyu commented Nov 22, 2018

k8s-ci-robot commented Jun 6, 2018 •

edited

Loading