From b78134ebe931179f7272ba81e7edd11e4ef07d9d Mon Sep 17 00:00:00 2001 From: Mike Petersen Date: Wed, 11 Nov 2020 09:33:26 -0800 Subject: [PATCH 1/2] adds --scale range option --- docs/cmd/kn_service_apply.md | 2 +- docs/cmd/kn_service_create.md | 2 +- docs/cmd/kn_service_update.md | 2 +- .../service/configuration_edit_flags.go | 40 ++++- pkg/kn/commands/service/create_test.go | 113 ++++++++++++++ pkg/kn/commands/service/update_test.go | 138 ++++++++++++++++++ 6 files changed, 289 insertions(+), 8 deletions(-) diff --git a/docs/cmd/kn_service_apply.md b/docs/cmd/kn_service_apply.md index 3fe951ccba..b98e0484b1 100644 --- a/docs/cmd/kn_service_apply.md +++ b/docs/cmd/kn_service_apply.md @@ -63,7 +63,7 @@ kn service apply s0 --filename my-svc.yml --pull-secret string Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace. --request strings The resource requirement requests for this Service. For example, 'cpu=100m,memory=256Mi'. You can use this flag multiple times. To unset a resource request, append "-" to the resource name, e.g. '--request cpu-'. --revision-name string The revision name to set. Must start with the service name and a dash as a prefix. Empty revision name will result in the server generating a name for the revision. Accepts golang templates, allowing {{.Service}} for the service name, {{.Generation}} for the generation, and {{.Random [n]}} for n random consonants. (default "{{.Service}}-{{.Random 5}}-{{.Generation}}") - --scale int Minimum and maximum number of replicas. + --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-min int Minimum number of replicas. diff --git a/docs/cmd/kn_service_create.md b/docs/cmd/kn_service_create.md index 682f5815d6..120e3dba53 100644 --- a/docs/cmd/kn_service_create.md +++ b/docs/cmd/kn_service_create.md @@ -83,7 +83,7 @@ kn service create NAME --image IMAGE --pull-secret string Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace. --request strings The resource requirement requests for this Service. For example, 'cpu=100m,memory=256Mi'. You can use this flag multiple times. To unset a resource request, append "-" to the resource name, e.g. '--request cpu-'. --revision-name string The revision name to set. Must start with the service name and a dash as a prefix. Empty revision name will result in the server generating a name for the revision. Accepts golang templates, allowing {{.Service}} for the service name, {{.Generation}} for the generation, and {{.Random [n]}} for n random consonants. (default "{{.Service}}-{{.Random 5}}-{{.Generation}}") - --scale int Minimum and maximum number of replicas. + --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-min int Minimum number of replicas. diff --git a/docs/cmd/kn_service_update.md b/docs/cmd/kn_service_update.md index 6ec2b7039c..2b68fead21 100644 --- a/docs/cmd/kn_service_update.md +++ b/docs/cmd/kn_service_update.md @@ -66,7 +66,7 @@ kn service update NAME --pull-secret string Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace. --request strings The resource requirement requests for this Service. For example, 'cpu=100m,memory=256Mi'. You can use this flag multiple times. To unset a resource request, append "-" to the resource name, e.g. '--request cpu-'. --revision-name string The revision name to set. Must start with the service name and a dash as a prefix. Empty revision name will result in the server generating a name for the revision. Accepts golang templates, allowing {{.Service}} for the service name, {{.Generation}} for the generation, and {{.Random [n]}} for n random consonants. (default "{{.Service}}-{{.Random 5}}-{{.Generation}}") - --scale int Minimum and maximum number of replicas. + --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-min int Minimum number of replicas. diff --git a/pkg/kn/commands/service/configuration_edit_flags.go b/pkg/kn/commands/service/configuration_edit_flags.go index d8cbcc0c67..19afcc78c4 100644 --- a/pkg/kn/commands/service/configuration_edit_flags.go +++ b/pkg/kn/commands/service/configuration_edit_flags.go @@ -37,7 +37,7 @@ type ConfigurationEditFlags struct { PodSpecFlags knflags.PodSpecFlags // Direct field manipulation - Scale int + Scale string MinScale int MaxScale int ConcurrencyTarget int @@ -86,7 +86,11 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) { command.Flags().MarkHidden("max-scale") p.markFlagMakesRevision("max-scale") - command.Flags().IntVar(&p.Scale, "scale", 0, "Minimum and maximum number of replicas.") + command.Flags().StringVar(&p.Scale, "scale", "", + "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)") p.markFlagMakesRevision("scale") command.Flags().IntVar(&p.MinScale, "scale-min", 0, "Minimum number of replicas.") @@ -264,15 +268,41 @@ func (p *ConfigurationEditFlags) Apply( return fmt.Errorf("only --scale or --scale-max can be specified") } else if cmd.Flags().Changed("scale-min") { return fmt.Errorf("only --scale or --scale-min can be specified") - } else { - err = servinglib.UpdateMaxScale(template, p.Scale) + } + if strings.Contains(p.Scale, "..") { + scaleParts := strings.Split(p.Scale, "..") + if scaleParts[0] != "" { + scaleMin, err := strconv.Atoi(scaleParts[0]) + if err != nil { + return err + } + err = servinglib.UpdateMinScale(template, scaleMin) + if err != nil { + return err + } + } + if scaleParts[1] != "" { + scaleMax, err := strconv.Atoi(scaleParts[1]) + if err != nil { + return err + } + err = servinglib.UpdateMaxScale(template, scaleMax) + if err != nil { + return err + } + } + } else if scaleMin, err := strconv.Atoi(p.Scale); err == nil { + scaleMax := scaleMin + err = servinglib.UpdateMaxScale(template, scaleMax) if err != nil { return err } - err = servinglib.UpdateMinScale(template, p.Scale) + err = servinglib.UpdateMinScale(template, scaleMin) if err != nil { return err } + } else { + return fmt.Errorf("Scale must be of the format x..y or x") } } diff --git a/pkg/kn/commands/service/create_test.go b/pkg/kn/commands/service/create_test.go index 79250fd79a..0654e5b50e 100644 --- a/pkg/kn/commands/service/create_test.go +++ b/pkg/kn/commands/service/create_test.go @@ -482,6 +482,119 @@ func TestServiceCreateScaleWithMinScaleSet(t *testing.T) { } +func TestServiceCreateScaleRange(t *testing.T) { + action, created, _, err := fakeServiceCreate([]string{ + "service", "create", "foo", "--image", "gcr.io/foo/bar:baz", + "--scale", "1..5", "--no-wait"}, false) + + if err != nil { + t.Fatal(err) + } + if !action.Matches("create", "services") { + t.Fatal("Bad action ", action) + } + + template := &created.Spec.Template + + actualAnnos := template.Annotations + expectedAnnos := []string{ + "autoscaling.knative.dev/minScale", "1", + "autoscaling.knative.dev/maxScale", "5", + } + + for i := 0; i < len(expectedAnnos); i += 2 { + anno := expectedAnnos[i] + if actualAnnos[anno] != expectedAnnos[i+1] { + t.Fatalf("Unexpected annotation value for %s : %s (actual) != %s (expected)", + anno, actualAnnos[anno], expectedAnnos[i+1]) + } + } +} + +func TestServiceCreateScaleRangeOnlyMin(t *testing.T) { + action, created, _, err := fakeServiceCreate([]string{ + "service", "create", "foo", "--image", "gcr.io/foo/bar:baz", + "--scale", "1..", "--no-wait"}, false) + + if err != nil { + t.Fatal(err) + } + if !action.Matches("create", "services") { + t.Fatal("Bad action ", action) + } + + template := &created.Spec.Template + + actualAnnos := template.Annotations + expectedAnnos := []string{ + "autoscaling.knative.dev/minScale", "1", + } + + for i := 0; i < len(expectedAnnos); i += 2 { + anno := expectedAnnos[i] + if actualAnnos[anno] != expectedAnnos[i+1] { + t.Fatalf("Unexpected annotation value for %s : %s (actual) != %s (expected)", + anno, actualAnnos[anno], expectedAnnos[i+1]) + } + } +} + +func TestServiceCreateScaleRangeOnlyMax(t *testing.T) { + action, created, _, err := fakeServiceCreate([]string{ + "service", "create", "foo", "--image", "gcr.io/foo/bar:baz", + "--scale", "..5", "--no-wait"}, false) + + if err != nil { + t.Fatal(err) + } + if !action.Matches("create", "services") { + t.Fatal("Bad action ", action) + } + + template := &created.Spec.Template + + actualAnnos := template.Annotations + expectedAnnos := []string{ + "autoscaling.knative.dev/maxScale", "5", + } + + for i := 0; i < len(expectedAnnos); i += 2 { + anno := expectedAnnos[i] + if actualAnnos[anno] != expectedAnnos[i+1] { + t.Fatalf("Unexpected annotation value for %s : %s (actual) != %s (expected)", + anno, actualAnnos[anno], expectedAnnos[i+1]) + } + } +} + +func TestServiceCreateScaleRangeOnlyMinWrongSeparator(t *testing.T) { + _, _, _, err := fakeServiceCreate([]string{ + "service", "create", "foo", "--image", "gcr.io/foo/bar:baz", + "--scale", "1--", "--no-wait"}, true) + if err == nil { + t.Fatal(err) + } + expectedErrMsg := "Scale must be of the format x..y or x" + if !strings.Contains(err.Error(), expectedErrMsg) { + t.Errorf("Invalid error output, expected: %s, got : '%s'", expectedErrMsg, err) + } + +} + +func TestServiceCreateScaleRangeOnlyMaxWrongSeparator(t *testing.T) { + _, _, _, err := fakeServiceCreate([]string{ + "service", "create", "foo", "--image", "gcr.io/foo/bar:baz", + "--scale", "--1", "--no-wait"}, true) + if err == nil { + t.Fatal(err) + } + expectedErrMsg := "Scale must be of the format x..y or x" + if !strings.Contains(err.Error(), expectedErrMsg) { + t.Errorf("Invalid error output, expected: %s, got : '%s'", expectedErrMsg, err) + } + +} + func TestServiceCreateRequestsLimitsCPUMemory(t *testing.T) { action, created, _, err := fakeServiceCreate([]string{ "service", "create", "foo", "--image", "gcr.io/foo/bar:baz", diff --git a/pkg/kn/commands/service/update_test.go b/pkg/kn/commands/service/update_test.go index ee5f184faf..023743fe02 100644 --- a/pkg/kn/commands/service/update_test.go +++ b/pkg/kn/commands/service/update_test.go @@ -448,6 +448,144 @@ func TestServiceUpdateScaleWithMinScaleSet(t *testing.T) { } } + +func TestServiceUpdateScaleWithRange(t *testing.T) { + original := newEmptyService() + + action, updated, _, err := fakeServiceUpdate(original, []string{ + "service", "update", "foo", + "--scale", "1..5", "--no-wait"}) + + if err != nil { + t.Fatal(err) + } else if !action.Matches("update", "services") { + t.Fatal("Bad action ", action) + } + + template := updated.Spec.Template + if err != nil { + t.Fatal(err) + } + + actualAnnos := template.Annotations + expectedAnnos := []string{ + "autoscaling.knative.dev/minScale", "1", + "autoscaling.knative.dev/maxScale", "5", + } + + for i := 0; i < len(expectedAnnos); i += 2 { + anno := expectedAnnos[i] + if actualAnnos[anno] != expectedAnnos[i+1] { + t.Fatalf("Unexpected annotation value for %s : %s (actual) != %s (expected)", + anno, actualAnnos[anno], expectedAnnos[i+1]) + } + } + +} + +func TestServiceUpdateScaleMinWithRange(t *testing.T) { + original := newEmptyService() + + action, updated, _, err := fakeServiceUpdate(original, []string{ + "service", "update", "foo", + "--scale", "1..", "--no-wait"}) + + if err != nil { + t.Fatal(err) + } else if !action.Matches("update", "services") { + t.Fatal("Bad action ", action) + } + + template := updated.Spec.Template + if err != nil { + t.Fatal(err) + } + + actualAnnos := template.Annotations + expectedAnnos := []string{ + "autoscaling.knative.dev/minScale", "1", + } + + for i := 0; i < len(expectedAnnos); i += 2 { + anno := expectedAnnos[i] + if actualAnnos[anno] != expectedAnnos[i+1] { + t.Fatalf("Unexpected annotation value for %s : %s (actual) != %s (expected)", + anno, actualAnnos[anno], expectedAnnos[i+1]) + } + } + +} + +func TestServiceUpdateScaleMaxWithRange(t *testing.T) { + original := newEmptyService() + + action, updated, _, err := fakeServiceUpdate(original, []string{ + "service", "update", "foo", + "--scale", "..5", "--no-wait"}) + + if err != nil { + t.Fatal(err) + } else if !action.Matches("update", "services") { + t.Fatal("Bad action ", action) + } + + template := updated.Spec.Template + if err != nil { + t.Fatal(err) + } + + actualAnnos := template.Annotations + expectedAnnos := []string{ + "autoscaling.knative.dev/maxScale", "5", + } + + for i := 0; i < len(expectedAnnos); i += 2 { + anno := expectedAnnos[i] + if actualAnnos[anno] != expectedAnnos[i+1] { + t.Fatalf("Unexpected annotation value for %s : %s (actual) != %s (expected)", + anno, actualAnnos[anno], expectedAnnos[i+1]) + } + } + +} + +func TestServiceUpdateScaleRangeOnlyMinWrongSeparator(t *testing.T) { + original := newEmptyService() + + _, _, _, err := fakeServiceUpdate(original, []string{ + "service", "update", "foo", + "--scale", "1--", "--no-wait"}) + + if err == nil { + t.Fatal("Expected error, got nil") + } + + expectedErrMsg := "Scale must be of the format x..y or x" + + if !strings.Contains(err.Error(), expectedErrMsg) { + t.Errorf("Invalid error output, expected: %s, got : '%s'", expectedErrMsg, err) + } + +} + +func TestServiceUpdateScaleRangeOnlyMaxWrongSeparator(t *testing.T) { + original := newEmptyService() + + _, _, _, err := fakeServiceUpdate(original, []string{ + "service", "update", "foo", + "--scale", "--1", "--no-wait"}) + + if err == nil { + t.Fatal("Expected error, got nil") + } + + expectedErrMsg := "Scale must be of the format x..y or x" + + if !strings.Contains(err.Error(), expectedErrMsg) { + t.Errorf("Invalid error output, expected: %s, got : '%s'", expectedErrMsg, err) + } +} + func TestServiceUpdateEnv(t *testing.T) { orig := newEmptyService() From 1f81d8d1f9bec7fa51e2afd2c13b6b5ede753b5a Mon Sep 17 00:00:00 2001 From: Mike Petersen Date: Wed, 11 Nov 2020 11:22:36 -0800 Subject: [PATCH 2/2] added scale range tests with negative values --- pkg/kn/commands/service/create_test.go | 26 ++++++++++++++++++ pkg/kn/commands/service/update_test.go | 38 ++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/pkg/kn/commands/service/create_test.go b/pkg/kn/commands/service/create_test.go index 0654e5b50e..a3885ee40e 100644 --- a/pkg/kn/commands/service/create_test.go +++ b/pkg/kn/commands/service/create_test.go @@ -539,6 +539,19 @@ func TestServiceCreateScaleRangeOnlyMin(t *testing.T) { } } +func TestServiceCreateScaleRangeOnlyMinNegative(t *testing.T) { + _, _, _, err := fakeServiceCreate([]string{ + "service", "create", "foo", "--image", "gcr.io/foo/bar:baz", + "--scale", "-1..", "--no-wait"}, true) + if err == nil { + t.Fatal(err) + } + expectedErrMsg := "expected 0 <= -1 <= 2147483647: autoscaling.knative.dev/minScale" + if !strings.Contains(err.Error(), expectedErrMsg) { + t.Errorf("Invalid error output, expected: %s, got : '%s'", expectedErrMsg, err) + } +} + func TestServiceCreateScaleRangeOnlyMax(t *testing.T) { action, created, _, err := fakeServiceCreate([]string{ "service", "create", "foo", "--image", "gcr.io/foo/bar:baz", @@ -567,6 +580,19 @@ func TestServiceCreateScaleRangeOnlyMax(t *testing.T) { } } +func TestServiceCreateScaleRangeOnlyMaxNegative(t *testing.T) { + _, _, _, err := fakeServiceCreate([]string{ + "service", "create", "foo", "--image", "gcr.io/foo/bar:baz", + "--scale", "..-5", "--no-wait"}, true) + if err == nil { + t.Fatal(err) + } + expectedErrMsg := "expected 0 <= -5 <= 2147483647: autoscaling.knative.dev/maxScale" + if !strings.Contains(err.Error(), expectedErrMsg) { + t.Errorf("Invalid error output, expected: %s, got : '%s'", expectedErrMsg, err) + } +} + func TestServiceCreateScaleRangeOnlyMinWrongSeparator(t *testing.T) { _, _, _, err := fakeServiceCreate([]string{ "service", "create", "foo", "--image", "gcr.io/foo/bar:baz", diff --git a/pkg/kn/commands/service/update_test.go b/pkg/kn/commands/service/update_test.go index 023743fe02..cf2df87a25 100644 --- a/pkg/kn/commands/service/update_test.go +++ b/pkg/kn/commands/service/update_test.go @@ -516,6 +516,25 @@ func TestServiceUpdateScaleMinWithRange(t *testing.T) { } +func TestServiceUpdateScaleMinWithRangeNegative(t *testing.T) { + original := newEmptyService() + + _, _, _, err := fakeServiceUpdate(original, []string{ + "service", "update", "foo", + "--scale", "-1..", "--no-wait"}) + + if err == nil { + t.Fatal("Expected error, got nil") + } + + expectedErrMsg := "expected 0 <= -1 <= 2147483647: autoscaling.knative.dev/minScale" + + if !strings.Contains(err.Error(), expectedErrMsg) { + t.Errorf("Invalid error output, expected: %s, got : '%s'", expectedErrMsg, err) + } + +} + func TestServiceUpdateScaleMaxWithRange(t *testing.T) { original := newEmptyService() @@ -549,6 +568,25 @@ func TestServiceUpdateScaleMaxWithRange(t *testing.T) { } +func TestServiceUpdateScaleMaxWithRangeNegative(t *testing.T) { + original := newEmptyService() + + _, _, _, err := fakeServiceUpdate(original, []string{ + "service", "update", "foo", + "--scale", "..-5", "--no-wait"}) + + if err == nil { + t.Fatal("Expected error, got nil") + } + + expectedErrMsg := "expected 0 <= -5 <= 2147483647: autoscaling.knative.dev/maxScale" + + if !strings.Contains(err.Error(), expectedErrMsg) { + t.Errorf("Invalid error output, expected: %s, got : '%s'", expectedErrMsg, err) + } + +} + func TestServiceUpdateScaleRangeOnlyMinWrongSeparator(t *testing.T) { original := newEmptyService()