From 901a16ecbcaaf41c11318920f8a047c918b6c30d Mon Sep 17 00:00:00 2001 From: Jan Chaloupka Date: Fri, 10 Dec 2021 15:38:17 +0100 Subject: [PATCH] Do not collect the metrics when the metrics server is not enabled --- pkg/descheduler/descheduler.go | 1 + pkg/descheduler/evictions/evictions.go | 26 ++++++++++++------- pkg/descheduler/evictions/evictions_test.go | 2 +- pkg/descheduler/strategies/duplicates_test.go | 2 ++ pkg/descheduler/strategies/failedpods_test.go | 1 + .../strategies/node_affinity_test.go | 1 + pkg/descheduler/strategies/node_taint_test.go | 1 + .../highnodeutilization_test.go | 2 ++ .../lownodeutilization_test.go | 2 ++ .../strategies/pod_antiaffinity_test.go | 1 + .../strategies/pod_lifetime_test.go | 1 + .../strategies/toomanyrestarts_test.go | 1 + .../topologyspreadconstraint_test.go | 1 + test/e2e/e2e_duplicatepods_test.go | 1 + test/e2e/e2e_test.go | 2 ++ test/e2e/e2e_toomanyrestarts_test.go | 1 + 16 files changed, 36 insertions(+), 10 deletions(-) diff --git a/pkg/descheduler/descheduler.go b/pkg/descheduler/descheduler.go index b18e250ff..eaa403f24 100644 --- a/pkg/descheduler/descheduler.go +++ b/pkg/descheduler/descheduler.go @@ -271,6 +271,7 @@ func RunDeschedulerStrategies(ctx context.Context, rs *options.DeschedulerServer evictSystemCriticalPods, ignorePvcPods, evictBarePods, + !rs.DisableMetrics, ) for name, strategy := range deschedulerPolicy.Strategies { diff --git a/pkg/descheduler/evictions/evictions.go b/pkg/descheduler/evictions/evictions.go index d1d0380b7..dc2300417 100644 --- a/pkg/descheduler/evictions/evictions.go +++ b/pkg/descheduler/evictions/evictions.go @@ -61,6 +61,7 @@ type PodEvictor struct { evictLocalStoragePods bool evictSystemCriticalPods bool ignorePvcPods bool + metricsEnabled bool } func NewPodEvictor( @@ -74,6 +75,7 @@ func NewPodEvictor( evictSystemCriticalPods bool, ignorePvcPods bool, evictFailedBarePods bool, + metricsEnabled bool, ) *PodEvictor { var nodePodCount = make(nodePodEvictedCount) var namespacePodCount = make(namespacePodEvictCount) @@ -95,6 +97,7 @@ func NewPodEvictor( evictSystemCriticalPods: evictSystemCriticalPods, evictFailedBarePods: evictFailedBarePods, ignorePvcPods: ignorePvcPods, + metricsEnabled: metricsEnabled, } } @@ -121,20 +124,26 @@ func (pe *PodEvictor) EvictPod(ctx context.Context, pod *v1.Pod, node *v1.Node, reason += " (" + strings.Join(reasons, ", ") + ")" } if pe.maxPodsToEvictPerNode != nil && pe.nodepodCount[node]+1 > *pe.maxPodsToEvictPerNode { - metrics.PodsEvicted.With(map[string]string{"result": "maximum number of pods per node reached", "strategy": strategy, "namespace": pod.Namespace, "node": node.Name}).Inc() + if pe.metricsEnabled { + metrics.PodsEvicted.With(map[string]string{"result": "maximum number of pods per node reached", "strategy": strategy, "namespace": pod.Namespace, "node": node.Name}).Inc() + } return false, fmt.Errorf("Maximum number %v of evicted pods per %q node reached", *pe.maxPodsToEvictPerNode, node.Name) } if pe.maxPodsToEvictPerNamespace != nil && pe.namespacePodCount[pod.Namespace]+1 > *pe.maxPodsToEvictPerNamespace { - metrics.PodsEvicted.With(map[string]string{"result": "maximum number of pods per namespace reached", "strategy": strategy, "namespace": pod.Namespace, "node": node.Name}).Inc() + if pe.metricsEnabled { + metrics.PodsEvicted.With(map[string]string{"result": "maximum number of pods per namespace reached", "strategy": strategy, "namespace": pod.Namespace, "node": node.Name}).Inc() + } return false, fmt.Errorf("Maximum number %v of evicted pods per %q namespace reached", *pe.maxPodsToEvictPerNamespace, pod.Namespace) } - err := evictPod(ctx, pe.client, pod, pe.policyGroupVersion, pe.dryRun) + err := evictPod(ctx, pe.client, pod, pe.policyGroupVersion) if err != nil { // err is used only for logging purposes klog.ErrorS(err, "Error evicting pod", "pod", klog.KObj(pod), "reason", reason) - metrics.PodsEvicted.With(map[string]string{"result": "error", "strategy": strategy, "namespace": pod.Namespace, "node": node.Name}).Inc() + if pe.metricsEnabled { + metrics.PodsEvicted.With(map[string]string{"result": "error", "strategy": strategy, "namespace": pod.Namespace, "node": node.Name}).Inc() + } return false, nil } @@ -149,15 +158,14 @@ func (pe *PodEvictor) EvictPod(ctx context.Context, pod *v1.Pod, node *v1.Node, 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)) - metrics.PodsEvicted.With(map[string]string{"result": "success", "strategy": strategy, "namespace": pod.Namespace, "node": node.Name}).Inc() + if pe.metricsEnabled { + metrics.PodsEvicted.With(map[string]string{"result": "success", "strategy": strategy, "namespace": pod.Namespace, "node": node.Name}).Inc() + } } return true, nil } -func evictPod(ctx context.Context, client clientset.Interface, pod *v1.Pod, policyGroupVersion string, dryRun bool) error { - if dryRun { - return nil - } +func evictPod(ctx context.Context, client clientset.Interface, pod *v1.Pod, policyGroupVersion string) error { deleteOptions := &metav1.DeleteOptions{} // GracePeriodSeconds ? eviction := &policy.Eviction{ diff --git a/pkg/descheduler/evictions/evictions_test.go b/pkg/descheduler/evictions/evictions_test.go index 4a16412e3..d7e8b46dd 100644 --- a/pkg/descheduler/evictions/evictions_test.go +++ b/pkg/descheduler/evictions/evictions_test.go @@ -62,7 +62,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") 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) } diff --git a/pkg/descheduler/strategies/duplicates_test.go b/pkg/descheduler/strategies/duplicates_test.go index 82d8087aa..f2b6467d3 100644 --- a/pkg/descheduler/strategies/duplicates_test.go +++ b/pkg/descheduler/strategies/duplicates_test.go @@ -301,6 +301,7 @@ func TestFindDuplicatePods(t *testing.T) { false, false, false, + false, ) RemoveDuplicatePods(ctx, fakeClient, testCase.strategy, testCase.nodes, podEvictor, getPodsAssignedToNode) @@ -727,6 +728,7 @@ func TestRemoveDuplicatesUniformly(t *testing.T) { false, false, false, + false, ) RemoveDuplicatePods(ctx, fakeClient, testCase.strategy, testCase.nodes, podEvictor, getPodsAssignedToNode) diff --git a/pkg/descheduler/strategies/failedpods_test.go b/pkg/descheduler/strategies/failedpods_test.go index 7f310ad59..367418499 100644 --- a/pkg/descheduler/strategies/failedpods_test.go +++ b/pkg/descheduler/strategies/failedpods_test.go @@ -265,6 +265,7 @@ func TestRemoveFailedPods(t *testing.T) { false, false, false, + false, ) RemoveFailedPods(ctx, fakeClient, tc.strategy, tc.nodes, podEvictor, getPodsAssignedToNode) diff --git a/pkg/descheduler/strategies/node_affinity_test.go b/pkg/descheduler/strategies/node_affinity_test.go index ab2efc489..df2b636e1 100644 --- a/pkg/descheduler/strategies/node_affinity_test.go +++ b/pkg/descheduler/strategies/node_affinity_test.go @@ -226,6 +226,7 @@ func TestRemovePodsViolatingNodeAffinity(t *testing.T) { false, false, false, + false, ) RemovePodsViolatingNodeAffinity(ctx, fakeClient, tc.strategy, tc.nodes, podEvictor, getPodsAssignedToNode) diff --git a/pkg/descheduler/strategies/node_taint_test.go b/pkg/descheduler/strategies/node_taint_test.go index 6d67ede10..0dc722214 100644 --- a/pkg/descheduler/strategies/node_taint_test.go +++ b/pkg/descheduler/strategies/node_taint_test.go @@ -263,6 +263,7 @@ func TestDeletePodsViolatingNodeTaints(t *testing.T) { tc.evictSystemCriticalPods, false, false, + false, ) strategy := api.DeschedulerStrategy{ diff --git a/pkg/descheduler/strategies/nodeutilization/highnodeutilization_test.go b/pkg/descheduler/strategies/nodeutilization/highnodeutilization_test.go index 1a2f0865c..7d4c8bd60 100644 --- a/pkg/descheduler/strategies/nodeutilization/highnodeutilization_test.go +++ b/pkg/descheduler/strategies/nodeutilization/highnodeutilization_test.go @@ -467,6 +467,7 @@ func TestHighNodeUtilization(t *testing.T) { false, false, false, + false, ) strategy := api.DeschedulerStrategy{ @@ -671,6 +672,7 @@ func TestHighNodeUtilizationWithTaints(t *testing.T) { false, false, false, + false, ) HighNodeUtilization(ctx, fakeClient, strategy, item.nodes, podEvictor, getPodsAssignedToNode) diff --git a/pkg/descheduler/strategies/nodeutilization/lownodeutilization_test.go b/pkg/descheduler/strategies/nodeutilization/lownodeutilization_test.go index 07961d160..8cdcb299b 100644 --- a/pkg/descheduler/strategies/nodeutilization/lownodeutilization_test.go +++ b/pkg/descheduler/strategies/nodeutilization/lownodeutilization_test.go @@ -724,6 +724,7 @@ func TestLowNodeUtilization(t *testing.T) { false, false, false, + false, ) strategy := api.DeschedulerStrategy{ @@ -1036,6 +1037,7 @@ func TestLowNodeUtilizationWithTaints(t *testing.T) { false, false, false, + false, ) LowNodeUtilization(ctx, fakeClient, strategy, item.nodes, podEvictor, getPodsAssignedToNode) diff --git a/pkg/descheduler/strategies/pod_antiaffinity_test.go b/pkg/descheduler/strategies/pod_antiaffinity_test.go index d70184793..df5f33057 100644 --- a/pkg/descheduler/strategies/pod_antiaffinity_test.go +++ b/pkg/descheduler/strategies/pod_antiaffinity_test.go @@ -213,6 +213,7 @@ func TestPodAntiAffinity(t *testing.T) { false, false, false, + false, ) strategy := api.DeschedulerStrategy{ Params: &api.StrategyParameters{ diff --git a/pkg/descheduler/strategies/pod_lifetime_test.go b/pkg/descheduler/strategies/pod_lifetime_test.go index a30a26a79..697250cba 100644 --- a/pkg/descheduler/strategies/pod_lifetime_test.go +++ b/pkg/descheduler/strategies/pod_lifetime_test.go @@ -302,6 +302,7 @@ func TestPodLifeTime(t *testing.T) { false, tc.ignorePvcPods, false, + false, ) PodLifeTime(ctx, fakeClient, tc.strategy, tc.nodes, podEvictor, getPodsAssignedToNode) diff --git a/pkg/descheduler/strategies/toomanyrestarts_test.go b/pkg/descheduler/strategies/toomanyrestarts_test.go index 39c9ef7dd..aa7bec2e7 100644 --- a/pkg/descheduler/strategies/toomanyrestarts_test.go +++ b/pkg/descheduler/strategies/toomanyrestarts_test.go @@ -238,6 +238,7 @@ func TestRemovePodsHavingTooManyRestarts(t *testing.T) { false, false, false, + false, ) RemovePodsHavingTooManyRestarts(ctx, fakeClient, tc.strategy, tc.nodes, podEvictor, getPodsAssignedToNode) diff --git a/pkg/descheduler/strategies/topologyspreadconstraint_test.go b/pkg/descheduler/strategies/topologyspreadconstraint_test.go index 6e6675855..22a863651 100644 --- a/pkg/descheduler/strategies/topologyspreadconstraint_test.go +++ b/pkg/descheduler/strategies/topologyspreadconstraint_test.go @@ -906,6 +906,7 @@ func TestTopologySpreadConstraint(t *testing.T) { false, false, false, + false, ) RemovePodsViolatingTopologySpreadConstraint(ctx, fakeClient, tc.strategy, tc.nodes, podEvictor, getPodsAssignedToNode) podsEvicted := podEvictor.TotalEvicted() diff --git a/test/e2e/e2e_duplicatepods_test.go b/test/e2e/e2e_duplicatepods_test.go index f3c991b25..cbb56e676 100644 --- a/test/e2e/e2e_duplicatepods_test.go +++ b/test/e2e/e2e_duplicatepods_test.go @@ -151,6 +151,7 @@ func TestRemoveDuplicates(t *testing.T) { false, false, false, + false, ) t.Log("Running DeschedulerStrategy strategy") diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index ac56bd4da..a832d463b 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -203,6 +203,7 @@ func runPodLifetimeStrategy( evictCritical, false, false, + false, ), getPodsAssignedToNode, ) @@ -1341,5 +1342,6 @@ func initPodEvictorOrFail(t *testing.T, clientSet clientset.Interface, nodes []* false, false, false, + false, ) } diff --git a/test/e2e/e2e_toomanyrestarts_test.go b/test/e2e/e2e_toomanyrestarts_test.go index dfd3389f1..077b6db3b 100644 --- a/test/e2e/e2e_toomanyrestarts_test.go +++ b/test/e2e/e2e_toomanyrestarts_test.go @@ -141,6 +141,7 @@ func TestTooManyRestarts(t *testing.T) { false, false, false, + false, ) // Run RemovePodsHavingTooManyRestarts strategy t.Log("Running RemovePodsHavingTooManyRestarts strategy")