Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move --autoscale-window to --scale-window #1489

Merged
merged 5 commits into from
Oct 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions CHANGELOG.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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%"]
|===
Expand Down
3 changes: 2 additions & 1 deletion docs/cmd/kn_service_apply.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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-.
Expand Down
3 changes: 2 additions & 1 deletion docs/cmd/kn_service_create.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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).
Expand Down
3 changes: 2 additions & 1 deletion docs/cmd/kn_service_update.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down
12 changes: 8 additions & 4 deletions pkg/kn/commands/service/configuration_edit_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ type ConfigurationEditFlags struct {
ConcurrencyTarget int
ConcurrencyLimit int
ConcurrencyUtilization int
AutoscaleWindow string
ScaleWindow string
Labels []string
LabelsService []string
LabelsRevision []string
Expand Down Expand Up @@ -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)")

Expand Down Expand Up @@ -314,8 +318,8 @@ func (p *ConfigurationEditFlags) Apply(
}
}

if cmd.Flags().Changed("autoscale-window") {
err = servinglib.UpdateAutoscaleWindow(template, p.AutoscaleWindow)
if cmd.Flags().Changed("scale-window") || cmd.Flags().Changed("autoscale-window") {
err = servinglib.UpdateScaleWindow(template, p.ScaleWindow)
if err != nil {
return err
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/kn/commands/service/create_mock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))

Expand Down Expand Up @@ -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"))

Expand All @@ -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"))

Expand Down Expand Up @@ -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"))

Expand Down
12 changes: 12 additions & 0 deletions pkg/kn/commands/service/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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",
Expand Down
6 changes: 3 additions & 3 deletions pkg/serving/config_changes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
func UpdateAutoscaleWindow(template *servingv1.RevisionTemplateSpec, window string) error {
// UpdateScaleWindow updates the autoscale window annotation
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)
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/serving/config_changes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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")
assert.Check(t, util.ContainsAll(err.Error(), "invalid duration", "autoscale-window"))
err = UpdateScaleWindow(template, "blub")
assert.Check(t, util.ContainsAll(err.Error(), "invalid duration", "scale-window"))
}

func TestUpdateConcurrencyTarget(t *testing.T) {
Expand Down
16 changes: 9 additions & 7 deletions test/e2e/service_options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
serviceCreateWithOptions(r, "svc4", "--autoscale-window", "1m")
validateAutoscaleWindow(r, "svc4", "1m")
test.ServiceUpdate(r, "svc4", "--autoscale-window", "15s")
validateAutoscaleWindow(r, "svc4", "15s")
test.ServiceDelete(r, "svc4")
t.Log("create, update and validate service with scale window option")
for _, option := range []string{"--scale-window", "--autoscale-window"} {
serviceCreateWithOptions(r, "svc4", option, "1m")
validateScaleWindow(r, "svc4", "1m")
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")
Expand Down Expand Up @@ -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)
Expand Down