Added support for limits and requests of CPU and memory (#78)

* Added support for limits and requests of CPU and memory. Examples:
1. kn service create mysvc --image dev.local/ns/image:latest --request-cpu 100m --requests-memory 64Mi
2. kn service create mysvc --image dev.local/ns/image:latest --limits-cpu 1000m --limits-memory 1024Mi

You can also include and/or omit the requests and limits. So the following should also work:
kn service create mysvc --image dev.local/ns/image:latest  --request-cpu 1000m --limits-memory 1024Mi

* Addressed comments about maintaining existing requests and limits and added more tests
* TestServiceCreateWithRequests
* TestServiceCreateWithLimits
* TestServiceCreateRequestsLimitsCPU
* TestServiceCreateRequestsLimitsMemory
* TestServiceCreateRequestsLimitsCPUMemory

* updating dependencies and formatting tests code

go.mod

@@ -20,6 +20,7 @@ require (
 	github.com/imdario/mergo v0.3.6 // indirect
 	github.com/inconshreveable/mousetrap v1.0.0 // indirect
 	github.com/json-iterator/go v1.1.5 // indirect
+	github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024 // indirect
 	github.com/knative/build v0.3.0 // indirect
 	github.com/knative/pkg v0.0.0-20190110005142-b6044a7d1795 // indirect
 	github.com/knative/serving v0.3.0

go.sum

@@ -58,6 +58,8 @@ github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NH
 github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8=
 github.com/json-iterator/go v1.1.5 h1:gL2yXlmiIo4+t+y32d4WGwOjKGYcGOuyrg46vadswDE=
 github.com/json-iterator/go v1.1.5/go.mod h1:+SdeFBvtyEkXs7REEP0seUULqWtbJapLOCVDaaPEHmU=
+github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024 h1:rBMNdlhTLzJjJSDIjNEXX1Pz3Hmwmz91v+zycvx9PJc=
+github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024/go.mod h1:6v2b51hI/fHJwM22ozAgKL4VKDeJcHhJFhtBdhmNjmU=
 github.com/knative/build v0.3.0 h1:WRWGzJZmFxBPjehIEcsa0pgsKE6j5BGuDqvFRJRQt8I=
 github.com/knative/build v0.3.0/go.mod h1:/sU74ZQkwlYA5FwYDJhYTy61i/Kn+5eWfln2jDbw3Qo=
 github.com/knative/pkg v0.0.0-20190110005142-b6044a7d1795 h1:mTaIClRJEjRuTFOc4Gsicdqe/OwEla0Ano9cllUNoU0=

pkg/kn/commands/configuration_edit_flags.go

@@ -21,11 +21,19 @@ import (
 	servinglib "github.com/knative/client/pkg/serving"
 	servingv1alpha1 "github.com/knative/serving/pkg/apis/serving/v1alpha1"
 	"github.com/spf13/cobra"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
 )
 
 type ConfigurationEditFlags struct {
-	Image string
+	Image                      string
-	Env   []string
+	Env                        []string
+	RequestsFlags, LimitsFlags ResourceFlags
+}
+
+type ResourceFlags struct {
+	CPU    string
+	Memory string
 }
 
 func (p *ConfigurationEditFlags) AddFlags(command *cobra.Command) {
@@ -33,6 +41,10 @@ func (p *ConfigurationEditFlags) AddFlags(command *cobra.Command) {
 	command.Flags().StringArrayVarP(&p.Env, "env", "e", []string{},
 		"Environment variable to set. NAME=value; you may provide this flag "+
 			"any number of times to set multiple environment variables.")
+	command.Flags().StringVar(&p.RequestsFlags.CPU, "requests-cpu", "", "The requested CPU (e.g., 250m).")
+	command.Flags().StringVar(&p.RequestsFlags.Memory, "requests-memory", "", "The requested CPU (e.g., 64Mi).")
+	command.Flags().StringVar(&p.LimitsFlags.CPU, "limits-cpu", "", "The limits on the requested CPU (e.g., 1000m).")
+	command.Flags().StringVar(&p.LimitsFlags.Memory, "limits-memory", "", "The limits on the requested CPU (e.g., 1024Mi).")
 	command.MarkFlagRequired("image")
 }
 
@@ -55,5 +67,41 @@ func (p *ConfigurationEditFlags) Apply(config *servingv1alpha1.ConfigurationSpec
 	if err != nil {
 		return err
 	}
+	limitsResources, err := p.computeResources(p.LimitsFlags)
+	if err != nil {
+		return err
+	}
+	requestsResources, err := p.computeResources(p.RequestsFlags)
+	if err != nil {
+		return err
+	}
+	err = servinglib.UpdateResources(config, requestsResources, limitsResources)
+	if err != nil {
+		return err
+	}
 	return nil
 }
+
+func (p *ConfigurationEditFlags) computeResources(resourceFlags ResourceFlags) (corev1.ResourceList, error) {
+	resourceList := corev1.ResourceList{}
+
+	if resourceFlags.CPU != "" {
+		cpuQuantity, err := resource.ParseQuantity(resourceFlags.CPU)
+		if err != nil {
+			return corev1.ResourceList{}, err
+		}
+
+		resourceList[corev1.ResourceCPU] = cpuQuantity
+	}
+
+	if resourceFlags.Memory != "" {
+		memoryQuantity, err := resource.ParseQuantity(resourceFlags.Memory)
+		if err != nil {
+			return corev1.ResourceList{}, err
+		}
+
+		resourceList[corev1.ResourceMemory] = memoryQuantity
+	}
+
+	return resourceList, nil
+}

pkg/kn/commands/service_create.go

@@ -17,12 +17,10 @@ package commands
 import (
 	"errors"
 
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-
-	servingv1alpha1 "github.com/knative/serving/pkg/apis/serving/v1alpha1"
-
 	serving_lib "github.com/knative/client/pkg/serving"
+	servingv1alpha1 "github.com/knative/serving/pkg/apis/serving/v1alpha1"
 	"github.com/spf13/cobra"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func NewServiceCreateCommand(p *KnParams) *cobra.Command {

pkg/kn/commands/service_create_test.go

@@ -21,12 +21,13 @@ import (
 	"reflect"
 	"testing"
 
-	servinglib "github.com/knative/client/pkg/serving"
-
 	"github.com/knative/serving/pkg/apis/serving/v1alpha1"
-	serving "github.com/knative/serving/pkg/client/clientset/versioned/typed/serving/v1alpha1"
 	"github.com/knative/serving/pkg/client/clientset/versioned/typed/serving/v1alpha1/fake"
+	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/apimachinery/pkg/runtime"
+	servinglib "github.com/knative/client/pkg/serving"
+	serving "github.com/knative/serving/pkg/client/clientset/versioned/typed/serving/v1alpha1"
+	corev1 "k8s.io/api/core/v1"
 	client_testing "k8s.io/client-go/testing"
 )
 
@@ -110,5 +111,179 @@ func TestServiceCreateEnv(t *testing.T) {
 		expectedEnvVars) {
 		t.Fatalf("wrong env vars %v", conf.RevisionTemplate.Spec.Container.Env)
 	}
-
+}
+
+func TestServiceCreateWithRequests(t *testing.T) {
+	action, created, _, err := fakeServiceCreate([]string{
+		"service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "--requests-cpu", "250m", "--requests-memory", "64Mi"})
+
+	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"),
+	}
+
+	conf, err := servinglib.GetConfiguration(created)
+
+	if err != nil {
+		t.Fatal(err)
+	} else if !reflect.DeepEqual(
+		conf.RevisionTemplate.Spec.Container.Resources.Requests,
+		expectedRequestsVars) {
+		t.Fatalf("wrong requests vars %v", conf.RevisionTemplate.Spec.Container.Resources.Requests)
+	}
+}
+
+func TestServiceCreateWithLimits(t *testing.T) {
+	action, created, _, err := fakeServiceCreate([]string{
+		"service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "--limits-cpu", "1000m", "--limits-memory", "1024Mi"})
+
+	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"),
+	}
+
+	conf, err := servinglib.GetConfiguration(created)
+
+	if err != nil {
+		t.Fatal(err)
+	} else if !reflect.DeepEqual(
+		conf.RevisionTemplate.Spec.Container.Resources.Limits,
+		expectedLimitsVars) {
+		t.Fatalf("wrong limits vars %v", conf.RevisionTemplate.Spec.Container.Resources.Limits)
+	}
+}
+
+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"})
+
+	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"),
+	}
+
+	conf, err := servinglib.GetConfiguration(created)
+
+	if err != nil {
+		t.Fatal(err)
+	} else {
+		if !reflect.DeepEqual(
+			conf.RevisionTemplate.Spec.Container.Resources.Requests,
+			expectedRequestsVars) {
+			t.Fatalf("wrong requests vars %v", conf.RevisionTemplate.Spec.Container.Resources.Requests)
+		}
+
+		if !reflect.DeepEqual(
+			conf.RevisionTemplate.Spec.Container.Resources.Limits,
+			expectedLimitsVars) {
+			t.Fatalf("wrong limits vars %v", conf.RevisionTemplate.Spec.Container.Resources.Limits)
+		}
+	}
+}
+
+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"})
+
+	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"),
+	}
+
+	conf, err := servinglib.GetConfiguration(created)
+
+	if err != nil {
+		t.Fatal(err)
+	} else {
+		if !reflect.DeepEqual(
+			conf.RevisionTemplate.Spec.Container.Resources.Requests,
+			expectedRequestsVars) {
+			t.Fatalf("wrong requests vars %v", conf.RevisionTemplate.Spec.Container.Resources.Requests)
+		}
+
+		if !reflect.DeepEqual(
+			conf.RevisionTemplate.Spec.Container.Resources.Limits,
+			expectedLimitsVars) {
+			t.Fatalf("wrong limits vars %v", conf.RevisionTemplate.Spec.Container.Resources.Limits)
+		}
+	}
+}
+
+func TestServiceCreateRequestsLimitsCPUMemory(t *testing.T) {
+	action, created, _, err := fakeServiceCreate([]string{
+		"service", "create", "foo", "--image", "gcr.io/foo/bar:baz",
+		"--requests-cpu", "250m", "--limits-cpu", "1000m",
+		"--requests-memory", "64Mi", "--limits-memory", "1024Mi"})
+
+	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"),
+	}
+
+	expectedLimitsVars := corev1.ResourceList{
+		corev1.ResourceCPU:    parseQuantity(t, "1000m"),
+		corev1.ResourceMemory: parseQuantity(t, "1024Mi"),
+	}
+
+	conf, err := servinglib.GetConfiguration(created)
+
+	if err != nil {
+		t.Fatal(err)
+	} else {
+		if !reflect.DeepEqual(
+			conf.RevisionTemplate.Spec.Container.Resources.Requests,
+			expectedRequestsVars) {
+			t.Fatalf("wrong requests vars %v", conf.RevisionTemplate.Spec.Container.Resources.Requests)
+		}
+
+		if !reflect.DeepEqual(
+			conf.RevisionTemplate.Spec.Container.Resources.Limits,
+			expectedLimitsVars) {
+			t.Fatalf("wrong limits vars %v", conf.RevisionTemplate.Spec.Container.Resources.Limits)
+		}
+	}
+}
+
+func parseQuantity(t *testing.T, quantityString string) resource.Quantity {
+	quantity, err := resource.ParseQuantity(quantityString)
+	if err != nil {
+		t.Fatal(err)
+	}
+	return quantity
 }

pkg/serving/config_changes.go

@@ -17,9 +17,8 @@ package serving
 import (
 	"fmt"
 
-	corev1 "k8s.io/api/core/v1"
-
 	servingv1alpha1 "github.com/knative/serving/pkg/apis/serving/v1alpha1"
+	corev1 "k8s.io/api/core/v1"
 )
 
 // Give the configuration all the env var values listed in the given map of
@@ -67,3 +66,23 @@ func UpdateImage(config *servingv1alpha1.ConfigurationSpec, image string) error
 	config.RevisionTemplate.Spec.Container.Image = image
 	return nil
 }
