Merge pull request #78 from zalando-incubator/return-err

When traffic switching is used, require a backend for the RPS metric

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

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)
+			if tc.expectError {
-			require.Len(t, collected, 1, "the number of metrics returned is not 1")
+				require.Error(t, err)
-			require.EqualValues(t, tc.collectedMetric, collected[0].Custom.Value.Value(), "the returned metric is not expected value")
+			} 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")
+			}
 		})
 	}
 }