feat(service create): Added --no-wait and --wait-timeout

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

docs/cmd/kn_service_create.md

@@ -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

pkg/kn/commands/service/service_create.go

@@ -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",
@@ -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 := p.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
+}

pkg/kn/commands/service/service_create_test.go

@@ -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"
@@ -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 {
@@ -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") {
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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") {
@@ -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)

pkg/kn/commands/service/wait_args.go

@@ -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
+}