diff --git a/pkg/descheduler/evictions/evictions.go b/pkg/descheduler/evictions/evictions.go index d234c833c..131ed602c 100644 --- a/pkg/descheduler/evictions/evictions.go +++ b/pkg/descheduler/evictions/evictions.go @@ -83,25 +83,26 @@ func (pe *PodEvictor) TotalEvicted() int { // EvictPod returns non-nil error only when evicting a pod on a node is not // possible (due to maxPodsToEvict constraint). Success is true when the pod // is evicted on the server side. -func (pe *PodEvictor) EvictPod(ctx context.Context, pod *v1.Pod, node *v1.Node) (success bool, err error) { +func (pe *PodEvictor) EvictPod(ctx context.Context, pod *v1.Pod, node *v1.Node) (bool, error) { if pe.maxPodsToEvict > 0 && pe.nodepodCount[node]+1 > pe.maxPodsToEvict { return false, fmt.Errorf("Maximum number %v of evicted pods per %q node reached", pe.maxPodsToEvict, node.Name) } - success, err = EvictPod(ctx, pe.client, pod, pe.policyGroupVersion, pe.dryRun) - if success { - pe.nodepodCount[node]++ - klog.V(1).Infof("Evicted pod: %#v in namespace %#v", pod.Name, pod.Namespace) - return success, nil + err := evictPod(ctx, pe.client, pod, pe.policyGroupVersion, pe.dryRun) + if err != nil { + // err is used only for logging purposes + klog.Errorf("Error evicting pod: %#v in namespace %#v (%#v)", pod.Name, pod.Namespace, err) + return false, nil } - // err is used only for logging purposes - klog.Errorf("Error evicting pod: %#v in namespace %#v (%#v)", pod.Name, pod.Namespace, err) - return false, nil + + pe.nodepodCount[node]++ + klog.V(1).Infof("Evicted pod: %#v in namespace %#v", pod.Name, pod.Namespace) + return true, nil } -func EvictPod(ctx context.Context, client clientset.Interface, pod *v1.Pod, policyGroupVersion string, dryRun bool) (bool, error) { +func evictPod(ctx context.Context, client clientset.Interface, pod *v1.Pod, policyGroupVersion string, dryRun bool) error { if dryRun { - return true, nil + return nil } deleteOptions := &metav1.DeleteOptions{} // GracePeriodSeconds ? @@ -124,13 +125,13 @@ func EvictPod(ctx context.Context, client clientset.Interface, pod *v1.Pod, poli eventBroadcaster.StartRecordingToSink(&clientcorev1.EventSinkImpl{Interface: client.CoreV1().Events(pod.Namespace)}) r := eventBroadcaster.NewRecorder(scheme.Scheme, v1.EventSource{Component: "sigs.k8s.io.descheduler"}) r.Event(pod, v1.EventTypeNormal, "Descheduled", "pod evicted by sigs.k8s.io/descheduler") - return true, nil + return nil } if apierrors.IsTooManyRequests(err) { - return false, fmt.Errorf("error when evicting pod (ignoring) %q: %v", pod.Name, err) + return fmt.Errorf("error when evicting pod (ignoring) %q: %v", pod.Name, err) } if apierrors.IsNotFound(err) { - return false, fmt.Errorf("pod not found when evicting %q: %v", pod.Name, err) + return fmt.Errorf("pod not found when evicting %q: %v", pod.Name, err) } - return false, err + return err } diff --git a/pkg/descheduler/evictions/evictions_test.go b/pkg/descheduler/evictions/evictions_test.go index a1e87f8fe..0d6bf1bb8 100644 --- a/pkg/descheduler/evictions/evictions_test.go +++ b/pkg/descheduler/evictions/evictions_test.go @@ -20,7 +20,7 @@ import ( "context" "testing" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/fake" core "k8s.io/client-go/testing" @@ -36,21 +36,21 @@ func TestEvictPod(t *testing.T) { node *v1.Node pod *v1.Pod pods []v1.Pod - want bool + want error }{ { description: "test pod eviction - pod present", node: node1, pod: pod1, pods: []v1.Pod{*pod1}, - want: true, + want: nil, }, { description: "test pod eviction - pod absent", node: node1, pod: pod1, pods: []v1.Pod{*test.BuildTestPod("p2", 400, 0, "node1", nil), *test.BuildTestPod("p3", 450, 0, "node1", nil)}, - want: true, + want: nil, }, } @@ -59,7 +59,7 @@ func TestEvictPod(t *testing.T) { fakeClient.Fake.AddReactor("list", "pods", func(action core.Action) (bool, runtime.Object, error) { return true, &v1.PodList{Items: test.pods}, nil }) - got, _ := EvictPod(ctx, fakeClient, test.pod, "v1", false) + got := evictPod(ctx, fakeClient, test.pod, "v1", false) if got != test.want { t.Errorf("Test error for Desc: %s. Expected %v pod eviction to be %v, got %v", test.description, test.pod.Name, test.want, got) }