Merge pull request #75144 from Huang-Wei/followup-74222

kubelet: updated logic of verifying a static critical pod

pkg/kubelet/eviction/eviction_manager.go

@@ -57,6 +57,8 @@ type managerImpl struct {
 	config Config
 	// the function to invoke to kill a pod
 	killPodFunc KillPodFunc
+	// the function to get the mirror pod by a given statid pod
+	mirrorPodFunc MirrorPodFunc
 	// the interface that knows how to do image gc
 	imageGC ImageGC
 	// the interface that knows how to do container gc
@@ -99,6 +101,7 @@ func NewManager(
 	summaryProvider stats.SummaryProvider,
 	config Config,
 	killPodFunc KillPodFunc,
+	mirrorPodFunc MirrorPodFunc,
 	imageGC ImageGC,
 	containerGC ContainerGC,
 	recorder record.EventRecorder,
@@ -108,6 +111,7 @@ func NewManager(
 	manager := &managerImpl{
 		clock:                        clock,
 		killPodFunc:                  killPodFunc,
+		mirrorPodFunc:                mirrorPodFunc,
 		imageGC:                      imageGC,
 		containerGC:                  containerGC,
 		config:                       config,
@@ -545,9 +549,19 @@ func (m *managerImpl) evictPod(pod *v1.Pod, gracePeriodOverride int64, evictMsg
 	// If the pod is marked as critical and static, and support for critical pod annotations is enabled,
 	// do not evict such pods. Static pods are not re-admitted after evictions.
 	// https://github.com/kubernetes/kubernetes/issues/40573 has more details.
-	if kubelettypes.IsCriticalPod(pod) && kubepod.IsStaticPod(pod) {
+	if kubepod.IsStaticPod(pod) {
-		klog.Errorf("eviction manager: cannot evict a critical static pod %s", format.Pod(pod))
+		// need mirrorPod to check its "priority" value; static pod doesn't carry it
-		return false
+		if mirrorPod, ok := m.mirrorPodFunc(pod); ok && mirrorPod != nil {
+			// skip only when it's a static and critical pod
+			if kubelettypes.IsCriticalPod(mirrorPod) {
+				klog.Errorf("eviction manager: cannot evict a critical static pod %s", format.Pod(pod))
+				return false
+			}
+		} else {
+			// we should never hit this
+			klog.Errorf("eviction manager: cannot get mirror pod from static pod %s, so cannot evict it", format.Pod(pod))
+			return false
+		}
 	}
 	status := v1.PodStatus{
 		Phase:   v1.PodFailed,

pkg/kubelet/eviction/eviction_manager_test.go

@@ -1164,6 +1164,11 @@ func TestCriticalPodsAreNotEvicted(t *testing.T) {
 	activePodsFunc := func() []*v1.Pod {
 		return pods
 	}
+	mirrorPodFunc := func(staticPod *v1.Pod) (*v1.Pod, bool) {
+		mirrorPod := staticPod.DeepCopy()
+		mirrorPod.Annotations[kubelettypes.ConfigSourceAnnotationKey] = kubelettypes.ApiserverSource
+		return mirrorPod, true
+	}
 
 	fakeClock := clock.NewFakeClock(time.Now())
 	podKiller := &mockPodKiller{}
@@ -1198,6 +1203,7 @@ func TestCriticalPodsAreNotEvicted(t *testing.T) {
 	manager := &managerImpl{
 		clock:                        fakeClock,
 		killPodFunc:                  podKiller.killPodNow,
+		mirrorPodFunc:                mirrorPodFunc,
 		imageGC:                      diskGC,
 		containerGC:                  diskGC,
 		config:                       config,

pkg/kubelet/eviction/types.go

@@ -94,6 +94,10 @@ type ContainerGC interface {
 // gracePeriodOverride - the grace period override to use instead of what is on the pod spec
 type KillPodFunc func(pod *v1.Pod, status v1.PodStatus, gracePeriodOverride *int64) error
 
+// MirrorPodFunc returns the mirror pod for the given static pod and
+// whether it was known to the pod manager.
+type MirrorPodFunc func(*v1.Pod) (*v1.Pod, bool)
+
 // ActivePodsFunc returns pods bound to the kubelet that are active (i.e. non-terminal state)
 type ActivePodsFunc func() []*v1.Pod
 

pkg/kubelet/kubelet.go

@@ -822,7 +822,7 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
 	klet.podKillingCh = make(chan *kubecontainer.PodPair, podKillingChannelCapacity)
 
 	// setup eviction manager
-	evictionManager, evictionAdmitHandler := eviction.NewManager(klet.resourceAnalyzer, evictionConfig, killPodNow(klet.podWorkers, kubeDeps.Recorder), klet.imageManager, klet.containerGC, kubeDeps.Recorder, nodeRef, klet.clock)
+	evictionManager, evictionAdmitHandler := eviction.NewManager(klet.resourceAnalyzer, evictionConfig, killPodNow(klet.podWorkers, kubeDeps.Recorder), klet.podManager.GetMirrorPodByPod, klet.imageManager, klet.containerGC, kubeDeps.Recorder, nodeRef, klet.clock)
 
 	klet.evictionManager = evictionManager
 	klet.admitHandlers.AddPodAdmitHandler(evictionAdmitHandler)

pkg/kubelet/kubelet_pods.go

@@ -89,19 +89,7 @@ func (kl *Kubelet) listPodsFromDisk() ([]types.UID, error) {
 
 // GetActivePods returns non-terminal pods
 func (kl *Kubelet) GetActivePods() []*v1.Pod {
-	allPods, mirrorPods := kl.podManager.GetPodsAndMirrorPods()
+	allPods := kl.podManager.GetPods()
-	mirrorPodSet := make(map[string]*v1.Pod)
-	for _, p := range mirrorPods {
-		mirrorPodSet[kubecontainer.GetPodFullName(p)] = p
-	}
-	for i := range allPods {
-		podFullName := kubecontainer.GetPodFullName(allPods[i])
-		// replace static pod with mirror pod as some info (e.g. spec.Priority)
-		// is needed to make further decisions (e.g. eviction)
-		if mirrorPod, ok := mirrorPodSet[podFullName]; ok {
-			allPods[i] = mirrorPod
-		}
-	}
 	activePods := kl.filterOutTerminatedPods(allPods)
 	return activePods
 }

pkg/kubelet/kubelet_test.go

@@ -305,7 +305,7 @@ func newTestKubeletWithImageList(
 		Namespace: "",
 	}
 	// setup eviction manager
-	evictionManager, evictionAdmitHandler := eviction.NewManager(kubelet.resourceAnalyzer, eviction.Config{}, killPodNow(kubelet.podWorkers, fakeRecorder), kubelet.imageManager, kubelet.containerGC, fakeRecorder, nodeRef, kubelet.clock)
+	evictionManager, evictionAdmitHandler := eviction.NewManager(kubelet.resourceAnalyzer, eviction.Config{}, killPodNow(kubelet.podWorkers, fakeRecorder), kubelet.podManager.GetMirrorPodByPod, kubelet.imageManager, kubelet.containerGC, fakeRecorder, nodeRef, kubelet.clock)
 
 	kubelet.evictionManager = evictionManager
 	kubelet.admitHandlers.AddPodAdmitHandler(evictionAdmitHandler)

pkg/kubelet/runonce_test.go

@@ -120,7 +120,8 @@ func TestRunOnce(t *testing.T) {
 	fakeKillPodFunc := func(pod *v1.Pod, podStatus v1.PodStatus, gracePeriodOverride *int64) error {
 		return nil
 	}
-	evictionManager, evictionAdmitHandler := eviction.NewManager(kb.resourceAnalyzer, eviction.Config{}, fakeKillPodFunc, nil, nil, kb.recorder, nodeRef, kb.clock)
+	fakeMirrodPodFunc := func(*v1.Pod) (*v1.Pod, bool) { return nil, false }
+	evictionManager, evictionAdmitHandler := eviction.NewManager(kb.resourceAnalyzer, eviction.Config{}, fakeKillPodFunc, fakeMirrodPodFunc, nil, nil, kb.recorder, nodeRef, kb.clock)
 
 	kb.evictionManager = evictionManager
 	kb.admitHandlers.AddPodAdmitHandler(evictionAdmitHandler)