Fix mapping of Prometheus query for new metric configuration

Signed-off-by: Mikkel Oscar Lyderik Larsen <mikkel.larsen@zalando.de>
This commit is contained in:
Mikkel Oscar Lyderik Larsen
2020-11-16 17:29:21 +01:00
parent df0ed1fca4
commit 4506701b7c
2 changed files with 206 additions and 7 deletions

View File

@ -97,16 +97,22 @@ func NewPrometheusCollector(client kubernetes.Interface, promAPI promv1.API, hpa
return nil, fmt.Errorf("selector for prometheus query is not specified")
}
queryName, ok := config.Config[prometheusQueryNameLabelKey]
if !ok {
return nil, fmt.Errorf("query name not specified on metric")
}
if v, ok := config.Config[queryName]; ok {
if v, ok := config.Config["query"]; ok {
// TODO: validate query
c.query = v
} else {
return nil, fmt.Errorf("no prometheus query defined for metric")
// support legacy behavior of mapping query name to metric
queryName, ok := config.Config[prometheusQueryNameLabelKey]
if !ok {
return nil, fmt.Errorf("query or query name not specified on metric")
}
if v, ok := config.Config[queryName]; ok {
// TODO: validate query
c.query = v
} else {
return nil, fmt.Errorf("no prometheus query defined for metric")
}
}
// Use custom Prometheus URL if defined in HPA annotation.

View File

@ -0,0 +1,193 @@
package collector
import (
"testing"
"github.com/stretchr/testify/require"
autoscalingv2 "k8s.io/api/autoscaling/v2beta2"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
func TestNewPrometheusCollector(t *testing.T) {
for _, tc := range []struct {
msg string
hpa *autoscalingv2.HorizontalPodAutoscaler
// config *MetricConfig
valid bool
expectedQuery string
}{
{
msg: "valid external metric configuration should work",
hpa: &autoscalingv2.HorizontalPodAutoscaler{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
"metric-config.external.rps.prometheus/query": "sum(rate(rps[1m]))",
},
},
Spec: autoscalingv2.HorizontalPodAutoscalerSpec{
Metrics: []autoscalingv2.MetricSpec{
{
Type: autoscalingv2.ExternalMetricSourceType,
External: &autoscalingv2.ExternalMetricSource{
Metric: autoscalingv2.MetricIdentifier{
Name: "rps",
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{"type": "prometheus"},
},
},
},
},
},
},
},
expectedQuery: "sum(rate(rps[1m]))",
},
{
msg: "missing query for external metric should not work",
hpa: &autoscalingv2.HorizontalPodAutoscaler{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
"metric-config.external.rps.prometheus/not-query": "sum(rate(rps[1m]))",
},
},
Spec: autoscalingv2.HorizontalPodAutoscalerSpec{
Metrics: []autoscalingv2.MetricSpec{
{
Type: autoscalingv2.ExternalMetricSourceType,
External: &autoscalingv2.ExternalMetricSource{
Metric: autoscalingv2.MetricIdentifier{
Name: "rps",
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{"type": "prometheus"},
},
},
},
},
},
},
},
expectedQuery: "",
},
{
msg: "valid legacy external metric configuration should work",
hpa: &autoscalingv2.HorizontalPodAutoscaler{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
"metric-config.external.prometheus-query.prometheus/rps": "sum(rate(rps[1m]))",
},
},
Spec: autoscalingv2.HorizontalPodAutoscalerSpec{
Metrics: []autoscalingv2.MetricSpec{
{
Type: autoscalingv2.ExternalMetricSourceType,
External: &autoscalingv2.ExternalMetricSource{
Metric: autoscalingv2.MetricIdentifier{
Name: PrometheusMetricNameLegacy,
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{prometheusQueryNameLabelKey: "rps"},
},
},
},
},
},
},
},
expectedQuery: "sum(rate(rps[1m]))",
},
{
msg: "invalid legacy external metric configuration with wrong query name should not work",
hpa: &autoscalingv2.HorizontalPodAutoscaler{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
"metric-config.external.prometheus-query.prometheus/rps": "sum(rate(rps[1m]))",
},
},
Spec: autoscalingv2.HorizontalPodAutoscalerSpec{
Metrics: []autoscalingv2.MetricSpec{
{
Type: autoscalingv2.ExternalMetricSourceType,
External: &autoscalingv2.ExternalMetricSource{
Metric: autoscalingv2.MetricIdentifier{
Name: PrometheusMetricNameLegacy,
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{prometheusQueryNameLabelKey: "not-rps"},
},
},
},
},
},
},
},
expectedQuery: "",
},
{
msg: "invalid legacy external metric configuration with missing selector should not work",
hpa: &autoscalingv2.HorizontalPodAutoscaler{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
"metric-config.external.prometheus-query.prometheus/rps": "sum(rate(rps[1m]))",
},
},
Spec: autoscalingv2.HorizontalPodAutoscalerSpec{
Metrics: []autoscalingv2.MetricSpec{
{
Type: autoscalingv2.ExternalMetricSourceType,
External: &autoscalingv2.ExternalMetricSource{
Metric: autoscalingv2.MetricIdentifier{
Name: PrometheusMetricNameLegacy,
},
},
},
},
},
},
expectedQuery: "",
},
{
msg: "invalid legacy external metric configuration with missing query name should not work",
hpa: &autoscalingv2.HorizontalPodAutoscaler{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
"metric-config.external.prometheus-query.prometheus/rps": "sum(rate(rps[1m]))",
},
},
Spec: autoscalingv2.HorizontalPodAutoscalerSpec{
Metrics: []autoscalingv2.MetricSpec{
{
Type: autoscalingv2.ExternalMetricSourceType,
External: &autoscalingv2.ExternalMetricSource{
Metric: autoscalingv2.MetricIdentifier{
Name: PrometheusMetricNameLegacy,
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{"not-query-name": "not-rps"},
},
},
},
},
},
},
},
expectedQuery: "",
},
} {
t.Run(tc.msg, func(t *testing.T) {
collectorFactory := NewCollectorFactory()
promPlugin, err := NewPrometheusCollectorPlugin(nil, "http://prometheus")
require.NoError(t, err)
collectorFactory.RegisterExternalCollector([]string{PrometheusMetricType, PrometheusMetricNameLegacy}, promPlugin)
configs, err := ParseHPAMetrics(tc.hpa)
require.NoError(t, err)
require.Len(t, configs, 1)
collector, err := collectorFactory.NewCollector(tc.hpa, configs[0], 0)
if tc.expectedQuery != "" {
require.NoError(t, err)
c, ok := collector.(*PrometheusCollector)
require.True(t, ok)
require.Equal(t, tc.expectedQuery, c.query)
} else {
require.Error(t, err)
}
})
}
}