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.

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

[WIP] Added range options to --scale #986

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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_create.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion docs/cmd/kn_service_update.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
42 changes: 38 additions & 4 deletions pkg/kn/commands/service/configuration_edit_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package service

import (
"errors"
"fmt"
"strconv"
"strings"
Expand All @@ -38,7 +39,7 @@ type ConfigurationEditFlags struct {
PodSpecFlags knflags.PodSpecFlags

// Direct field manipulation
Scale int
Scale string
MinScale int
MaxScale int
ConcurrencyTarget int
Expand Down Expand Up @@ -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.")
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
110 changes: 110 additions & 0 deletions pkg/kn/commands/service/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
138 changes: 138 additions & 0 deletions pkg/kn/commands/service/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down