Move `--autoscale-window` to `--scale-window` (#1489)

* Move `--autoscale-window` to `--scale-window`

and deprecate `--autoscale-window`

* regenerate docs

* fix issue that --scale-window was not picked up

* add some test

* add some additional test

CHANGELOG.adoc

@@ -11,6 +11,18 @@
 |
 | https://github.com/knative/client/pull/[#]
 ////
+
+## v1.0.0
+[cols="1,10,3", options="header", width="100%"]
+|===
+| | Description | PR
+
+| ✨
+| Deprecate `--autoscale-window` in favor of `--scale-window`
+| https://github.com/knative/client/pull/1454[#1454]
+
+|===
+
 ## v0.26.0 (2021-09-22)
 [cols="1,10,3", options="header", width="100%"]
 |===

docs/cmd/kn_service_apply.md

@@ -33,7 +33,7 @@ kn service apply s0 --filename my-svc.yml
       --annotation-revision stringArray   Revision annotation to set. name=value; you may provide this flag any number of times to set multiple annotations. To unset, specify the annotation name followed by a "-" (e.g., name-). This flag takes precedence over the "annotation" flag.
       --annotation-service stringArray    Service annotation to set. name=value; you may provide this flag any number of times to set multiple annotations. To unset, specify the annotation name followed by a "-" (e.g., name-). This flag takes precedence over the "annotation" flag.
       --arg stringArray                   Add argument to the container command. Example: --arg myArg1 --arg --myArg2 --arg myArg3=3. You can use this flag multiple times.
-      --autoscale-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)
+      --autoscale-window string           Deprecated option, please use --scale-window
       --cluster-local                     Specify that the service be private. (--no-cluster-local will make the service publicly available)
       --cmd stringArray                   Specify command to be used as entrypoint instead of default one. Example: --cmd /app/start or --cmd sh --cmd /app/start.sh or --cmd /app/start --arg myArg to pass additional arguments.
       --concurrency-limit int             Hard Limit of concurrent requests to be processed by a single replica.
@@ -66,6 +66,7 @@ kn service apply s0 --filename my-svc.yml
       --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-min int                     Minimum number of replicas.
+      --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.
       --user int                          The user ID to run the container (e.g., 1001).
       --volume stringArray                Add a volume from a ConfigMap (prefix cm: or config-map:) or a Secret (prefix secret: or sc:). Example: --volume myvolume=cm:myconfigmap or --volume myvolume=secret:mysecret. You can use this flag multiple times. To unset a ConfigMap/Secret reference, append "-" to the name, e.g. --volume myvolume-.

docs/cmd/kn_service_create.md

@@ -58,7 +58,7 @@ kn service create NAME --image IMAGE
       --annotation-revision stringArray   Revision annotation to set. name=value; you may provide this flag any number of times to set multiple annotations. To unset, specify the annotation name followed by a "-" (e.g., name-). This flag takes precedence over the "annotation" flag.
       --annotation-service stringArray    Service annotation to set. name=value; you may provide this flag any number of times to set multiple annotations. To unset, specify the annotation name followed by a "-" (e.g., name-). This flag takes precedence over the "annotation" flag.
       --arg stringArray                   Add argument to the container command. Example: --arg myArg1 --arg --myArg2 --arg myArg3=3. You can use this flag multiple times.
-      --autoscale-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)
+      --autoscale-window string           Deprecated option, please use --scale-window
       --cluster-local                     Specify that the service be private. (--no-cluster-local will make the service publicly available)
       --cmd stringArray                   Specify command to be used as entrypoint instead of default one. Example: --cmd /app/start or --cmd sh --cmd /app/start.sh or --cmd /app/start --arg myArg to pass additional arguments.
       --concurrency-limit int             Hard Limit of concurrent requests to be processed by a single replica.
@@ -91,6 +91,7 @@ kn service create NAME --image IMAGE
       --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-min int                     Minimum number of replicas.
+      --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.
       --target string                     Work on local directory instead of a remote cluster (experimental)
       --user int                          The user ID to run the container (e.g., 1001).