+
+func UpdateResources(config *servingv1alpha1.ConfigurationSpec, requestsResourceList corev1.ResourceList, limitsResourceList corev1.ResourceList) error {
+	if config.RevisionTemplate.Spec.Container.Resources.Requests == nil {
+		config.RevisionTemplate.Spec.Container.Resources.Requests = corev1.ResourceList{}
+	}
+
+	for k, v := range requestsResourceList {
+		config.RevisionTemplate.Spec.Container.Resources.Requests[k] = v
+	}
+
+	if config.RevisionTemplate.Spec.Container.Resources.Limits == nil {
+		config.RevisionTemplate.Spec.Container.Resources.Limits = corev1.ResourceList{}
+	}
+
+	for k, v := range limitsResourceList {
+		config.RevisionTemplate.Spec.Container.Resources.Limits[k] = v
+	}
+
+	return nil
+}

vendor/modules.txt

@@ -208,6 +208,7 @@ k8s.io/api/storage/v1
 k8s.io/api/storage/v1alpha1
 k8s.io/api/storage/v1beta1
 # k8s.io/apimachinery v0.0.0-20190104073114-849b284f3b75
+k8s.io/apimachinery/pkg/api/resource
 k8s.io/apimachinery/pkg/apis/meta/v1
 k8s.io/apimachinery/pkg/runtime/schema
 k8s.io/apimachinery/pkg/api/equality
@@ -218,7 +219,6 @@ k8s.io/apimachinery/pkg/util/validation
 k8s.io/apimachinery/pkg/runtime/serializer
 k8s.io/apimachinery/pkg/types
 k8s.io/apimachinery/pkg/watch
-k8s.io/apimachinery/pkg/api/resource
 k8s.io/apimachinery/pkg/conversion
 k8s.io/apimachinery/pkg/fields
 k8s.io/apimachinery/pkg/labels