From 71a8e99d1f49c48c3ba2137e9e352c219f8eca5e Mon Sep 17 00:00:00 2001 From: Katyanna Moura Date: Wed, 12 Jan 2022 10:56:03 +0100 Subject: [PATCH] Fix panic when trying to add key to nil map Bug was introduced trying to solved the following issue: > When scenarios where two HPAs reference the same object but have different metric calculation (e.g. ingresses with different weights), kube-metrics-adapter calculates the two metrics but always overrides one of the metrics when saving it to the store. This commit fixes a issue where without the added `return` the system was continuing in an invalid states where `labels2metric` wasn't properly initialized, causing the system to panic. Signed-off-by: Katyanna Moura --- pkg/provider/metric_store.go | 1 + pkg/provider/metric_store_test.go | 63 +++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/pkg/provider/metric_store.go b/pkg/provider/metric_store.go index 38b0536..6614e8f 100644 --- a/pkg/provider/metric_store.go +++ b/pkg/provider/metric_store.go @@ -185,6 +185,7 @@ func (s *MetricStore) insertCustomMetric(value custom_metrics.MetricValue) { object2label[object] = labelsHashToCustomMetricStore{ labelsKey: customMetric, } + return } labels2metric[labelsKey] = customMetric diff --git a/pkg/provider/metric_store_test.go b/pkg/provider/metric_store_test.go index 39e4c70..3a0e4dd 100644 --- a/pkg/provider/metric_store_test.go +++ b/pkg/provider/metric_store_test.go @@ -894,6 +894,69 @@ func TestCustomMetricsStorageErrors(t *testing.T) { }, }, }, + // Regression test covering https://github.com/zalando-incubator/kube-metrics-adapter/issues/379 + { + test: "test multiple metrics with label added to metric name", + insert: []collector.CollectedMetric{ + { + Type: autoscalingv2.MetricSourceType("Object"), + Custom: custom_metrics.MetricValue{ + Metric: newMetricIdentifier("metric-per-unit", metav1.LabelSelector{}), + Value: *resource.NewQuantity(0, ""), + DescribedObject: custom_metrics.ObjectReference{ + Name: "metricObject", + Namespace: "default", + Kind: "Deployment", + APIVersion: "apps/v1", + }, + }, + }, + { + Type: autoscalingv2.MetricSourceType("Object"), + Custom: custom_metrics.MetricValue{ + Metric: newMetricIdentifier("metric-per-unit", metav1.LabelSelector{}), + Value: *resource.NewQuantity(1, ""), + DescribedObject: custom_metrics.ObjectReference{ + Name: "metricObject-000", + Namespace: "default", + Kind: "Deployment", + APIVersion: "apps/v1", + }, + }, + }, + }, + list: []provider.CustomMetricInfo{ + { + GroupResource: schema.GroupResource{}, + Namespaced: true, + Metric: "metric-per-unit", + }, + }, + byName: struct { + name types.NamespacedName + info provider.CustomMetricInfo + }{ + name: types.NamespacedName{Name: "metricObject-000", Namespace: "default"}, + info: provider.CustomMetricInfo{ + GroupResource: schema.GroupResource{}, + Namespaced: true, + Metric: "metric-per-unit", + }, + }, + byLabel: struct { + namespace string + selector labels.Selector + info provider.CustomMetricInfo + }{ + namespace: "default", + selector: labels.Everything(), + info: provider.CustomMetricInfo{ + GroupResource: schema.GroupResource{}, + Namespaced: true, + Metric: "metric-per-unit", + }, + }, + }, } for _, tc := range multiValueTests {