Repository: knative/client
Pull request #1321 (2021-05-18 11:49:20)

Revert "Refactor waiting loop to use resourceVersion
## Description

Per subject revert PR#1301

/assign @rhuss 


knative-prow-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please ask for approval from **dsimansk** after the PR has been reviewed.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fclient).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/knative/client/blob/main/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["dsimansk"]} -->
knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-client-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[pkg/eventing/v1/client.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_client/1321/pull-knative-client-go-coverage/1394621137354231808/artifacts/line-cov.html#file0) | 86.0% | 85.8% | -0.1
[pkg/serving/v1/client.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_client/1321/pull-knative-client-go-coverage/1394621137354231808/artifacts/line-cov.html#file1) | 69.0% | 69.3% | 0.3
[pkg/wait/wait_for_ready.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_client/1321/pull-knative-client-go-coverage/1394621137354231808/artifacts/line-cov.html#file3) | 81.0% | 76.1% | -4.9

codecov[bot]: # [Codecov](https://codecov.io/gh/knative/client/pull/1321?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) Report
> Merging [#1321](https://codecov.io/gh/knative/client/pull/1321?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) (14d7cbc) into [main](https://codecov.io/gh/knative/client/commit/be8f246b79c5ebb626fb703203b82917f72fcd2f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) (be8f246) will **decrease** coverage by `0.06%`.
> The diff coverage is `52.94%`.

[![Impacted file tree graph](https://codecov.io/gh/knative/client/pull/1321/graphs/tree.svg?width=650&height=150&src=pr&token=bkTp6BzJiV&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative)](https://codecov.io/gh/knative/client/pull/1321?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative)

```diff
@@            Coverage Diff             @@
##             main    #1321      +/-   ##
==========================================
- Coverage   73.04%   72.97%   -0.07%     
==========================================
  Files         160      160              
  Lines        8161     8151      -10     
==========================================
- Hits         5961     5948      -13     
- Misses       1473     1476       +3     
  Partials      727      727              
```


| [Impacted Files](https://codecov.io/gh/knative/client/pull/1321?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) | Coverage Δ | |
|---|---|---|
| [pkg/wait/poll\_watcher.go](https://codecov.io/gh/knative/client/pull/1321/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative#diff-cGtnL3dhaXQvcG9sbF93YXRjaGVyLmdv) | `55.88% <0.00%> (+0.80%)` | :arrow_up: |
| [pkg/serving/v1/client.go](https://codecov.io/gh/knative/client/pull/1321/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative#diff-cGtnL3NlcnZpbmcvdjEvY2xpZW50Lmdv) | `57.60% <47.36%> (+1.32%)` | :arrow_up: |
| [pkg/eventing/v1/client.go](https://codecov.io/gh/knative/client/pull/1321/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative#diff-cGtnL2V2ZW50aW5nL3YxL2NsaWVudC5nbw==) | `79.16% <71.42%> (-0.18%)` | :arrow_down: |
| [pkg/wait/wait\_for\_ready.go](https://codecov.io/gh/knative/client/pull/1321/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative#diff-cGtnL3dhaXQvd2FpdF9mb3JfcmVhZHkuZ28=) | `66.34% <80.00%> (-7.34%)` | :arrow_down: |
| [pkg/wait/test\_wait\_helper.go](https://codecov.io/gh/knative/client/pull/1321/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative#diff-cGtnL3dhaXQvdGVzdF93YWl0X2hlbHBlci5nbw==) | `95.00% <0.00%> (-5.00%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/knative/client/pull/1321?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/knative/client/pull/1321?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative). Last update [be8f246...14d7cbc](https://codecov.io/gh/knative/client/pull/1321?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative).

rhuss: /lgtm
/approve
knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/client/pull/1321#issuecomment-843108179" title="Approved">rhuss</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fclient).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/knative/client/blob/main/OWNERS)~~ [rhuss]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc
index 4035bd5b..9ef510fc 100644
--- a/CHANGELOG.adoc
+++ b/CHANGELOG.adoc
@@ -29,10 +29,6 @@
 | Fix for serviceaccounts "default" not found flaky issue
 | https://github.com/knative/client/pull/1306[#1306]
 
-| ✨
-| Refactor waiting loop to use resourceVersion
-| https://github.com/knative/client/pull/1301[#1301]
-
 | ✨
 | Update Eventing Sources `APIServerSource, ContainerSource, SinkBinding` API to v1
 | https://github.com/knative/client/pull/1299[#1299]
diff --git a/pkg/eventing/v1/client.go b/pkg/eventing/v1/client.go
index 6401346f..deb908fd 100644
--- a/pkg/eventing/v1/client.go
+++ b/pkg/eventing/v1/client.go
@@ -16,7 +16,6 @@ package v1
 
 import (
 	"context"
-	"fmt"
 	"time"
 
 	apis_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -242,27 +241,25 @@ func (c *knEventingClient) GetBroker(ctx context.Context, name string) (*eventin
 }
 
 // WatchBroker is used to create watcher object
-func (c *knEventingClient) WatchBroker(ctx context.Context, name string, initialVersion string, timeout time.Duration) (watch.Interface, error) {
-	return wait.NewWatcherWithVersion(ctx, c.client.Brokers(c.namespace).Watch, c.client.RESTClient(), c.namespace, "brokers", name, initialVersion, timeout)
+func (c *knEventingClient) WatchBroker(ctx context.Context, name string, timeout time.Duration) (watch.Interface, error) {
+	return wait.NewWatcher(ctx, c.client.Brokers(c.namespace).Watch, c.client.RESTClient(), c.namespace, "brokers", name, timeout)
 }
 
 // DeleteBroker is used to delete an instance of broker and wait for completion until given timeout
 // For `timeout == 0` delete is performed async without any wait
 func (c *knEventingClient) DeleteBroker(ctx context.Context, name string, timeout time.Duration) error {
-	broker, err := c.GetBroker(ctx, name)
-	if err != nil {
-		return err
-	}
-	if broker.GetDeletionTimestamp() != nil {
-		return fmt.Errorf("can't delete broker '%s' because it has been already marked for deletion", name)
-	}
 	if timeout == 0 {
 		return c.deleteBroker(ctx, name, apis_v1.DeletePropagationBackground)
 	}
 	waitC := make(chan error)
+	watcher, err := c.WatchBroker(ctx, name, timeout)
+	if err != nil {
+		return nil
+	}
+	defer watcher.Stop()
 	go func() {
-		waitForEvent := wait.NewWaitForEvent("broker", c.WatchBroker, func(evt *watch.Event) bool { return evt.Type == watch.Deleted })
-		err, _ := waitForEvent.Wait(ctx, name, broker.ResourceVersion, wait.Options{Timeout: &timeout}, wait.NoopMessageCallback())
+		waitForEvent := wait.NewWaitForEvent("broker", func(evt *watch.Event) bool { return evt.Type == watch.Deleted })
+		err, _ := waitForEvent.Wait(ctx, watcher, name, wait.Options{Timeout: &timeout}, wait.NoopMessageCallback())
 		waitC <- err
 	}()
 	err = c.deleteBroker(ctx, name, apis_v1.DeletePropagationForeground)
diff --git a/pkg/serving/v1/client.go b/pkg/serving/v1/client.go
index caf303e7..a0310a6b 100644
--- a/pkg/serving/v1/client.go
+++ b/pkg/serving/v1/client.go
@@ -197,12 +197,12 @@ func (cl *knServingClient) GetService(ctx context.Context, name string) (*servin
 	return service, nil
 }
 
-func (cl *knServingClient) WatchServiceWithVersion(ctx context.Context, name string, initialVersion string, timeout time.Duration) (watch.Interface, error) {
-	return wait.NewWatcherWithVersion(ctx, cl.client.Services(cl.namespace).Watch, cl.client.RESTClient(), cl.namespace, "services", name, initialVersion, timeout)
+func (cl *knServingClient) WatchService(ctx context.Context, name string, timeout time.Duration) (watch.Interface, error) {
+	return wait.NewWatcher(ctx, cl.client.Services(cl.namespace).Watch, cl.client.RESTClient(), cl.namespace, "services", name, timeout)
 }
 
-func (cl *knServingClient) WatchRevisionWithVersion(ctx context.Context, name string, initialVersion string, timeout time.Duration) (watch.Interface, error) {
-	return wait.NewWatcherWithVersion(ctx, cl.client.Revisions(cl.namespace).Watch, cl.client.RESTClient(), cl.namespace, "revision", name, initialVersion, timeout)
+func (cl *knServingClient) WatchRevision(ctx context.Context, name string, timeout time.Duration) (watch.Interface, error) {
+	return wait.NewWatcher(ctx, cl.client.Revisions(cl.namespace).Watch, cl.client.RESTClient(), cl.namespace, "revision", name, timeout)
 }
 
 // List services
@@ -314,28 +314,24 @@ func (cl *knServingClient) ApplyService(ctx context.Context, modifiedService *se
 // Param `timeout` represents a duration to wait for a delete op to finish.
 // For `timeout == 0` delete is performed async without any wait.
 func (cl *knServingClient) DeleteService(ctx context.Context, serviceName string, timeout time.Duration) error {
-	service, err := cl.GetService(ctx, serviceName)
-	if err != nil {
-		return err
-	}
-	if service.GetDeletionTimestamp() != nil {
-		return fmt.Errorf("can't delete service '%s' because it has been already marked for deletion", serviceName)
-	}
 	if timeout == 0 {
 		return cl.deleteService(ctx, serviceName, v1.DeletePropagationBackground)
 	}
-
 	waitC := make(chan error)
+	watcher, err := cl.WatchService(ctx, serviceName, timeout)
+	if err != nil {
+		return nil
+	}
+	defer watcher.Stop()
 	go func() {
-		waitForEvent := wait.NewWaitForEvent("service", cl.WatchServiceWithVersion, func(evt *watch.Event) bool { return evt.Type == watch.Deleted })
-		err, _ := waitForEvent.Wait(ctx, serviceName, service.ResourceVersion, wait.Options{Timeout: &timeout}, wait.NoopMessageCallback())
+		waitForEvent := wait.NewWaitForEvent("service", func(evt *watch.Event) bool { return evt.Type == watch.Deleted })
+		err, _ := waitForEvent.Wait(ctx, watcher, serviceName, wait.Options{Timeout: &timeout}, wait.NoopMessageCallback())
 		waitC <- err
 	}()
 	err = cl.deleteService(ctx, serviceName, v1.DeletePropagationForeground)
 	if err != nil {
 		return err
 	}
-
 	return <-waitC
 }
 
@@ -354,16 +350,13 @@ func (cl *knServingClient) deleteService(ctx context.Context, serviceName string
 
 // Wait for a service to become ready, but not longer than provided timeout
 func (cl *knServingClient) WaitForService(ctx context.Context, name string, timeout time.Duration, msgCallback wait.MessageCallback) (error, time.Duration) {
-	waitForReady := wait.NewWaitForReady("service", cl.WatchServiceWithVersion, serviceConditionExtractor)
-
-	service, err := cl.GetService(ctx, name)
+	watcher, err := cl.WatchService(ctx, name, timeout)
 	if err != nil {
-		if apierrors.IsNotFound(err) {
-			return waitForReady.Wait(ctx, name, "", wait.Options{Timeout: &timeout}, msgCallback)
-		}
-		return err, 0
+		return err, timeout
 	}
-	return waitForReady.Wait(ctx, name, service.ResourceVersion, wait.Options{Timeout: &timeout}, msgCallback)
+	defer watcher.Stop()
+	waitForReady := wait.NewWaitForReady("service", serviceConditionExtractor)
+	return waitForReady.Wait(ctx, watcher, name, wait.Options{Timeout: &timeout}, msgCallback)
 }
 
 // Get the configuration for a service
@@ -476,9 +469,14 @@ func (cl *knServingClient) DeleteRevision(ctx context.Context, name string, time
 		return cl.deleteRevision(ctx, name)
 	}
 	waitC := make(chan error)
+	watcher, err := cl.WatchRevision(ctx, name, timeout)
+	if err != nil {
+		return err
+	}
+	defer watcher.Stop()
 	go func() {
-		waitForEvent := wait.NewWaitForEvent("revision", cl.WatchRevisionWithVersion, func(evt *watch.Event) bool { return evt.Type == watch.Deleted })
-		err, _ := waitForEvent.Wait(ctx, name, revision.ResourceVersion, wait.Options{Timeout: &timeout}, wait.NoopMessageCallback())
+		waitForEvent := wait.NewWaitForEvent("revision", func(evt *watch.Event) bool { return evt.Type == watch.Deleted })
+		err, _ := waitForEvent.Wait(ctx, watcher, name, wait.Options{Timeout: &timeout}, wait.NoopMessageCallback())
 		waitC <- err
 	}()
 	err = cl.deleteRevision(ctx, name)
diff --git a/pkg/wait/poll_watcher.go b/pkg/wait/poll_watcher.go
index c88950b8..86189c21 100644
--- a/pkg/wait/poll_watcher.go
+++ b/pkg/wait/poll_watcher.go
@@ -69,16 +69,16 @@ func newTickerPollInterval(d time.Duration) *tickerPollInterval {
 	return &tickerPollInterval{time.NewTicker(d)}
 }
 
-// NewWatcherWithVersion makes a watch.Interface on the given resource in the client,
+// NewWatcher makes a watch.Interface on the given resource in the client,
 // falling back to polling if the server does not support Watch.
-func NewWatcherWithVersion(ctx context.Context, watchFunc watchF, c rest.Interface, ns string, resource string, name string, initialResourceVersion string, timeout time.Duration) (watch.Interface, error) {
-	native, err := nativeWatchWithVersion(ctx, watchFunc, name, initialResourceVersion, timeout)
+func NewWatcher(ctx context.Context, watchFunc watchF, c rest.Interface, ns string, resource string, name string, timeout time.Duration) (watch.Interface, error) {
+	native, err := nativeWatch(ctx, watchFunc, name, timeout)
 	if err == nil {
 		return native, nil
 	}
 	polling := &pollingWatcher{
 		c, ns, resource, name, timeout, make(chan bool), make(chan watch.Event), &sync.WaitGroup{},
-		newTickerPollInterval(time.Second), nativePoll(ctx, c, ns, resource, name)}
+		newTickerPollInterval(pollInterval), nativePoll(ctx, c, ns, resource, name)}
 	polling.start()
 	return polling, nil
 }
@@ -161,10 +161,9 @@ func (w *pollingWatcher) Stop() {
 	close(w.done)
 }
 
-func nativeWatchWithVersion(ctx context.Context, watchFunc watchF, name string, initialVersion string, timeout time.Duration) (watch.Interface, error) {
+func nativeWatch(ctx context.Context, watchFunc watchF, name string, timeout time.Duration) (watch.Interface, error) {
 	opts := v1.ListOptions{
-		ResourceVersion: initialVersion,
-		FieldSelector:   fields.OneTermEqualSelector("metadata.name", name).String(),
+		FieldSelector: fields.OneTermEqualSelector("metadata.name", name).String(),
 	}
 	opts.Watch = true
 	addWatchTimeout(&opts, timeout)
diff --git a/pkg/wait/wait_for_ready.go b/pkg/wait/wait_for_ready.go
index edee29f1..ba6a27c1 100644
--- a/pkg/wait/wait_for_ready.go
+++ b/pkg/wait/wait_for_ready.go
@@ -28,16 +28,14 @@ import (
 
 // Callbacks and configuration used while waiting
 type waitForReadyConfig struct {
-	watchMaker          WatchMaker
 	conditionsExtractor ConditionsExtractor
 	kind                string
 }
 
 // Callbacks and configuration used while waiting for event
 type waitForEvent struct {
-	watchMaker WatchMaker
-	eventDone  EventDone
-	kind       string
+	eventDone EventDone
+	kind      string
 }
 
 // EventDone is a marker to stop actual waiting on given event state
@@ -50,7 +48,7 @@ type Wait interface {
 	// Wait on resource the resource with this name
 	// and write event messages for unknown event to the status writer.
 	// Returns an error (if any) and the overall time it took to wait
-	Wait(ctx context.Context, name string, initialVersion string, options Options, msgCallback MessageCallback) (error, time.Duration)
+	Wait(ctx context.Context, watcher watch.Interface, name string, options Options, msgCallback MessageCallback) (error, time.Duration)
 }
 
 type Options struct {
@@ -63,7 +61,7 @@ type Options struct {
 }
 
 // Create watch which is used when waiting for Ready condition
-type WatchMaker func(ctx context.Context, name string, initialVersion string, timeout time.Duration) (watch.Interface, error)
+type WatchMaker func(name string, timeout time.Duration) (watch.Interface, error)
 
 // Extract conditions from a runtime object
 type ConditionsExtractor func(obj runtime.Object) (apis.Conditions, error)
@@ -72,21 +70,19 @@ type ConditionsExtractor func(obj runtime.Object) (apis.Conditions, error)
 type MessageCallback func(durationSinceState time.Duration, message string)
 
 // NewWaitForReady waits until the condition is set to Ready == True
-func NewWaitForReady(kind string, watchMaker WatchMaker, extractor ConditionsExtractor) Wait {
+func NewWaitForReady(kind string, extractor ConditionsExtractor) Wait {
 	return &waitForReadyConfig{
 		kind:                kind,
-		watchMaker:          watchMaker,
 		conditionsExtractor: extractor,
 	}
 }
 
 // NewWaitForEvent creates a Wait object which waits until a specific event (i.e. when
 // the EventDone function returns true)
-func NewWaitForEvent(kind string, watchMaker WatchMaker, eventDone EventDone) Wait {
+func NewWaitForEvent(kind string, eventDone EventDone) Wait {
 	return &waitForEvent{
-		kind:       kind,
-		watchMaker: watchMaker,
-		eventDone:  eventDone,
+		kind:      kind,
+		eventDone: eventDone,
 	}
 }
 
@@ -113,13 +109,14 @@ func NoopMessageCallback() MessageCallback {
 // (e.g. "service"), `timeout` is a timeout after which the watch should be cancelled if no
 // target state has been entered yet and `out` is used for printing out status messages
 // msgCallback gets called for every event with an 'Ready' condition == UNKNOWN with the event's message.
-func (w *waitForReadyConfig) Wait(ctx context.Context, name string, initialVersion string, options Options, msgCallback MessageCallback) (error, time.Duration) {
+func (w *waitForReadyConfig) Wait(ctx context.Context, watcher watch.Interface, name string, options Options, msgCallback MessageCallback) (error, time.Duration) {
+
 	timeout := options.timeoutWithDefault()
 	timeoutTimer := time.NewTimer(timeout)
 	defer timeoutTimer.Stop()
 	for {
 		start := time.Now()
-		retry, timeoutReached, err := w.waitForReadyCondition(ctx, name, initialVersion, start, timeoutTimer, options.errorWindowWithDefault(), options, msgCallback)
+		retry, timeoutReached, err := w.waitForReadyCondition(ctx, watcher, start, timeoutTimer, options.errorWindowWithDefault(), msgCallback)
 
 		if err != nil {
 			return err, time.Since(start)
@@ -143,14 +140,10 @@ func (w *waitForReadyConfig) Wait(ctx context.Context, name string, initialVersi
 // An errorWindow can be specified which takes into account of intermediate "false" ready conditions. So before returning
 // an error, this methods waits for the errorWindow duration and if an "True" or "Unknown" event arrives in the meantime
 // for the "Ready" condition, then the method continues to wait.
-func (w *waitForReadyConfig) waitForReadyCondition(ctx context.Context, name string, initialVersion string, start time.Time,
-	timeoutTimer *time.Timer, errorWindow time.Duration, options Options, msgCallback MessageCallback) (retry bool, timeoutReached bool, err error) {
+func (w *waitForReadyConfig) waitForReadyCondition(
+	ctx context.Context, watcher watch.Interface, start time.Time, timeoutTimer *time.Timer, errorWindow time.Duration, msgCallback MessageCallback,
+) (retry bool, timeoutReached bool, err error) {
 
-	watcher, err := w.watchMaker(ctx, name, initialVersion, options.timeoutWithDefault())
-	if err != nil {
-		return false, false, err
-	}
-	defer watcher.Stop()
 	// channel used to transport the error that has been received
 	errChan := make(chan error)
 
@@ -177,7 +170,8 @@ func (w *waitForReadyConfig) waitForReadyCondition(ctx context.Context, name str
 			return false, false, err
 		case event, ok := <-watcher.ResultChan():
 			if !ok || event.Object == nil {
-				return true, false, nil
+				// retry only if the channel is still open
+				return ok, false, nil
 			}
 
 			// Check whether resource is in sync already (meta.generation == status.observedGeneration)
@@ -246,13 +240,7 @@ func (w *waitForReadyConfig) waitForReadyCondition(ctx context.Context, name str
 }
 
 // Wait until the expected EventDone is satisfied
-func (w *waitForEvent) Wait(ctx context.Context, name string, initialVersion string, options Options, msgCallback MessageCallback) (error, time.Duration) {
-	watcher, err := w.watchMaker(ctx, name, initialVersion, options.timeoutWithDefault())
-	if err != nil {
-		return err, 0
-	}
-	defer watcher.Stop()
-
+func (w *waitForEvent) Wait(ctx context.Context, watcher watch.Interface, name string, options Options, msgCallback MessageCallback) (error, time.Duration) {
 	timeout := options.timeoutWithDefault()
 	start := time.Now()
 	// channel used to transport the error


Test patch:
diff --git a/pkg/eventing/v1/client_test.go b/pkg/eventing/v1/client_test.go
index 0c596b80..ca133726 100644
--- a/pkg/eventing/v1/client_test.go
+++ b/pkg/eventing/v1/client_test.go
@@ -245,21 +245,13 @@ func TestBrokerDelete(t *testing.T) {
 	var name = "fooBroker"
 	server, client := setup()
 
-	server.AddReactor("get", "brokers",
-		func(a client_testing.Action) (bool, runtime.Object, error) {
-			name := a.(client_testing.GetAction).GetName()
-			if name == "notFound" {
-				return true, nil, errors.NewNotFound(eventingv1.Resource("broker"), "notFound")
-			}
-			return false, nil, nil
-		})
 	server.AddReactor("delete", "brokers",
 		func(a client_testing.Action) (bool, runtime.Object, error) {
 			name := a.(client_testing.DeleteAction).GetName()
 			if name == "errorBroker" {
 				return true, nil, fmt.Errorf("error while deleting broker %s", name)
 			}
-			return false, nil, nil
+			return true, nil, nil
 		})
 
 	err := client.DeleteBroker(context.Background(), name, 0)
@@ -267,10 +259,6 @@ func TestBrokerDelete(t *testing.T) {
 
 	err = client.DeleteBroker(context.Background(), "errorBroker", 0)
 	assert.ErrorContains(t, err, "errorBroker", 0)
-
-	err = client.DeleteBroker(context.Background(), "notFound", 0)
-	assert.ErrorContains(t, err, "not found", 0)
-	assert.ErrorContains(t, err, "notFound", 0)
 }
 
 func TestBrokerDeleteWithWait(t *testing.T) {
diff --git a/pkg/serving/v1/client_test.go b/pkg/serving/v1/client_test.go
index dbe43313..7306b542 100644
--- a/pkg/serving/v1/client_test.go
+++ b/pkg/serving/v1/client_test.go
@@ -192,15 +192,6 @@ func TestDeleteService(t *testing.T) {
 		nonExistingServiceName = "no-service"
 	)
 
-	serving.AddReactor("get", "services",
-		func(a clienttesting.Action) (bool, runtime.Object, error) {
-			name := a.(clienttesting.GetAction).GetName()
-			if name == serviceName {
-				// Don't handle existing service, just continue to next
-				return false, nil, nil
-			}
-			return true, nil, errors.NewNotFound(servingv1.Resource("service"), name)
-		})
 	serving.AddReactor("delete", "services",
 		func(a clienttesting.Action) (bool, runtime.Object, error) {
 			name := a.(clienttesting.DeleteAction).GetName()
@@ -210,7 +201,7 @@ func TestDeleteService(t *testing.T) {
 			if name == serviceName {
 				return true, nil, nil
 			}
-			return false, nil, nil
+			return true, nil, errors.NewNotFound(servingv1.Resource("service"), name)
 		})
 	serving.AddWatchReactor("services",
 		func(a clienttesting.Action) (bool, watch.Interface, error) {
@@ -231,7 +222,6 @@ func TestDeleteService(t *testing.T) {
 
 	t.Run("trying to delete non-existing service returns error", func(t *testing.T) {
 		err := client.DeleteService(context.Background(), nonExistingServiceName, time.Duration(10)*time.Second)
-		println(err.Error())
 		assert.ErrorContains(t, err, "not found")
 		assert.ErrorContains(t, err, nonExistingServiceName)
 	})
diff --git a/pkg/wait/wait_for_ready_test.go b/pkg/wait/wait_for_ready_test.go
index b40b493f..6fbcefdb 100644
--- a/pkg/wait/wait_for_ready_test.go
+++ b/pkg/wait/wait_for_ready_test.go
@@ -17,7 +17,6 @@ package wait
 import (
 	"context"
 	"errors"
-	"fmt"
 	"testing"
 	"time"
 
@@ -40,13 +39,9 @@ type waitForReadyTestCase struct {
 func TestWaitCancellation(t *testing.T) {
 	fakeWatchApi := NewFakeWatch([]watch.Event{})
 	fakeWatchApi.Start()
-	wfe := NewWaitForEvent("foobar",
-		func(ctx context.Context, name string, initialVersion string, timeout time.Duration) (watch.Interface, error) {
-			return fakeWatchApi, nil
-		},
-		func(e *watch.Event) bool {
-			return false
-		})
+	wfe := NewWaitForEvent("foobar", func(e *watch.Event) bool {
+		return false
+	})
 
 	timeout := time.Second * 5
 
@@ -56,7 +51,7 @@ func TestWaitCancellation(t *testing.T) {
 		time.Sleep(time.Millisecond * 500)
 		cancel()
 	}()
-	err, _ := wfe.Wait(ctx, "foobar", "", Options{Timeout: &timeout}, NoopMessageCallback())
+	err, _ := wfe.Wait(ctx, fakeWatchApi, "foobar", Options{Timeout: &timeout}, NoopMessageCallback())
 	assert.Assert(t, errors.Is(err, context.Canceled))
 
 	ctx, cancel = context.WithCancel(context.Background())
@@ -67,13 +62,10 @@ func TestWaitCancellation(t *testing.T) {
 	}()
 	wfr := NewWaitForReady(
 		"blub",
-		func(ctx context.Context, name string, initialVersion string, timeout time.Duration) (watch.Interface, error) {
-			return fakeWatchApi, nil
-		},
 		func(obj runtime.Object) (apis.Conditions, error) {
 			return apis.Conditions(obj.(*servingv1.Service).Status.Conditions), nil
 		})
-	err, _ = wfr.Wait(ctx, "foobar", "", Options{Timeout: &timeout}, NoopMessageCallback())
+	err, _ = wfr.Wait(ctx, fakeWatchApi, "foobar", Options{Timeout: &timeout}, NoopMessageCallback())
 	assert.Assert(t, errors.Is(err, context.Canceled))
 }
 
@@ -81,65 +73,15 @@ func TestAddWaitForReady(t *testing.T) {
 
 	for i, tc := range prepareTestCases("test-service") {
 		fakeWatchApi := NewFakeWatch(tc.events)
-		waitForReady := NewWaitForReady(
-			"blub",
-			func(ctx context.Context, name string, initialVersion string, timeout time.Duration) (watch.Interface, error) {
-				return fakeWatchApi, nil
-			},
-			func(obj runtime.Object) (apis.Conditions, error) {
-				return apis.Conditions(obj.(*servingv1.Service).Status.Conditions), nil
-			})
-		fakeWatchApi.Start()
-		var msgs []string
-		err, _ := waitForReady.Wait(context.Background(), "foobar", "", Options{Timeout: &tc.timeout}, func(_ time.Duration, msg string) {
-			msgs = append(msgs, msg)
-		})
-		close(fakeWatchApi.eventChan)
-
-		if tc.errorText == "" && err != nil {
-			t.Errorf("%d: Error received %v", i, err)
-			continue
-		}
-		if tc.errorText != "" {
-			if err == nil {
-				t.Errorf("%d: No error but expected one", i)
-			} else {
-				assert.ErrorContains(t, err, tc.errorText)
-			}
-		}
-
-		// check messages
-		assert.Assert(t, cmp.DeepEqual(tc.messagesExpected, msgs), "%d: Messages expected to be equal", i)
-
-		if fakeWatchApi.StopCalled != 1 {
-			t.Errorf("%d: Exactly one 'stop' should be called, but got %d", i, fakeWatchApi.StopCalled)
-		}
 
-	}
-}
-
-func TestAddWaitForReadyWithChannelClose(t *testing.T) {
-	for i, tc := range prepareTestCases("test-service") {
-		fakeWatchApi := NewFakeWatch(tc.events)
-		counter := 0
 		waitForReady := NewWaitForReady(
 			"blub",
-			func(ctx context.Context, name string, initialVersion string, timeout time.Duration) (watch.Interface, error) {
-				if counter == 0 {
-					close(fakeWatchApi.eventChan)
-					counter++
-					return fakeWatchApi, nil
-				}
-				fakeWatchApi.eventChan = make(chan watch.Event)
-				fakeWatchApi.Start()
-				return fakeWatchApi, nil
-			},
 			func(obj runtime.Object) (apis.Conditions, error) {
 				return apis.Conditions(obj.(*servingv1.Service).Status.Conditions), nil
 			})
+		fakeWatchApi.Start()
 		var msgs []string
-
-		err, _ := waitForReady.Wait(context.Background(), "foobar", "", Options{Timeout: &tc.timeout}, func(_ time.Duration, msg string) {
+		err, _ := waitForReady.Wait(context.Background(), fakeWatchApi, "foobar", Options{Timeout: &tc.timeout}, func(_ time.Duration, msg string) {
 			msgs = append(msgs, msg)
 		})
 		close(fakeWatchApi.eventChan)
@@ -159,80 +101,23 @@ func TestAddWaitForReadyWithChannelClose(t *testing.T) {
 		// check messages
 		assert.Assert(t, cmp.DeepEqual(tc.messagesExpected, msgs), "%d: Messages expected to be equal", i)
 
-		if fakeWatchApi.StopCalled != 2 {
-			t.Errorf("%d: Exactly one 'stop' should be called, but got %d", i, fakeWatchApi.StopCalled)
+		if fakeWatchApi.StopCalled != 0 {
+			t.Errorf("%d: Exactly zero 'stop' should be called, but got %d", i, fakeWatchApi.StopCalled)
 		}
 
 	}
 }
 
-func TestWaitTimeout(t *testing.T) {
-	fakeWatchApi := NewFakeWatch([]watch.Event{})
-	timeout := time.Second * 3
-	wfe := NewWaitForEvent("foobar",
-		func(ctx context.Context, name string, initialVersion string, timeout time.Duration) (watch.Interface, error) {
-			return fakeWatchApi, nil
-		},
-		func(e *watch.Event) bool {
-			return false
-		})
-
-	err, _ := wfe.Wait(context.Background(), "foobar", "", Options{Timeout: &timeout}, NoopMessageCallback())
-	assert.ErrorContains(t, err, "not ready")
-	assert.Assert(t, fakeWatchApi.StopCalled == 1)
-
-	fakeWatchApi = NewFakeWatch([]watch.Event{})
-	wfr := NewWaitForReady(
-		"blub",
-		func(ctx context.Context, name string, initialVersion string, timeout time.Duration) (watch.Interface, error) {
-			return fakeWatchApi, nil
-		},
-		func(obj runtime.Object) (apis.Conditions, error) {
-			return apis.Conditions(obj.(*servingv1.Service).Status.Conditions), nil
-		})
-	err, _ = wfr.Wait(context.Background(), "foobar", "", Options{Timeout: &timeout}, NoopMessageCallback())
-	assert.ErrorContains(t, err, "not ready")
-	assert.Assert(t, fakeWatchApi.StopCalled == 1)
-}
-
-func TestWaitWatchError(t *testing.T) {
-	timeout := time.Second * 3
-	wfe := NewWaitForEvent("foobar",
-		func(ctx context.Context, name string, initialVersion string, timeout time.Duration) (watch.Interface, error) {
-			return nil, fmt.Errorf("error creating watcher")
-		},
-		func(e *watch.Event) bool {
-			return false
-		})
-
-	err, _ := wfe.Wait(context.Background(), "foobar", "", Options{Timeout: &timeout}, NoopMessageCallback())
-	assert.ErrorContains(t, err, "error creating watcher")
-
-	wfr := NewWaitForReady(
-		"blub",
-		func(ctx context.Context, name string, initialVersion string, timeout time.Duration) (watch.Interface, error) {
-			return nil, fmt.Errorf("error creating watcher")
-		},
-		func(obj runtime.Object) (apis.Conditions, error) {
-			return apis.Conditions(obj.(*servingv1.Service).Status.Conditions), nil
-		})
-	err, _ = wfr.Wait(context.Background(), "foobar", "", Options{Timeout: &timeout}, NoopMessageCallback())
-	assert.ErrorContains(t, err, "error creating watcher")
-}
-
 func TestAddWaitForDelete(t *testing.T) {
 	for i, tc := range prepareDeleteTestCases("test-service") {
 		fakeWatchAPI := NewFakeWatch(tc.events)
 
 		waitForEvent := NewWaitForEvent(
 			"blub",
-			func(ctx context.Context, name string, initialVersion string, timeout time.Duration) (watch.Interface, error) {
-				return fakeWatchAPI, nil
-			},
 			func(evt *watch.Event) bool { return evt.Type == watch.Deleted })
 		fakeWatchAPI.Start()
 
-		err, _ := waitForEvent.Wait(context.Background(), "foobar", "", Options{Timeout: &tc.timeout}, NoopMessageCallback())
+		err, _ := waitForEvent.Wait(context.Background(), fakeWatchAPI, "foobar", Options{Timeout: &tc.timeout}, NoopMessageCallback())
 		close(fakeWatchAPI.eventChan)
 
 		if tc.errorText == "" && err != nil {
@@ -247,8 +132,8 @@ func TestAddWaitForDelete(t *testing.T) {
 			}
 		}
 
-		if fakeWatchAPI.StopCalled != 1 {
-			t.Errorf("%d: Exactly one 'stop' should be called, but got %d", i, fakeWatchAPI.StopCalled)
+		if fakeWatchAPI.StopCalled != 0 {
+			t.Errorf("%d: Exactly zero 'stop' should be called, but got %d", i, fakeWatchAPI.StopCalled)
 		}
 	}
 }
@@ -257,10 +142,10 @@ func TestAddWaitForDelete(t *testing.T) {
 func prepareTestCases(name string) []waitForReadyTestCase {
 	return []waitForReadyTestCase{
 		errorTest(name),
-		tc(peNormal, name, 5*time.Second, ""),
-		tc(peWrongGeneration, name, 5*time.Second, "timeout"),
-		tc(peTimeout, name, 5*time.Second, "timeout"),
-		tc(peReadyFalseWithinErrorWindow, name, 5*time.Second, ""),
+		tc(peNormal, name, time.Second, ""),
+		tc(peWrongGeneration, name, 1*time.Second, "timeout"),
+		tc(peTimeout, name, time.Second, "timeout"),
+		tc(peReadyFalseWithinErrorWindow, name, time.Second, ""),
 	}
 }
 
@@ -279,7 +164,7 @@ func errorTest(name string) waitForReadyTestCase {
 
 	return waitForReadyTestCase{
 		events:           events,
-		timeout:          5 * time.Second,
+		timeout:          3 * time.Second,
 		errorText:        "FakeError",
 		messagesExpected: []string{"msg1", "Test Error"},
 	}