Merge pull request #55913 from wackxu/refauto

Automatic merge from submit-queue (batch tested with PRs 55900, 55995, 55913, 55467, 55376). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Use structured generator for kubectl autoscale

**What this PR does / why we need it**:

Use structured generator for kubectl autoscale

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
part of  # https://github.com/kubernetes/kubectl/issues/138

**Special notes for your reviewer**:
/assign @mengqiy 

**Release note**:

```release-note
Use structured generator for kubectl autoscale 
```

pkg/kubectl/autoscale.go

@@ -18,98 +18,68 @@ 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{}
-
-func (HorizontalPodAutoscalerV1) ParamNames() []GeneratorParam {
-	return []GeneratorParam{
-		{"default-name", true},
-		{"name", false},
-		{"scaleRef-kind", false},
-		{"scaleRef-name", false},
-		{"scaleRef-apiVersion", false},
-		{"min", false},
-		{"max", true},
-		{"cpu-percent", false},
-	}
+// HorizontalPodAutoscalerV1Generator supports stable generation of a horizontal pod autoscaler.
+type HorizontalPodAutoscalerGeneratorV1 struct {
+	Name               string
+	ScaleRefKind       string
+	ScaleRefName       string
+	ScaleRefApiVersion string
+	MinReplicas        int32
+	MaxReplicas        int32
+	CPUPercent         int32
 }
 
-func (HorizontalPodAutoscalerV1) Generate(genericParams map[string]interface{}) (runtime.Object, error) {
-	return generateHPA(genericParams)
-}
+// Ensure it supports the generator pattern that uses parameters specified during construction.
+var _ StructuredGenerator = &HorizontalPodAutoscalerGeneratorV1{}
 
-func generateHPA(genericParams map[string]interface{}) (runtime.Object, error) {
-	params := map[string]string{}
-	for key, value := range genericParams {
-		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 {
+// StructuredGenerate outputs a horizontal pod autoscaler object using the configured fields.
+func (s *HorizontalPodAutoscalerGeneratorV1) StructuredGenerate() (runtime.Object, error) {
+	if err := s.validate(); 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"],
-				Name:       params["scaleRef-name"],
-				APIVersion: params["scaleRef-apiVersion"],
+				Kind:       s.ScaleRefKind,
+				Name:       s.ScaleRefName,
+				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 {
-		c := int32(cpu)
+	if s.CPUPercent >= 0 {
+		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 < 1 {
+		return fmt.Errorf("'max' is a required parameter and must be at least 1")
+	}
+	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
-		params    map[string]interface{}
-		expected  *autoscalingv1.HorizontalPodAutoscaler
-		expectErr bool
+		name               string
+		HPAName            string
+		scaleRefKind       string
+		scaleRefName       string
+		scaleRefApiVersion string
+		minReplicas        int32
+		maxReplicas        int32
+		CPUPercent         int32
+		expected           *autoscalingv1.HorizontalPodAutoscaler
+		expectErr          bool
 	}{
 		{
-			name: "valid case",
-			params: map[string]interface{}{
-				"name":                "foo",
-				"min":                 "1",
-				"max":                 "10",
-				"cpu-percent":         "80",
-				"scaleRef-kind":       "kind",
-				"scaleRef-name":       "name",
-				"scaleRef-apiVersion": "apiVersion",
-			},
+			name:               "valid case",
+			HPAName:            "foo",
+			minReplicas:        1,
+			maxReplicas:        10,
+			CPUPercent:         80,
+			scaleRefKind:       "kind",
+			scaleRefName:       "name",
+			scaleRefApiVersion: "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",
-			params: map[string]interface{}{
-				"scaleRef-kind":       "kind",
-				"scaleRef-name":       "name",
-				"scaleRef-apiVersion": "apiVersion",
-			},
-			expectErr: true,
+			name:               "'name' is a required parameter",
+			scaleRefKind:       "kind",
+			scaleRefName:       "name",
+			scaleRefApiVersion: "apiVersion",
+			expectErr:          true,
 		},
 		{
-			name: "'max' is a required parameter",
-			params: map[string]interface{}{
-				"default-name":        "foo",
-				"scaleRef-kind":       "kind",
-				"scaleRef-name":       "name",
-				"scaleRef-apiVersion": "apiVersion",
-			},
-			expectErr: true,
+			name:               "'max' is a required parameter",
+			HPAName:            "foo",
+			scaleRefKind:       "kind",
+			scaleRefName:       "name",
+			scaleRefApiVersion: "apiVersion",
+			expectErr:          true,
 		},
 		{
-			name: "'max' must be greater than or equal to 'min'",
-			params: map[string]interface{}{
-				"name":                "foo",
-				"min":                 "10",
-				"max":                 "1",
-				"scaleRef-kind":       "kind",
-				"scaleRef-name":       "name",
-				"scaleRef-apiVersion": "apiVersion",
-			},
-			expectErr: true,
+			name:               "'max' must be greater than or equal to 'min'",
+			HPAName:            "foo",
+			minReplicas:        10,
+			maxReplicas:        1,
+			scaleRefKind:       "kind",
+			scaleRefName:       "name",
+			scaleRefApiVersion: "apiVersion",
+			expectErr:          true,
 		},
 		{
-			name: "cpu-percent must be an integer if specified",
-			params: map[string]interface{}{
-				"name":                "foo",
-				"min":                 "1",
-				"max":                 "10",
-				"cpu-percent":         "",
-				"scaleRef-kind":       "kind",
-				"scaleRef-name":       "name",
-				"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,
+			name:               "'max' must be at least 1",
+			HPAName:            "foo",
+			minReplicas:        1,
+			maxReplicas:        -10,
+			scaleRefKind:       "kind",
+			scaleRefName:       "name",
+			scaleRefApiVersion: "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
 		}

pkg/kubectl/cmd/autoscale.go

@@ -64,11 +64,11 @@ func NewCmdAutoscale(f cmdutil.Factory, out io.Writer) *cobra.Command {
 		ArgAliases: argAliases,
 	}
 	cmdutil.AddPrinterFlags(cmd)
-	cmd.Flags().String("generator", "horizontalpodautoscaler/v1", i18n.T("The name of the API generator to use. Currently there is only 1 generator."))
-	cmd.Flags().Int("min", -1, "The lower limit for the number of pods that can be set by the autoscaler. If it's not specified or negative, the server will apply a default value.")
-	cmd.Flags().Int("max", -1, "The upper limit for the number of pods that can be set by the autoscaler. Required.")
+	cmd.Flags().String("generator", cmdutil.HorizontalPodAutoscalerV1GeneratorName, i18n.T("The name of the API generator to use. Currently there is only 1 generator."))
+	cmd.Flags().Int32("min", -1, "The lower limit for the number of pods that can be set by the autoscaler. If it's not specified or negative, the server will apply a default value.")
+	cmd.Flags().Int32("max", -1, "The upper limit for the number of pods that can be set by the autoscaler. Required.")
 	cmd.MarkFlagRequired("max")
-	cmd.Flags().Int("cpu-percent", -1, fmt.Sprintf("The target average CPU utilization (represented as a percent of requested CPU) over all the pods. If it's not specified or negative, a default autoscaling policy will be used."))
+	cmd.Flags().Int32("cpu-percent", -1, fmt.Sprintf("The target average CPU utilization (represented as a percent of requested CPU) over all the pods. If it's not specified or negative, a default autoscaling policy will be used."))
 	cmd.Flags().String("name", "", i18n.T("The name for the newly created object. If not specified, the name of the input resource will be used."))
 	cmdutil.AddDryRunFlag(cmd)
 	usage := "identifying the resource to autoscale."
@@ -103,15 +103,6 @@ func RunAutoscale(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []s
 		return err
 	}
 
-	// Get the generator, setup and validate all required parameters
-	generatorName := cmdutil.GetFlagString(cmd, "generator")
-	generators := f.Generators("autoscale")
-	generator, found := generators[generatorName]
-	if !found {
-		return cmdutil.UsageErrorf(cmd, "generator %q not found.", generatorName)
-	}
-	names := generator.ParamNames()
-
 	count := 0
 	err = r.Visit(func(info *resource.Info, err error) error {
 		if err != nil {
@@ -123,24 +114,25 @@ func RunAutoscale(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []s
 			return err
 		}
 
-		name := info.Name
-		params := kubectl.MakeParams(cmd, names)
-		params["default-name"] = name
-
-		params["scaleRef-kind"] = mapping.GroupVersionKind.Kind
-		params["scaleRef-name"] = name
-		params["scaleRef-apiVersion"] = mapping.GroupVersionKind.GroupVersion().String()
-
-		if err = kubectl.ValidateParams(names, params); err != nil {
-			return err
-		}
-		// Check for invalid flags used against the present generator.
-		if err := kubectl.EnsureFlagsValid(cmd, generators, generatorName); err != nil {
-			return err
+		// get the generator
+		var generator kubectl.StructuredGenerator
+		switch generatorName := cmdutil.GetFlagString(cmd, "generator"); generatorName {
+		case cmdutil.HorizontalPodAutoscalerV1GeneratorName:
+			generator = &kubectl.HorizontalPodAutoscalerGeneratorV1{
+				Name:               info.Name,
+				MinReplicas:        cmdutil.GetFlagInt32(cmd, "min"),
+				MaxReplicas:        cmdutil.GetFlagInt32(cmd, "max"),
+				CPUPercent:         cmdutil.GetFlagInt32(cmd, "cpu-percent"),
+				ScaleRefName:       info.Name,
+				ScaleRefKind:       mapping.GroupVersionKind.Kind,
+				ScaleRefApiVersion: mapping.GroupVersionKind.GroupVersion().String(),
+			}
+		default:
+			return errUnsupportedGenerator(cmd, generatorName)
 		}
 
 		// Generate new object
-		object, err := generator.Generate(params)
+		object, err := generator.StructuredGenerate()
 		if err != nil {
 			return err
 		}
@@ -194,7 +186,7 @@ func RunAutoscale(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []s
 
 func validateFlags(cmd *cobra.Command) error {
 	errs := []error{}
-	max, min := cmdutil.GetFlagInt(cmd, "max"), cmdutil.GetFlagInt(cmd, "min")
+	max, min := cmdutil.GetFlagInt32(cmd, "max"), cmdutil.GetFlagInt32(cmd, "min")
 	if max < 1 {
 		errs = append(errs, fmt.Errorf("--max=MAXPODS is required and must be at least 1, max: %d", max))
 	}

pkg/kubectl/cmd/util/factory_client_access.go

@@ -538,10 +538,6 @@ func DefaultGenerators(cmdName string) map[string]kubectl.Generator {
 			CronJobV2Alpha1GeneratorName:       kubectl.CronJobV2Alpha1{},
 			CronJobV1Beta1GeneratorName:        kubectl.CronJobV1Beta1{},
 		}
-	case "autoscale":
-		generator = map[string]kubectl.Generator{
-			HorizontalPodAutoscalerV1GeneratorName: kubectl.HorizontalPodAutoscalerV1{},
-		}
 	case "namespace":
 		generator = map[string]kubectl.Generator{
 			NamespaceV1GeneratorName: kubectl.NamespaceGeneratorV1{},