Correctly handle zero-weight backends (#70)

Signed-off-by: Alexey Ermakov <alexey.ermakov@zalando.de>

pkg/collector/skipper_collector.go

@@ -3,6 +3,7 @@ package collector
 import (
 	"encoding/json"
 	"fmt"
+	"math"
 	"strings"
 	"time"
 
@@ -81,38 +82,37 @@ func NewSkipperCollector(client kubernetes.Interface, plugin CollectorPlugin, hp
 	}, nil
 }
 
-func getAnnotationWeight(backendWeights string, backend string) (float64, bool) {
+func getAnnotationWeight(backendWeights string, backend string) float64 {
 	var weightsMap map[string]int
 	err := json.Unmarshal([]byte(backendWeights), &weightsMap)
 	if err != nil {
-		return 0, false
+		return 0
 	}
 	if weight, ok := weightsMap[backend]; ok {
-		return float64(weight) / 100, true
+		return float64(weight) / 100
 	}
-	return 0, false
+	return 0
 }
 
 func getWeights(ingressAnnotations map[string]string, backendAnnotations []string, backend string) float64 {
-	var maxWeight float64 = -1
+	maxWeight := 0.0
-	weightSet := false
+	annotationsPresent := false
+
 	for _, anno := range backendAnnotations {
 		if weightsMap, ok := ingressAnnotations[anno]; ok {
-			weight, isPresent := getAnnotationWeight(weightsMap, backend)
+			annotationsPresent = true
-			if isPresent {
+			maxWeight = math.Max(maxWeight, getAnnotationWeight(weightsMap, backend))
-				weightSet = true
-				if weight > maxWeight {
-					maxWeight = weight
 		}
 	}
-		}
+
-	}
+	// Fallback for ingresses that don't use traffic switching
-	if weightSet {
+	if !annotationsPresent {
-		return maxWeight
-	}
 		return 1.0
 	}
 
+	return maxWeight
+}
+
 // getCollector returns a collector for getting the metrics.
 func (c *SkipperCollector) getCollector() (Collector, error) {
 	ingress, err := c.client.ExtensionsV1beta1().Ingresses(c.objectReference.Namespace).Get(c.objectReference.Name, metav1.GetOptions{})

pkg/collector/skipper_collector_test.go

@@ -177,7 +177,7 @@ func TestSkipperCollector(t *testing.T) {
 			msg:                "test backend is not set",
 			metrics:            []int{100, 1500, 700},
 			ingressName:        "dummy-ingress",
-			collectedMetric:    1500,
+			collectedMetric:    0,
 			namespace:          "default",
 			backend:            "backend3",
 			backendWeights:     map[string]map[string]int{testBackendWeightsAnnotation: {"backend2": 100, "backend1": 0}},
@@ -185,6 +185,18 @@ func TestSkipperCollector(t *testing.T) {
 			readyReplicas:      1,
 			backendAnnotations: []string{testBackendWeightsAnnotation},
 		},
+		{
+			msg:                "test no annotations set",
+			metrics:            []int{100, 1500, 700},
+			ingressName:        "dummy-ingress",
+			collectedMetric:    1500,
+			namespace:          "default",
+			backend:            "backend3",
+			backendWeights:     map[string]map[string]int{testBackendWeightsAnnotation: {"backend2": 100, "backend1": 0}},
+			replicas:           1,
+			readyReplicas:      1,
+			backendAnnotations: []string{},
+		},
 		{
 			msg:             "test partial backend annotations",
 			metrics:         []int{100, 1500, 700},
@@ -201,7 +213,7 @@ func TestSkipperCollector(t *testing.T) {
 			backendAnnotations: []string{testBackendWeightsAnnotation, testStacksetWeightsAnnotation},
 		},
 	} {
-		t.Run(tc.msg, func(tt *testing.T) {
+		t.Run(tc.msg, func(t *testing.T) {
 			client := fake.NewSimpleClientset()
 			err := makeIngress(client, tc.namespace, tc.ingressName, tc.backend, tc.backendWeights)
 			require.NoError(t, err)
@@ -211,9 +223,9 @@ func TestSkipperCollector(t *testing.T) {
 			_, err = newDeployment(client, tc.namespace, tc.backend, tc.replicas, tc.readyReplicas)
 			require.NoError(t, err)
 			collector, err := NewSkipperCollector(client, plugin, hpa, config, time.Minute, tc.backendAnnotations, tc.backend)
-			require.NoError(tt, err, "failed to create skipper collector: %v", err)
+			require.NoError(t, err, "failed to create skipper collector: %v", err)
 			collected, err := collector.GetMetrics()
-			require.NoError(tt, err, "failed to collect metrics: %v", err)
+			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")
 		})