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.

Sign up for GitHub

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

Jump to bottom

adds --scale range option #1114

Merged
merged 2 commits into from
Nov 24, 2020
Merged

adds --scale range option #1114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b78134e
adds --scale range option
Nov 11, 2020
1f81d8d
added scale range tests with negative values
Nov 11, 2020
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
2 changes: 1 addition & 1 deletion docs/cmd/kn_service_apply.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion docs/cmd/kn_service_create.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion docs/cmd/kn_service_update.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
40 changes: 35 additions & 5 deletions pkg/kn/commands/service/configuration_edit_flags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ type ConfigurationEditFlags struct {
PodSpecFlags knflags.PodSpecFlags

// Direct field manipulation
Scale int
Scale string
MinScale int
MaxScale int
ConcurrencyTarget int
Expand Down Expand Up @@ -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.")
Expand Down Expand Up @@ -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")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return fmt.Errorf("Scale must be of the format x..y or x")
return fmt.Errorf("scale must be of the format x..y or x")

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

error messages should begin with lower case

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I make this edit and create a new PR, or commit suggestion? Not sure how it works after a merge.

}
}

Expand Down
139 changes: 139 additions & 0 deletions pkg/kn/commands/service/create_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,145 @@ 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 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",
"--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 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",
"--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",
Expand Down
Loading