Skip to content

Adds --force flag to service create command (service replace) #79

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
knative-prow-robot merged 4 commits into from
May 15, 2019
Merged

Adds --force flag to service create command (service replace) #79

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
02526c7
Adds --force flag to service create command / service replace
navidshaikh May 9, 2019
bf3e686
Keeps the resourceVersion of service with --force flag
navidshaikh May 10, 2019
71ca713
Updates the tests
navidshaikh May 10, 2019
d676c68
Removes unnecessary comments
navidshaikh May 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pkg/kn/commands/configuration_edit_flags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ type ConfigurationEditFlags struct {
Image string
Env []string
RequestsFlags, LimitsFlags ResourceFlags
ForceCreate bool
Copy link
Copy Markdown

@cppforlife cppforlife May 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i wonder if at some point it would make sense to have

type ConfigurationCreateFlags struct {
  ConfigurationEditFlags
  ForceCreate bool
  ...
}

to better separate create from update at the compile time. wdyt?

Copy link
Copy Markdown
Contributor Author

@navidshaikh navidshaikh May 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1
@cppforlife : Do you want me to update the same PR? or log an issue and address is post v0.1 ?

Copy link
Copy Markdown

@cppforlife cppforlife May 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@navidshaikh up to you on the timeline. i think current implementation is fine.

Copy link
Copy Markdown
Contributor Author

@navidshaikh navidshaikh May 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logged the suggestion to be addressed later #110 , thanks @cppforlife

Also rebased the PR to resolve the conflicts.

}

type ResourceFlags struct {
Expand All @@ -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.")
Comment thread
navidshaikh marked this conversation as resolved.
command.MarkFlagRequired("image")
}

Expand Down
38 changes: 33 additions & 5 deletions pkg/kn/commands/service_create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -35,7 +36,19 @@ 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

# 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

# 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

# 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`,

RunE: func(cmd *cobra.Command, args []string) (err error) {
if len(args) != 1 {
Expand Down Expand Up @@ -70,11 +83,26 @@ func NewServiceCreateCommand(p *KnParams) *cobra.Command {
if err != nil {
return err
}
_, err = client.Services(namespace).Create(&service)
if err != nil {
return err
var serviceExists bool = false
if editFlags.ForceCreate {
existingService, err := client.Services(namespace).Get(args[0], v1.GetOptions{})
if err == nil {
serviceExists = true
service.ResourceVersion = existingService.ResourceVersion
_, 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)
}
}
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
},
}
Expand Down
60 changes: 60 additions & 0 deletions pkg/kn/commands/service_create_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,3 +290,63 @@ 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)
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 !strings.Contains(output, "foo") || !strings.Contains(output, "default") {
t.Fatalf("wrong output: %s", output)
}
}

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)
}
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 !strings.Contains(output, "foo") || !strings.Contains(output, "default") {
t.Fatalf("wrong output: %s", output)
}
}