Repository: knative/client (go)

Issue:
Add support to allow scaling metric type (concurrency, rps, cpu, memory)
<!-- If you need to report a security issue with Knative, send an email to knative-security@googlegroups.com. -->

### Feature request
Currently,  I can't set the autoscaler metric with rps.    How about add a new option `--autoscaler-metric` ?
### Use case

Currently, I can't achieve below knative creation with `kn`
```
cat <<EOF | kubectl apply -n default -f -
apiVersion: serving.knative.dev/v1
kind: Service
metadata:
  name: ceperftesthttp-rps
spec:
  template:
    metadata:
      annotations:
        autoscaling.knative.dev/metric: rps
        autoscaling.knative.dev/target: "100"
        autoscaling.knative.dev/minScale: "3"
        autoscaling.knative.dev/maxScale: "50"        
    spec:
      containerConcurrency: 100
      containers:
        - image: .....
EOF
```

### UI Example

how about using 
`kn service create --autoscaler-metric rps` for above?  The default value for `--autoscaler-metric` should be `concurrency`

Pull request:
Added scale-metric flag to configure metric name
## Description

Added `--scale-metric` flag to configure the metric name
<!-- Please add a short summary about what the pull request is going to bring on the table -->

## Changes

<!-- Please add list of more detailed changes. These changes should be reflected also in the commit messages -->

* Added flag
* Added unit test
* Validation already present in serving

## Reference

<!-- Please add the corresponding issue number which this pull request is about to fix -->
Fixes #1205 

<!--
Please add an entry to CHANGELOG.adoc file, too, as part of your Pull Request.

In the following cases, add a short description of PR to the unreleased section in CHANGELOG.adoc:

- 🎁 New feature
- 🐛 Bug fix
- ✨ Feature Update
- 🐣 Refactoring
- 🗑️ Remove feature or internal logic

See other entries in CHANGELOG.adoc as an example for how to add the entry, including a reference to the corresponding issue/PR

PLEASE DON'T ADD THAT LINE HERE IN THE PULL-REQUEST DESCRIPTION BUT DIRECTLY IN CHANGELOG.ADOC AND ADD CHANGELOG.ADOC AS PART OF YOUR PULL-REQUEST.
-->

<!--
To automatically lint go code in this pull request uncomment the line below. You get feedback as comments on your pull request then -->

<!--
/lint
-->


Patch:
diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc
index a6c7501e..ec3cf426 100644
--- a/CHANGELOG.adoc
+++ b/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%"]
 |===
diff --git a/docs/cmd/kn_service_apply.md b/docs/cmd/kn_service_apply.md
index 32a49f76..35c7234c 100644
--- a/docs/cmd/kn_service_apply.md
+++ b/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.
diff --git a/docs/cmd/kn_service_create.md b/docs/cmd/kn_service_create.md
index 50925c26..d522c335 100644
--- a/docs/cmd/kn_service_create.md
+++ b/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.
diff --git a/docs/cmd/kn_service_update.md b/docs/cmd/kn_service_update.md
index 5ef26057..86f46fcc 100644
--- a/docs/cmd/kn_service_update.md
+++ b/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.
diff --git a/pkg/kn/commands/service/configuration_edit_flags.go b/pkg/kn/commands/service/configuration_edit_flags.go
index 17b13e08..089485c5 100644
--- a/pkg/kn/commands/service/configuration_edit_flags.go
+++ b/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 {
diff --git a/pkg/serving/config_changes.go b/pkg/serving/config_changes.go
index 6c6b8803..56847797 100644
--- a/pkg/serving/config_changes.go
+++ b/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 {


Test patch:
diff --git a/pkg/kn/commands/service/configuration_edit_flags_test.go b/pkg/kn/commands/service/configuration_edit_flags_test.go
index b2cd8726..e5596656 100644
--- a/pkg/kn/commands/service/configuration_edit_flags_test.go
+++ b/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)
+}
diff --git a/pkg/serving/config_changes_test.go b/pkg/serving/config_changes_test.go
index e971d908..1aa21e83 100644
--- a/pkg/serving/config_changes_test.go
+++ b/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())