Added scale-metric flag to configure metric name (#1653)

* Added scale-metric flag to configure metric name

* Updated flag description

* code gen fix

* Added test for coverage

* Updated Changelog

CHANGELOG.adoc

@@ -11,6 +11,28 @@
 |
 | https://github.com/knative/client/pull/[#]
 ////
+## v1.4.0
+[cols="1,10,3", options="header", width="100%"]
+|===
+| | Description | PR
+
+| 🎁
+| Added --timeout flag to service create, update, apply
+| https://github.com/knative/client/pull/1643[#1643]
+
+| 🎁
+| Added --pull-policy flag to service create, update, apply
+| https://github.com/knative/client/pull/1644[#1644]
+
+| 🎁
+| Added --wait-window flag to configure error window between errors during service create
+| https://github.com/knative/client/pull/1645[#1645]
+
+| 🎁
+| Added --scale-metric flag to configure metric name
+| https://github.com/knative/client/pull/1653[#1653]
+
+|===
 ## v1.3.0 (2022-03-08)
 [cols="1,10,3", options="header", width="100%"]
 |===

docs/cmd/kn_service_apply.md

@@ -63,8 +63,9 @@ kn service apply s0 --filename my-svc.yml
       --scale string                      Set the Minimum and Maximum number of replicas. You can use this flag to set both to a single value, or set a range with min/max values, or set either min or max values without specifying the other. Example: --scale 5 (scale-min = 5, scale-max = 5) or --scale 1..5 (scale-min = 1, scale-max = 5) or --scale 1.. (scale-min = 1, scale-max = unchanged) or --scale ..5 (scale-min = unchanged, scale-max = 5)
       --scale-init int                    Initial number of replicas with which a service starts. Can be 0 or a positive integer.
       --scale-max int                     Maximum number of replicas.
+      --scale-metric string               Set the name of the metric the PodAutoscaler should scale on. Example: --scale-metric rps (to scale on rps) or --scale-metric concurrency (to scale on concurrency). The default metric is concurrency.
       --scale-min int                     Minimum number of replicas.
-      --scale-target int                  Recommendation for when to scale up based on the concurrent number of incoming request. Defaults to --concurrency-limit when given.
+      --scale-target int                  Recommendation for what metric value the PodAutoscaler should attempt to maintain. Use with --scale-metric flag to configure the metric name for which the target value should be maintained. Default metric name is concurrency. The flag defaults to --concurrency-limit when given.
       --scale-utilization int             Percentage of concurrent requests utilization before scaling up. (default 70)
       --scale-window string               Duration to look back for making auto-scaling decisions. The service is scaled to zero if no request was received in during that time. (eg: 10s)
       --service-account string            Service account name to set. An empty argument ("") clears the service account. The referenced service account must exist in the service's namespace.

docs/cmd/kn_service_create.md

@@ -88,8 +88,9 @@ kn service create NAME --image IMAGE
       --scale string                      Set the Minimum and Maximum number of replicas. You can use this flag to set both to a single value, or set a range with min/max values, or set either min or max values without specifying the other. Example: --scale 5 (scale-min = 5, scale-max = 5) or --scale 1..5 (scale-min = 1, scale-max = 5) or --scale 1.. (scale-min = 1, scale-max = unchanged) or --scale ..5 (scale-min = unchanged, scale-max = 5)
       --scale-init int                    Initial number of replicas with which a service starts. Can be 0 or a positive integer.
       --scale-max int                     Maximum number of replicas.
+      --scale-metric string               Set the name of the metric the PodAutoscaler should scale on. Example: --scale-metric rps (to scale on rps) or --scale-metric concurrency (to scale on concurrency). The default metric is concurrency.
       --scale-min int                     Minimum number of replicas.
-      --scale-target int                  Recommendation for when to scale up based on the concurrent number of incoming request. Defaults to --concurrency-limit when given.
+      --scale-target int                  Recommendation for what metric value the PodAutoscaler should attempt to maintain. Use with --scale-metric flag to configure the metric name for which the target value should be maintained. Default metric name is concurrency. The flag defaults to --concurrency-limit when given.
       --scale-utilization int             Percentage of concurrent requests utilization before scaling up. (default 70)
       --scale-window string               Duration to look back for making auto-scaling decisions. The service is scaled to zero if no request was received in during that time. (eg: 10s)
       --service-account string            Service account name to set. An empty argument ("") clears the service account. The referenced service account must exist in the service's namespace.

docs/cmd/kn_service_update.md

@@ -75,8 +75,9 @@ kn service update NAME
       --scale string                      Set the Minimum and Maximum number of replicas. You can use this flag to set both to a single value, or set a range with min/max values, or set either min or max values without specifying the other. Example: --scale 5 (scale-min = 5, scale-max = 5) or --scale 1..5 (scale-min = 1, scale-max = 5) or --scale 1.. (scale-min = 1, scale-max = unchanged) or --scale ..5 (scale-min = unchanged, scale-max = 5)
       --scale-init int                    Initial number of replicas with which a service starts. Can be 0 or a positive integer.
       --scale-max int                     Maximum number of replicas.
+      --scale-metric string               Set the name of the metric the PodAutoscaler should scale on. Example: --scale-metric rps (to scale on rps) or --scale-metric concurrency (to scale on concurrency). The default metric is concurrency.
       --scale-min int                     Minimum number of replicas.
-      --scale-target int                  Recommendation for when to scale up based on the concurrent number of incoming request. Defaults to --concurrency-limit when given.
+      --scale-target int                  Recommendation for what metric value the PodAutoscaler should attempt to maintain. Use with --scale-metric flag to configure the metric name for which the target value should be maintained. Default metric name is concurrency. The flag defaults to --concurrency-limit when given.
       --scale-utilization int             Percentage of concurrent requests utilization before scaling up. (default 70)
       --scale-window string               Duration to look back for making auto-scaling decisions. The service is scaled to zero if no request was received in during that time. (eg: 10s)
       --service-account string            Service account name to set. An empty argument ("") clears the service account. The referenced service account must exist in the service's namespace.

pkg/kn/commands/service/configuration_edit_flags.go

@@ -42,6 +42,7 @@ type ConfigurationEditFlags struct {
 	MinScale            int
 	MaxScale            int
 	ScaleTarget         int
+	ScaleMetric         string
 	ConcurrencyLimit    int
 	ScaleUtilization    int
 	ScaleWindow         string
@@ -93,6 +94,10 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) {
 	command.Flags().IntVar(&p.MaxScale, "scale-max", 0, "Maximum number of replicas.")
 	p.markFlagMakesRevision("scale-max")
 
+	command.Flags().StringVar(&p.ScaleMetric, "scale-metric", "", "Set the name of the metric the PodAutoscaler should scale on. "+
+		"Example: --scale-metric rps (to scale on rps) or --scale-metric concurrency (to scale on concurrency). The default metric is concurrency.")
+	p.markFlagMakesRevision("scale-metric")
+
 	// DEPRECATED since 1.0
 	command.Flags().StringVar(&p.ScaleWindow, "autoscale-window", "", "Deprecated option, please use --scale-window")
 	p.markFlagMakesRevision("autoscale-window")
@@ -111,8 +116,10 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) {
 	command.Flags().MarkHidden("concurrency-target")
 
 	command.Flags().IntVar(&p.ScaleTarget, "scale-target", 0,
-		"Recommendation for when to scale up based on the concurrent number of incoming request. "+
-			"Defaults to --concurrency-limit when given.")
+		"Recommendation for what metric value the PodAutoscaler should attempt to maintain. "+
+			"Use with --scale-metric flag to configure the metric name for which the target value should be maintained. "+
+			"Default metric name is concurrency. "+
+			"The flag defaults to --concurrency-limit when given.")
 	p.markFlagMakesRevision("scale-target")
 
 	command.Flags().IntVar(&p.ConcurrencyLimit, "concurrency-limit", 0,
@@ -347,6 +354,10 @@ func (p *ConfigurationEditFlags) Apply(
 		}
 	}
 
+	if cmd.Flags().Changed("scale-metric") {
+		servinglib.UpdateScaleMetric(template, p.ScaleMetric)
+	}
+
 	if cmd.Flags().Changed("concurrency-limit") {
 		err = servinglib.UpdateConcurrencyLimit(template, int64(p.ConcurrencyLimit))
 		if err != nil {

pkg/kn/commands/service/configuration_edit_flags_test.go

@@ -47,3 +47,16 @@ func TestApplyPullPolicyFlagError(t *testing.T) {
 	err := editFlags.Apply(&svc, nil, cmd)
 	assert.Assert(t, util.ContainsAll(err.Error(), "invalid", "InvalidPolicy", "Valid arguments (case insensitive): Always | Never | IfNotPresent"))
 }
+
+func TestScaleMetric(t *testing.T) {
+	var editFlags ConfigurationEditFlags
+	knParams := &commands.KnParams{}
+	cmd, _, _ := commands.CreateTestKnCommand(NewServiceCreateCommand(knParams), knParams)
+
+	editFlags.AddCreateFlags(cmd)
+	svc := createTestService("test-svc", []string{"test-svc-00001", "test-svc-00002"}, goodConditions())
+	cmd.SetArgs([]string{"--scale-metric", "rps"})
+	cmd.Execute()
+	err := editFlags.Apply(&svc, nil, cmd)
+	assert.NilError(t, err)
+}

pkg/serving/config_changes.go

@@ -214,6 +214,14 @@ func UpdateRevisionTemplateAnnotation(template *servingv1.RevisionTemplateSpec,
 	return UpdateRevisionTemplateAnnotations(template, map[string]string{annotation: value}, []string{})
 }
 
+// UpdateScaleMetric updates the metric annotation for the given Revision Template
+func UpdateScaleMetric(template *servingv1.RevisionTemplateSpec, metric string) {
+	if template.Annotations == nil {
+		template.Annotations = make(map[string]string)
+	}
+	template.Annotations[autoscaling.MetricAnnotationKey] = metric
+}
+
 // =======================================================================================
 
 func updateAnnotations(annotations map[string]string, toUpdate map[string]string, toRemove []string) error {

pkg/serving/config_changes_test.go

@@ -456,6 +456,65 @@ func TestUpdateAnnotationsRemoveExisting(t *testing.T) {
 	assert.DeepEqual(t, expected, actual)
 }
 
+func TestUpdateScaleMetric(t *testing.T) {
+	type args struct {
+		template *servingv1.RevisionTemplateSpec
+		metric   string
+	}
+	tests := []struct {
+		name string
+		args args
+	}{
+		{
+			"Update scale metric to rps",
+			args{
+				&servingv1.RevisionTemplateSpec{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "template-foo",
+						Namespace: "default",
+						Annotations: map[string]string{
+							autoscaling.MetricAnnotationKey: "rps",
+						},
+					},
+					Spec: servingv1.RevisionSpec{
+						PodSpec: corev1.PodSpec{
+							Containers: []corev1.Container{{}},
+						},
+					},
+				},
+				"rps",
+			},
+		},
+		{
+			"Update scale metric to cpu",
+			args{
+				&servingv1.RevisionTemplateSpec{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "template-foo",
+						Namespace: "default",
+						Annotations: map[string]string{
+							autoscaling.MetricAnnotationKey: "concurrency",
+						},
+					},
+					Spec: servingv1.RevisionSpec{
+						PodSpec: corev1.PodSpec{
+							Containers: []corev1.Container{{}},
+						},
+					},
+				},
+				"concurrency",
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			actualTemplate, _ := getRevisionTemplate()
+			UpdateScaleMetric(actualTemplate, tt.args.metric)
+			assert.DeepEqual(t, tt.args.template, actualTemplate)
+		})
+	}
+}
+
 func TestString(t *testing.T) {
 	vt := ConfigMapVolumeSourceType
 	assert.Equal(t, "config-map", vt.String())