From e167ecbb9ea26a097be89b443c478d03a8e244ee Mon Sep 17 00:00:00 2001
From: likakuli <1154584512@qq.com>
Date: Wed, 30 Aug 2023 17:15:34 +0800
Subject: [PATCH] fix: only invoke AddReference first time when to sync same
 pod to minimize unnecessary API requests to the API server for the
 configmap/secret get API

Signed-off-by: likakuli <1154584512@qq.com>
---
 .../util/manager/cache_based_manager.go       | 30 ++++++++++++-------
 .../util/manager/cache_based_manager_test.go  |  6 ++--
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/pkg/kubelet/util/manager/cache_based_manager.go b/pkg/kubelet/util/manager/cache_based_manager.go
index 160f9ca0d29..c8f93481251 100644
--- a/pkg/kubelet/util/manager/cache_based_manager.go
+++ b/pkg/kubelet/util/manager/cache_based_manager.go
@@ -224,21 +224,29 @@ func (c *cacheBasedManager) RegisterPod(pod *v1.Pod) {
 	names := c.getReferencedObjects(pod)
 	c.lock.Lock()
 	defer c.lock.Unlock()
-	for name := range names {
-		c.objectStore.AddReference(pod.Namespace, name, pod.UID)
-	}
 	var prev *v1.Pod
 	key := objectKey{namespace: pod.Namespace, name: pod.Name, uid: pod.UID}
 	prev = c.registeredPods[key]
 	c.registeredPods[key] = pod
-	if prev != nil {
-		for name := range c.getReferencedObjects(prev) {
-			// On an update, the .Add() call above will have re-incremented the
-			// ref count of any existing object, so any objects that are in both
-			// names and prev need to have their ref counts decremented. Any that
-			// are only in prev need to be completely removed. This unconditional
-			// call takes care of both cases.
-			c.objectStore.DeleteReference(prev.Namespace, name, prev.UID)
+	// To minimize unnecessary API requests to the API server for the configmap/secret get API
+	// only invoke AddReference the first time RegisterPod is called for a pod.
+	if prev == nil {
+		for name := range names {
+			c.objectStore.AddReference(pod.Namespace, name, pod.UID)
+		}
+	} else {
+		prevNames := c.getReferencedObjects(prev)
+		// Add new references
+		for name := range names {
+			if !prevNames.Has(name) {
+				c.objectStore.AddReference(pod.Namespace, name, pod.UID)
+			}
+		}
+		// Remove dropped references
+		for prevName := range prevNames {
+			if !names.Has(prevName) {
+				c.objectStore.DeleteReference(pod.Namespace, prevName, pod.UID)
+			}
 		}
 	}
 }
diff --git a/pkg/kubelet/util/manager/cache_based_manager_test.go b/pkg/kubelet/util/manager/cache_based_manager_test.go
index 59936f50d31..38bb2439b9a 100644
--- a/pkg/kubelet/util/manager/cache_based_manager_test.go
+++ b/pkg/kubelet/util/manager/cache_based_manager_test.go
@@ -336,7 +336,7 @@ type secretsToAttach struct {
 }
 
 func podWithSecrets(ns, podName string, toAttach secretsToAttach) *v1.Pod {
-	return podWithSecretsAndUID(ns, podName, "", toAttach)
+	return podWithSecretsAndUID(ns, podName, fmt.Sprintf("%s/%s", ns, podName), toAttach)
 }
 
 func podWithSecretsAndUID(ns, podName, podUID string, toAttach secretsToAttach) *v1.Pod {
@@ -415,13 +415,13 @@ func TestCacheInvalidation(t *testing.T) {
 		},
 	}
 	manager.RegisterPod(podWithSecrets("ns1", "name1", s2))
-	// All secrets should be invalidated - this should trigger get operations.
+	// Fetch only s3 and s20 secrets - this should trigger get operations.
 	store.Get("ns1", "s1")
 	store.Get("ns1", "s2")
 	store.Get("ns1", "s20")
 	store.Get("ns1", "s3")
 	actions = fakeClient.Actions()
-	assert.Equal(t, 4, len(actions), "unexpected actions: %#v", actions)
+	assert.Equal(t, 2, len(actions), "unexpected actions: %#v", actions)
 	fakeClient.ClearActions()
 
 	// Create a new pod that is refencing the first three secrets - those should