Use Patch instead of Put to sync pod status #62306

freehan · 2018-04-10T00:43:30Z

ref: https://github.com/kubernetes/community/blob/master/keps/sig-network/0007-pod-ready%2B%2B.md

Use Patch instead of Put to sync pod status

krmayankk · 2018-04-10T07:48:40Z

pkg/kubelet/status/status_manager.go

+
+	oldStatus := pod.Status.DeepCopy()
+	newPod, patchBytes, err := statusutil.PatchPodStatus(m.kubeClient, pod.Namespace, pod.Name, *oldStatus, mergePodStatus(*oldStatus, status.status))
+	glog.V(3).Infof("Patch status for pod %q with %q", format.Pod(pod), patchBytes)


is the pod status updated only by kubelet ? is there any reason to handle conflicts ?

krzyzacy · 2018-04-11T19:10:02Z

/retest

krzyzacy · 2018-04-11T19:11:21Z

pkg/util/pod/pod.go

+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/api/core/v1"
+	clientset "k8s.io/client-go/kubernetes"
+	"k8s.io/kubernetes/staging/src/k8s.io/apimachinery/pkg/util/strategicpatch"


pod/pod.go: import of k8s.io/kubernetes/staging/src/k8s.io/apimachinery/pkg/util/strategicpatch, which is not a direct dependency

caesarxuchao · 2018-04-11T19:36:35Z

staging/src/k8s.io/client-go/testing/fixture.go

+			if err != nil {
+				return true, nil, err
+			}
+			// Only supports strategic merge patch


I think this is ok as a starting point.

caesarxuchao · 2018-04-11T19:37:46Z

staging/src/k8s.io/client-go/testing/fixture.go

