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 openfaas#209 Signed-off-by: Lucas Roesler <[email protected]>
LucasRoesler · Jun 23, 2018 · 99e468f · 99e468f
1 parent d2a9878
commit 99e468f
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 50 deletions.
diff --git a/handlers/delete.go b/handlers/delete.go
@@ -64,7 +64,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
@@ -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
-
 // ValidateDeployRequest validates that the service name is valid for Kubernetes
 func ValidateDeployRequest(request *requests.CreateFunctionRequest) error {
 	// Regex for RFC-1123 validation:
@@ -135,22 +131,9 @@ func makeDeploymentSpec(request requests.CreateFunctionRequest, existingSecrets
 		probe = nil
 	}
 
-	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 {
@@ -178,7 +161,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,
@@ -245,7 +228,7 @@ func makeServiceSpec(request requests.CreateFunctionRequest) *v1.Service {
 		Spec: v1.ServiceSpec{
 			Type: v1.ServiceTypeClusterIP,
 			Selector: map[string]string{
-				"faas_function": request.Service,
+				OFFunctionNameLabel: request.Service,
 			},
 			Ports: []v1.ServicePort{
 				{
@@ -346,16 +329,3 @@ 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
-}
diff --git a/handlers/labels.go 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/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
@@ -52,23 +52,12 @@ func MakeUpdateHandler(functionNamespace string, clientset *kubernetes.Clientset
 
 			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)
 
 			resources, resourceErr := createResources(request)
 			if resourceErr != nil {