Continue admitting the pod regardless of return value from killPodFunc

pkg/kubelet/preemption/preemption.go

@@ -107,7 +107,9 @@ func (c *CriticalPodAdmissionHandler) evictPodsToFreeRequests(admitPod *v1.Pod,
 		// this is a blocking call and should only return when the pod and its containers are killed.
 		err := c.killPodFunc(pod, status, nil)
 		if err != nil {
-			return fmt.Errorf("preemption: pod %s failed to evict %v", format.Pod(pod), err)
+			klog.Warningf("preemption: pod %s failed to evict %v", format.Pod(pod), err)
+			// In future syncPod loops, the kubelet will retry the pod deletion steps that it was stuck on.
+			continue
 		}
 		klog.Infof("preemption: pod %s evicted successfully", format.Pod(pod))
 	}

pkg/kubelet/preemption/preemption_test.go

@@ -46,11 +46,12 @@ const (
 )
 
 type fakePodKiller struct {
-	killedPods []*v1.Pod
+	killedPods          []*v1.Pod
+	errDuringPodKilling bool
 }
 
-func newFakePodKiller() *fakePodKiller {
-	return &fakePodKiller{killedPods: []*v1.Pod{}}
+func newFakePodKiller(errPodKilling bool) *fakePodKiller {
+	return &fakePodKiller{killedPods: []*v1.Pod{}, errDuringPodKilling: errPodKilling}
 }
 
 func (f *fakePodKiller) clear() {
@@ -62,6 +63,10 @@ func (f *fakePodKiller) getKilledPods() []*v1.Pod {
 }
 
 func (f *fakePodKiller) killPodNow(pod *v1.Pod, status v1.PodStatus, gracePeriodOverride *int64) error {
+	if f.errDuringPodKilling {
+		f.killedPods = []*v1.Pod{}
+		return fmt.Errorf("problem killing pod %v", pod)
+	}
 	f.killedPods = append(f.killedPods, pod)
 	return nil
 }
@@ -90,6 +95,45 @@ func getTestCriticalPodAdmissionHandler(podProvider *fakePodProvider, podKiller
 	}
 }
 
+func TestEvictPodsToFreeRequestsWithError(t *testing.T) {
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExperimentalCriticalPodAnnotation, true)()
+	type testRun struct {
+		testName              string
+		inputPods             []*v1.Pod
+		insufficientResources admissionRequirementList
+		expectErr             bool
+		expectedOutput        []*v1.Pod
+	}
+	podProvider := newFakePodProvider()
+	podKiller := newFakePodKiller(true)
+	criticalPodAdmissionHandler := getTestCriticalPodAdmissionHandler(podProvider, podKiller)
+	allPods := getTestPods()
+	runs := []testRun{
+		{
+			testName: "multiple pods eviction error",
+			inputPods: []*v1.Pod{
+				allPods[critical], allPods[bestEffort], allPods[burstable], allPods[highRequestBurstable],
+				allPods[guaranteed], allPods[highRequestGuaranteed]},
+			insufficientResources: getAdmissionRequirementList(0, 550, 0),
+			expectErr:             false,
+			expectedOutput:        nil,
+		},
+	}
+	for _, r := range runs {
+		podProvider.setPods(r.inputPods)
+		outErr := criticalPodAdmissionHandler.evictPodsToFreeRequests(allPods[critical], r.insufficientResources)
+		outputPods := podKiller.getKilledPods()
+		if !r.expectErr && outErr != nil {
+			t.Errorf("evictPodsToFreeRequests returned an unexpected error during the %s test.  Err: %v", r.testName, outErr)
+		} else if r.expectErr && outErr == nil {
+			t.Errorf("evictPodsToFreeRequests expected an error but returned a successful output=%v during the %s test.", outputPods, r.testName)
+		} else if !podListEqual(r.expectedOutput, outputPods) {
+			t.Errorf("evictPodsToFreeRequests expected %v but got %v during the %s test.", r.expectedOutput, outputPods, r.testName)
+		}
+		podKiller.clear()
+	}
+}
+
 func TestEvictPodsToFreeRequests(t *testing.T) {
 	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExperimentalCriticalPodAnnotation, true)()
 	type testRun struct {
@@ -100,7 +144,7 @@ func TestEvictPodsToFreeRequests(t *testing.T) {
 		expectedOutput        []*v1.Pod
 	}
 	podProvider := newFakePodProvider()
-	podKiller := newFakePodKiller()
+	podKiller := newFakePodKiller(false)
 	criticalPodAdmissionHandler := getTestCriticalPodAdmissionHandler(podProvider, podKiller)
 	allPods := getTestPods()
 	runs := []testRun{