@@ -68,23 +71,22 @@ type ObjectScheme interface {

 // ObjectReaction returns a ReactionFunc that applies core.Action to
 // the given tracker.
-func ObjectReaction(tracker ObjectTracker) ReactionFunc {
+func ObjectReaction(objectTracker ObjectTracker) ReactionFunc {


Why renaming?

caesarxuchao · 2018-04-11T19:39:49Z

staging/src/k8s.io/client-go/testing/fixture.go

+				return true, nil, err
+			}
+
+			obj, err = objectTracker.Get(gvr, ns, action.GetName())


Is the Get necessary? Can you just return obj?

freehan · 2018-04-16T23:48:42Z

/assign thockin for approval

k8s-ci-robot · 2018-04-16T23:48:43Z

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

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.

freehan · 2018-04-16T23:50:19Z

@caesarxuchao @mengqiy for reviewing Patch helper function and fake kube client changes
@yujuhong for reviewing kubelet changes
@thockin for approval

k8s-github-robot · 2018-05-29T23:47:07Z

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

@caesarxuchao @freehan @mengqiy @thockin @yujuhong

Pull Request Labels

sig/network sig/node: Pull Request will be escalated to these SIGs if needed.
priority/important-soon: Escalate to the pull request owners and SIG owner; move out of milestone after several unsuccessful escalation attempts.
kind/feature: New functionality.

Help

k8s-ci-robot · 2018-05-30T00:10:15Z

New changes are detected. LGTM label has been removed.

freehan · 2018-05-30T00:10:45Z

Rebased. Reapplying labels.

freehan · 2018-05-30T03:44:07Z

/test pull-kubernetes-bazel-test

fejta-bot · 2018-05-30T05:53:18Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

fejta-bot · 2018-05-30T08:20:20Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

fejta-bot · 2018-05-30T10:47:16Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

fejta-bot · 2018-05-30T13:14:19Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

fejta-bot · 2018-05-30T16:02:19Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

…od status

k8s-ci-robot · 2018-05-30T18:34:12Z

New changes are detected. LGTM label has been removed.

freehan · 2018-05-30T18:35:46Z

Rebased. Reapplying labels

fejta-bot · 2018-05-30T22:20:18Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

k8s-github-robot · 2018-05-30T23:09:33Z

Automatic merge from submit-queue (batch tested with PRs 58920, 58327, 60577, 49388, 62306). If you want to cherry-pick this change to another branch, please follow the instructions here.

k8s-ci-robot · 2018-05-30T23:57:50Z

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

Test name	Commit	Details	Rerun command
pull-kubernetes-kubemark-e2e-gce-big	`85e0d05`	link	`/test pull-kubernetes-kubemark-e2e-gce-big`

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.

@yujuhong

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Teach Kubelet about Pod Ready++ Follow up PR of #62306 and #64057, **Only the last 3 commits are new.** Will rebase once the previous ones are merged. ref: https://github.com/kubernetes/community/blob/master/keps/sig-network/0007-pod-ready%2B%2B.md kind/feature priority/important-soon sig/network sig/node /assign @yujuhong ```release-note NONE ```

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

k8s-ci-robot requested review from dchen1107, ericchiang and yifan-gu April 10, 2018 00:43

freehan assigned yujuhong and mengqiy Apr 10, 2018

freehan force-pushed the pod-status-patch2 branch from 46eb193 to d1309b4 Compare April 10, 2018 00:47

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Apr 10, 2018

krmayankk reviewed Apr 10, 2018

View reviewed changes

freehan force-pushed the pod-status-patch2 branch from d1309b4 to 20de2bc Compare April 11, 2018 18:37

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 11, 2018

freehan force-pushed the pod-status-patch2 branch 2 times, most recently from 1fd4e4f to 42eb210 Compare April 11, 2018 19:01

krzyzacy reviewed Apr 11, 2018

View reviewed changes

freehan force-pushed the pod-status-patch2 branch from 42eb210 to 5921d9d Compare April 11, 2018 19:13

freehan assigned caesarxuchao Apr 11, 2018

caesarxuchao reviewed Apr 11, 2018

View reviewed changes

freehan force-pushed the pod-status-patch2 branch 3 times, most recently from 2a72d6e to 6ffa55f Compare April 12, 2018 00:44

k8s-ci-robot assigned thockin Apr 16, 2018

k8s-github-robot removed the milestone/needs-approval label May 29, 2018

freehan force-pushed the pod-status-patch2 branch from 9f2aef0 to bf6020f Compare May 30, 2018 00:10

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

freehan added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 30, 2018

freehan added 7 commits May 30, 2018 11:15

add utils to patch pod status

212a16e

include patch permission for kubelets

9fe2c53

change kubelet status manager to use patch instead of put to update p…

35777c3

…od status

add Patch support in fake kubeClient

8b3b4e4

fix unit tests using Patch in fake client

cb9ac04

make update

78b8633

add utils for pod condition

85e0d05

freehan force-pushed the pod-status-patch2 branch from bf6020f to 85e0d05 Compare May 30, 2018 18:34

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

freehan added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 30, 2018

k8s-github-robot merged commit ea92879 into kubernetes:master May 30, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Patch instead of Put to sync pod status #62306

Use Patch instead of Put to sync pod status #62306

freehan commented Apr 10, 2018 •

edited

Loading

krmayankk Apr 10, 2018

krzyzacy commented Apr 11, 2018

krzyzacy Apr 11, 2018

caesarxuchao Apr 11, 2018

caesarxuchao Apr 11, 2018

caesarxuchao Apr 11, 2018

freehan commented Apr 16, 2018

k8s-ci-robot commented Apr 16, 2018

freehan commented Apr 16, 2018

k8s-github-robot commented May 29, 2018

k8s-ci-robot commented May 30, 2018

freehan commented May 30, 2018

freehan commented May 30, 2018

fejta-bot commented May 30, 2018

fejta-bot commented May 30, 2018

fejta-bot commented May 30, 2018

fejta-bot commented May 30, 2018

fejta-bot commented May 30, 2018

k8s-ci-robot commented May 30, 2018

freehan commented May 30, 2018

fejta-bot commented May 30, 2018

k8s-github-robot commented May 30, 2018

k8s-ci-robot commented May 30, 2018

Use Patch instead of Put to sync pod status #62306

Use Patch instead of Put to sync pod status #62306

Conversation

freehan commented Apr 10, 2018 • edited Loading

krmayankk Apr 10, 2018

Choose a reason for hiding this comment

krzyzacy commented Apr 11, 2018

krzyzacy Apr 11, 2018

Choose a reason for hiding this comment

caesarxuchao Apr 11, 2018

Choose a reason for hiding this comment

caesarxuchao Apr 11, 2018

Choose a reason for hiding this comment

caesarxuchao Apr 11, 2018

Choose a reason for hiding this comment

freehan commented Apr 16, 2018

k8s-ci-robot commented Apr 16, 2018

freehan commented Apr 16, 2018

k8s-github-robot commented May 29, 2018

k8s-ci-robot commented May 30, 2018

freehan commented May 30, 2018

freehan commented May 30, 2018

fejta-bot commented May 30, 2018

fejta-bot commented May 30, 2018

fejta-bot commented May 30, 2018

fejta-bot commented May 30, 2018

fejta-bot commented May 30, 2018

k8s-ci-robot commented May 30, 2018

freehan commented May 30, 2018

fejta-bot commented May 30, 2018

k8s-github-robot commented May 30, 2018

k8s-ci-robot commented May 30, 2018

freehan commented Apr 10, 2018 •

edited

Loading