Use structured generator for kubectl autoscale

pkg/kubectl/autoscale.go

@@ -18,98 +18,69 @@ package kubectl
 
 import (
 	"fmt"
-	"strconv"
 
 	autoscalingv1 "k8s.io/api/autoscaling/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 )
 
-type HorizontalPodAutoscalerV1 struct{}
+// HorizontalPodAutoscalerV1Generator supports stable generation of a horizontal pod autoscaler.
-
+type HorizontalPodAutoscalerGeneratorV1 struct {
-func (HorizontalPodAutoscalerV1) ParamNames() []GeneratorParam {
+	Name               string
-	return []GeneratorParam{
+	ScaleRefKind       string
-		{"default-name", true},
+	ScaleRefName       string
-		{"name", false},
+	ScaleRefApiVersion string
-		{"scaleRef-kind", false},
+	MinReplicas        int32
-		{"scaleRef-name", false},
+	MaxReplicas        int32
-		{"scaleRef-apiVersion", false},
+	CPUPercent         int32
-		{"min", false},
-		{"max", true},
-		{"cpu-percent", false},
-	}
 }
 
-func (HorizontalPodAutoscalerV1) Generate(genericParams map[string]interface{}) (runtime.Object, error) {
+// Ensure it supports the generator pattern that uses parameters specified during construction.
-	return generateHPA(genericParams)
+var _ StructuredGenerator = &HorizontalPodAutoscalerGeneratorV1{}
-}
 
-func generateHPA(genericParams map[string]interface{}) (runtime.Object, error) {
+// StructuredGenerate outputs a horizontal pod autoscaler object using the configured fields.
-	params := map[string]string{}
+func (s *HorizontalPodAutoscalerGeneratorV1) StructuredGenerate() (runtime.Object, error) {
-	for key, value := range genericParams {
+	if err := s.validate(); err != nil {
-		strVal, isString := value.(string)
-		if !isString {
-			return nil, fmt.Errorf("expected string, saw %v for '%s'", value, key)
-		}
-		params[key] = strVal
-	}
-
-	name, found := params["name"]
-	if !found || len(name) == 0 {
-		name, found = params["default-name"]
-		if !found || len(name) == 0 {
-			return nil, fmt.Errorf("'name' is a required parameter.")
-		}
-	}
-	minString, found := params["min"]
-	min := -1
-	var err error
-	if found {
-		if min, err = strconv.Atoi(minString); err != nil {
-			return nil, err
-		}
-	}
-	maxString, found := params["max"]
-	if !found {
-		return nil, fmt.Errorf("'max' is a required parameter.")
-	}
-	max, err := strconv.Atoi(maxString)
-	if err != nil {
 		return nil, err
 	}
 
-	if min > max {
-		return nil, fmt.Errorf("'max' must be greater than or equal to 'min'.")
-	}
-
-	cpuString, found := params["cpu-percent"]
-	cpu := -1
-	if found {
-		if cpu, err = strconv.Atoi(cpuString); err != nil {
-			return nil, err
-		}
-	}
-
 	scaler := autoscalingv1.HorizontalPodAutoscaler{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: name,
+			Name: s.Name,
 		},
 		Spec: autoscalingv1.HorizontalPodAutoscalerSpec{
 			ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
-				Kind:       params["scaleRef-kind"],
+				Kind:       s.ScaleRefKind,
-				Name:       params["scaleRef-name"],
+				Name:       s.ScaleRefName,
-				APIVersion: params["scaleRef-apiVersion"],
+				APIVersion: s.ScaleRefApiVersion,
 			},
-			MaxReplicas: int32(max),
+			MaxReplicas: s.MaxReplicas,
 		},
 	}
-	if min > 0 {
+
-		v := int32(min)
+	if s.MinReplicas > 0 {
+		v := int32(s.MinReplicas)
 		scaler.Spec.MinReplicas = &v
 	}
-	if cpu >= 0 {
+	if s.CPUPercent >= 0 {
-		c := int32(cpu)
+		c := int32(s.CPUPercent)
 		scaler.Spec.TargetCPUUtilizationPercentage = &c
 	}
+
 	return &scaler, nil
 }
+
+// validate check if the caller has set the right fields.
+func (s HorizontalPodAutoscalerGeneratorV1) validate() error {
+	if len(s.Name) == 0 {
+		return fmt.Errorf("name must be specified")
+	}
+	if s.MaxReplicas <= 0 {
+		return fmt.Errorf("'max' is a required parameter and must be greater than zero")
+	}
+	if s.MinReplicas > s.MaxReplicas {
+		return fmt.Errorf("'max' must be greater than or equal to 'min'")
+	}
+	return nil
+}
+

pkg/kubectl/autoscale_test.go

@@ -26,22 +26,26 @@ import (
 
 func TestHPAGenerate(t *testing.T) {
 	tests := []struct {
-		name      string
+		name               string
-		params    map[string]interface{}
+		HPAName            string
-		expected  *autoscalingv1.HorizontalPodAutoscaler
+		scaleRefKind       string
-		expectErr bool
+		scaleRefName       string
+		scaleRefApiVersion string
+		minReplicas        int32
+		maxReplicas        int32
+		CPUPercent         int32
+		expected           *autoscalingv1.HorizontalPodAutoscaler
+		expectErr          bool
 	}{
 		{
-			name: "valid case",
+			name:               "valid case",
-			params: map[string]interface{}{
+			HPAName:            "foo",
-				"name":                "foo",
+			minReplicas:        1,
-				"min":                 "1",
+			maxReplicas:        10,
-				"max":                 "10",
+			CPUPercent:         80,
-				"cpu-percent":         "80",
+			scaleRefKind:       "kind",
-				"scaleRef-kind":       "kind",
+			scaleRefName:       "name",
-				"scaleRef-name":       "name",
+			scaleRefApiVersion: "apiVersion",
-				"scaleRef-apiVersion": "apiVersion",
-			},
 			expected: &autoscalingv1.HorizontalPodAutoscaler{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "foo",
@@ -60,79 +64,53 @@ func TestHPAGenerate(t *testing.T) {
 			expectErr: false,
 		},
 		{
-			name: "'name' is a required parameter",
+			name:               "'name' is a required parameter",
-			params: map[string]interface{}{
+			scaleRefKind:       "kind",
-				"scaleRef-kind":       "kind",
+			scaleRefName:       "name",
-				"scaleRef-name":       "name",
+			scaleRefApiVersion: "apiVersion",
-				"scaleRef-apiVersion": "apiVersion",
+			expectErr:          true,
-			},
-			expectErr: true,
 		},
 		{
-			name: "'max' is a required parameter",
+			name:               "'max' is a required parameter",
-			params: map[string]interface{}{
+			HPAName:            "foo",
-				"default-name":        "foo",
+			scaleRefKind:       "kind",
-				"scaleRef-kind":       "kind",
+			scaleRefName:       "name",
-				"scaleRef-name":       "name",
+			scaleRefApiVersion: "apiVersion",
-				"scaleRef-apiVersion": "apiVersion",
+			expectErr:          true,
-			},
-			expectErr: true,
 		},
 		{
-			name: "'max' must be greater than or equal to 'min'",
+			name:               "'max' must be greater than or equal to 'min'",
-			params: map[string]interface{}{
+			HPAName:            "foo",
-				"name":                "foo",
+			minReplicas:        10,
-				"min":                 "10",
+			maxReplicas:        1,
-				"max":                 "1",
+			scaleRefKind:       "kind",
-				"scaleRef-kind":       "kind",
+			scaleRefName:       "name",
-				"scaleRef-name":       "name",
+			scaleRefApiVersion: "apiVersion",
-				"scaleRef-apiVersion": "apiVersion",
+			expectErr:          true,
-			},
-			expectErr: true,
 		},
 		{
-			name: "cpu-percent must be an integer if specified",
+			name:               "'max' must be greater than zero",
-			params: map[string]interface{}{
+			HPAName:            "foo",
-				"name":                "foo",
+			minReplicas:        1,
-				"min":                 "1",
+			maxReplicas:        -10,
-				"max":                 "10",
+			scaleRefKind:       "kind",
-				"cpu-percent":         "",
+			scaleRefName:       "name",
-				"scaleRef-kind":       "kind",
+			scaleRefApiVersion: "apiVersion",
-				"scaleRef-name":       "name",
+			expectErr:          true,
-				"scaleRef-apiVersion": "apiVersion",
-			},
-			expectErr: true,
-		},
-		{
-			name: "'min' must be an integer if specified",
-			params: map[string]interface{}{
-				"name":                "foo",
-				"min":                 "foo",
-				"max":                 "10",
-				"cpu-percent":         "60",
-				"scaleRef-kind":       "kind",
-				"scaleRef-name":       "name",
-				"scaleRef-apiVersion": "apiVersion",
-			},
-			expectErr: true,
-		},
-		{
-			name: "'max' must be an integer if specified",
-			params: map[string]interface{}{
-				"name":                "foo",
-				"min":                 "1",
-				"max":                 "bar",
-				"cpu-percent":         "90",
-				"scaleRef-kind":       "kind",
-				"scaleRef-name":       "name",
-				"scaleRef-apiVersion": "apiVersion",
-			},
-			expectErr: true,
 		},
 	}
-	generator := HorizontalPodAutoscalerV1{}
+
 	for _, test := range tests {
-		obj, err := generator.Generate(test.params)
+		generator := HorizontalPodAutoscalerGeneratorV1{
+			Name:               test.HPAName,
+			ScaleRefKind:       test.scaleRefKind,
+			ScaleRefName:       test.scaleRefName,
+			ScaleRefApiVersion: test.scaleRefApiVersion,
+			MinReplicas:        test.minReplicas,
+			MaxReplicas:        test.maxReplicas,
+			CPUPercent:         test.CPUPercent,
+		}
+		obj, err := generator.StructuredGenerate()
 		if test.expectErr && err != nil {
 			continue
 		}