diff --git a/pkg/collector/skipper_collector.go b/pkg/collector/skipper_collector.go index b5d6534..cf7482a 100644 --- a/pkg/collector/skipper_collector.go +++ b/pkg/collector/skipper_collector.go @@ -2,6 +2,7 @@ package collector import ( "encoding/json" + "errors" "fmt" "math" "strings" @@ -20,6 +21,10 @@ const ( rpsMetricBackendSeparator = "," ) +var ( + errBackendNameMissing = errors.New("backend name must be specified for requests-per-second when traffic switching is used") +) + // SkipperCollectorPlugin is a collector plugin for initializing metrics // collectors for getting skipper ingress metrics. type SkipperCollectorPlugin struct { @@ -94,7 +99,7 @@ func getAnnotationWeight(backendWeights string, backend string) float64 { return 0 } -func getWeights(ingressAnnotations map[string]string, backendAnnotations []string, backend string) float64 { +func getWeights(ingressAnnotations map[string]string, backendAnnotations []string, backend string) (float64, error) { maxWeight := 0.0 annotationsPresent := false @@ -107,10 +112,15 @@ func getWeights(ingressAnnotations map[string]string, backendAnnotations []strin // Fallback for ingresses that don't use traffic switching if !annotationsPresent { - return 1.0 + return 1.0, nil } - return maxWeight + // Require backend name here + if backend != "" { + return maxWeight, nil + } + + return 0.0, errBackendNameMissing } // getCollector returns a collector for getting the metrics. @@ -120,7 +130,10 @@ func (c *SkipperCollector) getCollector() (Collector, error) { return nil, err } - backendWeight := getWeights(ingress.Annotations, c.backendAnnotations, c.backend) + backendWeight, err := getWeights(ingress.Annotations, c.backendAnnotations, c.backend) + if err != nil { + return nil, err + } config := c.config var collector Collector diff --git a/pkg/collector/skipper_collector_test.go b/pkg/collector/skipper_collector_test.go index 0ca5982..b8ff845 100644 --- a/pkg/collector/skipper_collector_test.go +++ b/pkg/collector/skipper_collector_test.go @@ -106,6 +106,7 @@ func TestSkipperCollector(t *testing.T) { backend string ingressName string collectedMetric int + expectError bool namespace string backendWeights map[string]map[string]int replicas int32 @@ -197,6 +198,30 @@ func TestSkipperCollector(t *testing.T) { readyReplicas: 1, backendAnnotations: []string{testBackendWeightsAnnotation}, }, + { + msg: "test annotations are set but backend is missing", + metrics: []int{100, 1500, 700}, + ingressName: "dummy-ingress", + expectError: true, + namespace: "default", + backend: "", + backendWeights: map[string]map[string]int{testBackendWeightsAnnotation: {"backend2": 100, "backend1": 0}}, + replicas: 1, + readyReplicas: 1, + backendAnnotations: []string{testBackendWeightsAnnotation}, + }, + { + msg: "test annotations are missing and backend is unset", + metrics: []int{100, 1500, 700}, + ingressName: "dummy-ingress", + collectedMetric: 1500, + namespace: "default", + backend: "", + backendWeights: nil, + replicas: 1, + readyReplicas: 1, + backendAnnotations: []string{testBackendWeightsAnnotation}, + }, { msg: "test partial backend annotations", metrics: []int{100, 1500, 700}, @@ -225,9 +250,13 @@ func TestSkipperCollector(t *testing.T) { collector, err := NewSkipperCollector(client, plugin, hpa, config, time.Minute, tc.backendAnnotations, tc.backend) require.NoError(t, err, "failed to create skipper collector: %v", err) collected, err := collector.GetMetrics() - require.NoError(t, err, "failed to collect metrics: %v", err) - require.Len(t, collected, 1, "the number of metrics returned is not 1") - require.EqualValues(t, tc.collectedMetric, collected[0].Custom.Value.Value(), "the returned metric is not expected value") + if tc.expectError { + require.Error(t, err) + } else { + require.NoError(t, err, "failed to collect metrics: %v", err) + require.Len(t, collected, 1, "the number of metrics returned is not 1") + require.EqualValues(t, tc.collectedMetric, collected[0].Custom.Value.Value(), "the returned metric is not expected value") + } }) } }