From 02526c74e93ea10d4d2c78127a314e2239d134c6 Mon Sep 17 00:00:00 2001 From: Navid Shaikh Date: Thu, 9 May 2019 19:10:14 +0530 Subject: [PATCH 1/4] Adds --force flag to service create command / service replace Fixes #13 --- pkg/kn/commands/configuration_edit_flags.go | 2 + pkg/kn/commands/service_create.go | 20 ++++++- pkg/kn/commands/service_create_test.go | 63 +++++++++++++++++++++ 3 files changed, 84 insertions(+), 1 deletion(-) diff --git a/pkg/kn/commands/configuration_edit_flags.go b/pkg/kn/commands/configuration_edit_flags.go index 816f273955..2cc4b7240b 100644 --- a/pkg/kn/commands/configuration_edit_flags.go +++ b/pkg/kn/commands/configuration_edit_flags.go @@ -29,6 +29,7 @@ type ConfigurationEditFlags struct { Image string Env []string RequestsFlags, LimitsFlags ResourceFlags + ForceCreate bool } type ResourceFlags struct { @@ -49,6 +50,7 @@ func (p *ConfigurationEditFlags) AddUpdateFlags(command *cobra.Command) { func (p *ConfigurationEditFlags) AddCreateFlags(command *cobra.Command) { p.AddUpdateFlags(command) + command.Flags().BoolVar(&p.ForceCreate, "force", false, "Create service forcefully, replaces existing service if any.") command.MarkFlagRequired("image") } diff --git a/pkg/kn/commands/service_create.go b/pkg/kn/commands/service_create.go index 6b40e997d3..8eb1256105 100644 --- a/pkg/kn/commands/service_create.go +++ b/pkg/kn/commands/service_create.go @@ -22,6 +22,7 @@ import ( servingv1alpha1 "github.com/knative/serving/pkg/apis/serving/v1alpha1" "github.com/spf13/cobra" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func NewServiceCreateCommand(p *KnParams) *cobra.Command { @@ -35,7 +36,18 @@ func NewServiceCreateCommand(p *KnParams) *cobra.Command { kn service create mysvc --image dev.local/ns/image:latest # Create a service with multiple environment variables - kn service create mysvc --env KEY1=VALUE1 --env KEY2=VALUE2 --image dev.local/ns/image:latest`, + kn service create mysvc --env KEY1=VALUE1 --env KEY2=VALUE2 --image dev.local/ns/image:latest + + # Replace a service 's1' with image dev.local/ns/image:v2 using --force flag + kn service create --force s1 --image dev.local/ns/image:v2 + + # Replace environment variables of service 's1' using --force flag + kn service create --force s1 --env KEY1=NEW_VALUE1 --env NEW_KEY2=NEW_VALUE2 --image dev.local/ns/image:v1 + + # Reset resources to default ones of a service 's1' using --force flag + # (earlier configured resource requests and limits will be replaced with default) + # (earlier configured environment variables will be cleared too if any) + kn service create --force s1 --image dev.local/ns/image:v1`, RunE: func(cmd *cobra.Command, args []string) (err error) { if len(args) != 1 { @@ -70,6 +82,12 @@ func NewServiceCreateCommand(p *KnParams) *cobra.Command { if err != nil { return err } + // check if --force flag is given + if force, err := cmd.Flags().GetBool("force"); err != nil { + return err + } else if force { + client.Services(namespace).Delete(args[0], &v1.DeleteOptions{}) + } _, err = client.Services(namespace).Create(&service) if err != nil { return err diff --git a/pkg/kn/commands/service_create_test.go b/pkg/kn/commands/service_create_test.go index 8ee41e2784..e8102cfef4 100644 --- a/pkg/kn/commands/service_create_test.go +++ b/pkg/kn/commands/service_create_test.go @@ -290,3 +290,66 @@ func parseQuantity(t *testing.T, quantityString string) resource.Quantity { } return quantity } + +func TestServiceCreateImageForce(t *testing.T) { + _, _, _, err := fakeServiceCreate([]string{ + "service", "create", "foo", "--image", "gcr.io/foo/bar:v1"}) + if err != nil { + t.Fatal(err) + } + action, created, output, err := fakeServiceCreate([]string{ + "service", "create", "foo", "--force", "--image", "gcr.io/foo/bar:v2"}) + if err != nil { + t.Fatal(err) + } else if !action.Matches("create", "services") { + t.Fatalf("Bad action %v", action) + } + conf, err := servinglib.GetConfiguration(created) + expectedOutput := "Service 'foo' is successfully created in namespace 'default'.\n" + if err != nil { + t.Fatal(err) + } else if conf.RevisionTemplate.Spec.Container.Image != "gcr.io/foo/bar:v2" { + t.Fatalf("wrong image set: %v", conf.RevisionTemplate.Spec.Container.Image) + } else if output != expectedOutput { + t.Fatalf("wrong output: %s, expected: %s", output, expectedOutput) + } +} + +func TestServiceCreateEnvForce(t *testing.T) { + _, _, _, err := fakeServiceCreate([]string{ + "service", "create", "foo", "--image", "gcr.io/foo/bar:v1", "-e", "A=DOGS", "--env", "B=WOLVES"}) + if err != nil { + t.Fatal(err) + } + action, created, output, err := fakeServiceCreate([]string{ + "service", "create", "foo", "--force", "--image", "gcr.io/foo/bar:v2", "-e", "A=CATS", "--env", "B=LIONS"}) + + if err != nil { + t.Fatal(err) + } else if !action.Matches("create", "services") { + t.Fatalf("Bad action %v", action) + } + + expectedEnvVars := map[string]string{ + "A": "CATS", + "B": "LIONS"} + + conf, err := servinglib.GetConfiguration(created) + actualEnvVars, err := servinglib.EnvToMap(conf.RevisionTemplate.Spec.Container.Env) + if err != nil { + t.Fatal(err) + } + + expectedOutput := "Service 'foo' is successfully created in namespace 'default'.\n" + if err != nil { + t.Fatal(err) + } else if conf.RevisionTemplate.Spec.Container.Image != "gcr.io/foo/bar:v2" { + t.Fatalf("wrong image set: %v", conf.RevisionTemplate.Spec.Container.Image) + } else if !reflect.DeepEqual( + actualEnvVars, + expectedEnvVars) { + t.Fatalf("wrong env vars:%v", conf.RevisionTemplate.Spec.Container.Env) + } else if output != expectedOutput { + t.Fatalf("wrong output: %s, expected: %s", output, expectedOutput) + } +} From bf3e686758f43ef01b7716189971a3ae82ea2603 Mon Sep 17 00:00:00 2001 From: Navid Shaikh Date: Fri, 10 May 2019 18:13:42 +0530 Subject: [PATCH 2/4] Keeps the resourceVersion of service with --force flag --- pkg/kn/commands/service_create.go | 37 ++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/pkg/kn/commands/service_create.go b/pkg/kn/commands/service_create.go index 8eb1256105..587215407e 100644 --- a/pkg/kn/commands/service_create.go +++ b/pkg/kn/commands/service_create.go @@ -38,13 +38,14 @@ func NewServiceCreateCommand(p *KnParams) *cobra.Command { # Create a service with multiple environment variables kn service create mysvc --env KEY1=VALUE1 --env KEY2=VALUE2 --image dev.local/ns/image:latest - # Replace a service 's1' with image dev.local/ns/image:v2 using --force flag + # Create or replace a service 's1' with image dev.local/ns/image:v2 using --force flag + # if service 's1' doesn't exist, it's just a normal create operation kn service create --force s1 --image dev.local/ns/image:v2 - # Replace environment variables of service 's1' using --force flag + # Create or replace environment variables of service 's1' using --force flag kn service create --force s1 --env KEY1=NEW_VALUE1 --env NEW_KEY2=NEW_VALUE2 --image dev.local/ns/image:v1 - # Reset resources to default ones of a service 's1' using --force flag + # Create or replace default resources of a service 's1' using --force flag # (earlier configured resource requests and limits will be replaced with default) # (earlier configured environment variables will be cleared too if any) kn service create --force s1 --image dev.local/ns/image:v1`, @@ -82,17 +83,31 @@ func NewServiceCreateCommand(p *KnParams) *cobra.Command { if err != nil { return err } + var serviceExists bool = false // check if --force flag is given - if force, err := cmd.Flags().GetBool("force"); err != nil { - return err - } else if force { - client.Services(namespace).Delete(args[0], &v1.DeleteOptions{}) + if editFlags.ForceCreate { + // --force flag is given, lets check if the service exists + existingService, err := client.Services(namespace).Get(args[0], v1.GetOptions{}) + if err == nil { + serviceExists = true + // copy over the resource version + service.ResourceVersion = existingService.ResourceVersion + // lets update the service + _, err = client.Services(namespace).Update(&service) + if err != nil { + return err + } + fmt.Fprintf(cmd.OutOrStdout(), "Service '%s' successfully replaced in namespace '%s'.\n", args[0], namespace) + } } - _, err = client.Services(namespace).Create(&service) - if err != nil { - return err + // if service doesn't exist, perform a normal create operation + if !serviceExists { + _, err = client.Services(namespace).Create(&service) + if err != nil { + return err + } + fmt.Fprintf(cmd.OutOrStdout(), "Service '%s' successfully created in namespace '%s'.\n", args[0], namespace) } - fmt.Fprintf(cmd.OutOrStdout(), "Service '%s' successfully created in namespace '%s'.\n", args[0], namespace) return nil }, } From 71ca7131265066b6d72614d6cf07a02aba112fa5 Mon Sep 17 00:00:00 2001 From: Navid Shaikh Date: Fri, 10 May 2019 18:24:15 +0530 Subject: [PATCH 3/4] Updates the tests --- pkg/kn/commands/service_create_test.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/pkg/kn/commands/service_create_test.go b/pkg/kn/commands/service_create_test.go index e8102cfef4..4cca9b59af 100644 --- a/pkg/kn/commands/service_create_test.go +++ b/pkg/kn/commands/service_create_test.go @@ -305,13 +305,12 @@ func TestServiceCreateImageForce(t *testing.T) { t.Fatalf("Bad action %v", action) } conf, err := servinglib.GetConfiguration(created) - expectedOutput := "Service 'foo' is successfully created in namespace 'default'.\n" if err != nil { t.Fatal(err) } else if conf.RevisionTemplate.Spec.Container.Image != "gcr.io/foo/bar:v2" { t.Fatalf("wrong image set: %v", conf.RevisionTemplate.Spec.Container.Image) - } else if output != expectedOutput { - t.Fatalf("wrong output: %s, expected: %s", output, expectedOutput) + } else if !strings.Contains(output, "foo") || !strings.Contains(output, "default") { + t.Fatalf("wrong output: %s", output) } } @@ -339,8 +338,6 @@ func TestServiceCreateEnvForce(t *testing.T) { if err != nil { t.Fatal(err) } - - expectedOutput := "Service 'foo' is successfully created in namespace 'default'.\n" if err != nil { t.Fatal(err) } else if conf.RevisionTemplate.Spec.Container.Image != "gcr.io/foo/bar:v2" { @@ -349,7 +346,7 @@ func TestServiceCreateEnvForce(t *testing.T) { actualEnvVars, expectedEnvVars) { t.Fatalf("wrong env vars:%v", conf.RevisionTemplate.Spec.Container.Env) - } else if output != expectedOutput { - t.Fatalf("wrong output: %s, expected: %s", output, expectedOutput) + } else if !strings.Contains(output, "foo") || !strings.Contains(output, "default") { + t.Fatalf("wrong output: %s", output) } } From d676c68f11b2464bf07698ac346ef6d68908077b Mon Sep 17 00:00:00 2001 From: Navid Shaikh Date: Mon, 13 May 2019 18:04:49 +0530 Subject: [PATCH 4/4] Removes unnecessary comments --- pkg/kn/commands/service_create.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pkg/kn/commands/service_create.go b/pkg/kn/commands/service_create.go index 587215407e..9159f07180 100644 --- a/pkg/kn/commands/service_create.go +++ b/pkg/kn/commands/service_create.go @@ -84,15 +84,11 @@ func NewServiceCreateCommand(p *KnParams) *cobra.Command { return err } var serviceExists bool = false - // check if --force flag is given if editFlags.ForceCreate { - // --force flag is given, lets check if the service exists existingService, err := client.Services(namespace).Get(args[0], v1.GetOptions{}) if err == nil { serviceExists = true - // copy over the resource version service.ResourceVersion = existingService.ResourceVersion - // lets update the service _, err = client.Services(namespace).Update(&service) if err != nil { return err @@ -100,7 +96,6 @@ func NewServiceCreateCommand(p *KnParams) *cobra.Command { fmt.Fprintf(cmd.OutOrStdout(), "Service '%s' successfully replaced in namespace '%s'.\n", args[0], namespace) } } - // if service doesn't exist, perform a normal create operation if !serviceExists { _, err = client.Services(namespace).Create(&service) if err != nil {