From 49c7a26a251901ba32d5fe3670d507e02dec5369 Mon Sep 17 00:00:00 2001 From: Mike Petersen Date: Fri, 21 Aug 2020 11:10:28 -0700 Subject: [PATCH] Fixed issues related to reviews and cleaned up some of the logic --- docs/cmd/kn_service_create.md | 2 +- docs/cmd/kn_service_update.md | 2 +- .../service/configuration_edit_flags.go | 32 ++++++++-------- pkg/kn/commands/service/update_test.go | 37 ------------------- 4 files changed, 17 insertions(+), 56 deletions(-) diff --git a/docs/cmd/kn_service_create.md b/docs/cmd/kn_service_create.md index d42828e5b2..f9ebc02ee5 100644 --- a/docs/cmd/kn_service_create.md +++ b/docs/cmd/kn_service_create.md @@ -86,7 +86,7 @@ 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 string Minimum and maximum number of replicas. Example: --scale 5 or --scale 1..5 or --scale 1.. or --scale ..5. + --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 1..5 or --scale 1.. or --scale ..5. --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 fafbd6a645..8609e2c3db 100644 --- a/docs/cmd/kn_service_update.md +++ b/docs/cmd/kn_service_update.md @@ -69,7 +69,7 @@ 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 string Minimum and maximum number of replicas. Example: --scale 5 or --scale 1..5 or --scale 1.. or --scale ..5. + --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 1..5 or --scale 1.. or --scale ..5. --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 e9de07439e..e99e809118 100644 --- a/pkg/kn/commands/service/configuration_edit_flags.go +++ b/pkg/kn/commands/service/configuration_edit_flags.go @@ -83,7 +83,7 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) { command.Flags().MarkHidden("max-scale") p.markFlagMakesRevision("max-scale") - command.Flags().StringVar(&p.Scale, "scale", "", "Minimum and maximum number of replicas.") + command.Flags().StringVar(&p.Scale, "scale", "1", "Minimum and maximum number of replicas.") p.markFlagMakesRevision("scale") command.Flags().IntVar(&p.MinScale, "scale-min", 0, "Minimum number of replicas.") @@ -335,45 +335,43 @@ func (p *ConfigurationEditFlags) Apply( return fmt.Errorf("only --scale or --scale-min can be specified") } else { if !strings.Contains(p.Scale, "..") { - scaleInt, _ := strconv.Atoi(p.Scale) - err = servinglib.UpdateMaxScale(template, scaleInt) + scaleMin, err := strconv.Atoi(p.Scale) if err != nil { return err } - err = servinglib.UpdateMinScale(template, scaleInt) + scaleMax := scaleMin + err = servinglib.UpdateMaxScale(template, scaleMax) if err != nil { return err } - } else if len(p.Scale) == 4 { - scaleParts := strings.Split(p.Scale, "..") - scaleMin, _ := strconv.Atoi(scaleParts[0]) - scaleMax, _ := strconv.Atoi(scaleParts[1]) err = servinglib.UpdateMinScale(template, scaleMin) if err != nil { return err } - err = servinglib.UpdateMaxScale(template, scaleMax) + } else { + scaleParts := strings.Split(p.Scale, "..") + scaleMin, err := strconv.Atoi(scaleParts[0]) if err != nil { return err } - } else { - scaleParts := strings.Split(p.Scale, "") - if scaleParts[0] == "." { - scaleParts = strings.Split(p.Scale, "..") - scaleMax, _ := strconv.Atoi(scaleParts[1]) + scaleMax, err := strconv.Atoi(scaleParts[1]) + if err != nil { + return err + } + if scaleMax > 0 { err = servinglib.UpdateMaxScale(template, scaleMax) if err != nil { return err } - } else { - scaleParts = strings.Split(p.Scale, "..") - scaleMin, _ := strconv.Atoi(scaleParts[0]) + } + if scaleMin > 0 { err = servinglib.UpdateMinScale(template, scaleMin) if err != nil { return err } } } + } } diff --git a/pkg/kn/commands/service/update_test.go b/pkg/kn/commands/service/update_test.go index 64ea14e68f..e7718e1e8e 100644 --- a/pkg/kn/commands/service/update_test.go +++ b/pkg/kn/commands/service/update_test.go @@ -546,43 +546,6 @@ func TestServiceUpdateScaleMaxWithRange(t *testing.T) { } -func TestServiceUpdateScaleWithMinScaleNegativeSet(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 TestServiceUpdateScaleWithMaxScaleNegativeSet(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/maxScale" - - if !strings.Contains(err.Error(), expectedErrMsg) { - t.Errorf("Invalid error output, expected: %s, got : '%s'", expectedErrMsg, err) - } -} - func TestServiceUpdateEnv(t *testing.T) { orig := newEmptyService()