forked from openfaas/faas-netes
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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 <roesler.lucas@gmail.com>
- Loading branch information
1 parent
a65b904
commit b154215
Showing
6 changed files
with
155 additions
and
50 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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) | ||
| } | ||
| } | ||
|
|
||
| }) | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters