From a65d8aeb7622a56b373f9d5c3762c07a4b776d21 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Mon, 16 Dec 2019 14:27:32 -0500 Subject: [PATCH] Add UID precondition to kubelet pod status patch updates --- pkg/kubelet/status/status_manager.go | 4 ++-- pkg/util/pod/BUILD | 2 ++ pkg/util/pod/pod.go | 12 +++++++----- pkg/util/pod/pod_test.go | 21 ++++++++++++--------- 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/pkg/kubelet/status/status_manager.go b/pkg/kubelet/status/status_manager.go index 3029d926c1d..1cb34e06a41 100644 --- a/pkg/kubelet/status/status_manager.go +++ b/pkg/kubelet/status/status_manager.go @@ -24,7 +24,7 @@ import ( clientset "k8s.io/client-go/kubernetes" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -540,7 +540,7 @@ func (m *manager) syncPod(uid types.UID, status versionedPodStatus) { } oldStatus := pod.Status.DeepCopy() - newPod, patchBytes, err := statusutil.PatchPodStatus(m.kubeClient, pod.Namespace, pod.Name, *oldStatus, mergePodStatus(*oldStatus, status.status)) + newPod, patchBytes, err := statusutil.PatchPodStatus(m.kubeClient, pod.Namespace, pod.Name, pod.UID, *oldStatus, mergePodStatus(*oldStatus, status.status)) klog.V(3).Infof("Patch status for pod %q with %q", format.Pod(pod), patchBytes) if err != nil { klog.Warningf("Failed to update status for pod %q: %v", format.Pod(pod), err) diff --git a/pkg/util/pod/BUILD b/pkg/util/pod/BUILD index 41921752595..19dadbd6f86 100644 --- a/pkg/util/pod/BUILD +++ b/pkg/util/pod/BUILD @@ -7,6 +7,7 @@ go_library( visibility = ["//visibility:public"], deps = [ "//staging/src/k8s.io/api/core/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/strategicpatch:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", @@ -20,6 +21,7 @@ go_test( deps = [ "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", "//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library", ], ) diff --git a/pkg/util/pod/pod.go b/pkg/util/pod/pod.go index 079a9b63c9a..984f0ea673d 100644 --- a/pkg/util/pod/pod.go +++ b/pkg/util/pod/pod.go @@ -20,15 +20,16 @@ import ( "encoding/json" "fmt" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/strategicpatch" clientset "k8s.io/client-go/kubernetes" ) // PatchPodStatus patches pod status. -func PatchPodStatus(c clientset.Interface, namespace, name string, oldPodStatus, newPodStatus v1.PodStatus) (*v1.Pod, []byte, error) { - patchBytes, err := preparePatchBytesForPodStatus(namespace, name, oldPodStatus, newPodStatus) +func PatchPodStatus(c clientset.Interface, namespace, name string, uid types.UID, oldPodStatus, newPodStatus v1.PodStatus) (*v1.Pod, []byte, error) { + patchBytes, err := preparePatchBytesForPodStatus(namespace, name, uid, oldPodStatus, newPodStatus) if err != nil { return nil, nil, err } @@ -40,7 +41,7 @@ func PatchPodStatus(c clientset.Interface, namespace, name string, oldPodStatus, return updatedPod, patchBytes, nil } -func preparePatchBytesForPodStatus(namespace, name string, oldPodStatus, newPodStatus v1.PodStatus) ([]byte, error) { +func preparePatchBytesForPodStatus(namespace, name string, uid types.UID, oldPodStatus, newPodStatus v1.PodStatus) ([]byte, error) { oldData, err := json.Marshal(v1.Pod{ Status: oldPodStatus, }) @@ -49,7 +50,8 @@ func preparePatchBytesForPodStatus(namespace, name string, oldPodStatus, newPodS } newData, err := json.Marshal(v1.Pod{ - Status: newPodStatus, + ObjectMeta: metav1.ObjectMeta{UID: uid}, // only put the uid in the new object to ensure it appears in the patch as a precondition + Status: newPodStatus, }) if err != nil { return nil, fmt.Errorf("failed to Marshal newData for pod %q/%q: %v", namespace, name, err) diff --git a/pkg/util/pod/pod_test.go b/pkg/util/pod/pod_test.go index af0278fa090..9c2b7e4f11a 100644 --- a/pkg/util/pod/pod_test.go +++ b/pkg/util/pod/pod_test.go @@ -17,18 +17,21 @@ limitations under the License. package pod import ( + "fmt" "testing" - "fmt" - "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes/fake" "reflect" + + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes/fake" ) func TestPatchPodStatus(t *testing.T) { ns := "ns" name := "name" + uid := types.UID("myuid") client := &fake.Clientset{} client.CoreV1().Pods(ns).Create(&v1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -45,7 +48,7 @@ func TestPatchPodStatus(t *testing.T) { { "no change", func(input v1.PodStatus) v1.PodStatus { return input }, - []byte(fmt.Sprintf(`{}`)), + []byte(fmt.Sprintf(`{"metadata":{"uid":"myuid"}}`)), }, { "message change", @@ -53,7 +56,7 @@ func TestPatchPodStatus(t *testing.T) { input.Message = "random message" return input }, - []byte(fmt.Sprintf(`{"status":{"message":"random message"}}`)), + []byte(fmt.Sprintf(`{"metadata":{"uid":"myuid"},"status":{"message":"random message"}}`)), }, { "pod condition change", @@ -61,7 +64,7 @@ func TestPatchPodStatus(t *testing.T) { input.Conditions[0].Status = v1.ConditionFalse return input }, - []byte(fmt.Sprintf(`{"status":{"$setElementOrder/conditions":[{"type":"Ready"},{"type":"PodScheduled"}],"conditions":[{"status":"False","type":"Ready"}]}}`)), + []byte(fmt.Sprintf(`{"metadata":{"uid":"myuid"},"status":{"$setElementOrder/conditions":[{"type":"Ready"},{"type":"PodScheduled"}],"conditions":[{"status":"False","type":"Ready"}]}}`)), }, { "additional init container condition", @@ -74,11 +77,11 @@ func TestPatchPodStatus(t *testing.T) { } return input }, - []byte(fmt.Sprintf(`{"status":{"initContainerStatuses":[{"image":"","imageID":"","lastState":{},"name":"init-container","ready":true,"restartCount":0,"state":{}}]}}`)), + []byte(fmt.Sprintf(`{"metadata":{"uid":"myuid"},"status":{"initContainerStatuses":[{"image":"","imageID":"","lastState":{},"name":"init-container","ready":true,"restartCount":0,"state":{}}]}}`)), }, } for _, tc := range testCases { - _, patchBytes, err := PatchPodStatus(client, ns, name, getPodStatus(), tc.mutate(getPodStatus())) + _, patchBytes, err := PatchPodStatus(client, ns, name, uid, getPodStatus(), tc.mutate(getPodStatus())) if err != nil { t.Errorf("unexpected error: %v", err) }