kubelet: Avoid sending no-op patches

In an e2e run, out of 1857 pod status updates executed by the
Kubelet 453 (25%) were no-ops - they only contained the UID of
the pod and no status changes. If the patch is a no-op we can
avoid invoking the server and continue.
This commit is contained in:
Clayton Coleman
2020-02-26 17:05:33 -05:00
parent 5ceddce539
commit b252865479
4 changed files with 51 additions and 29 deletions

View File

@@ -17,6 +17,7 @@ limitations under the License.
package pod
import (
"bytes"
"context"
"encoding/json"
"fmt"
@@ -28,26 +29,29 @@ import (
clientset "k8s.io/client-go/kubernetes"
)
// PatchPodStatus patches pod status.
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)
// PatchPodStatus patches pod status. It returns true and avoids an update if the patch contains no changes.
func PatchPodStatus(c clientset.Interface, namespace, name string, uid types.UID, oldPodStatus, newPodStatus v1.PodStatus) (*v1.Pod, []byte, bool, error) {
patchBytes, unchanged, err := preparePatchBytesForPodStatus(namespace, name, uid, oldPodStatus, newPodStatus)
if err != nil {
return nil, nil, err
return nil, nil, false, err
}
if unchanged {
return nil, patchBytes, true, nil
}
updatedPod, err := c.CoreV1().Pods(namespace).Patch(context.TODO(), name, types.StrategicMergePatchType, patchBytes, metav1.PatchOptions{}, "status")
if err != nil {
return nil, nil, fmt.Errorf("failed to patch status %q for pod %q/%q: %v", patchBytes, namespace, name, err)
return nil, nil, false, fmt.Errorf("failed to patch status %q for pod %q/%q: %v", patchBytes, namespace, name, err)
}
return updatedPod, patchBytes, nil
return updatedPod, patchBytes, false, nil
}
func preparePatchBytesForPodStatus(namespace, name string, uid types.UID, oldPodStatus, newPodStatus v1.PodStatus) ([]byte, error) {
func preparePatchBytesForPodStatus(namespace, name string, uid types.UID, oldPodStatus, newPodStatus v1.PodStatus) ([]byte, bool, error) {
oldData, err := json.Marshal(v1.Pod{
Status: oldPodStatus,
})
if err != nil {
return nil, fmt.Errorf("failed to Marshal oldData for pod %q/%q: %v", namespace, name, err)
return nil, false, fmt.Errorf("failed to Marshal oldData for pod %q/%q: %v", namespace, name, err)
}
newData, err := json.Marshal(v1.Pod{
@@ -55,12 +59,12 @@ func preparePatchBytesForPodStatus(namespace, name string, uid types.UID, oldPod
Status: newPodStatus,
})
if err != nil {
return nil, fmt.Errorf("failed to Marshal newData for pod %q/%q: %v", namespace, name, err)
return nil, false, fmt.Errorf("failed to Marshal newData for pod %q/%q: %v", namespace, name, err)
}
patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, v1.Pod{})
if err != nil {
return nil, fmt.Errorf("failed to CreateTwoWayMergePatch for pod %q/%q: %v", namespace, name, err)
return nil, false, fmt.Errorf("failed to CreateTwoWayMergePatch for pod %q/%q: %v", namespace, name, err)
}
return patchBytes, nil
return patchBytes, bytes.Equal(patchBytes, []byte(fmt.Sprintf(`{"metadata":{"uid":%q}}`, uid))), nil
}

View File

@@ -44,11 +44,13 @@ func TestPatchPodStatus(t *testing.T) {
testCases := []struct {
description string
mutate func(input v1.PodStatus) v1.PodStatus
expectUnchanged bool
expectedPatchBytes []byte
}{
{
"no change",
func(input v1.PodStatus) v1.PodStatus { return input },
true,
[]byte(fmt.Sprintf(`{"metadata":{"uid":"myuid"}}`)),
},
{
@@ -57,6 +59,7 @@ func TestPatchPodStatus(t *testing.T) {
input.Message = "random message"
return input
},
false,
[]byte(fmt.Sprintf(`{"metadata":{"uid":"myuid"},"status":{"message":"random message"}}`)),
},
{
@@ -65,6 +68,7 @@ func TestPatchPodStatus(t *testing.T) {
input.Conditions[0].Status = v1.ConditionFalse
return input
},
false,
[]byte(fmt.Sprintf(`{"metadata":{"uid":"myuid"},"status":{"$setElementOrder/conditions":[{"type":"Ready"},{"type":"PodScheduled"}],"conditions":[{"status":"False","type":"Ready"}]}}`)),
},
{
@@ -78,17 +82,23 @@ func TestPatchPodStatus(t *testing.T) {
}
return input
},
false,
[]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, uid, getPodStatus(), tc.mutate(getPodStatus()))
if err != nil {
t.Errorf("unexpected error: %v", err)
}
if !reflect.DeepEqual(patchBytes, tc.expectedPatchBytes) {
t.Errorf("for test case %q, expect patchBytes: %q, got: %q\n", tc.description, tc.expectedPatchBytes, patchBytes)
}
t.Run(tc.description, func(t *testing.T) {
_, patchBytes, unchanged, err := PatchPodStatus(client, ns, name, uid, getPodStatus(), tc.mutate(getPodStatus()))
if err != nil {
t.Errorf("unexpected error: %v", err)
}
if unchanged != tc.expectUnchanged {
t.Errorf("unexpected change: %t", unchanged)
}
if !reflect.DeepEqual(patchBytes, tc.expectedPatchBytes) {
t.Errorf("expect patchBytes: %q, got: %q\n", tc.expectedPatchBytes, patchBytes)
}
})
}
}