Skip to content

Commit

Permalink
Refactor label parsing to set OF labels last
Browse files Browse the repository at this point in the history
**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]>
  • Loading branch information
LucasRoesler committed Jun 23, 2018
1 parent d2a9878 commit f733c8e
Show file tree
Hide file tree
Showing 6 changed files with 155 additions and 50 deletions.
2 changes: 1 addition & 1 deletion handlers/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
38 changes: 4 additions & 34 deletions handlers/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"os"
"path/filepath"
"regexp"
"strconv"
"strings"

"github.com/openfaas/faas/gateway/requests"
Expand All @@ -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:
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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{
{
Expand Down Expand Up @@ -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
}
55 changes: 55 additions & 0 deletions handlers/labels.go
Original file line number Diff line number Diff line change
@@ -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)
}
91 changes: 91 additions & 0 deletions handlers/lables_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}

})
}
}
2 changes: 1 addition & 1 deletion handlers/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
17 changes: 3 additions & 14 deletions handlers/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit f733c8e

Please sign in to comment.