Update flag names to --request and --limit (knative#872) (knative#873)

* Update flag names to --request and --limit

 - Use singular flag names and support comma separated or repeated flag values
 - Update tests and docs

* Update CHANGELOG

* Update the flag description and examples

* Remove release 0.15.0 header from CHANGELOG

CHANGELOG.adoc

@@ -12,12 +12,16 @@
 | https://github.com/knative/client/pull/[#]
 ////
 
-## v0.15.0 (2020-06-02)
+## v0.15.1 (2020-06-03)
 
 [cols="1,10,3", options="header", width="100%"]
 |===
 | | Description | PR
 
+| 🐛
+| Update flag names to --request and --limit
+| https://github.com/knative/client/pull/872[#872]
+
 | 🐛
 | Fix `kn source -h`
 | https://github.com/knative/client/pull/846[#846]

docs/cmd/kn_service_create.md

@@ -44,7 +44,7 @@ kn service create NAME --image IMAGE [flags]
   # Create a service with 250MB memory, 200m CPU requests and a GPU resource limit
   # [https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/]
   # [https://kubernetes.io/docs/tasks/manage-gpus/scheduling-gpus/]
-  kn service create s4gpu --image knativesamples/hellocuda-go --requests memory=250Mi,cpu=200m --limits nvidia.com/gpu=1
+  kn service create s4gpu --image knativesamples/hellocuda-go --request memory=250Mi,cpu=200m --limit nvidia.com/gpu=1
 ```
 
 ### Options
@@ -67,9 +67,9 @@ kn service create NAME --image IMAGE [flags]
   -l, --label stringArray             Labels to set for both Service and Revision. name=value; you may provide this flag any number of times to set multiple labels. To unset, specify the label name followed by a "-" (e.g., name-).
       --label-revision stringArray    Revision label to set. name=value; you may provide this flag any number of times to set multiple labels. To unset, specify the label name followed by a "-" (e.g., name-). This flag takes precedence over "label" flag.
       --label-service stringArray     Service label to set. name=value; you may provide this flag any number of times to set multiple labels. To unset, specify the label name followed by a "-" (e.g., name-). This flag takes precedence over "label" flag.
-      --limits string                 The resource requirement limits for this Service. For example, 'cpu=100m,memory=256Mi'.
-      --limits-cpu string             DEPRECATED: please use --limits instead. The limits on the requested CPU (e.g., 1000m).
-      --limits-memory string          DEPRECATED: please use --limits instead. The limits on the requested memory (e.g., 1024Mi).
+      --limit strings                 The resource requirement limits for this Service. For example, 'cpu=100m,memory=256Mi'. You can use this flag multiple times. To unset a resource limit, append "-" to the resource name, e.g. '--limit memory-'.
+      --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.
@@ -80,9 +80,9 @@ kn service create NAME --image IMAGE [flags]
       --no-wait                       Do not wait for 'service create' operation to be completed.
   -p, --port int32                    The port where application listens on.
       --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.
-      --requests string               The resource requirement requests for this Service. For example, 'cpu=100m,memory=256Mi'.
-      --requests-cpu string           DEPRECATED: please use --requests instead. The requested CPU (e.g., 250m).
-      --requests-memory string        DEPRECATED: please use --requests instead. The requested memory (e.g., 64Mi).
+      --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-'.
+      --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}}")
       --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).

docs/cmd/kn_service_update.md

@@ -20,8 +20,8 @@ kn service update NAME [flags]
   # Update a service 'svc' with new port
   kn service update svc --port 80
 
-  # Updates a service 'svc' with new requests and limits parameters
-  kn service update svc --requests-cpu 500m --limits-memory 1024Mi
+  # Updates a service 'svc' with new request and limit parameters
+  kn service update svc --request cpu=500m --limit memory=1024Mi --limit cpu=1000m
 
   # Assign tag 'latest' and 'stable' to revisions 'echo-v2' and 'echo-v1' respectively
   kn service update svc --tag echo-v2=latest --tag echo-v1=stable
@@ -54,9 +54,9 @@ kn service update NAME [flags]
   -l, --label stringArray             Labels to set for both Service and Revision. name=value; you may provide this flag any number of times to set multiple labels. To unset, specify the label name followed by a "-" (e.g., name-).
       --label-revision stringArray    Revision label to set. name=value; you may provide this flag any number of times to set multiple labels. To unset, specify the label name followed by a "-" (e.g., name-). This flag takes precedence over "label" flag.
       --label-service stringArray     Service label to set. name=value; you may provide this flag any number of times to set multiple labels. To unset, specify the label name followed by a "-" (e.g., name-). This flag takes precedence over "label" flag.
-      --limits string                 The resource requirement limits for this Service. For example, 'cpu=100m,memory=256Mi'.
-      --limits-cpu string             DEPRECATED: please use --limits instead. The limits on the requested CPU (e.g., 1000m).
-      --limits-memory string          DEPRECATED: please use --limits instead. The limits on the requested memory (e.g., 1024Mi).
+      --limit strings                 The resource requirement limits for this Service. For example, 'cpu=100m,memory=256Mi'. You can use this flag multiple times. To unset a resource limit, append "-" to the resource name, e.g. '--limit memory-'.
+      --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.
@@ -67,9 +67,9 @@ kn service update NAME [flags]
       --no-wait                       Do not wait for 'service update' operation to be completed.
   -p, --port int32                    The port where application listens on.
       --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.
-      --requests string               The resource requirement requests for this Service. For example, 'cpu=100m,memory=256Mi'.
-      --requests-cpu string           DEPRECATED: please use --requests instead. The requested CPU (e.g., 250m).
-      --requests-memory string        DEPRECATED: please use --requests instead. The requested memory (e.g., 64Mi).
+      --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-'.
+      --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}}")
       --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.

pkg/kn/commands/service/configuration_edit_flags.go

@@ -145,27 +145,38 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) {
 			"You can use this flag multiple times.")
 	p.markFlagMakesRevision("arg")
 
-	command.Flags().StringVar(&p.Resources.Limits, "limits", "", "The resource requirement limits for this Service. For example, 'cpu=100m,memory=256Mi'.")
-	p.markFlagMakesRevision("limits")
-	command.Flags().StringVar(&p.Resources.Requests, "requests", "", "The resource requirement requests for this Service. For example, 'cpu=100m,memory=256Mi'.")
-	p.markFlagMakesRevision("requests")
+	command.Flags().StringSliceVar(&p.Resources.Limits,
+		"limit",
+		nil,
+		"The resource requirement limits for this Service. For example, 'cpu=100m,memory=256Mi'. "+
+			"You can use this flag multiple times. "+
+			"To unset a resource limit, append \"-\" to the resource name, e.g. '--limit memory-'.")
+	p.markFlagMakesRevision("limit")
+
+	command.Flags().StringSliceVar(&p.Resources.Requests,
+		"request",
+		nil,
+		"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-'.")
+	p.markFlagMakesRevision("request")
 
 	command.Flags().StringVar(&p.RequestsFlags.CPU, "requests-cpu", "",
-		"DEPRECATED: please use --requests instead. The requested CPU (e.g., 250m).")
+		"DEPRECATED: please use --request instead. The requested CPU (e.g., 250m).")
 	p.markFlagMakesRevision("requests-cpu")
 
 	command.Flags().StringVar(&p.RequestsFlags.Memory, "requests-memory", "",
-		"DEPRECATED: please use --requests instead. The requested memory (e.g., 64Mi).")
+		"DEPRECATED: please use --request instead. The requested memory (e.g., 64Mi).")
 	p.markFlagMakesRevision("requests-memory")
 
 	// TODO: Flag marked deprecated in release v0.15.0, remove in release v0.18.0
 	command.Flags().StringVar(&p.LimitsFlags.CPU, "limits-cpu", "",
-		"DEPRECATED: please use --limits instead. The limits on the requested CPU (e.g., 1000m).")
+		"DEPRECATED: please use --limit instead. The limits on the requested CPU (e.g., 1000m).")
 	p.markFlagMakesRevision("limits-cpu")
 
 	// TODO: Flag marked deprecated in release v0.15.0, remove in release v0.18.0
 	command.Flags().StringVar(&p.LimitsFlags.Memory, "limits-memory", "",
-		"DEPRECATED: please use --limits instead. The limits on the requested memory (e.g., 1024Mi).")
+		"DEPRECATED: please use --limit instead. The limits on the requested memory (e.g., 1024Mi).")
 	p.markFlagMakesRevision("limits-memory")
 
 	command.Flags().IntVar(&p.MinScale, "min-scale", 0, "Minimal number of replicas.")
@@ -352,17 +363,17 @@ func (p *ConfigurationEditFlags) Apply(
 	}
 
 	if cmd.Flags().Changed("limits-cpu") || cmd.Flags().Changed("limits-memory") {
-		if cmd.Flags().Changed("limits") {
-			return fmt.Errorf("only one of (DEPRECATED) --limits-cpu / --limits-memory and --limits can be specified")
+		if cmd.Flags().Changed("limit") {
+			return fmt.Errorf("only one of (DEPRECATED) --limits-cpu / --limits-memory and --limit can be specified")
 		}
-		fmt.Fprintf(cmd.OutOrStdout(), "\nWARNING: flags --limits-cpu / --limits-memory are deprecated and going to be removed in future release, please use --limits instead.\n\n")
+		fmt.Fprintf(cmd.OutOrStdout(), "\nWARNING: flags --limits-cpu / --limits-memory are deprecated and going to be removed in future release, please use --limit instead.\n\n")
 	}
 
 	if cmd.Flags().Changed("requests-cpu") || cmd.Flags().Changed("requests-memory") {
-		if cmd.Flags().Changed("requests") {
-			return fmt.Errorf("only one of (DEPRECATED) --requests-cpu / --requests-memory and --requests can be specified")
+		if cmd.Flags().Changed("request") {
+			return fmt.Errorf("only one of (DEPRECATED) --requests-cpu / --requests-memory and --request can be specified")
 		}
-		fmt.Fprintf(cmd.OutOrStdout(), "\nWARNING: flags --requests-cpu / --requests-memory are deprecated and going to be removed in future release, please use --requests instead.\n\n")
+		fmt.Fprintf(cmd.OutOrStdout(), "\nWARNING: flags --requests-cpu / --requests-memory are deprecated and going to be removed in future release, please use --request instead.\n\n")
 	}
 
 	limitsResources, err := p.computeResources(p.LimitsFlags)
@@ -378,12 +389,12 @@ func (p *ConfigurationEditFlags) Apply(
 		return err
 	}
 
-	err = p.Resources.Validate()
+	requestsToRemove, limitsToRemove, err := p.Resources.Validate()
 	if err != nil {
 		return err
 	}
 
-	err = servinglib.UpdateResources(template, p.Resources.ResourceRequirements)
+	err = servinglib.UpdateResources(template, p.Resources.ResourceRequirements, requestsToRemove, limitsToRemove)
 	if err != nil {
 		return err
 	}

pkg/kn/commands/service/create.go

@@ -64,7 +64,7 @@ var create_example = `
   # Create a service with 250MB memory, 200m CPU requests and a GPU resource limit
   # [https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/]
   # [https://kubernetes.io/docs/tasks/manage-gpus/scheduling-gpus/]
-  kn service create s4gpu --image knativesamples/hellocuda-go --requests memory=250Mi,cpu=200m --limits nvidia.com/gpu=1`
+  kn service create s4gpu --image knativesamples/hellocuda-go --request memory=250Mi,cpu=200m --limit nvidia.com/gpu=1`
 
 func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command {
 	var editFlags ConfigurationEditFlags

pkg/kn/commands/service/create_mock_test.go

@@ -434,8 +434,8 @@ func TestServiceCreateWithResources(t *testing.T) {
 	r.CreateService(service, nil)
 
 	output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz",
-		"--requests", "cpu=250m,memory=64Mi",
-		"--limits", "cpu=1000m,memory=1024Mi,nvidia.com/gpu=1",
+		"--request", "cpu=250m,memory=64Mi",
+		"--limit", "cpu=1000m,memory=1024Mi,nvidia.com/gpu=1",
 		"--no-wait", "--revision-name=")
 	assert.NilError(t, err)
 	assert.Assert(t, util.ContainsAll(output, "created", "foo", "default"))
@@ -469,8 +469,8 @@ func TestServiceCreateWithResourcesWarning(t *testing.T) {
 		"--limits-cpu", "1000m",
 		"--no-wait", "--revision-name=")
 	assert.NilError(t, err)
-	assert.Assert(t, util.ContainsAll(output, "WARNING", "--requests-cpu", "--requests-memory", "deprecated", "removed", "--requests", "instead"))
-	assert.Assert(t, util.ContainsAll(output, "WARNING", "--limits-cpu", "--limits-memory", "deprecated", "removed", "--limits", "instead"))
+	assert.Assert(t, util.ContainsAll(output, "WARNING", "--requests-cpu", "--requests-memory", "deprecated", "removed", "--request", "instead"))
+	assert.Assert(t, util.ContainsAll(output, "WARNING", "--limits-cpu", "--limits-memory", "deprecated", "removed", "--limit", "instead"))
 	assert.Assert(t, util.ContainsAll(output, "created", "foo", "default"))
 
 	r.Validate()
@@ -482,10 +482,10 @@ func TestServiceCreateWithResourcesError(t *testing.T) {
 
 	output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz",
 		"--requests-memory", "64Mi",
-		"--requests", "memory=64Mi",
+		"--request", "memory=64Mi",
 		"--no-wait", "--revision-name=")
 	assert.Assert(t, err != nil)
-	assert.Assert(t, util.ContainsAll(output, "only one of", "DEPRECATED", "--requests-cpu", "--requests-memory", "--requests", "can be specified"))
+	assert.Assert(t, util.ContainsAll(output, "only one of", "DEPRECATED", "--requests-cpu", "--requests-memory", "--request", "can be specified"))
 
 	r.Validate()
 }

pkg/kn/commands/service/create_test.go

@@ -268,7 +268,7 @@ func TestServiceCreateWithDeprecatedRequests(t *testing.T) {
 func TestServiceCreateWithRequests(t *testing.T) {
 	action, created, _, err := fakeServiceCreate([]string{
 		"service", "create", "foo", "--image", "gcr.io/foo/bar:baz",
-		"--requests", "cpu=250m,memory=64Mi",
+		"--request", "cpu=250m,memory=64Mi",
 		"--no-wait"}, false)
 
 	if err != nil {
@@ -324,7 +324,7 @@ func TestServiceCreateWithDeprecatedLimits(t *testing.T) {
 func TestServiceCreateWithLimits(t *testing.T) {
 	action, created, _, err := fakeServiceCreate([]string{
 		"service", "create", "foo", "--image", "gcr.io/foo/bar:baz",
-		"--limits", "cpu=1000m,memory=1024Mi",
+		"--limit", "cpu=1000m", "--limit", "memory=1024Mi",
 		"--no-wait"}, false)
 
 	if err != nil {
@@ -391,7 +391,7 @@ func TestServiceCreateDeprecatedRequestsLimitsCPU(t *testing.T) {
 func TestServiceCreateRequestsLimitsCPU(t *testing.T) {
 	action, created, _, err := fakeServiceCreate([]string{
 		"service", "create", "foo", "--image", "gcr.io/foo/bar:baz",
-		"--requests", "cpu=250m", "--limits", "cpu=1000m",
+		"--request", "cpu=250m", "--limit", "cpu=1000m",
 		"--no-wait"}, false)
 
 	if err != nil {
@@ -471,8 +471,8 @@ func TestServiceCreateRequestsLimitsMemory(t *testing.T) {
 	action, created, _, err := fakeServiceCreate([]string{
 		"service", "create", "foo",
 		"--image", "gcr.io/foo/bar:baz",
-		"--requests", "memory=64Mi",
-		"--limits", "memory=1024Mi", "--no-wait"}, false)
+		"--request", "memory=64Mi",
+		"--limit", "memory=1024Mi", "--no-wait"}, false)
 
 	if err != nil {
 		t.Fatal(err)

pkg/kn/commands/service/update.go

@@ -36,8 +36,8 @@ var updateExample = `
   # Update a service 'svc' with new port
   kn service update svc --port 80
 
-  # Updates a service 'svc' with new requests and limits parameters
-  kn service update svc --requests-cpu 500m --limits-memory 1024Mi
+  # Updates a service 'svc' with new request and limit parameters
+  kn service update svc --request cpu=500m --limit memory=1024Mi --limit cpu=1000m
 
   # Assign tag 'latest' and 'stable' to revisions 'echo-v2' and 'echo-v1' respectively
   kn service update svc --tag echo-v2=latest --tag echo-v1=stable