docs/cmd/kn_service_update.md

@@ -43,7 +43,7 @@ kn service update NAME
       --annotation-revision stringArray   Revision annotation to set. name=value; you may provide this flag any number of times to set multiple annotations. To unset, specify the annotation name followed by a "-" (e.g., name-). This flag takes precedence over the "annotation" flag.
       --annotation-service stringArray    Service annotation to set. name=value; you may provide this flag any number of times to set multiple annotations. To unset, specify the annotation name followed by a "-" (e.g., name-). This flag takes precedence over the "annotation" flag.
       --arg stringArray                   Add argument to the container command. Example: --arg myArg1 --arg --myArg2 --arg myArg3=3. You can use this flag multiple times.
-      --autoscale-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)
+      --autoscale-window string           Deprecated option, please use --scale-window
       --cluster-local                     Specify that the service be private. (--no-cluster-local will make the service publicly available)
       --cmd stringArray                   Specify command to be used as entrypoint instead of default one. Example: --cmd /app/start or --cmd sh --cmd /app/start.sh or --cmd /app/start --arg myArg to pass additional arguments.
       --concurrency-limit int             Hard Limit of concurrent requests to be processed by a single replica.
@@ -74,6 +74,7 @@ kn service update NAME
       --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-min int                     Minimum number of replicas.
+      --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.
       --tag strings                       Set tag (format: --tag revisionRef=tagName) where revisionRef can be a revision or '@latest' string representing latest ready revision. This flag can be specified multiple times.
       --target string                     Work on local directory instead of a remote cluster (experimental)

pkg/kn/commands/service/configuration_edit_flags.go

@@ -44,7 +44,7 @@ type ConfigurationEditFlags struct {
 	ConcurrencyTarget      int
 	ConcurrencyLimit       int
 	ConcurrencyUtilization int
-	AutoscaleWindow        string
+	ScaleWindow            string
 	Labels                 []string
 	LabelsService          []string
 	LabelsRevision         []string
@@ -100,9 +100,13 @@ 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.AutoscaleWindow, "autoscale-window", "", "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)")
+	// DEPRECATED since 1.0
+	command.Flags().StringVar(&p.ScaleWindow, "autoscale-window", "", "Deprecated option, please use --scale-window")
 	p.markFlagMakesRevision("autoscale-window")
 
+	command.Flags().StringVar(&p.ScaleWindow, "scale-window", "", "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)")
+	p.markFlagMakesRevision("scale-window")
+
 	knflags.AddBothBoolFlagsUnhidden(command.Flags(), &p.ClusterLocal, "cluster-local", "", false,
 		"Specify that the service be private. (--no-cluster-local will make the service publicly available)")
 
@@ -314,8 +318,8 @@ func (p *ConfigurationEditFlags) Apply(
 		}
 	}
 
-	if cmd.Flags().Changed("autoscale-window") {
+	if cmd.Flags().Changed("scale-window") || cmd.Flags().Changed("autoscale-window") {
-		err = servinglib.UpdateAutoscaleWindow(template, p.AutoscaleWindow)
+		err = servinglib.UpdateScaleWindow(template, p.ScaleWindow)
 		if err != nil {
 			return err
 		}

pkg/kn/commands/service/create_mock_test.go

@@ -539,7 +539,7 @@ func TestServiceCreateWithAnnotations(t *testing.T) {
 	output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz",
 		"--annotation", "foo=bar",
 		"--annotation", autoscaling.InitialScaleAnnotationKey+"=1",
-		"--no-wait", "--revision-name=")
+		"--no-wait")
 	assert.NilError(t, err)
 	assert.Assert(t, util.ContainsAll(output, "created", "foo", "default"))
 
@@ -570,7 +570,7 @@ func TestServiceCreateWithRevisionAndServiceAnnotations(t *testing.T) {
 	output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz",
 		"--annotation-service", "foo=bar",
 		"--annotation-revision", autoscaling.InitialScaleAnnotationKey+"=1",
-		"--no-wait", "--revision-name=")
+		"--no-wait")
 	assert.NilError(t, err)
 	assert.Assert(t, util.ContainsAll(output, "created", "foo", "default"))
 
