diff --git a/docs/cmd/kn_service_create.md b/docs/cmd/kn_service_create.md index 0ffb240b2e..580517bdf1 100644 --- a/docs/cmd/kn_service_create.md +++ b/docs/cmd/kn_service_create.md @@ -86,8 +86,8 @@ kn service create NAME --image IMAGE --requests-cpu string DEPRECATED: please use --request instead. The requested CPU (e.g., 250m). --requests-memory string DEPRECATED: please use --request instead. The requested memory (e.g., 64Mi). --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-init int Initial number of replicas with which a service starts. Can be 0 or a positive integer. + --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 or --scale-max int Maximum number of replicas. --scale-min int Minimum number of replicas. --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 007a029233..f047a5315c 100644 --- a/docs/cmd/kn_service_update.md +++ b/docs/cmd/kn_service_update.md @@ -69,8 +69,8 @@ kn service update NAME --requests-cpu string DEPRECATED: please use --request instead. The requested CPU (e.g., 250m). --requests-memory string DEPRECATED: please use --request instead. The requested memory (e.g., 64Mi). --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-init int Initial number of replicas with which a service starts. Can be 0 or a positive integer. + --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 or --scale-max int Maximum number of replicas. --scale-min int Minimum number of replicas. --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 d76d2381f4..7ddb3f2257 100644 --- a/pkg/kn/commands/service/configuration_edit_flags.go +++ b/pkg/kn/commands/service/configuration_edit_flags.go @@ -15,6 +15,7 @@ package service import ( + "errors" "fmt" "strconv" "strings" @@ -38,7 +39,7 @@ type ConfigurationEditFlags struct { PodSpecFlags knflags.PodSpecFlags // Direct field manipulation - Scale int + Scale string MinScale int MaxScale int ConcurrencyTarget int @@ -85,7 +86,7 @@ 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", "1", "Minimum and maximum number of replicas. (e.g., 1..5)") p.markFlagMakesRevision("scale") command.Flags().IntVar(&p.MinScale, "scale-min", 0, "Minimum number of replicas.") @@ -339,11 +340,15 @@ func (p *ConfigurationEditFlags) Apply( } 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) + scaleMin, scaleMax, err := p.scaleConversion(p.Scale) if err != nil { return err } - err = servinglib.UpdateMinScale(template, p.Scale) + err = servinglib.UpdateMaxScale(template, scaleMax) + if err != nil { + return err + } + err = servinglib.UpdateMinScale(template, scaleMin) if err != nil { return err } @@ -509,3 +514,32 @@ func (p *ConfigurationEditFlags) AnyMutation(cmd *cobra.Command) bool { } return false } + +func (p *ConfigurationEditFlags) scaleConversion(scale string) (scaleMin int, scaleMax int, err error) { + if len(scale) <= 2 { + if !strings.Contains(scale, "..") { + scaleMin, err = strconv.Atoi(scale) + if err != nil { + return 0, 0, err + } + scaleMax = scaleMin + } + } else if strings.Contains(scale, "..") { + scaleParts := strings.Split(scale, "..") + if scaleParts[0] != "" { + scaleMin, err = strconv.Atoi(scaleParts[0]) + if err != nil { + return 0, 0, err + } + } + if scaleParts[1] != "" { + scaleMax, err = strconv.Atoi(scaleParts[1]) + if err != nil { + return 0, 0, err + } + } + } else { + return 0, 0, errors.New("Scale must be of the format x..y or x") + } + return scaleMin, scaleMax, err +} diff --git a/pkg/kn/commands/service/create_test.go b/pkg/kn/commands/service/create_test.go index aa03b2b5ab..94a92867a6 100644 --- a/pkg/kn/commands/service/create_test.go +++ b/pkg/kn/commands/service/create_test.go @@ -617,6 +617,116 @@ 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) + } else if !action.Matches("create", "services") { + t.Fatalf("Bad action %v", 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) + } else if !action.Matches("create", "services") { + t.Fatalf("Bad action %v", 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) + } else if !action.Matches("create", "services") { + t.Fatalf("Bad action %v", 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 a696824153..a1a114bfe0 100644 --- a/pkg/kn/commands/service/update_test.go +++ b/pkg/kn/commands/service/update_test.go @@ -446,6 +446,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.Fatalf("Bad action %v", 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.Fatalf("Bad action %v", 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.Fatalf("Bad action %v", 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()