Merge pull request #32232 from Random-Liu/avoid-syncpod-when-no-mirrorpod

Automatic merge from submit-queue

Avoid unnecessary status update when there is no corresponding mirror pod

Fixes https://github.com/kubernetes/kubernetes/issues/32191.

This PR changes status manager to skip update when there is no mirror pod for a static pod.
We need this because:
1) When static pod terminates and mirror pod is deleted, this will avoid extra `syncPod`.
2) During mirror pod creation and recreation, this will avoid unnecessary `syncPod`.

Mark P1 to match the original issue.

@wojtek-t @yujuhong 
/cc @kubernetes/sig-node

pkg/kubelet/pod/pod_manager.go

@@ -74,10 +74,9 @@ type Manager interface {
 	// associated static pods. This method sends deletion requests to the API
 	// server, but does NOT modify the internal pod storage in basicManager.
 	DeleteOrphanedMirrorPods()
-	// TranslatePodUID returns the UID which is the mirror pod or static pod
-	// of the pod with the given UID.  If the UID belongs to a mirror pod,
-	// returns the UID of its static pod.  If the UID belongs to a static pod,
-	// returns the UID of its mirror pod.  Otherwise, returns the original UID.
+	// TranslatePodUID returns the actual UID of a pod. If the UID belongs to
+	// a mirror pod, returns the UID of its static pod. Otherwise, returns the
+	// original UID.
 	//
 	// All public-facing functions should perform this translation for UIDs
 	// because user may provide a mirror pod UID, which is not recognized by
@@ -237,6 +236,17 @@ func (pm *basicManager) GetUIDTranslations() (podToMirror, mirrorToPod map[types
 
 	podToMirror = make(map[types.UID]types.UID, len(pm.translationByUID))
 	mirrorToPod = make(map[types.UID]types.UID, len(pm.translationByUID))
+	// Insert empty translation mapping for all static pods.
+	for uid, pod := range pm.podByUID {
+		if !IsStaticPod(pod) {
+			continue
+		}
+		podToMirror[uid] = ""
+	}
+	// Fill in translations. Notice that if there is no mirror pod for a
+	// static pod, its uid will be translated into empty string "". This
+	// is WAI, from the caller side we can know that the static pod doesn't
+	// have a corresponding mirror pod instead of using static pod uid directly.
 	for k, v := range pm.translationByUID {
 		mirrorToPod[k] = v
 		podToMirror[v] = k

pkg/kubelet/status/status_manager.go

@@ -372,6 +372,10 @@ func (m *manager) syncBatch() {
 		for uid, status := range m.podStatuses {
 			syncedUID := uid
 			if mirrorUID, ok := podToMirror[uid]; ok {
+				if mirrorUID == "" {
+					glog.V(5).Infof("Static pod %q (%s/%s) does not have a corresponding mirror pod; skipping", uid, status.podName, status.podNamespace)
+					continue
+				}
 				syncedUID = mirrorUID
 			}
 			if m.needsUpdate(syncedUID, status) {

pkg/kubelet/status/status_manager_test.go

@@ -477,7 +477,7 @@ func TestStatusEquality(t *testing.T) {
 	}
 }
 
-func TestStaticPodStatus(t *testing.T) {
+func TestStaticPod(t *testing.T) {
 	staticPod := getTestPod()
 	staticPod.Annotations = map[string]string{kubetypes.ConfigSourceAnnotationKey: "file"}
 	mirrorPod := getTestPod()
@@ -488,24 +488,36 @@ func TestStaticPodStatus(t *testing.T) {
 	}
 	client := fake.NewSimpleClientset(mirrorPod)
 	m := newTestManager(client)
+
+	// Create the static pod
 	m.podManager.AddPod(staticPod)
-	m.podManager.AddPod(mirrorPod)
-	// Verify setup.
 	assert.True(t, kubepod.IsStaticPod(staticPod), "SetUp error: staticPod")
-	assert.True(t, kubepod.IsMirrorPod(mirrorPod), "SetUp error: mirrorPod")
-	assert.Equal(t, m.podManager.TranslatePodUID(mirrorPod.UID), staticPod.UID)
 
 	status := getRandomPodStatus()
 	now := unversioned.Now()
 	status.StartTime = &now
-
 	m.SetPodStatus(staticPod, status)
+
+	// Should be able to get the static pod status from status manager
 	retrievedStatus := expectPodStatus(t, m, staticPod)
 	normalizeStatus(staticPod, &status)
 	assert.True(t, isStatusEqual(&status, &retrievedStatus), "Expected: %+v, Got: %+v", status, retrievedStatus)
+
+	// Should not sync pod because there is no corresponding mirror pod for the static pod.
+	m.testSyncBatch()
+	verifyActions(t, m.kubeClient, []core.Action{})
+	client.ClearActions()
+
+	// Create the mirror pod
+	m.podManager.AddPod(mirrorPod)
+	assert.True(t, kubepod.IsMirrorPod(mirrorPod), "SetUp error: mirrorPod")
+	assert.Equal(t, m.podManager.TranslatePodUID(mirrorPod.UID), staticPod.UID)
+
+	// Should be able to get the mirror pod status from status manager
 	retrievedStatus, _ = m.GetPodStatus(mirrorPod.UID)
 	assert.True(t, isStatusEqual(&status, &retrievedStatus), "Expected: %+v, Got: %+v", status, retrievedStatus)
-	// Should translate mirrorPod / staticPod UID.
+
+	// Should sync pod because the corresponding mirror pod is created
 	m.testSyncBatch()
 	verifyActions(t, m.kubeClient, []core.Action{
 		core.GetActionImpl{ActionImpl: core.ActionImpl{Verb: "get", Resource: unversioned.GroupVersionResource{Resource: "pods"}}},
@@ -517,17 +529,17 @@ func TestStaticPodStatus(t *testing.T) {
 	assert.True(t, isStatusEqual(&status, &updatedPod.Status), "Expected: %+v, Got: %+v", status, updatedPod.Status)
 	client.ClearActions()
 
-	// No changes.
+	// Should not sync pod because nothing is changed.
 	m.testSyncBatch()
 	verifyActions(t, m.kubeClient, []core.Action{})
 
-	// Mirror pod identity changes.
+	// Change mirror pod identity.
 	m.podManager.DeletePod(mirrorPod)
 	mirrorPod.UID = "new-mirror-pod"
 	mirrorPod.Status = api.PodStatus{}
 	m.podManager.AddPod(mirrorPod)
 
-	// Expect no update to mirror pod, since UID has changed.
+	// Should not update to mirror pod, because UID has changed.
 	m.testSyncBatch()
 	verifyActions(t, m.kubeClient, []core.Action{
 		core.GetActionImpl{ActionImpl: core.ActionImpl{Verb: "get", Resource: unversioned.GroupVersionResource{Resource: "pods"}}},