From 085693eff2e4995aef536f6cb7d474a42412dcf0 Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <liggitt@google.com>
Date: Wed, 26 Jan 2022 15:58:24 -0500
Subject: [PATCH] Move kubelet secret and configmap manager calls to sync_Pod
 functions

---
 pkg/kubelet/kubelet.go         | 18 +++++++++++++++++
 pkg/kubelet/pod/pod_manager.go | 36 ----------------------------------
 2 files changed, 18 insertions(+), 36 deletions(-)

diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go
index 6bdb3b99c85..64fcc61060d 100644
--- a/pkg/kubelet/kubelet.go
+++ b/pkg/kubelet/kubelet.go
@@ -1598,6 +1598,16 @@ func (kl *Kubelet) syncPod(ctx context.Context, updateType kubetypes.SyncPodType
 		return fmt.Errorf("%s: %v", NetworkNotReadyErrorMsg, err)
 	}
 
+	// ensure the kubelet knows about referenced secrets or configmaps used by the pod
+	if !kl.podWorkers.IsPodTerminationRequested(pod.UID) {
+		if kl.secretManager != nil {
+			kl.secretManager.RegisterPod(pod)
+		}
+		if kl.configMapManager != nil {
+			kl.configMapManager.RegisterPod(pod)
+		}
+	}
+
 	// Create Cgroups for the pod and apply resource parameters
 	// to them if cgroups-per-qos flag is enabled.
 	pcm := kl.containerManager.NewPodContainerManager()
@@ -1827,6 +1837,14 @@ func (kl *Kubelet) syncTerminatedPod(ctx context.Context, pod *v1.Pod, podStatus
 	}
 	klog.V(4).InfoS("Pod termination unmounted volumes", "pod", klog.KObj(pod), "podUID", pod.UID)
 
+	// After volume unmount is complete, let the secret and configmap managers know we're done with this pod
+	if kl.secretManager != nil {
+		kl.secretManager.UnregisterPod(pod)
+	}
+	if kl.configMapManager != nil {
+		kl.configMapManager.UnregisterPod(pod)
+	}
+
 	// Note: we leave pod containers to be reclaimed in the background since dockershim requires the
 	// container for retrieving logs and we want to make sure logs are available until the pod is
 	// physically deleted.
diff --git a/pkg/kubelet/pod/pod_manager.go b/pkg/kubelet/pod/pod_manager.go
index c94e563620c..ce342ac7209 100644
--- a/pkg/kubelet/pod/pod_manager.go
+++ b/pkg/kubelet/pod/pod_manager.go
@@ -159,10 +159,6 @@ func (pm *basicManager) UpdatePod(pod *v1.Pod) {
 	pm.updatePodsInternal(pod)
 }
 
-func isPodInTerminatedState(pod *v1.Pod) bool {
-	return pod.Status.Phase == v1.PodFailed || pod.Status.Phase == v1.PodSucceeded
-}
-
 // updateMetrics updates the metrics surfaced by the pod manager.
 // oldPod or newPod may be nil to signify creation or deletion.
 func updateMetrics(oldPod, newPod *v1.Pod) {
@@ -187,32 +183,6 @@ func updateMetrics(oldPod, newPod *v1.Pod) {
 // lock.
 func (pm *basicManager) updatePodsInternal(pods ...*v1.Pod) {
 	for _, pod := range pods {
-		if pm.secretManager != nil {
-			if isPodInTerminatedState(pod) {
-				// Pods that are in terminated state and no longer running can be
-				// ignored as they no longer require access to secrets.
-				// It is especially important in watch-based manager, to avoid
-				// unnecessary watches for terminated pods waiting for GC.
-				pm.secretManager.UnregisterPod(pod)
-			} else {
-				// TODO: Consider detecting only status update and in such case do
-				// not register pod, as it doesn't really matter.
-				pm.secretManager.RegisterPod(pod)
-			}
-		}
-		if pm.configMapManager != nil {
-			if isPodInTerminatedState(pod) {
-				// Pods that are in terminated state and no longer running can be
-				// ignored as they no longer require access to configmaps.
-				// It is especially important in watch-based manager, to avoid
-				// unnecessary watches for terminated pods waiting for GC.
-				pm.configMapManager.UnregisterPod(pod)
-			} else {
-				// TODO: Consider detecting only status update and in such case do
-				// not register pod, as it doesn't really matter.
-				pm.configMapManager.RegisterPod(pod)
-			}
-		}
 		podFullName := kubecontainer.GetPodFullName(pod)
 		// This logic relies on a static pod and its mirror to have the same name.
 		// It is safe to type convert here due to the IsMirrorPod guard.
@@ -239,12 +209,6 @@ func (pm *basicManager) DeletePod(pod *v1.Pod) {
 	updateMetrics(pod, nil)
 	pm.lock.Lock()
 	defer pm.lock.Unlock()
-	if pm.secretManager != nil {
-		pm.secretManager.UnregisterPod(pod)
-	}
-	if pm.configMapManager != nil {
-		pm.configMapManager.UnregisterPod(pod)
-	}
 	podFullName := kubecontainer.GetPodFullName(pod)
 	// It is safe to type convert here due to the IsMirrorPod guard.
 	if kubetypes.IsMirrorPod(pod) {