Skip to content

Commit

Permalink
feat(service create): Added --no-wait and --wait-timeout
Browse files Browse the repository at this point in the history 
By default, `kn service create` blocks until the service is either
created or an error occured during service creation.

With the option --no-wait the behaviour can be switched to an
async mode so that that kn returns immediately after the service is
created without waiting for a successful Ready status condition.

The timeout for how long to wait can be configured with --wait-timeout
If a timeout occur, that doesn't mean that the service is not created,
but the wait just returns. The default value is 60 seconds.

In wait mode, print out the service URL as a last line (so that it can be used together with `tail -1`) to extract the service URL after the service is created.

Fixes knative#54
  • Loading branch information
@rhuss
rhuss committed Jun 9, 2019
1 parent 1b471d5 commit 25571ed
Show file tree
Hide file tree
Showing 9 changed files with 602 additions and 55 deletions.
2 changes: 2 additions & 0 deletions docs/cmd/kn_service_create.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,10 @@ kn service create NAME --image IMAGE [flags]
--max-scale int Maximal number of replicas.
--min-scale int Minimal number of replicas.
-n, --namespace string List the requested object(s) in given namespace.
--no-wait Don't wait for service to become ready.
--requests-cpu string The requested CPU (e.g., 250m).
--requests-memory string The requested CPU (e.g., 64Mi).
--wait-timeout int Seconds to wait before stop checking for service to become ready (default: 60). (default 60)
```

### Options inherited from parent commands
Expand Down
141 changes: 105 additions & 36 deletions pkg/kn/commands/service/service_create.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,20 @@ package service
import (
"errors"
"fmt"

"github.com/knative/client/pkg/kn/commands"
servingv1alpha1 "github.com/knative/serving/pkg/apis/serving/v1alpha1"
serving_v1alpha1_api "github.com/knative/serving/pkg/apis/serving/v1alpha1"
serving_v1alpha1_client "github.com/knative/serving/pkg/client/clientset/versioned/typed/serving/v1alpha1"
"github.com/spf13/cobra"
"io"
corev1 "k8s.io/api/core/v1"
api_errors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command {
var editFlags ConfigurationEditFlags
var waitFlags commands.WaitFlags

serviceCreateCommand := &cobra.Command{
Use: "create NAME --image IMAGE",
Expand All @@ -53,66 +56,132 @@ func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command {

RunE: func(cmd *cobra.Command, args []string) (err error) {
if len(args) != 1 {
return errors.New("requires the service name.")
return errors.New("'service create' requires the service name given as single argument")
}
name := args[0]
if editFlags.Image == "" {
return errors.New("requires the image name to run.")
return errors.New("'service create' requires the image name to run provided with the --image option")
}

namespace, err := commands.GetNamespace(cmd)
if err != nil {
return err
}

service := servingv1alpha1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: args[0],
Namespace: namespace,
},
service, err := constructService(cmd, editFlags, name, namespace)
if err != nil {
return err
}

service.Spec.DeprecatedRunLatest = &servingv1alpha1.RunLatestType{
Configuration: servingv1alpha1.ConfigurationSpec{
DeprecatedRevisionTemplate: &servingv1alpha1.RevisionTemplateSpec{
Spec: servingv1alpha1.RevisionSpec{
DeprecatedContainer: &corev1.Container{},
},
},
},
client, err := p.ServingFactory()
if err != nil {
return err
}

err = editFlags.Apply(&service, cmd)
serviceExists, err := serviceExists(client, service.Name, namespace)
if err != nil {
return err
}
client, err := p.ServingFactory()

if editFlags.ForceCreate && serviceExists {
err = replaceService(client, service, namespace, cmd.OutOrStdout())
} else {
err = createService(client, service, namespace, cmd.OutOrStdout())
}
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 {

if waitFlags.IsWait(true) {
waitForReady := newServiceWaitForReady(client, namespace)
if err := waitForReady.Wait(name, waitFlags.Timeout, cmd.OutOrStdout()); err != nil {
return err
}
fmt.Fprintf(cmd.OutOrStdout(), "Service '%s' successfully created in namespace '%s'.\n", args[0], namespace)
return showUrl(client, name, namespace, cmd.OutOrStdout())
}

return nil
},
}
commands.AddNamespaceFlags(serviceCreateCommand.Flags(), false)
editFlags.AddCreateFlags(serviceCreateCommand)
waitFlags.AddWaitFlags(serviceCreateCommand, true, 60, "service")
return serviceCreateCommand
}

func createService(client serving_v1alpha1_client.ServingV1alpha1Interface, service *serving_v1alpha1_api.Service, namespace string, out io.Writer) error {
_, err := client.Services(namespace).Create(service)
if err != nil {
return err
}
fmt.Fprintf(out, "Service '%s' successfully created in namespace '%s'.\n", service.Name, namespace)
return nil
}

func replaceService(client serving_v1alpha1_client.ServingV1alpha1Interface, service *serving_v1alpha1_api.Service, namespace string, out io.Writer) error {
existingService, err := client.Services(namespace).Get(service.Name, v1.GetOptions{})
if err != nil {
return err
}
service.ResourceVersion = existingService.ResourceVersion
_, err = client.Services(namespace).Update(service)
if err != nil {
return err
}
fmt.Fprintf(out, "Service '%s' successfully replaced in namespace '%s'.\n", service.Name, namespace)
return nil
}

func serviceExists(client serving_v1alpha1_client.ServingV1alpha1Interface, name string, namespace string) (bool, error) {
_, err := client.Services(namespace).Get(name, v1.GetOptions{})
if api_errors.IsNotFound(err) {
return false, nil
}
if err != nil {
return false, err
}
return true, nil
}

// Create service struct from provided options
func constructService(cmd *cobra.Command, editFlags ConfigurationEditFlags, name string, namespace string) (*serving_v1alpha1_api.Service,
error) {

service := serving_v1alpha1_api.Service{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: namespace,
},
}

// TODO: Should it always be `runLatest` ?
service.Spec.DeprecatedRunLatest = &serving_v1alpha1_api.RunLatestType{
Configuration: serving_v1alpha1_api.ConfigurationSpec{
DeprecatedRevisionTemplate: &serving_v1alpha1_api.RevisionTemplateSpec{
Spec: serving_v1alpha1_api.RevisionSpec{
DeprecatedContainer: &corev1.Container{},
},
},
},
}

err := editFlags.Apply(&service, cmd)
if err != nil {
return nil, err
}
return &service, nil
}

func showUrl(client serving_v1alpha1_client.ServingV1alpha1Interface, serviceName string, namespace string, out io.Writer) error {
service, err := client.Services(namespace).Get(serviceName, v1.GetOptions{})
if err != nil {
return fmt.Errorf("cannot fetch service %s in namespace %s for extracting the URL", serviceName, namespace)
}
url := service.Status.URL.String()
if url == "" {
url = service.Status.DeprecatedDomain
}
fmt.Fprintln(out, "\nService URL:")
fmt.Fprintf(out, "%s\n", url)
return nil
}
39 changes: 24 additions & 15 deletions pkg/kn/commands/service/service_create_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ package service
import (
"errors"
"fmt"
api_errors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime/schema"
"reflect"
"strings"
"testing"
Expand All @@ -30,19 +32,26 @@ import (
client_testing "k8s.io/client-go/testing"
)

func fakeServiceCreate(args []string) (
func fakeServiceCreate(args []string, withExistingService bool) (
action client_testing.Action,
created *v1alpha1.Service,
output string,
err error) {
knParams := &commands.KnParams{}
cmd, fakeServing, buf := commands.CreateTestKnCommand(NewServiceCommand(knParams), knParams)
fakeServing.AddReactor("*", "*",
fakeServing.AddReactor("get", "services",
func(a client_testing.Action) (bool, runtime.Object, error) {
if withExistingService {
return true, &v1alpha1.Service{}, nil
}
return true, nil, api_errors.NewNotFound(schema.GroupResource{}, "")
})
fakeServing.AddReactor("create", "services",
func(a client_testing.Action) (bool, runtime.Object, error) {
createAction, ok := a.(client_testing.CreateAction)
action = createAction
if !ok {
return true, nil, fmt.Errorf("wrong kind of action %v", action)
return true, nil, fmt.Errorf("wrong kind of action %v", a)
}
created, ok = createAction.GetObject().(*v1alpha1.Service)
if !ok {
Expand All @@ -61,7 +70,7 @@ func fakeServiceCreate(args []string) (

func TestServiceCreateImage(t *testing.T) {
action, created, output, err := fakeServiceCreate([]string{
"service", "create", "foo", "--image", "gcr.io/foo/bar:baz"})
"service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "--no-wait"}, false)
if err != nil {
t.Fatal(err)
} else if !action.Matches("create", "services") {
Expand All @@ -80,7 +89,7 @@ func TestServiceCreateImage(t *testing.T) {

func TestServiceCreateEnv(t *testing.T) {
action, created, _, err := fakeServiceCreate([]string{
"service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "-e", "A=DOGS", "--env", "B=WOLVES"})
"service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "-e", "A=DOGS", "--env", "B=WOLVES", "--no-wait"}, false)

if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -111,7 +120,7 @@ func TestServiceCreateEnv(t *testing.T) {

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"})
"service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "--requests-cpu", "250m", "--requests-memory", "64Mi", "--no-wait"}, false)

if err != nil {
t.Fatal(err)
Expand All @@ -137,7 +146,7 @@ func TestServiceCreateWithRequests(t *testing.T) {

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"})
"service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "--limits-cpu", "1000m", "--limits-memory", "1024Mi", "--no-wait"}, false)

if err != nil {
t.Fatal(err)
Expand All @@ -163,7 +172,7 @@ func TestServiceCreateWithLimits(t *testing.T) {

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"})
"service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "--requests-cpu", "250m", "--limits-cpu", "1000m", "--no-wait"}, false)

if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -200,7 +209,7 @@ func TestServiceCreateRequestsLimitsCPU(t *testing.T) {

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"})
"service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "--requests-memory", "64Mi", "--limits-memory", "1024Mi", "--no-wait"}, false)

if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -238,7 +247,7 @@ func TestServiceCreateRequestsLimitsMemory(t *testing.T) {
func TestServiceCreateMaxMinScale(t *testing.T) {
action, created, _, err := fakeServiceCreate([]string{
"service", "create", "foo", "--image", "gcr.io/foo/bar:baz",
"--min-scale", "1", "--max-scale", "5", "--concurrency-target", "10", "--concurrency-limit", "100"})
"--min-scale", "1", "--max-scale", "5", "--concurrency-target", "10", "--concurrency-limit", "100", "--no-wait"}, false)

if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -275,7 +284,7 @@ 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"})
"--requests-memory", "64Mi", "--limits-memory", "1024Mi", "--no-wait"}, false)

if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -322,12 +331,12 @@ func parseQuantity(t *testing.T, quantityString string) resource.Quantity {

func TestServiceCreateImageForce(t *testing.T) {
_, _, _, err := fakeServiceCreate([]string{
"service", "create", "foo", "--image", "gcr.io/foo/bar:v1"})
"service", "create", "foo", "--image", "gcr.io/foo/bar:v1", "--no-wait"}, false)
if err != nil {
t.Fatal(err)
}
action, created, output, err := fakeServiceCreate([]string{
"service", "create", "foo", "--force", "--image", "gcr.io/foo/bar:v2"})
"service", "create", "foo", "--force", "--image", "gcr.io/foo/bar:v2", "--no-wait"}, false)
if err != nil {
t.Fatal(err)
} else if !action.Matches("create", "services") {
Expand All @@ -345,12 +354,12 @@ func TestServiceCreateImageForce(t *testing.T) {

func TestServiceCreateEnvForce(t *testing.T) {
_, _, _, err := fakeServiceCreate([]string{
"service", "create", "foo", "--image", "gcr.io/foo/bar:v1", "-e", "A=DOGS", "--env", "B=WOLVES"})
"service", "create", "foo", "--image", "gcr.io/foo/bar:v1", "-e", "A=DOGS", "--env", "B=WOLVES", "--no-wait"}, false)
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"})
"service", "create", "foo", "--force", "--image", "gcr.io/foo/bar:v2", "-e", "A=CATS", "--env", "B=LIONS", "--no-wait"}, false)

if err != nil {
t.Fatal(err)
Expand Down
28 changes: 28 additions & 0 deletions pkg/kn/commands/service/wait_args.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package service

import (
"fmt"
"github.com/knative/client/pkg/wait"
"github.com/knative/pkg/apis"
serving_v1alpha1_api "github.com/knative/serving/pkg/apis/serving/v1alpha1"
serving_v1alpha1_client "github.com/knative/serving/pkg/client/clientset/versioned/typed/serving/v1alpha1"
"k8s.io/apimachinery/pkg/runtime"
)

// Create wait arguments for a Knative service which can be used to wait for
// a create/update options to be finished
// Can be used by `service_create` and `service_update`, hence this extra file
func newServiceWaitForReady(client serving_v1alpha1_client.ServingV1alpha1Interface, namespace string) wait.WaitForReady {
return wait.NewWaitForReady(
"service",
client.Services(namespace).Watch,
serviceConditionExtractor)
}

func serviceConditionExtractor(obj runtime.Object) (apis.Conditions, error) {
service, ok := obj.(*serving_v1alpha1_api.Service)
if !ok {
return nil, fmt.Errorf("%v is not a service", obj)
}
return apis.Conditions(service.Status.Conditions), nil
}
Loading

0 comments on commit 25571ed

Please sign in to comment.