Skip to content

Commit

Permalink
Deprecates --max-scale/--min-scale for more consistent naming (knativ…
Browse files Browse the repository at this point in the history
…e#958)

* Depreciates --max-scale/--min-scale for more consistent naming

* Added back old flags and marked hidden
  • Loading branch information
Mike Petersen authored and rhuss committed Sep 9, 2020
1 parent d78f53e commit 99e738d
Show file tree
Hide file tree
Showing 8 changed files with 33 additions and 25 deletions.
2 changes: 1 addition & 1 deletion cmd/kn/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func TestUnknownCommands(t *testing.T) {
expectedError []string
}{
{
[]string{"service", "udpate", "test", "--min-scale=0"},
[]string{"service", "udpate", "test", "--scale-min=0"},
[]string{"service"},
[]string{"unknown sub-command", "udpate"},
},
Expand Down
4 changes: 2 additions & 2 deletions docs/cmd/kn_service_create.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,6 @@ kn service create NAME --image IMAGE
--limits-cpu string DEPRECATED: please use --limit instead. The limits on the requested CPU (e.g., 1000m).
--limits-memory string DEPRECATED: please use --limit instead. The limits on the requested memory (e.g., 1024Mi).
--lock-to-digest Keep the running image for the service constant when not explicitly specifying the image. (--no-lock-to-digest pulls the image tag afresh with each new revision) (default true)
--max-scale int Maximal number of replicas.
--min-scale int Minimal number of replicas.
--mount stringArray Mount a ConfigMap (prefix cm: or config-map:), a Secret (prefix secret: or sc:), or an existing Volume (without any prefix) on the specified directory. Example: --mount /mydir=cm:myconfigmap, --mount /mydir=secret:mysecret, or --mount /mydir=myvolume. When a configmap or a secret is specified, a corresponding volume is automatically generated. You can use this flag multiple times. For unmounting a directory, append "-", e.g. --mount /mydir-, which also removes any auto-generated volume.
-n, --namespace string Specify the namespace to operate in.
--no-cluster-local Do not specify that the service be private. (--no-cluster-local will make the service publicly available) (default true)
Expand All @@ -89,6 +87,8 @@ kn service create NAME --image IMAGE
--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-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.
--user int The user ID to run the container (e.g., 1001).
--volume stringArray Add a volume from a ConfigMap (prefix cm: or config-map:) or a Secret (prefix secret: or sc:). Example: --volume myvolume=cm:myconfigmap or --volume myvolume=secret:mysecret. You can use this flag multiple times. To unset a ConfigMap/Secret reference, append "-" to the name, e.g. --volume myvolume-.
Expand Down
4 changes: 2 additions & 2 deletions docs/cmd/kn_service_update.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,6 @@ kn service update NAME
--limits-cpu string DEPRECATED: please use --limit instead. The limits on the requested CPU (e.g., 1000m).
--limits-memory string DEPRECATED: please use --limit instead. The limits on the requested memory (e.g., 1024Mi).
--lock-to-digest Keep the running image for the service constant when not explicitly specifying the image. (--no-lock-to-digest pulls the image tag afresh with each new revision) (default true)
--max-scale int Maximal number of replicas.
--min-scale int Minimal number of replicas.
--mount stringArray Mount a ConfigMap (prefix cm: or config-map:), a Secret (prefix secret: or sc:), or an existing Volume (without any prefix) on the specified directory. Example: --mount /mydir=cm:myconfigmap, --mount /mydir=secret:mysecret, or --mount /mydir=myvolume. When a configmap or a secret is specified, a corresponding volume is automatically generated. You can use this flag multiple times. For unmounting a directory, append "-", e.g. --mount /mydir-, which also removes any auto-generated volume.
-n, --namespace string Specify the namespace to operate in.
--no-cluster-local Do not specify that the service be private. (--no-cluster-local will make the service publicly available) (default true)
Expand All @@ -72,6 +70,8 @@ kn service update NAME
--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-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.
--tag strings Set tag (format: --tag revisionRef=tagName) where revisionRef can be a revision or '@latest' string representing latest ready revision. This flag can be specified multiple times.
--traffic strings Set traffic distribution (format: --traffic revisionRef=percent) where revisionRef can be a revision or a tag or '@latest' string representing latest ready revision. This flag can be given multiple times with percent summing up to 100%.
Expand Down
4 changes: 2 additions & 2 deletions docs/operations/autoscaling.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,5 @@ flags to configure the autoscaling behavior.
| :------------------------- | :-------------------------------------------------------------------------------------------------------------------------- |
| `--concurrency-limit int` | Hard limit of concurrent requests to be processed by a single replica. |
| `--concurrency-target int` | Recommendation for when to scale up based on the concurrent number of incoming requests. Defaults to `--concurrency-limit`. |
| `--max-scale int` | Maximum number of replicas. |
| `--min-scale int` | Minimum number of replicas. |
| `--scale-max int` | Maximum number of replicas. |
| `--scale-min int` | Minimum number of replicas. |
20 changes: 14 additions & 6 deletions pkg/kn/commands/service/configuration_edit_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,14 +183,22 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) {
p.markFlagMakesRevision("limits-memory")

command.Flags().IntVar(&p.MinScale, "min-scale", 0, "Minimal number of replicas.")
command.Flags().MarkHidden("min-scale")
p.markFlagMakesRevision("min-scale")

command.Flags().IntVar(&p.MaxScale, "max-scale", 0, "Maximal number of replicas.")
command.Flags().MarkHidden("max-scale")
p.markFlagMakesRevision("max-scale")

command.Flags().IntVar(&p.Scale, "scale", 0, "Minimum and maximum number of replicas.")
p.markFlagMakesRevision("scale")

command.Flags().IntVar(&p.MinScale, "scale-min", 0, "Minimum number of replicas.")
p.markFlagMakesRevision("scale-min")

command.Flags().IntVar(&p.MaxScale, "scale-max", 0, "Maximum number of replicas.")
p.markFlagMakesRevision("scale-max")

command.Flags().StringVar(&p.AutoscaleWindow, "autoscale-window", "", "Duration to look back for making auto-scaling decisions. The service is scaled to zero if no request was received in during that time. (eg: 10s)")
p.markFlagMakesRevision("autoscale-window")

Expand Down Expand Up @@ -430,25 +438,25 @@ func (p *ConfigurationEditFlags) Apply(
}
}

if cmd.Flags().Changed("min-scale") {
if cmd.Flags().Changed("scale-min") {
err = servinglib.UpdateMinScale(template, p.MinScale)
if err != nil {
return err
}
}

if cmd.Flags().Changed("max-scale") {
if cmd.Flags().Changed("scale-max") {
err = servinglib.UpdateMaxScale(template, p.MaxScale)
if err != nil {
return err
}
}

if cmd.Flags().Changed("scale") {
if cmd.Flags().Changed("max-scale") {
return fmt.Errorf("only --scale or --max-scale can be specified")
} else if cmd.Flags().Changed("min-scale") {
return fmt.Errorf("only --scale or --min-scale can be specified")
if cmd.Flags().Changed("scale-max") {
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 err != nil {
Expand Down
10 changes: 5 additions & 5 deletions pkg/kn/commands/service/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ func TestServiceCreateRequestsLimitsMemory(t *testing.T) {
func TestServiceCreateMaxMinScale(t *testing.T) {
action, created, _, err := fakeServiceCreate([]string{
"service", "create", "foo", "--image", "gcr.io/foo/bar:baz",
"--min-scale", "1", "--max-scale", "5",
"--scale-min", "1", "--scale-max", "5",
"--concurrency-target", "10", "--concurrency-limit", "100",
"--concurrency-utilization", "50",
"--no-wait"}, false)
Expand Down Expand Up @@ -592,11 +592,11 @@ func TestServiceCreateScaleWithNegativeValue(t *testing.T) {
func TestServiceCreateScaleWithMaxScaleSet(t *testing.T) {
_, _, _, err := fakeServiceCreate([]string{
"service", "create", "foo", "--image", "gcr.io/foo/bar:baz",
"--scale", "5", "--max-scale", "2", "--no-wait"}, true)
"--scale", "5", "--scale-max", "2", "--no-wait"}, true)
if err == nil {
t.Fatal(err)
}
expectedErrMsg := "only --scale or --max-scale can be specified"
expectedErrMsg := "only --scale or --scale-max can be specified"
if !strings.Contains(err.Error(), expectedErrMsg) {
t.Errorf("Invalid error output, expected: %s, got : '%s'", expectedErrMsg, err)
}
Expand All @@ -606,11 +606,11 @@ func TestServiceCreateScaleWithMaxScaleSet(t *testing.T) {
func TestServiceCreateScaleWithMinScaleSet(t *testing.T) {
_, _, _, err := fakeServiceCreate([]string{
"service", "create", "foo", "--image", "gcr.io/foo/bar:baz",
"--scale", "5", "--min-scale", "2", "--no-wait"}, true)
"--scale", "5", "--scale-min", "2", "--no-wait"}, true)
if err == nil {
t.Fatal(err)
}
expectedErrMsg := "only --scale or --min-scale can be specified"
expectedErrMsg := "only --scale or --scale-min can be specified"
if !strings.Contains(err.Error(), expectedErrMsg) {
t.Errorf("Invalid error output, expected: %s, got : '%s'", expectedErrMsg, err)
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/kn/commands/service/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ func TestServiceUpdateMaxMinScale(t *testing.T) {

action, updated, _, err := fakeServiceUpdate(original, []string{
"service", "update", "foo",
"--min-scale", "1", "--max-scale", "5", "--concurrency-target", "10", "--concurrency-limit", "100", "--concurrency-utilization", "50", "--no-wait"})
"--scale-min", "1", "--scale-max", "5", "--concurrency-target", "10", "--concurrency-limit", "100", "--concurrency-utilization", "50", "--no-wait"})

if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -413,13 +413,13 @@ func TestServiceUpdateScaleWithMaxScaleSet(t *testing.T) {

_, _, _, err := fakeServiceUpdate(original, []string{
"service", "update", "foo",
"--scale", "5", "--max-scale", "2", "--no-wait"})
"--scale", "5", "--scale-max", "2", "--no-wait"})

if err == nil {
t.Fatal("Expected error, got nil")
}

expectedErrMsg := "only --scale or --max-scale can be specified"
expectedErrMsg := "only --scale or --scale-max can be specified"

if !strings.Contains(err.Error(), expectedErrMsg) {
t.Errorf("Invalid error output, expected: %s, got : '%s'", expectedErrMsg, err)
Expand All @@ -432,13 +432,13 @@ func TestServiceUpdateScaleWithMinScaleSet(t *testing.T) {

_, _, _, err := fakeServiceUpdate(original, []string{
"service", "update", "foo",
"--scale", "5", "--min-scale", "2", "--no-wait"})
"--scale", "5", "--scale-min", "2", "--no-wait"})

if err == nil {
t.Fatal("Expected error, got nil")
}

expectedErrMsg := "only --scale or --min-scale can be specified"
expectedErrMsg := "only --scale or --scale-min can be specified"

if !strings.Contains(err.Error(), expectedErrMsg) {
t.Errorf("Invalid error output, expected: %s, got : '%s'", expectedErrMsg, err)
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/service_options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,12 @@ func TestServiceOptions(t *testing.T) {
test.ServiceDelete(r, "svc1")

t.Log("create and validate service with min/max scale options")
serviceCreateWithOptions(r, "svc2", "--min-scale", "1", "--max-scale", "3")
serviceCreateWithOptions(r, "svc2", "--scale-min", "1", "--scale-max", "3")
validateServiceMinScale(r, "svc2", "1")
validateServiceMaxScale(r, "svc2", "3")

t.Log("update and validate service with max scale option")
test.ServiceUpdate(r, "svc2", "--max-scale", "2")
test.ServiceUpdate(r, "svc2", "--scale-max", "2")
validateServiceMaxScale(r, "svc2", "2")

t.Log("create and validate service with scale options")
Expand Down

0 comments on commit 99e738d

Please sign in to comment.