From 4b84de3594d3f124ff48788f0897c75aa1fe8ec3 Mon Sep 17 00:00:00 2001 From: Mike Petersen Date: Fri, 6 Nov 2020 09:02:23 -0800 Subject: [PATCH] got rid of helper functions, cleaned up code so it is easier to read, resolved a couple bugs --- .../service/configuration_edit_flags.go | 76 +++++++------------ pkg/kn/commands/service/create_test.go | 4 +- pkg/kn/commands/service/update_test.go | 2 +- 3 files changed, 31 insertions(+), 51 deletions(-) diff --git a/pkg/kn/commands/service/configuration_edit_flags.go b/pkg/kn/commands/service/configuration_edit_flags.go index f129a35662..439f3db28a 100644 --- a/pkg/kn/commands/service/configuration_edit_flags.go +++ b/pkg/kn/commands/service/configuration_edit_flags.go @@ -15,7 +15,6 @@ package service import ( - "errors" "fmt" "strconv" "strings" @@ -362,20 +361,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 { - scaleMin, scaleMax, err := p.scaleConversion(p.Scale) - if err != nil { - return err + } + 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 + } } - err = servinglib.UpdateMinScale(template, scaleMin) + 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.UpdateMaxScale(template, scaleMax) + err = servinglib.UpdateMinScale(template, scaleMin) if err != nil { return err } - + } else { + return fmt.Errorf("Scale must be of the format x..y or x") } } @@ -570,43 +590,3 @@ func (p *ConfigurationEditFlags) AnyMutation(cmd *cobra.Command) bool { } return false } - -// Helper function for --scale -func (p *ConfigurationEditFlags) scaleConversion(scale string) (scaleMin int, scaleMax int, err error) { - if len(scale) <= 2 { - scaleMin, scaleMax, err = p.scaleSingleValue(scale) - } else if strings.Contains(scale, "..") { - scaleMin, scaleMax, err = p.scaleRange(scale) - } else { - return 0, 0, errors.New("Scale must be of the format x..y or x") - } - return scaleMin, scaleMax, err -} - -func (p *ConfigurationEditFlags) scaleSingleValue(scale string) (scaleMin int, scaleMax int, err error) { - if !strings.Contains(scale, "..") { - scaleMin, err = strconv.Atoi(scale) - if err != nil { - return 0, 0, err - } - scaleMax = scaleMin - } - return scaleMin, scaleMax, err -} - -func (p *ConfigurationEditFlags) scaleRange(scale string) (scaleMin int, scaleMax int, err error) { - 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 - } - } - return scaleMin, scaleMax, err -} diff --git a/pkg/kn/commands/service/create_test.go b/pkg/kn/commands/service/create_test.go index bf78cdf3fc..cc416b5530 100644 --- a/pkg/kn/commands/service/create_test.go +++ b/pkg/kn/commands/service/create_test.go @@ -585,7 +585,7 @@ func TestServiceCreateScaleWithNegativeValue(t *testing.T) { if err == nil { t.Fatal(err) } - expectedErrMsg := "expected 0 <= -1 <= 2147483647: autoscaling.knative.dev/minScale" + 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) } @@ -1169,5 +1169,5 @@ func TestServiceCreateFromYAMLWithOverrideError(t *testing.T) { _, _, _, err = fakeServiceCreate([]string{ "service", "create", "foo", "--filename", tempFile, "--scale", "-1"}, false) assert.Assert(t, err != nil) - assert.Assert(t, util.ContainsAll(err.Error(), "expected 0 <= -1 <= 2147483647: autoscaling.knative.dev/minScale")) + assert.Assert(t, util.ContainsAll(err.Error(), "expected 0 <= -1 <= 2147483647: autoscaling.knative.dev/maxScale")) } diff --git a/pkg/kn/commands/service/update_test.go b/pkg/kn/commands/service/update_test.go index 54323d70c4..5cd16b0642 100644 --- a/pkg/kn/commands/service/update_test.go +++ b/pkg/kn/commands/service/update_test.go @@ -402,7 +402,7 @@ func TestServiceUpdateScaleWithNegativeValue(t *testing.T) { t.Fatal("Expected error, got nil") } - expectedErrMsg := "expected 0 <= -1 <= 2147483647: autoscaling.knative.dev/minScale" + 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)