feat(wait): add wait for delete operation

docs/cmd/kn_service_delete.md

@@ -24,8 +24,11 @@ kn service delete NAME [flags]
 ### Options
 
 ```
+      --async              DEPRECATED: please use --no-wait instead. Delete service and don't wait for it to become ready.
   -h, --help               help for delete
   -n, --namespace string   Specify the namespace to operate in.
+      --no-wait            Delete service and don't wait for it to become ready.
+      --wait-timeout int   Seconds to wait before giving up on waiting for service to be ready. (default 600)
 ```
 
 ### Options inherited from parent commands

pkg/kn/commands/service/delete.go

@@ -17,14 +17,18 @@ package service
 import (
 	"errors"
 	"fmt"
+	"time"
 
 	"github.com/spf13/cobra"
+	"k8s.io/apimachinery/pkg/watch"
 
 	"knative.dev/client/pkg/kn/commands"
 )
 
 // NewServiceDeleteCommand represent 'service delete' command
 func NewServiceDeleteCommand(p *commands.KnParams) *cobra.Command {
+	var waitFlags commands.WaitFlags
+
 	serviceDeleteCommand := &cobra.Command{
 		Use:   "delete NAME",
 		Short: "Delete a service.",
@@ -50,7 +54,19 @@ func NewServiceDeleteCommand(p *commands.KnParams) *cobra.Command {
 			}
 
 			for _, name := range args {
+				waitC := make(chan error)
+				defer close(waitC)
+				if !waitFlags.NoWait {
+					go func(s string, c chan error) {
+						err := client.WaitForEvent("service", s, time.Duration(waitFlags.TimeoutInSeconds)*time.Second,
+							func(evt *watch.Event) bool { return evt.Type == watch.Deleted })
+						c <- err
+					}(name, waitC)
+				}
 				err = client.DeleteService(name)
+				if err == nil && !waitFlags.NoWait {
+					err = <-waitC
+				}
 				if err != nil {
 					fmt.Fprintf(cmd.OutOrStdout(), "%s.\n", err)
 				} else {
@@ -61,5 +77,6 @@ func NewServiceDeleteCommand(p *commands.KnParams) *cobra.Command {
 		},
 	}
 	commands.AddNamespaceFlags(serviceDeleteCommand.Flags(), false)
+	waitFlags.AddConditionWaitFlags(serviceDeleteCommand, commands.WaitDefaultTimeout, "Delete", "service")
 	return serviceDeleteCommand
 }

pkg/kn/commands/service/delete_mock_test.go

@@ -21,6 +21,7 @@ import (
 
 	clientservingv1 "knative.dev/client/pkg/serving/v1"
 	"knative.dev/client/pkg/util"
+	"knative.dev/client/pkg/util/mock"
 )
 
 func TestServiceDeleteMock(t *testing.T) {
@@ -31,6 +32,8 @@ func TestServiceDeleteMock(t *testing.T) {
 	r := client.Recorder()
 
 	r.DeleteService("foo", nil)
+	// Wait for delete event
+	r.WaitForEvent("service", "foo", mock.Any(), mock.Any(), nil)
 
 	output, err := executeServiceCommand(client, "delete", "foo")
 	assert.NilError(t, err)
@@ -46,6 +49,10 @@ func TestMultipleServiceDeleteMock(t *testing.T) {
 
 	// Recording:
 	r := client.Recorder()
+	// Wait for delete event
+	r.WaitForEvent("service", "foo", mock.Any(), mock.Any(), nil)
+	r.WaitForEvent("service", "bar", mock.Any(), mock.Any(), nil)
+	r.WaitForEvent("service", "baz", mock.Any(), mock.Any(), nil)
 
 	r.DeleteService("foo", nil)
 	r.DeleteService("bar", nil)

pkg/kn/commands/service/delete_test.go

@@ -17,12 +17,17 @@ package service
 import (
 	"testing"
 
+	"github.com/pkg/errors"
 	"gotest.tools/assert"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/watch"
 	client_testing "k8s.io/client-go/testing"
+	clienttesting "k8s.io/client-go/testing"
 
 	"knative.dev/client/pkg/kn/commands"
 	"knative.dev/client/pkg/util"
+	"knative.dev/client/pkg/wait"
 )
 
 func fakeServiceDelete(args []string) (action client_testing.Action, name string, output string, err error) {
@@ -35,6 +40,17 @@ func fakeServiceDelete(args []string) (action client_testing.Action, name string
 			name = deleteAction.GetName()
 			return true, nil, nil
 		})
+	fakeServing.AddWatchReactor("services",
+		func(a client_testing.Action) (bool, watch.Interface, error) {
+			watchAction := a.(clienttesting.WatchAction)
+			_, found := watchAction.GetWatchRestrictions().Fields.RequiresExactMatch("metadata.name")
+			if !found {
+				return true, nil, errors.New("no field selector on metadata.name found")
+			}
+			w := wait.NewFakeWatch(getServiceDeleteEvents("test-service"))
+			w.Start()
+			return true, w, nil
+		})
 	cmd.SetArgs(args)
 	err = cmd.Execute()
 	if err != nil {
@@ -79,3 +95,11 @@ func TestMultipleServiceDelete(t *testing.T) {
 	assert.Check(t, util.ContainsAll(output, "Service", sevName2, "deleted", "namespace", commands.FakeNamespace))
 	assert.Check(t, util.ContainsAll(output, "Service", sevName3, "deleted", "namespace", commands.FakeNamespace))
 }
+
+func getServiceDeleteEvents(name string) []watch.Event {
+	return []watch.Event{
+		{watch.Added, wait.CreateTestServiceWithConditions(name, corev1.ConditionUnknown, corev1.ConditionUnknown, "", "msg1")},
+		{watch.Modified, wait.CreateTestServiceWithConditions(name, corev1.ConditionUnknown, corev1.ConditionTrue, "", "msg2")},
+		{watch.Deleted, wait.CreateTestServiceWithConditions(name, corev1.ConditionTrue, corev1.ConditionTrue, "", "")},
+	}
+}

pkg/serving/v1/client.go

@@ -75,6 +75,8 @@ type KnServingClient interface {
 	// Return error and how long has been waited
 	WaitForService(name string, timeout time.Duration, msgCallback wait.MessageCallback) (error, time.Duration)
 
+	WaitForEvent(kind, name string, timeout time.Duration, done wait.EventDone) error
+
 	// Get a configuration by name
 	GetConfiguration(name string) (*servingv1.Configuration, error)
 
@@ -271,6 +273,12 @@ func (cl *knServingClient) WaitForService(name string, timeout time.Duration, ms
 	return waitForReady.Wait(name, timeout, msgCallback)
 }
 
+func (cl *knServingClient) WaitForEvent(kind, name string, timeout time.Duration, done wait.EventDone) error {
+	waitForEvent := wait.NewWaitForEvent(kind, cl.WatchService, done)
+	err, _ := waitForEvent.Wait(name, timeout, wait.NoopMessageCallback())
+	return err
+}
+
 // Get the configuration for a service
 func (cl *knServingClient) GetConfiguration(name string) (*servingv1.Configuration, error) {
 	configuration, err := cl.client.Configurations(cl.namespace).Get(name, v1.GetOptions{})

pkg/serving/v1/client_mock.go

@@ -125,6 +125,16 @@ func (c *MockKnServingClient) WaitForService(name string, timeout time.Duration,
 	return mock.ErrorOrNil(call.Result[0]), call.Result[1].(time.Duration)
 }
 
+// Wait for a service to become ready, but not longer than provided timeout
+func (sr *ServingRecorder) WaitForEvent(kind, name interface{}, timeout interface{}, done interface{}, err error) {
+	sr.r.Add("WaitForEvent", []interface{}{kind, name, timeout, done}, []interface{}{err})
+}
+
+func (c *MockKnServingClient) WaitForEvent(kind, name string, timeout time.Duration, done wait.EventDone) error {
+	call := c.recorder.r.VerifyCall("WaitForEvent", kind, name, timeout, done)
+	return mock.ErrorOrNil(call.Result[0])
+}
+
 // Get a revision by name
 func (sr *ServingRecorder) GetRevision(name interface{}, revision *servingv1.Revision, err error) {
 	sr.r.Add("GetRevision", []interface{}{name}, []interface{}{revision, err})

pkg/wait/wait_for_ready.go

@@ -36,9 +36,17 @@ type waitForReadyConfig struct {
 	kind                string
 }
 
+type waitForEvent struct {
+	watchMaker WatchMaker
+	eventDone  EventDone
+	kind       string
+}
+
+type EventDone func(ev *watch.Event) bool
+
 // Interface used for waiting of a resource of a given name to reach a definitive
 // state in its "Ready" condition.
-type WaitForReady interface {
+type Wait interface {
 
 	// Wait on resource the resource with this name until a given timeout
 	// and write event messages for unknown event to the status writer.
@@ -56,14 +64,22 @@ type ConditionsExtractor func(obj runtime.Object) (apis.Conditions, error)
 type MessageCallback func(durationSinceState time.Duration, message string)
 
 // Constructor with resource type specific configuration
-func NewWaitForReady(kind string, watchMaker WatchMaker, extractor ConditionsExtractor) WaitForReady {
+func NewWaitForReady(kind string, watchMaker WatchMaker, extractor ConditionsExtractor) Wait {
 	return &waitForReadyConfig{
 		kind:                kind,
 		watchMaker:          watchMaker,
 		conditionsExtractor: extractor,
 	}
 }
 
+func NewWaitForEvent(kind string, watchMaker WatchMaker, eventDone EventDone) Wait {
+	return &waitForEvent{
+		kind:       kind,
+		watchMaker: watchMaker,
+		eventDone:  eventDone,
+	}
+}
+
 // A simple message callback which prints out messages line by line
 func SimpleMessageCallback(out io.Writer) MessageCallback {
 	oldMessage := ""
@@ -178,6 +194,29 @@ func (w *waitForReadyConfig) waitForReadyCondition(start time.Time, name string,
 	}
 }
 
+func (w *waitForEvent) Wait(name string, timeout time.Duration, msgCallback MessageCallback) (error, time.Duration) {
+	watcher, err := w.watchMaker(name, timeout)
+	if err != nil {
+		return err, 0
+	}
+	defer watcher.Stop()
+	start := time.Now()
+	// channel used to transport the error
+	errChan := make(chan error)
+	for {
+		select {
+		case <-time.After(timeout):
+			return nil, time.Since(start)
+		case err = <-errChan:
+			return err, time.Since(start)
+		case event := <-watcher.ResultChan():
+			if w.eventDone(&event) {
+				return nil, time.Since(start)
+			}
+		}
+	}
+}
+
 // Going over Unstructured to keep that function generally applicable.
 // Alternative implemenentation: Add a func-field to waitForReadyConfig which has to be
 // provided for every resource (like the conditions extractor)