@@ -596,7 +596,7 @@ func TestServiceCreateWithRevisionAnnotations(t *testing.T) {
 
 	output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz",
 		"--annotation-revision", autoscaling.InitialScaleAnnotationKey+"=1",
-		"--no-wait", "--revision-name=")
+		"--no-wait")
 	assert.NilError(t, err)
 	assert.Assert(t, util.ContainsAll(output, "created", "foo", "default"))
 
@@ -625,21 +625,21 @@ func TestServiceCreateWithServiceAnnotations(t *testing.T) {
 
 	output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz",
 		"--annotation-service", "foo=bar",
-		"--no-wait", "--revision-name=")
+		"--no-wait")
 	assert.NilError(t, err)
 	assert.Assert(t, util.ContainsAll(output, "created", "foo", "default"))
 
 	r.Validate()
 }
 
-func TestServiceCreateWithAutoScaleServiceAnnotationsError(t *testing.T) {
+func TestServiceCreateWithScaleServiceAnnotationsError(t *testing.T) {
 	client := knclient.NewMockKnServiceClient(t)
 
 	r := client.Recorder()
 
 	output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz",
 		"--annotation-service", autoscaling.InitialScaleAnnotationKey+"=1",
-		"--no-wait", "--revision-name=")
+		"--no-wait")
 	assert.Assert(t, err != nil)
 	assert.Assert(t, util.ContainsAll(output, "service can not have auto-scaling related annotation", "autoscaling.knative.dev/initialScale"))
 

pkg/kn/commands/service/create_test.go

