added tests to `$kn service update ...` and removed required --image (#81)

* added tests for service update with variations of --requests/limits-cpu --requests/limits-memory
* made image optional on service update since that was not the case and image should be optional

pkg/kn/commands/configuration_edit_flags.go

@@ -36,7 +36,7 @@ type ResourceFlags struct {
 	Memory string
 }
 
-func (p *ConfigurationEditFlags) AddFlags(command *cobra.Command) {
+func (p *ConfigurationEditFlags) AddUpdateFlags(command *cobra.Command) {
 	command.Flags().StringVar(&p.Image, "image", "", "Image to run.")
 	command.Flags().StringArrayVarP(&p.Env, "env", "e", []string{},
 		"Environment variable to set. NAME=value; you may provide this flag "+
@@ -45,6 +45,10 @@ func (p *ConfigurationEditFlags) AddFlags(command *cobra.Command) {
 	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).")
+}
+
+func (p *ConfigurationEditFlags) AddCreateFlags(command *cobra.Command) {
+	p.AddUpdateFlags(command)
 	command.MarkFlagRequired("image")
 }
 

pkg/kn/commands/service_create.go

@@ -78,6 +78,6 @@ func NewServiceCreateCommand(p *KnParams) *cobra.Command {
 		},
 	}
 	AddNamespaceFlags(serviceCreateCommand.Flags(), false)
-	editFlags.AddFlags(serviceCreateCommand)
+	editFlags.AddCreateFlags(serviceCreateCommand)
 	return serviceCreateCommand
 }

pkg/kn/commands/service_create_test.go

@@ -21,13 +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"
+	corev1 "k8s.io/api/core/v1"
 	"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"
 )
 

pkg/kn/commands/service_update.go

