Add --requests and --limits resource flags (#859)

* Add --requests and --limits resource flags

  Fixes #490

  - Add deprecation message for --requests-cpu, --requests-memory,
  --limits-cpu and --limits-memory flag usage.

  - Add error message if new and deprecated, requests and limits flags
    are used together.

* Add resources requests and limits example for service create

* Add mock style unit tests

* Update existing unit tests for deprecated flags

* Add integration tests

* Add CHANGELOG entry

* Fix lint warnings

* Add unit tests for resource flags

* Fix typo for service name in e2e tests

* Refactor resource validation utility into test lib

* Remove commented out code

* Add references about managing resources and GPU in example

* Update image references in examples

CHANGELOG.adoc

@@ -16,6 +16,10 @@
 |===
 | | Description | PR
 
+| 🎁
+| Add --requests and --limits flags for resource requirements
+| https://github.com/knative/client/pull/859[#859]
+
 | 🐣
 | Replaced non-standard errors package with standard library functions
 | https://github.com/knative/client/pull/853[#853]

docs/cmd/kn_service_create.md

@@ -39,7 +39,12 @@ kn service create NAME --image IMAGE [flags]
   kn service create s3 --image knativesamples/helloworld --annotation sidecar.istio.io/inject=false
 
   # Create a private service (that is a service with no external endpoint)
-  kn service create s4 --image knativesamples/helloworld --cluster-local
+  kn service create s1 --image knativesamples/helloworld --cluster-local
+
+  # 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
 ```
 
 ### Options
@@ -62,8 +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-cpu string             The limits on the requested CPU (e.g., 1000m).
+      --limits string                 The resource requirement limits for this Service. For example, 'cpu=100m,memory=256Mi'.
-      --limits-memory string          The limits on the requested memory (e.g., 1024Mi).
+      --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).
       --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.
@@ -74,8 +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-cpu string           The requested CPU (e.g., 250m).
+      --requests string               The resource requirement requests for this Service. For example, 'cpu=100m,memory=256Mi'.
-      --requests-memory string        The requested memory (e.g., 64Mi).
+      --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).
       --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

@@ -54,8 +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-cpu string             The limits on the requested CPU (e.g., 1000m).
+      --limits string                 The resource requirement limits for this Service. For example, 'cpu=100m,memory=256Mi'.
-      --limits-memory string          The limits on the requested memory (e.g., 1024Mi).
+      --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).
       --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.
@@ -66,8 +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-cpu string           The requested CPU (e.g., 250m).
+      --requests string               The resource requirement requests for this Service. For example, 'cpu=100m,memory=256Mi'.
-      --requests-memory string        The requested memory (e.g., 64Mi).
+      --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).
       --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.

lib/test/service.go

@@ -15,7 +15,14 @@
 package test
 
 import (
+	"encoding/json"
+	"strings"
+
 	"gotest.tools/assert"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
+	servingv1 "knative.dev/serving/pkg/apis/serving/v1"
+
 	"knative.dev/client/pkg/util"
 )
 
@@ -78,3 +85,30 @@ func ServiceDescribeWithJSONPath(r *KnRunResultCollector, serviceName, jsonpath
 	r.AssertNoError(out)
 	return out.Stdout
 }
+
+func ValidateServiceResources(r *KnRunResultCollector, serviceName string, requestsMemory, requestsCPU, limitsMemory, limitsCPU string) {
+	var err error
+	rlist := corev1.ResourceList{}
+	rlist[corev1.ResourceCPU], err = resource.ParseQuantity(requestsCPU)
+	assert.NilError(r.T(), err)
+	rlist[corev1.ResourceMemory], err = resource.ParseQuantity(requestsMemory)
+	assert.NilError(r.T(), err)
+
+	llist := corev1.ResourceList{}
+	llist[corev1.ResourceCPU], err = resource.ParseQuantity(limitsCPU)
+	assert.NilError(r.T(), err)
+	llist[corev1.ResourceMemory], err = resource.ParseQuantity(limitsMemory)
+	assert.NilError(r.T(), err)
+
+	out := r.KnTest().Kn().Run("service", "describe", serviceName, "-ojson")
+	data := json.NewDecoder(strings.NewReader(out.Stdout))
+	var service servingv1.Service
+	err = data.Decode(&service)
+	assert.NilError(r.T(), err)
+
+	serviceRequestResourceList := service.Spec.Template.Spec.Containers[0].Resources.Requests
+	assert.DeepEqual(r.T(), serviceRequestResourceList, rlist)
+
+	serviceLimitsResourceList := service.Spec.Template.Spec.Containers[0].Resources.Limits
+	assert.DeepEqual(r.T(), serviceLimitsResourceList, llist)
+}

pkg/kn/commands/service/configuration_edit_flags.go

@@ -25,7 +25,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
-	"knative.dev/client/pkg/kn/flags"
+	knflags "knative.dev/client/pkg/kn/flags"
 	servinglib "knative.dev/client/pkg/serving"
 	"knative.dev/client/pkg/util"
 	"knative.dev/serving/pkg/apis/serving"
@@ -43,7 +43,8 @@ type ConfigurationEditFlags struct {
 	Command string
 	Arg     []string
 
-	RequestsFlags, LimitsFlags ResourceFlags
+	RequestsFlags, LimitsFlags ResourceFlags // TODO: Flag marked deprecated in release v0.15.0, remove in release v0.18.0
+	Resources                  knflags.ResourceOptions
 	MinScale                   int
 	MaxScale                   int
 	ConcurrencyTarget          int
@@ -144,17 +145,27 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) {
 			"You can use this flag multiple times.")
 	p.markFlagMakesRevision("arg")
 
-	command.Flags().StringVar(&p.RequestsFlags.CPU, "requests-cpu", "", "The requested CPU (e.g., 250m).")
+	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().StringVar(&p.RequestsFlags.CPU, "requests-cpu", "",
+		"DEPRECATED: please use --requests instead. The requested CPU (e.g., 250m).")
 	p.markFlagMakesRevision("requests-cpu")
 
-	command.Flags().StringVar(&p.RequestsFlags.Memory, "requests-memory", "", "The requested memory (e.g., 64Mi).")
+	command.Flags().StringVar(&p.RequestsFlags.Memory, "requests-memory", "",
+		"DEPRECATED: please use --requests instead. The requested memory (e.g., 64Mi).")
 	p.markFlagMakesRevision("requests-memory")
 
-	command.Flags().StringVar(&p.LimitsFlags.CPU, "limits-cpu", "", "The limits on the requested CPU (e.g., 1000m).")
+	// 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).")
 	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", "",
-		"The limits on the requested memory (e.g., 1024Mi).")
+		"DEPRECATED: please use --limits 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.")
@@ -166,7 +177,7 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) {
 	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")
 
-	flags.AddBothBoolFlagsUnhidden(command.Flags(), &p.ClusterLocal, "cluster-local", "", false,
+	knflags.AddBothBoolFlagsUnhidden(command.Flags(), &p.ClusterLocal, "cluster-local", "", false,
 		"Specify that the service be private. (--no-cluster-local will make the service publicly available)")
 	//TODO: Need to also not change revision when already set (solution to issue #646)
 	p.markFlagMakesRevision("cluster-local")
@@ -214,7 +225,7 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) {
 			"{{.Generation}} for the generation, and {{.Random [n]}} for n random consonants.")
 	p.markFlagMakesRevision("revision-name")
 
-	flags.AddBothBoolFlagsUnhidden(command.Flags(), &p.LockToDigest, "lock-to-digest", "", true,
+	knflags.AddBothBoolFlagsUnhidden(command.Flags(), &p.LockToDigest, "lock-to-digest", "", true,
 		"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)")
 	// Don't mark as changing the revision.
@@ -340,6 +351,20 @@ func (p *ConfigurationEditFlags) Apply(
 		servinglib.UnsetUserImageAnnot(template)
 	}
 
+	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")
+		}
+		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")
+	}
+
+	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")
+		}
+		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")
+	}
+
 	limitsResources, err := p.computeResources(p.LimitsFlags)
 	if err != nil {
 		return err
@@ -348,7 +373,17 @@ func (p *ConfigurationEditFlags) Apply(
 	if err != nil {
 		return err
 	}
-	err = servinglib.UpdateResources(template, requestsResources, limitsResources)
+	err = servinglib.UpdateResourcesDeprecated(template, requestsResources, limitsResources)
+	if err != nil {
+		return err
+	}
+
+	err = p.Resources.Validate()
+	if err != nil {
+		return err
+	}
+
+	err = servinglib.UpdateResources(template, p.Resources.ResourceRequirements)
 	if err != nil {
 		return err
 	}

pkg/kn/commands/service/create.go

@@ -59,7 +59,12 @@ var create_example = `
   kn service create s3 --image knativesamples/helloworld --annotation sidecar.istio.io/inject=false
 
   # Create a private service (that is a service with no external endpoint)
-  kn service create s4 --image knativesamples/helloworld --cluster-local`
+  kn service create s1 --image knativesamples/helloworld --cluster-local
+
+  # 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`
 
 func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command {
 	var editFlags ConfigurationEditFlags

pkg/kn/commands/service/create_mock_test.go

@@ -409,6 +409,87 @@ func TestServiceCreateWithUser(t *testing.T) {
 	r.Validate()
 }
 
+func TestServiceCreateWithResources(t *testing.T) {
+	client := knclient.NewMockKnServiceClient(t)
+
+	r := client.Recorder()
+	r.GetService("foo", nil, errors.NewNotFound(servingv1.Resource("service"), "foo"))
+
+	service := getService("foo")
+	template := &service.Spec.Template
+
+	template.Spec.Containers[0].Resources.Requests = corev1.ResourceList{
+		corev1.ResourceCPU:    parseQuantity(t, "250m"),
+		corev1.ResourceMemory: parseQuantity(t, "64Mi"),
+	}
+
+	template.Spec.Containers[0].Resources.Limits = corev1.ResourceList{
+		corev1.ResourceCPU:                    parseQuantity(t, "1000m"),
+		corev1.ResourceMemory:                 parseQuantity(t, "1024Mi"),
+		corev1.ResourceName("nvidia.com/gpu"): parseQuantity(t, "1"),
+	}
+
+	template.Spec.Containers[0].Image = "gcr.io/foo/bar:baz"
+	template.Annotations = map[string]string{servinglib.UserImageAnnotationKey: "gcr.io/foo/bar:baz"}
+	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",
+		"--no-wait", "--revision-name=")
+	assert.NilError(t, err)
+	assert.Assert(t, util.ContainsAll(output, "created", "foo", "default"))
+
+	r.Validate()
+}
+
+func TestServiceCreateWithResourcesWarning(t *testing.T) {
+	client := knclient.NewMockKnServiceClient(t)
+
+	r := client.Recorder()
+	r.GetService("foo", nil, errors.NewNotFound(servingv1.Resource("service"), "foo"))
+
+	service := getService("foo")
+	template := &service.Spec.Template
+
+	template.Spec.Containers[0].Resources.Requests = corev1.ResourceList{
+		corev1.ResourceMemory: parseQuantity(t, "64Mi"),
+	}
+
+	template.Spec.Containers[0].Resources.Limits = corev1.ResourceList{
+		corev1.ResourceCPU: parseQuantity(t, "1000m"),
+	}
+
+	template.Spec.Containers[0].Image = "gcr.io/foo/bar:baz"
+	template.Annotations = map[string]string{servinglib.UserImageAnnotationKey: "gcr.io/foo/bar:baz"}
+	r.CreateService(service, nil)
+
+	output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz",
+		"--requests-memory", "64Mi",
+		"--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, "created", "foo", "default"))
+
+	r.Validate()
+}
+
+func TestServiceCreateWithResourcesError(t *testing.T) {
+	client := knclient.NewMockKnServiceClient(t)
+	r := client.Recorder()
+
+	output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz",
+		"--requests-memory", "64Mi",
+		"--requests", "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"))
+
+	r.Validate()
+}
+
 func getService(name string) *servingv1.Service {
 	service := &servingv1.Service{
 		ObjectMeta: metav1.ObjectMeta{

pkg/kn/commands/service/create_test.go

@@ -237,7 +237,7 @@ func TestServiceCreateEnv(t *testing.T) {
 	}
 }
 
-func TestServiceCreateWithRequests(t *testing.T) {
+func TestServiceCreateWithDeprecatedRequests(t *testing.T) {
 	action, created, _, err := fakeServiceCreate([]string{
 		"service", "create", "foo", "--image", "gcr.io/foo/bar:baz",
 		"--requests-cpu", "250m", "--requests-memory", "64Mi",
@@ -265,7 +265,35 @@ func TestServiceCreateWithRequests(t *testing.T) {
 	}
 }
 
-func TestServiceCreateWithLimits(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",
+		"--no-wait"}, false)
+
+	if err != nil {
+		t.Fatal(err)
+	} else if !action.Matches("create", "services") {
+		t.Fatalf("Bad action %v", action)
+	}
+
+	expectedRequestsVars := corev1.ResourceList{
+		corev1.ResourceCPU:    parseQuantity(t, "250m"),
+		corev1.ResourceMemory: parseQuantity(t, "64Mi"),
+	}
+
+	template := &created.Spec.Template
+
+	if err != nil {
+		t.Fatal(err)
+	} else if !reflect.DeepEqual(
+		template.Spec.Containers[0].Resources.Requests,
+		expectedRequestsVars) {
+		t.Fatalf("wrong requests vars %v", template.Spec.Containers[0].Resources.Requests)
+	}
+}
+
+func TestServiceCreateWithDeprecatedLimits(t *testing.T) {
 	action, created, _, err := fakeServiceCreate([]string{
 		"service", "create", "foo", "--image", "gcr.io/foo/bar:baz",
 		"--limits-cpu", "1000m", "--limits-memory", "1024Mi",
@@ -293,7 +321,35 @@ func TestServiceCreateWithLimits(t *testing.T) {
 	}
 }
 
-func TestServiceCreateRequestsLimitsCPU(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",
+		"--no-wait"}, false)
+
+	if err != nil {
+		t.Fatal(err)
+	} else if !action.Matches("create", "services") {
+		t.Fatalf("Bad action %v", action)
+	}
+
+	expectedLimitsVars := corev1.ResourceList{
+		corev1.ResourceCPU:    parseQuantity(t, "1000m"),
+		corev1.ResourceMemory: parseQuantity(t, "1024Mi"),
+	}
+
+	template := &created.Spec.Template
+
+	if err != nil {
+		t.Fatal(err)
+	} else if !reflect.DeepEqual(
+		template.Spec.Containers[0].Resources.Limits,
+		expectedLimitsVars) {
+		t.Fatalf("wrong limits vars %v", template.Spec.Containers[0].Resources.Limits)
+	}
+}
+
+func TestServiceCreateDeprecatedRequestsLimitsCPU(t *testing.T) {
 	action, created, _, err := fakeServiceCreate([]string{
 		"service", "create", "foo", "--image", "gcr.io/foo/bar:baz",
 		"--requests-cpu", "250m", "--limits-cpu", "1000m",
@@ -332,7 +388,46 @@ func TestServiceCreateRequestsLimitsCPU(t *testing.T) {
 	}
 }
 
-func TestServiceCreateRequestsLimitsMemory(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",
+		"--no-wait"}, false)
+
+	if err != nil {
+		t.Fatal(err)
+	} else if !action.Matches("create", "services") {
+		t.Fatalf("Bad action %v", action)
+	}
+
+	expectedRequestsVars := corev1.ResourceList{
+		corev1.ResourceCPU: parseQuantity(t, "250m"),
+	}
+
+	expectedLimitsVars := corev1.ResourceList{
+		corev1.ResourceCPU: parseQuantity(t, "1000m"),
+	}
+
+	template := &created.Spec.Template
+
+	if err != nil {
+		t.Fatal(err)
+	} else {
+		if !reflect.DeepEqual(
+			template.Spec.Containers[0].Resources.Requests,
+			expectedRequestsVars) {
+			t.Fatalf("wrong requests vars %v", template.Spec.Containers[0].Resources.Requests)
+		}
+
+		if !reflect.DeepEqual(
+			template.Spec.Containers[0].Resources.Limits,
+			expectedLimitsVars) {
+			t.Fatalf("wrong limits vars %v", template.Spec.Containers[0].Resources.Limits)
+		}
+	}
+}
+
+func TestServiceCreateDeprecatedRequestsLimitsMemory(t *testing.T) {
 	action, created, _, err := fakeServiceCreate([]string{
 		"service", "create", "foo",
 		"--image", "gcr.io/foo/bar:baz",
@@ -372,6 +467,46 @@ func TestServiceCreateRequestsLimitsMemory(t *testing.T) {
 	}
 }
 
+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)
+
+	if err != nil {
+		t.Fatal(err)
+	} else if !action.Matches("create", "services") {
+		t.Fatalf("Bad action %v", action)
+	}
+
+	expectedRequestsVars := corev1.ResourceList{
+		corev1.ResourceMemory: parseQuantity(t, "64Mi"),
+	}
+
+	expectedLimitsVars := corev1.ResourceList{
+		corev1.ResourceMemory: parseQuantity(t, "1024Mi"),
+	}
+
+	template := &created.Spec.Template
+
+	if err != nil {
+		t.Fatal(err)
+	} else {
+		if !reflect.DeepEqual(
+			template.Spec.Containers[0].Resources.Requests,
+			expectedRequestsVars) {
+			t.Fatalf("wrong requests vars %v", template.Spec.Containers[0].Resources.Requests)
+		}
+
+		if !reflect.DeepEqual(
+			template.Spec.Containers[0].Resources.Limits,
+			expectedLimitsVars) {
+			t.Fatalf("wrong limits vars %v", template.Spec.Containers[0].Resources.Limits)
+		}
+	}
+}
+
 func TestServiceCreateMaxMinScale(t *testing.T) {
 	action, created, _, err := fakeServiceCreate([]string{
 		"service", "create", "foo", "--image", "gcr.io/foo/bar:baz",

pkg/kn/flags/resources.go

@@ -0,0 +1,72 @@
+// Copyright © 2020 The Knative Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package flags
+
+import (
+	"fmt"
+	"strings"
+
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
+)
+
+// ResourceOptions to hold the container resource requirements values
+type ResourceOptions struct {
+	Requests             string
+	Limits               string
+	ResourceRequirements corev1.ResourceRequirements
+}
+
+// Validate parses the limits and requests parameters if specified and
+// sets ResourceRequirements for ResourceOptions or returns error if any
+func (o *ResourceOptions) Validate() (err error) {
+	limits, err := populateResourceListV1(o.Limits)
+	if err != nil {
+		return err
+	}
+	o.ResourceRequirements.Limits = limits
+
+	requests, err := populateResourceListV1(o.Requests)
+	if err != nil {
+		return err
+	}
+	o.ResourceRequirements.Requests = requests
+	return nil
+}
+
+// populateResourceListV1 takes strings of form <resourceName1>=<value1>,<resourceName1>=<value2>
+// and returns ResourceList.
+func populateResourceListV1(spec string) (corev1.ResourceList, error) {
+	// empty input gets a nil response to preserve generator test expected behaviors
+	if spec == "" {
+		return nil, nil
+	}
+
+	result := corev1.ResourceList{}
+	resourceStatements := strings.Split(spec, ",")
+	for _, resourceStatement := range resourceStatements {
+		parts := strings.Split(resourceStatement, "=")
+		if len(parts) != 2 {
+			return nil, fmt.Errorf("invalid argument syntax %v, expected <resource>=<value>", resourceStatement)
+		}
+		resourceName := corev1.ResourceName(parts[0])
+		resourceQuantity, err := resource.ParseQuantity(parts[1])
+		if err != nil {
+			return nil, err
+		}
+		result[resourceName] = resourceQuantity
+	}
+	return result, nil
+}

pkg/kn/flags/resources_test.go

@@ -0,0 +1,86 @@
+// Copyright © 2020 The Knative Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package flags
+
+import (
+	"testing"
+
+	"gotest.tools/assert"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
+)
+
+type resourceOptionsTestCase struct {
+	requests         string
+	limits           string
+	expectedRequests corev1.ResourceList
+	expectedLimits   corev1.ResourceList
+	expectedErr      bool
+}
+
+func parseQuantity(value string) resource.Quantity {
+	q, _ := resource.ParseQuantity(value)
+	return q
+}
+
+func TestResourceOptions(t *testing.T) {
+	cases := []*resourceOptionsTestCase{
+		{"memory=200Mi,cpu=200m",
+			"memory=1024Mi,cpu=500m",
+			corev1.ResourceList{corev1.ResourceMemory: parseQuantity("200Mi"),
+				corev1.ResourceCPU: parseQuantity("200m")},
+			corev1.ResourceList{corev1.ResourceMemory: parseQuantity("1024Mi"),
+				corev1.ResourceCPU: parseQuantity("500m")},
+			false,
+		},
+		{"",
+			"nvidia.com/gpu=1",
+			nil,
+			corev1.ResourceList{corev1.ResourceName("nvidia.com/gpu"): parseQuantity("1")},
+			false,
+		},
+
+		{"",
+			"memory:500Mi",
+			nil,
+			nil,
+			true,
+		},
+		{"memory:200Mi",
+			"",
+			nil,
+			nil,
+			true,
+		},
+		{"memory=200MB",
+			"",
+			nil,
+			nil,
+			true,
+		},
+	}
+	for _, c := range cases {
+		options := &ResourceOptions{}
+		options.Requests = c.requests
+		options.Limits = c.limits
+		err := options.Validate()
+
+		if c.expectedErr {
+			assert.Assert(t, err != nil)
+		} else {
+			assert.DeepEqual(t, options.ResourceRequirements, corev1.ResourceRequirements{Limits: c.expectedLimits, Requests: c.expectedRequests})
+		}
+	}
+}

pkg/serving/config_changes.go

@@ -355,8 +355,34 @@ func UpdateUser(template *servingv1.RevisionTemplateSpec, user int64) error {
 	return nil
 }
 
-// UpdateResources updates resources as requested
+// UpdateResources updates container resources for given revision template
-func UpdateResources(template *servingv1.RevisionTemplateSpec, requestsResourceList corev1.ResourceList, limitsResourceList corev1.ResourceList) error {
+func UpdateResources(template *servingv1.RevisionTemplateSpec, resources corev1.ResourceRequirements) error {
+	container, err := ContainerOfRevisionTemplate(template)
+	if err != nil {
+		return err
+	}
+
+	if container.Resources.Requests == nil {
+		container.Resources.Requests = corev1.ResourceList{}
+	}
+
+	for k, v := range resources.Requests {
+		container.Resources.Requests[k] = v
+	}
+
+	if container.Resources.Limits == nil {
+		container.Resources.Limits = corev1.ResourceList{}
+	}
+
+	for k, v := range resources.Limits {
+		container.Resources.Limits[k] = v
+	}
+
+	return nil
+}
+
+// UpdateResourcesDeprecated updates resources as requested
+func UpdateResourcesDeprecated(template *servingv1.RevisionTemplateSpec, requestsResourceList corev1.ResourceList, limitsResourceList corev1.ResourceList) error {
 	container, err := ContainerOfRevisionTemplate(template)
 	if err != nil {
 		return err

test/e2e/service_options_test.go

@@ -26,11 +26,10 @@ import (
 	"testing"
 
 	"gotest.tools/assert"
+	servingv1 "knative.dev/serving/pkg/apis/serving/v1"
 
 	"knative.dev/client/lib/test"
 	"knative.dev/client/pkg/util"
-
-	servingv1 "knative.dev/serving/pkg/apis/serving/v1"
 )
 
 func TestServiceOptions(t *testing.T) {
@@ -124,6 +123,10 @@ func TestServiceOptions(t *testing.T) {
 	t.Log("create and validate service and revision labels")
 	serviceCreateWithOptions(r, "svc7", "--label-service", "svc=helloworld-svc", "--label-revision", "rev=helloworld-rev")
 	validateLabels(r, "svc7", map[string]string{"svc": "helloworld-svc"}, map[string]string{"rev": "helloworld-rev"})
+
+	t.Log("create and validate service resource options")
+	serviceCreateWithOptions(r, "svc8", "--limits", "memory=500Mi,cpu=1000m", "--requests", "memory=250Mi,cpu=200m")
+	test.ValidateServiceResources(r, "svc8", "250Mi", "200m", "500Mi", "1000m")
 }
 
 func serviceCreateWithOptions(r *test.KnRunResultCollector, serviceName string, options ...string) {