From 310b5ec080a4df664db28dd08268675abe575aee Mon Sep 17 00:00:00 2001 From: Lucas Roesler Date: Sat, 23 Jun 2018 11:18:56 +0200 Subject: [PATCH 1/5] Refactor label parsing to set OF labels last **What** - Ensure that internal OF labels are set after user labels to ensure that users do no override these internal values - Refactor the getMinReplicaCount to work with the labels pointer, this helps simplify the control flow in the handler methods - Add constants for the label keys and update all references to those keys throughout Closes #209 Signed-off-by: Lucas Roesler --- handlers/delete.go | 2 +- handlers/deploy.go | 38 ++--------------- handlers/labels.go | 55 +++++++++++++++++++++++++ handlers/lables_test.go | 91 +++++++++++++++++++++++++++++++++++++++++ handlers/reader.go | 2 +- handlers/update.go | 17 ++------ 6 files changed, 155 insertions(+), 50 deletions(-) create mode 100644 handlers/labels.go create mode 100644 handlers/lables_test.go diff --git a/handlers/delete.go b/handlers/delete.go index 6ab9e7010..ecece6e88 100644 --- a/handlers/delete.go +++ b/handlers/delete.go @@ -66,7 +66,7 @@ func MakeDeleteHandler(functionNamespace string, clientset *kubernetes.Clientset func isFunction(deployment *v1beta1.Deployment) bool { if deployment != nil { - if _, found := deployment.Labels["faas_function"]; found { + if _, found := deployment.Labels[OFFunctionNameLabel]; found { return true } } diff --git a/handlers/deploy.go b/handlers/deploy.go index 7d5c4862a..46d732f70 100644 --- a/handlers/deploy.go +++ b/handlers/deploy.go @@ -12,7 +12,6 @@ import ( "os" "path/filepath" "regexp" - "strconv" "strings" "github.com/openfaas/faas/gateway/requests" @@ -28,9 +27,6 @@ import ( // watchdogPort for the OpenFaaS function watchdog const watchdogPort = 8080 -// initialReplicasCount how many replicas to start of creating for a function -const initialReplicasCount = 1 - // Regex for RFC-1123 validation: // k8s.io/kubernetes/pkg/util/validation/validation.go var validDNS = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$`) @@ -165,22 +161,9 @@ func makeDeploymentSpec(request requests.CreateFunctionRequest, existingSecrets FailureThreshold: 3, } - initialReplicas := int32p(initialReplicasCount) - labels := map[string]string{ - "faas_function": request.Service, - } - - if request.Labels != nil { - if min := getMinReplicaCount(*request.Labels); min != nil { - initialReplicas = min - } - for k, v := range *request.Labels { - labels[k] = v - } - } - + initialReplicas := getMinReplicaCount(request.Labels) + labels := parseLabels(request.Service, request.Labels) nodeSelector := createSelector(request.Constraints) - resources, resourceErr := createResources(request) if resourceErr != nil { @@ -210,7 +193,7 @@ func makeDeploymentSpec(request requests.CreateFunctionRequest, existingSecrets Spec: v1beta1.DeploymentSpec{ Selector: &metav1.LabelSelector{ MatchLabels: map[string]string{ - "faas_function": request.Service, + OFFunctionNameLabel: request.Service, }, }, Replicas: initialReplicas, @@ -283,7 +266,7 @@ func makeServiceSpec(request requests.CreateFunctionRequest) *corev1.Service { Spec: corev1.ServiceSpec{ Type: corev1.ServiceTypeClusterIP, Selector: map[string]string{ - "faas_function": request.Service, + OFFunctionNameLabel: request.Service, }, Ports: []corev1.ServicePort{ { @@ -399,19 +382,6 @@ func createResources(request requests.CreateFunctionRequest) (*apiv1.ResourceReq return resources, nil } -func getMinReplicaCount(labels map[string]string) *int32 { - if value, exists := labels["com.openfaas.scale.min"]; exists { - minReplicas, err := strconv.Atoi(value) - if err == nil && minReplicas > 0 { - return int32p(int32(minReplicas)) - } - - log.Println(err) - } - - return nil -} - // configureReadOnlyRootFilesystem will create or update the required settings and mounts to ensure // that the ReadOnlyRootFilesystem setting works as expected, meaning: // 1. when ReadOnlyRootFilesystem is true, the security context of the container will have ReadOnlyRootFilesystem also diff --git a/handlers/labels.go b/handlers/labels.go new file mode 100644 index 000000000..3adbab7a9 --- /dev/null +++ b/handlers/labels.go @@ -0,0 +1,55 @@ +package handlers + +import ( + "log" + "strconv" +) + +const ( + // initialReplicasCount how many replicas to start of creating for a function, this is + // also used as the default return value for getMinReplicaCount + initialReplicasCount = 1 + + // OFFunctionNameLabel is the label key used by OpenFaaS to store the function name + // on the resources managed by OpenFaaS for that function. This key is also used to + // denote that a resource is a "Function" + OFFunctionNameLabel = "faas_function" + // OFFunctionMinReplicaCount is a label that user's can set and will be passed to Kubernetes + // as the Deployment replicas value. + OFFunctionMinReplicaCount = "com.openfaas.scale.min" +) + +// parseLabels will copy the user request labels and ensure that any required internal labels +// are set appropriately. +func parseLabels(functionName string, requestLables *map[string]string) map[string]string { + labels := map[string]string{} + if requestLables != nil { + for k, v := range *requestLables { + labels[k] = v + } + } + + labels[OFFunctionNameLabel] = functionName + + return labels +} + +// 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 { + if labels == nil { + return int32p(initialReplicasCount) + } + + l := *labels + if value, exists := l[OFFunctionMinReplicaCount]; exists { + minReplicas, err := strconv.Atoi(value) + if err == nil && minReplicas > 0 { + return int32p(int32(minReplicas)) + } + + log.Println(err) + } + + return int32p(initialReplicasCount) +} diff --git a/handlers/lables_test.go b/handlers/lables_test.go new file mode 100644 index 000000000..043678543 --- /dev/null +++ b/handlers/lables_test.go @@ -0,0 +1,91 @@ +package handlers + +import "testing" + +func Test_getMinReplicaCount(t *testing.T) { + scenarios := []struct { + name string + labels *map[string]string + output int + }{ + { + name: "nil map returns default", + labels: nil, + output: initialReplicasCount, + }, + { + name: "empty map returns default", + labels: &map[string]string{}, + output: initialReplicasCount, + }, + { + name: "empty map returns default", + labels: &map[string]string{OFFunctionMinReplicaCount: "2"}, + output: 2, + }, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + output := getMinReplicaCount(s.labels) + if output == nil { + t.Errorf("getMinReplicaCount should not return nil pointer") + } + + value := int(*output) + if value != s.output { + t.Errorf("expected: %d, got %d", s.output, value) + } + }) + } +} + +func Test_parseLabels(t *testing.T) { + scenarios := []struct { + name string + labels *map[string]string + functionName string + output map[string]string + }{ + { + name: "nil map returns just the function name", + labels: nil, + functionName: "testFunc", + output: map[string]string{OFFunctionNameLabel: "testFunc"}, + }, + { + name: "empty map returns just the function name", + labels: &map[string]string{}, + functionName: "testFunc", + output: map[string]string{OFFunctionNameLabel: "testFunc"}, + }, + { + name: "non-empty map does not overwrite the function name label", + labels: &map[string]string{OFFunctionNameLabel: "anotherValue", "customLabel": "test"}, + functionName: "testFunc", + output: map[string]string{OFFunctionNameLabel: "testFunc", "customLabel": "test"}, + }, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + output := parseLabels(s.functionName, s.labels) + if output == nil { + t.Errorf("parseLabels should not return nil map") + } + + outputFuncName := output[OFFunctionNameLabel] + if outputFuncName != s.functionName { + t.Errorf("parseLabels should always set the function name: expected %s, got %s", s.functionName, outputFuncName) + } + + for key, value := range output { + expectedValue := s.output[key] + if value != expectedValue { + t.Errorf("Incorrect output label for %s, expected: %s, got %s", key, expectedValue, value) + } + } + + }) + } +} diff --git a/handlers/reader.go b/handlers/reader.go index 89db7f107..7d2aa1812 100644 --- a/handlers/reader.go +++ b/handlers/reader.go @@ -39,7 +39,7 @@ func getServiceList(functionNamespace string, clientset *kubernetes.Clientset) ( functions := []requests.Function{} listOpts := metav1.ListOptions{ - LabelSelector: "faas_function", + LabelSelector: OFFunctionNameLabel, } res, err := clientset.ExtensionsV1beta1().Deployments(functionNamespace).List(listOpts) diff --git a/handlers/update.go b/handlers/update.go index b70c7a5b1..0c27d966f 100644 --- a/handlers/update.go +++ b/handlers/update.go @@ -72,23 +72,12 @@ func updateDeploymentSpec( deployment.Spec.Template.Spec.NodeSelector = createSelector(request.Constraints) - labels := map[string]string{ - "faas_function": request.Service, - "uid": fmt.Sprintf("%d", time.Now().Nanosecond()), - } - - if request.Labels != nil { - if min := getMinReplicaCount(*request.Labels); min != nil { - deployment.Spec.Replicas = min - } - - for k, v := range *request.Labels { - labels[k] = v - } - } + labels := parseLabels(request.Service, request.Labels) + labels["uid"] = fmt.Sprintf("%d", time.Now().Nanosecond()) deployment.Labels = labels deployment.Spec.Template.ObjectMeta.Labels = labels + deployment.Spec.Replicas = getMinReplicaCount(request.Labels) deployment.Annotations = annotations deployment.Spec.Template.Annotations = annotations From 5344c15d8959d84046e767f0fe8c49cc208df6d0 Mon Sep 17 00:00:00 2001 From: Lucas Roesler Date: Sat, 23 Jun 2018 18:07:59 +0200 Subject: [PATCH 2/5] Update label constants to remove OF prefix Signed-off-by: Lucas Roesler --- handlers/delete.go | 2 +- handlers/deploy.go | 4 ++-- handlers/labels.go | 12 ++++++------ handlers/lables_test.go | 12 ++++++------ handlers/reader.go | 2 +- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/handlers/delete.go b/handlers/delete.go index ecece6e88..7b69fa120 100644 --- a/handlers/delete.go +++ b/handlers/delete.go @@ -66,7 +66,7 @@ func MakeDeleteHandler(functionNamespace string, clientset *kubernetes.Clientset func isFunction(deployment *v1beta1.Deployment) bool { if deployment != nil { - if _, found := deployment.Labels[OFFunctionNameLabel]; found { + if _, found := deployment.Labels[FunctionNameLabel]; found { return true } } diff --git a/handlers/deploy.go b/handlers/deploy.go index 46d732f70..e9d9d3fac 100644 --- a/handlers/deploy.go +++ b/handlers/deploy.go @@ -193,7 +193,7 @@ func makeDeploymentSpec(request requests.CreateFunctionRequest, existingSecrets Spec: v1beta1.DeploymentSpec{ Selector: &metav1.LabelSelector{ MatchLabels: map[string]string{ - OFFunctionNameLabel: request.Service, + FunctionNameLabel: request.Service, }, }, Replicas: initialReplicas, @@ -266,7 +266,7 @@ func makeServiceSpec(request requests.CreateFunctionRequest) *corev1.Service { Spec: corev1.ServiceSpec{ Type: corev1.ServiceTypeClusterIP, Selector: map[string]string{ - OFFunctionNameLabel: request.Service, + FunctionNameLabel: request.Service, }, Ports: []corev1.ServicePort{ { diff --git a/handlers/labels.go b/handlers/labels.go index 3adbab7a9..beb0b1190 100644 --- a/handlers/labels.go +++ b/handlers/labels.go @@ -10,13 +10,13 @@ const ( // also used as the default return value for getMinReplicaCount initialReplicasCount = 1 - // OFFunctionNameLabel is the label key used by OpenFaaS to store the function name + // FunctionNameLabel is the label key used by OpenFaaS to store the function name // on the resources managed by OpenFaaS for that function. This key is also used to // denote that a resource is a "Function" - OFFunctionNameLabel = "faas_function" - // OFFunctionMinReplicaCount is a label that user's can set and will be passed to Kubernetes + FunctionNameLabel = "faas_function" + // FunctionMinReplicaCount is a label that user's can set and will be passed to Kubernetes // as the Deployment replicas value. - OFFunctionMinReplicaCount = "com.openfaas.scale.min" + FunctionMinReplicaCount = "com.openfaas.scale.min" ) // parseLabels will copy the user request labels and ensure that any required internal labels @@ -29,7 +29,7 @@ func parseLabels(functionName string, requestLables *map[string]string) map[stri } } - labels[OFFunctionNameLabel] = functionName + labels[FunctionNameLabel] = functionName return labels } @@ -42,7 +42,7 @@ func getMinReplicaCount(labels *map[string]string) *int32 { } l := *labels - if value, exists := l[OFFunctionMinReplicaCount]; exists { + if value, exists := l[FunctionMinReplicaCount]; exists { minReplicas, err := strconv.Atoi(value) if err == nil && minReplicas > 0 { return int32p(int32(minReplicas)) diff --git a/handlers/lables_test.go b/handlers/lables_test.go index 043678543..80bd569e5 100644 --- a/handlers/lables_test.go +++ b/handlers/lables_test.go @@ -20,7 +20,7 @@ func Test_getMinReplicaCount(t *testing.T) { }, { name: "empty map returns default", - labels: &map[string]string{OFFunctionMinReplicaCount: "2"}, + labels: &map[string]string{FunctionMinReplicaCount: "2"}, output: 2, }, } @@ -51,19 +51,19 @@ func Test_parseLabels(t *testing.T) { name: "nil map returns just the function name", labels: nil, functionName: "testFunc", - output: map[string]string{OFFunctionNameLabel: "testFunc"}, + output: map[string]string{FunctionNameLabel: "testFunc"}, }, { name: "empty map returns just the function name", labels: &map[string]string{}, functionName: "testFunc", - output: map[string]string{OFFunctionNameLabel: "testFunc"}, + output: map[string]string{FunctionNameLabel: "testFunc"}, }, { name: "non-empty map does not overwrite the function name label", - labels: &map[string]string{OFFunctionNameLabel: "anotherValue", "customLabel": "test"}, + labels: &map[string]string{FunctionNameLabel: "anotherValue", "customLabel": "test"}, functionName: "testFunc", - output: map[string]string{OFFunctionNameLabel: "testFunc", "customLabel": "test"}, + output: map[string]string{FunctionNameLabel: "testFunc", "customLabel": "test"}, }, } @@ -74,7 +74,7 @@ func Test_parseLabels(t *testing.T) { t.Errorf("parseLabels should not return nil map") } - outputFuncName := output[OFFunctionNameLabel] + outputFuncName := output[FunctionNameLabel] if outputFuncName != s.functionName { t.Errorf("parseLabels should always set the function name: expected %s, got %s", s.functionName, outputFuncName) } diff --git a/handlers/reader.go b/handlers/reader.go index 7d2aa1812..12b63f2ff 100644 --- a/handlers/reader.go +++ b/handlers/reader.go @@ -39,7 +39,7 @@ func getServiceList(functionNamespace string, clientset *kubernetes.Clientset) ( functions := []requests.Function{} listOpts := metav1.ListOptions{ - LabelSelector: OFFunctionNameLabel, + LabelSelector: FunctionNameLabel, } res, err := clientset.ExtensionsV1beta1().Deployments(functionNamespace).List(listOpts) From 8d22ce8bc9b6e5057bb67df0d873261bd155d094 Mon Sep 17 00:00:00 2001 From: Lucas Roesler Date: Sat, 23 Jun 2018 18:16:04 +0200 Subject: [PATCH 3/5] Move the uid into teh common label method **What** - moves the uid label into a constant - sets it inside the labels method so that it is applied consistently Signed-off-by: Lucas Roesler --- handlers/labels.go | 6 ++++++ handlers/lables_test.go | 11 ++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/handlers/labels.go b/handlers/labels.go index beb0b1190..e208a0228 100644 --- a/handlers/labels.go +++ b/handlers/labels.go @@ -1,8 +1,10 @@ package handlers import ( + "fmt" "log" "strconv" + "time" ) const ( @@ -17,6 +19,9 @@ const ( // FunctionMinReplicaCount is a label that user's can set and will be passed to Kubernetes // as the Deployment replicas value. FunctionMinReplicaCount = "com.openfaas.scale.min" + // FunctionVersionUID is the lable key used to store the uid value for the deploy/update of a + // function, this is currently a unix timestamp. + FunctionVersionUID = "com.openfaas.uid" ) // parseLabels will copy the user request labels and ensure that any required internal labels @@ -30,6 +35,7 @@ func parseLabels(functionName string, requestLables *map[string]string) map[stri } labels[FunctionNameLabel] = functionName + labels[FunctionVersionUID] = fmt.Sprintf("%d", time.Now().Nanosecond()) return labels } diff --git a/handlers/lables_test.go b/handlers/lables_test.go index 80bd569e5..466760e4f 100644 --- a/handlers/lables_test.go +++ b/handlers/lables_test.go @@ -60,7 +60,7 @@ func Test_parseLabels(t *testing.T) { output: map[string]string{FunctionNameLabel: "testFunc"}, }, { - name: "non-empty map does not overwrite the function name label", + name: "non-empty map does not overwrite the openfaas internal labels", labels: &map[string]string{FunctionNameLabel: "anotherValue", "customLabel": "test"}, functionName: "testFunc", output: map[string]string{FunctionNameLabel: "testFunc", "customLabel": "test"}, @@ -79,8 +79,13 @@ func Test_parseLabels(t *testing.T) { t.Errorf("parseLabels should always set the function name: expected %s, got %s", s.functionName, outputFuncName) } - for key, value := range output { - expectedValue := s.output[key] + _, ok := output[FunctionVersionUID] + if !ok { + t.Errorf("parseLabels should set the FunctionVersionUID label") + } + + for key, expectedValue := range s.output { + value := output[key] if value != expectedValue { t.Errorf("Incorrect output label for %s, expected: %s, got %s", key, expectedValue, value) } From 3413f2540a2a674f4f31bbb7ab3323bde5f6181d Mon Sep 17 00:00:00 2001 From: Lucas Roesler Date: Sat, 23 Jun 2018 18:22:04 +0200 Subject: [PATCH 4/5] Rename parseLabels to buildLabels **What** - rename parseLabels to buildLables to make the name more semantic Signed-off-by: Lucas Roesler --- handlers/deploy.go | 2 +- handlers/labels.go | 4 ++-- handlers/lables_test.go | 2 +- handlers/update.go | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/handlers/deploy.go b/handlers/deploy.go index e9d9d3fac..bac904e85 100644 --- a/handlers/deploy.go +++ b/handlers/deploy.go @@ -162,7 +162,7 @@ func makeDeploymentSpec(request requests.CreateFunctionRequest, existingSecrets } initialReplicas := getMinReplicaCount(request.Labels) - labels := parseLabels(request.Service, request.Labels) + labels := buildLabels(request.Service, request.Labels) nodeSelector := createSelector(request.Constraints) resources, resourceErr := createResources(request) diff --git a/handlers/labels.go b/handlers/labels.go index e208a0228..58ac97f43 100644 --- a/handlers/labels.go +++ b/handlers/labels.go @@ -24,9 +24,9 @@ const ( FunctionVersionUID = "com.openfaas.uid" ) -// parseLabels will copy the user request labels and ensure that any required internal labels +// buildLabels will copy the user request labels and ensure that any required internal labels // are set appropriately. -func parseLabels(functionName string, requestLables *map[string]string) map[string]string { +func buildLabels(functionName string, requestLables *map[string]string) map[string]string { labels := map[string]string{} if requestLables != nil { for k, v := range *requestLables { diff --git a/handlers/lables_test.go b/handlers/lables_test.go index 466760e4f..5ca411f9a 100644 --- a/handlers/lables_test.go +++ b/handlers/lables_test.go @@ -69,7 +69,7 @@ func Test_parseLabels(t *testing.T) { for _, s := range scenarios { t.Run(s.name, func(t *testing.T) { - output := parseLabels(s.functionName, s.labels) + output := buildLabels(s.functionName, s.labels) if output == nil { t.Errorf("parseLabels should not return nil map") } diff --git a/handlers/update.go b/handlers/update.go index 0c27d966f..30a58f7bc 100644 --- a/handlers/update.go +++ b/handlers/update.go @@ -72,7 +72,7 @@ func updateDeploymentSpec( deployment.Spec.Template.Spec.NodeSelector = createSelector(request.Constraints) - labels := parseLabels(request.Service, request.Labels) + labels := buildLabels(request.Service, request.Labels) labels["uid"] = fmt.Sprintf("%d", time.Now().Nanosecond()) deployment.Labels = labels From 6edf7e35a42bfb3fe2a0763066bb917ecab11724 Mon Sep 17 00:00:00 2001 From: Lucas Roesler Date: Sat, 23 Jun 2018 18:52:48 +0200 Subject: [PATCH 5/5] Propagate getMinReplicaCount error to the handler **What** - Return parsing errors from getMinReplicaCount - Add tests for the expected errors Signed-off-by: Lucas Roesler --- handlers/deploy.go | 5 +++- handlers/labels.go | 18 +++++++++------ handlers/lables_test.go | 51 +++++++++++++++++++++++++++++++++++++++-- handlers/update.go | 7 +++++- 4 files changed, 70 insertions(+), 11 deletions(-) diff --git a/handlers/deploy.go b/handlers/deploy.go index bac904e85..0c2a63c7d 100644 --- a/handlers/deploy.go +++ b/handlers/deploy.go @@ -161,7 +161,10 @@ func makeDeploymentSpec(request requests.CreateFunctionRequest, existingSecrets FailureThreshold: 3, } - 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 index 58ac97f43..736cd3bcf 100644 --- 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 index 5ca411f9a..1f1371d8d 100644 --- 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 index 30a58f7bc..66c8df18f 100644 --- a/handlers/update.go +++ b/handlers/update.go @@ -77,7 +77,12 @@ func updateDeploymentSpec( deployment.Labels = labels deployment.Spec.Template.ObjectMeta.Labels = labels - deployment.Spec.Replicas = getMinReplicaCount(request.Labels) + + replicaCount, replicaErr := getMinReplicaCount(request.Labels) + if replicaErr != nil { + return replicaErr, http.StatusBadRequest + } + deployment.Spec.Replicas = replicaCount deployment.Annotations = annotations deployment.Spec.Template.Annotations = annotations