@@ -28,6 +28,12 @@ func NewServiceUpdateCommand(p *KnParams) *cobra.Command {
 	serviceUpdateCommand := &cobra.Command{
 		Use:   "update NAME",
 		Short: "Update a service.",
+		Example: `
+  # Updates a service 'mysvc' with new environment variables
+  kn service update mysvc --env KEY1=VALUE1 --env KEY2=VALUE2
+
+  # Updates a service 'mysvc' with new requests and limits parameters
+  kn service update mysvc --requests-cpu 500m --limits-memory 1024Mi`,
 		RunE: func(cmd *cobra.Command, args []string) (err error) {
 			if len(args) != 1 {
 				return errors.New("requires the service name.")
@@ -66,6 +72,6 @@ func NewServiceUpdateCommand(p *KnParams) *cobra.Command {
 		},
 	}
 	AddNamespaceFlags(serviceUpdateCommand.Flags(), false)
-	editFlags.AddFlags(serviceUpdateCommand)
+	editFlags.AddUpdateFlags(serviceUpdateCommand)
 	return serviceUpdateCommand
 }

pkg/kn/commands/service_update_test.go

@@ -18,13 +18,15 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"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"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	client_testing "k8s.io/client-go/testing"
@@ -69,7 +71,6 @@ func fakeServiceUpdate(original *v1alpha1.Service, args []string) (
 }
 
 func TestServiceUpdateImage(t *testing.T) {
-
 	orig := &v1alpha1.Service{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "Service",
@@ -85,7 +86,6 @@ func TestServiceUpdateImage(t *testing.T) {
 	}
 
 	config, err := servinglib.GetConfiguration(orig)
-
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -107,3 +107,153 @@ func TestServiceUpdateImage(t *testing.T) {
 		t.Fatalf("wrong image set: %v", conf.RevisionTemplate.Spec.Container.Image)
 	}
 }
+
+func TestServiceUpdateRequestsLimitsCPU(t *testing.T) {
+	service := createMockServiceWithResources(t, "250", "64Mi", "1000m", "1024Mi")
+
+	action, updated, _, err := fakeServiceUpdate(service, []string{
+		"service", "update", "foo", "--requests-cpu", "500m", "--limits-cpu", "1000m"})
+	if err != nil {
+		t.Fatal(err)
+	} else if !action.Matches("update", "services") {
+		t.Fatalf("Bad action %v", action)
+	}
+
+	expectedRequestsVars := corev1.ResourceList{
+		corev1.ResourceCPU:    resource.MustParse("500m"),
+		corev1.ResourceMemory: resource.MustParse("64Mi"),
+	}
+	expectedLimitsVars := corev1.ResourceList{
+		corev1.ResourceCPU:    resource.MustParse("1000m"),
+		corev1.ResourceMemory: resource.MustParse("1024Mi"),
+	}
+
+	newConfig, err := servinglib.GetConfiguration(updated)
+	if err != nil {
+		t.Fatal(err)
+	} else {
+		if !reflect.DeepEqual(
+			newConfig.RevisionTemplate.Spec.Container.Resources.Requests,
+			expectedRequestsVars) {
+			t.Fatalf("wrong requests vars %v", newConfig.RevisionTemplate.Spec.Container.Resources.Requests)
+		}
+
+		if !reflect.DeepEqual(
+			newConfig.RevisionTemplate.Spec.Container.Resources.Limits,
+			expectedLimitsVars) {
+			t.Fatalf("wrong limits vars %v", newConfig.RevisionTemplate.Spec.Container.Resources.Limits)
+		}
+	}
+}
+
+func TestServiceUpdateRequestsLimitsMemory(t *testing.T) {
+	service := createMockServiceWithResources(t, "100m", "64Mi", "1000m", "1024Mi")
+
+	action, updated, _, err := fakeServiceUpdate(service, []string{
+		"service", "update", "foo", "--requests-memory", "128Mi", "--limits-memory", "2048Mi"})
+	if err != nil {
+		t.Fatal(err)
+	} else if !action.Matches("update", "services") {
+		t.Fatalf("Bad action %v", action)
+	}
+
+	expectedRequestsVars := corev1.ResourceList{
+		corev1.ResourceCPU:    resource.MustParse("100m"),
+		corev1.ResourceMemory: resource.MustParse("128Mi"),
+	}
+	expectedLimitsVars := corev1.ResourceList{
+		corev1.ResourceCPU:    resource.MustParse("1000m"),
+		corev1.ResourceMemory: resource.MustParse("2048Mi"),
+	}
+
+	newConfig, err := servinglib.GetConfiguration(updated)
+	if err != nil {
+		t.Fatal(err)
+	} else {
+		if !reflect.DeepEqual(
+			newConfig.RevisionTemplate.Spec.Container.Resources.Requests,
+			expectedRequestsVars) {
+			t.Fatalf("wrong requests vars %v", newConfig.RevisionTemplate.Spec.Container.Resources.Requests)
+		}
+
+		if !reflect.DeepEqual(
+			newConfig.RevisionTemplate.Spec.Container.Resources.Limits,
+			expectedLimitsVars) {
+			t.Fatalf("wrong limits vars %v", newConfig.RevisionTemplate.Spec.Container.Resources.Limits)
+		}
+	}
+}
+
+func TestServiceUpdateRequestsLimitsCPU_and_Memory(t *testing.T) {
+	service := createMockServiceWithResources(t, "250m", "64Mi", "1000m", "1024Mi")
+
+	action, updated, _, err := fakeServiceUpdate(service, []string{
+		"service", "update", "foo",
+		"--requests-cpu", "500m", "--limits-cpu", "2000m",
+		"--requests-memory", "128Mi", "--limits-memory", "2048Mi"})
+	if err != nil {
+		t.Fatal(err)
+	} else if !action.Matches("update", "services") {
+		t.Fatalf("Bad action %v", action)
+	}
+
+	expectedRequestsVars := corev1.ResourceList{
+		corev1.ResourceCPU:    resource.MustParse("500m"),
+		corev1.ResourceMemory: resource.MustParse("128Mi"),
+	}
+	expectedLimitsVars := corev1.ResourceList{
+		corev1.ResourceCPU:    resource.MustParse("2000m"),
+		corev1.ResourceMemory: resource.MustParse("2048Mi"),
+	}
+
+	newConfig, err := servinglib.GetConfiguration(updated)
+	if err != nil {
+		t.Fatal(err)
+	} else {
+		if !reflect.DeepEqual(
+			newConfig.RevisionTemplate.Spec.Container.Resources.Requests,
+			expectedRequestsVars) {
+			t.Fatalf("wrong requests vars %v", newConfig.RevisionTemplate.Spec.Container.Resources.Requests)
+		}
+
+		if !reflect.DeepEqual(
+			newConfig.RevisionTemplate.Spec.Container.Resources.Limits,
+			expectedLimitsVars) {
+			t.Fatalf("wrong limits vars %v", newConfig.RevisionTemplate.Spec.Container.Resources.Limits)
+		}
+	}
+}
+
+func createMockServiceWithResources(t *testing.T, requestCPU, requestMemory, limitsCPU, limitsMemory string) *v1alpha1.Service {
+	service := &v1alpha1.Service{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "Service",
+			APIVersion: "knative.dev/v1alpha1",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "foo",
+			Namespace: "default",
+		},
+		Spec: v1alpha1.ServiceSpec{
+			RunLatest: &v1alpha1.RunLatestType{},
+		},
+	}
+
+	config, err := servinglib.GetConfiguration(service)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	config.RevisionTemplate.Spec.Container.Resources = corev1.ResourceRequirements{
+		Requests: corev1.ResourceList{
+			corev1.ResourceCPU:    resource.MustParse(requestCPU),
+			corev1.ResourceMemory: resource.MustParse(requestMemory),
+		},
+		Limits: corev1.ResourceList{
+			corev1.ResourceCPU:    resource.MustParse(limitsCPU),
+			corev1.ResourceMemory: resource.MustParse(limitsMemory),
+		},
+	}
+
+	return service
+}