feat(service create): Wait for a service to be ready when its created (…

…#156)

* 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 #54

* chore(service create): Tolerate if obeservedGeneration has not been set yet during startup

* chore(service create): Refactored based on review comments

* Introduced an --async flag (replacing --wait and --no-wait)
* Added proper retry handling on the list watch
* Updated help message

* chore(service wait): Added a new test for sync behaviour

docs/cmd/kn_service_create.md

@@ -36,6 +36,7 @@ kn service create NAME --image IMAGE [flags]
 ### Options
 
 ```
+      --async                    Create service and don't wait for it to become ready.
       --concurrency-limit int    Hard Limit of concurrent requests to be processed by a single replica.
       --concurrency-target int   Recommendation for when to scale up based on the concurrent number of incoming request. Defaults to --concurrency-limit when given.
   -e, --env stringArray          Environment variable to set. NAME=value; you may provide this flag any number of times to set multiple environment variables.
@@ -49,6 +50,7 @@ kn service create NAME --image IMAGE [flags]
   -n, --namespace string         List the requested object(s) in given namespace.
       --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 giving up on waiting for service to be ready (default: 60). (default 60)
 ```
 
 ### Options inherited from parent commands

pkg/kn/commands/service/service_create.go

@@ -17,17 +17,22 @@ 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"
+	"time"
+
 	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 +58,133 @@ 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 !waitFlags.Async {
+				waitForReady := newServiceWaitForReady(client, namespace)
+				err := waitForReady.Wait(name, time.Duration(waitFlags.TimeoutInSeconds)*time.Second, cmd.OutOrStdout())
 				if 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.AddConditionWaitFlags(serviceCreateCommand, 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: %v", serviceName, namespace, err)
+	}
+	url := service.Status.URL.String()
+	if url == "" {
+		url = service.Status.DeprecatedDomain
+	}
+	fmt.Fprintln(out, "\nService URL:")
+	fmt.Fprintf(out, "%s\n", url)
+	return nil
+}