@@ -393,6 +393,7 @@ func TestServiceCreateMaxMinScale(t *testing.T) {
 		"--scale-min", "1", "--scale-max", "5",
 		"--concurrency-target", "10", "--concurrency-limit", "100",
 		"--concurrency-utilization", "50",
+		"--scale-window", "10s",
 		"--no-wait"}, false)
 
 	if err != nil {
@@ -409,6 +410,7 @@ func TestServiceCreateMaxMinScale(t *testing.T) {
 		"autoscaling.knative.dev/maxScale", "5",
 		"autoscaling.knative.dev/target", "10",
 		"autoscaling.knative.dev/targetUtilizationPercentage", "50",
+		"autoscaling.knative.dev/window", "10s",
 	}
 
 	for i := 0; i < len(expectedAnnos); i += 2 {
@@ -466,6 +468,16 @@ func TestServiceCreateScaleWithNegativeValue(t *testing.T) {
 
 }
 
+func TestServiceCreateInvalidScaleWindow(t *testing.T) {
+	_, _, _, err := fakeServiceCreate([]string{
+		"service", "create", "foo", "--image", "gcr.io/foo/bar:baz",
+		"--scale-window", "bla", "--no-wait"}, true)
+	if err == nil {
+		t.Fatal(err)
+	}
+	assert.ErrorContains(t, err, "invalid duration")
+}
+
 func TestServiceCreateScaleWithMaxScaleSet(t *testing.T) {
 	_, _, _, err := fakeServiceCreate([]string{
 		"service", "create", "foo", "--image", "gcr.io/foo/bar:baz",

pkg/serving/config_changes.go

@@ -64,11 +64,11 @@ func UpdateMaxScale(template *servingv1.RevisionTemplateSpec, max int) error {
 	return UpdateRevisionTemplateAnnotation(template, autoscaling.MaxScaleAnnotationKey, strconv.Itoa(max))
 }
 
-// UpdateAutoscaleWindow updates the autoscale window annotation
+// UpdateScaleWindow updates the autoscale window annotation
-func UpdateAutoscaleWindow(template *servingv1.RevisionTemplateSpec, window string) error {
+func UpdateScaleWindow(template *servingv1.RevisionTemplateSpec, window string) error {
 	_, err := time.ParseDuration(window)
 	if err != nil {
-		return fmt.Errorf("invalid duration for 'autoscale-window': %w", err)
+		return fmt.Errorf("invalid duration for 'scale-window': %w", err)
 	}
 	return UpdateRevisionTemplateAnnotation(template, autoscaling.WindowAnnotationKey, window)
 }

pkg/serving/config_changes_test.go

@@ -31,7 +31,7 @@ import (
 	servingv1 "knative.dev/serving/pkg/apis/serving/v1"
 )
 
-func TestUpdateAutoscalingAnnotations(t *testing.T) {
+func TestUpdateScalingAnnotations(t *testing.T) {
 	template := &servingv1.RevisionTemplateSpec{}
 	updateConcurrencyConfiguration(template, 10, 100, 1000, 1000, 50)
 	annos := template.Annotations
@@ -49,7 +49,7 @@ func TestUpdateAutoscalingAnnotations(t *testing.T) {
 	}
 }
 
-func TestUpdateInvalidAutoscalingAnnotations(t *testing.T) {
+func TestUpdateInvalidScalingAnnotations(t *testing.T) {
 	template := &servingv1.RevisionTemplateSpec{}
 	updateConcurrencyConfiguration(template, 10, 100, 1000, 1000, 50)
 	// Update with invalid concurrency options
@@ -169,15 +169,15 @@ func TestUpdateMaxScale(t *testing.T) {
 	assert.ErrorContains(t, err, "maxScale")
 }
 
-func TestAutoscaleWindow(t *testing.T) {
+func TestScaleWindow(t *testing.T) {
 	template, _ := getRevisionTemplate()
-	err := UpdateAutoscaleWindow(template, "10s")
+	err := UpdateScaleWindow(template, "10s")
 	assert.NilError(t, err)
 	// Verify update is successful or not
 	checkAnnotationValue(t, template, autoscaling.WindowAnnotationKey, "10s")
 	// Update with invalid value
-	err = UpdateAutoscaleWindow(template, "blub")
+	err = UpdateScaleWindow(template, "blub")
-	assert.Check(t, util.ContainsAll(err.Error(), "invalid duration", "autoscale-window"))
+	assert.Check(t, util.ContainsAll(err.Error(), "invalid duration", "scale-window"))
 }
 
 func TestUpdateConcurrencyTarget(t *testing.T) {

test/e2e/service_options_test.go

@@ -108,12 +108,14 @@ func TestServiceOptions(t *testing.T) {
 	validateServiceAnnotations(r, "svc3a", map[string]string{"alpha": "direwolf", "brave": ""})
 	test.ServiceDelete(r, "svc3a")
 
-	t.Log("create, update and validate service with autoscale window option")
+	t.Log("create, update and validate service with scale window option")
-	serviceCreateWithOptions(r, "svc4", "--autoscale-window", "1m")
+	for _, option := range []string{"--scale-window", "--autoscale-window"} {
-	validateAutoscaleWindow(r, "svc4", "1m")
+		serviceCreateWithOptions(r, "svc4", option, "1m")
-	test.ServiceUpdate(r, "svc4", "--autoscale-window", "15s")
+		validateScaleWindow(r, "svc4", "1m")
-	validateAutoscaleWindow(r, "svc4", "15s")
+		test.ServiceUpdate(r, "svc4", option, "15s")
+		validateScaleWindow(r, "svc4", "15s")
 		test.ServiceDelete(r, "svc4")
+	}
 
 	t.Log("create, update and validate service with cmd and arg options")
 	serviceCreateWithOptions(r, "svc5", "--cmd", "/ko-app/helloworld")
@@ -272,7 +274,7 @@ func validateServiceConcurrencyUtilization(r *test.KnRunResultCollector, service
 	r.AssertNoError(out)
 }
 
-func validateAutoscaleWindow(r *test.KnRunResultCollector, serviceName, window string) {
+func validateScaleWindow(r *test.KnRunResultCollector, serviceName, window string) {
 	jsonpath := "jsonpath={.items[0].spec.template.metadata.annotations.autoscaling\\.knative\\.dev/window}"
 	out := r.KnTest().Kn().Run("service", "list", serviceName, "-o", jsonpath)
 	assert.Equal(r.T(), out.Stdout, window)