From 5dcc0f8641c79093255f9e55ec6d6bd98c6d61e2 Mon Sep 17 00:00:00 2001 From: "dr.max" Date: Wed, 1 May 2019 15:36:34 -0700 Subject: [PATCH] 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 | 6 +- pkg/kn/commands/service_create.go | 2 +- pkg/kn/commands/service_create_test.go | 6 +- pkg/kn/commands/service_update.go | 8 +- pkg/kn/commands/service_update_test.go | 156 +++++++++++++++++++- 5 files changed, 169 insertions(+), 9 deletions(-) diff --git a/pkg/kn/commands/configuration_edit_flags.go b/pkg/kn/commands/configuration_edit_flags.go index ab4835124..816f27395 100644 --- a/pkg/kn/commands/configuration_edit_flags.go +++ b/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") } diff --git a/pkg/kn/commands/service_create.go b/pkg/kn/commands/service_create.go index 1bf3acda1..b25e911a0 100644 --- a/pkg/kn/commands/service_create.go +++ b/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 } diff --git a/pkg/kn/commands/service_create_test.go b/pkg/kn/commands/service_create_test.go index 273e04e04..5a0fd0660 100644 --- a/pkg/kn/commands/service_create_test.go +++ b/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" ) diff --git a/pkg/kn/commands/service_update.go b/pkg/kn/commands/service_update.go index e302ac617..e818a211e 100644 --- a/pkg/kn/commands/service_update.go +++ b/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 } diff --git a/pkg/kn/commands/service_update_test.go b/pkg/kn/commands/service_update_test.go index 71c3dd9dd..d87353ba7 100644 --- a/pkg/kn/commands/service_update_test.go +++ b/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 +}