From 7680e3d079d757e1cbe76281b33b5abd88532f7c Mon Sep 17 00:00:00 2001 From: Sean Malloy Date: Wed, 10 Jun 2020 01:05:05 -0500 Subject: [PATCH 1/2] Use var declaration instead of short assignmnet This is a very minor refactor to use a var declaration for the reason variable. A var declaration is being used because the zero value for strings is an empty string. https://golang.org/ref/spec#The_zero_value --- pkg/descheduler/evictions/evictions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/descheduler/evictions/evictions.go b/pkg/descheduler/evictions/evictions.go index 0a7110ad6..bde3186a5 100644 --- a/pkg/descheduler/evictions/evictions.go +++ b/pkg/descheduler/evictions/evictions.go @@ -126,7 +126,7 @@ func (pe *PodEvictor) TotalEvicted() int { // 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, reasons ...string) (bool, error) { - reason := "" + var reason string if len(reasons) > 0 { reason = " (" + strings.Join(reasons, ", ") + ")" } From 4ff533ec174fa3163eb27aa7be3eb57c4de4527f Mon Sep 17 00:00:00 2001 From: Sean Malloy Date: Wed, 10 Jun 2020 01:13:17 -0500 Subject: [PATCH 2/2] Add pod eviction reason to k8s events Prior to this change the event created for every pod eviction was identical. Instead leverage the newly added eviction reason when creating k8s events. This makes it easier for end users to understand why the descheduler evicted a pod when inspecting k8s events. --- pkg/descheduler/evictions/evictions.go | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/pkg/descheduler/evictions/evictions.go b/pkg/descheduler/evictions/evictions.go index bde3186a5..d9b7046c2 100644 --- a/pkg/descheduler/evictions/evictions.go +++ b/pkg/descheduler/evictions/evictions.go @@ -146,6 +146,11 @@ func (pe *PodEvictor) EvictPod(ctx context.Context, pod *v1.Pod, node *v1.Node, klog.V(1).Infof("Evicted pod in dry run mode: %#v in namespace %#v%s", pod.Name, pod.Namespace, reason) } else { klog.V(1).Infof("Evicted pod: %#v in namespace %#v%s", pod.Name, pod.Namespace, reason) + eventBroadcaster := record.NewBroadcaster() + eventBroadcaster.StartLogging(klog.V(3).Infof) + eventBroadcaster.StartRecordingToSink(&clientcorev1.EventSinkImpl{Interface: pe.client.CoreV1().Events(pod.Namespace)}) + r := eventBroadcaster.NewRecorder(scheme.Scheme, v1.EventSource{Component: "sigs.k8s.io.descheduler"}) + r.Event(pod, v1.EventTypeNormal, "Descheduled", fmt.Sprintf("pod evicted by sigs.k8s.io/descheduler%s", reason)) } return true, nil } @@ -169,14 +174,6 @@ func evictPod(ctx context.Context, client clientset.Interface, pod *v1.Pod, poli } err := client.PolicyV1beta1().Evictions(eviction.Namespace).Evict(ctx, eviction) - if err == nil { - eventBroadcaster := record.NewBroadcaster() - eventBroadcaster.StartLogging(klog.V(3).Infof) - 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 nil - } if apierrors.IsTooManyRequests(err) { return fmt.Errorf("error when evicting pod (ignoring) %q: %v", pod.Name, err) }