Propagate getMinReplicaCount error to the handler

**What** - Return parsing errors from getMinReplicaCount - Add tests for the expected errors Signed-off-by: Lucas Roesler <[email protected]>
openfaas · Jun 23, 2018 · b24d6fe · b24d6fe
1 parent c64dec6
commit b24d6fe
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 11 deletions.
diff --git a/handlers/deploy.go b/handlers/deploy.go
@@ -131,7 +131,10 @@ func makeDeploymentSpec(request requests.CreateFunctionRequest, existingSecrets
 		probe = nil
 	}
 
-	initialReplicas := getMinReplicaCount(request.Labels)
+	initialReplicas, replicaErr := getMinReplicaCount(request.Labels)
+	if replicaErr != nil {
+		return nil, replicaErr
+	}
 	labels := buildLabels(request.Service, request.Labels)
 	nodeSelector := createSelector(request.Constraints)
 	resources, resourceErr := createResources(request)

diff --git a/handlers/labels.go b/handlers/labels.go
@@ -1,8 +1,8 @@
 package handlers
 
 import (
+	"errors"
 	"fmt"
-	"log"
 	"strconv"
 	"time"
 )
@@ -42,20 +42,24 @@ func buildLabels(functionName string, requestLables *map[string]string) map[stri
 
 // getMinReplicaCount extracts the functions minimum replica count from the user's
 // request labels. If the value is not found, this will return the default value, 1.
-func getMinReplicaCount(labels *map[string]string) *int32 {
+func getMinReplicaCount(labels *map[string]string) (*int32, error) {
 	if labels == nil {
-		return int32p(initialReplicasCount)
+		return int32p(initialReplicasCount), nil
 	}
 
 	l := *labels
 	if value, exists := l[FunctionMinReplicaCount]; exists {
 		minReplicas, err := strconv.Atoi(value)
-		if err == nil && minReplicas > 0 {
-			return int32p(int32(minReplicas))
+		if err != nil {
+			return nil, errors.New("could not parse the minimum replica value")
 		}
 
-		log.Println(err)
+		if minReplicas > 0 {
+			return int32p(int32(minReplicas)), nil
+		}
+
+		return nil, errors.New("replica count must be a positive integer")
 	}
 
-	return int32p(initialReplicasCount)
+	return int32p(initialReplicasCount), nil
 }
diff --git a/handlers/lables_test.go b/handlers/lables_test.go
@@ -1,6 +1,9 @@
 package handlers
 
-import "testing"
+import (
+	"strings"
+	"testing"
+)
 
 func Test_getMinReplicaCount(t *testing.T) {
 	scenarios := []struct {
@@ -27,7 +30,10 @@ func Test_getMinReplicaCount(t *testing.T) {
 
 	for _, s := range scenarios {
 		t.Run(s.name, func(t *testing.T) {
-			output := getMinReplicaCount(s.labels)
+			output, err := getMinReplicaCount(s.labels)
+			if err != nil {
+				t.Errorf("getMinReplicaCount should not error on an empty or valid label")
+			}
 			if output == nil {
 				t.Errorf("getMinReplicaCount should not return nil pointer")
 			}
@@ -40,6 +46,47 @@ func Test_getMinReplicaCount(t *testing.T) {
 	}
 }
 
+func Test_getMinReplicaCount_Error(t *testing.T) {
+	scenarios := []struct {
+		name   string
+		labels *map[string]string
+		msg    string
+	}{
+		{
+			name:   "negative values should return an error",
+			labels: &map[string]string{FunctionMinReplicaCount: "-2"},
+			msg:    "replica count must be a positive integer",
+		},
+		{
+			name:   "zero values should return an error",
+			labels: &map[string]string{FunctionMinReplicaCount: "0"},
+			msg:    "replica count must be a positive integer",
+		},
+		{
+			name:   "decimal values should return an error",
+			labels: &map[string]string{FunctionMinReplicaCount: "10.5"},
+			msg:    "could not parse the minimum replica value",
+		},
+		{
+			name:   "non-integer values should return an error",
+			labels: &map[string]string{FunctionMinReplicaCount: "test"},
+			msg:    "could not parse the minimum replica value",
+		},
+	}
+
+	for _, s := range scenarios {
+		t.Run(s.name, func(t *testing.T) {
+			output, err := getMinReplicaCount(s.labels)
+			if output != nil {
+				t.Errorf("getMinReplicaCount should return nil value on invalid input")
+			}
+			if !strings.Contains(err.Error(), s.msg) {
+				t.Errorf("unexpected error: expected '%s', got '%s'", s.msg, err.Error())
+			}
+		})
+	}
+}
+
 func Test_parseLabels(t *testing.T) {
 	scenarios := []struct {
 		name         string

diff --git a/handlers/update.go b/handlers/update.go
@@ -53,7 +53,14 @@ func MakeUpdateHandler(functionNamespace string, clientset *kubernetes.Clientset
 			labels := buildLabels(request.Service, request.Labels)
 			deployment.Labels = labels
 			deployment.Spec.Template.ObjectMeta.Labels = labels
-			deployment.Spec.Replicas = getMinReplicaCount(request.Labels)
+
+			replicaCount, replicaErr := getMinReplicaCount(request.Labels)
+			if replicaErr != nil {
+				w.WriteHeader(http.StatusBadRequest)
+				w.Write([]byte(replicaErr.Error()))
+				return
+			}
+			deployment.Spec.Replicas = replicaCount
 
 			resources, resourceErr := createResources(request)
 			if resourceErr != nil {