diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index c7d3bf4cdb..037d98a218 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -45,13 +45,16 @@ | https://github.com/knative/client/pull/639[#639] | 🧽 -| Refactor service `create_test.go` and `update_test.go` to -| remove unecessary `sync` parameter in setup call +| Refactor service `create_test.go` and `update_test.go` to remove unecessary `sync` parameter in setup call | https://github.com/knative/client/pull/656[#656] | 🐛 | Fix `--image` flag to only allow single occurence | https://github.com/knative/client/pull/647[#647] + +| 🧽 +| Add `--wait` and `--no-wait` to service delete operation. Change service delete to wait by default. +| https://github.com/knative/client/pull/682[#682] |=== ## v0.12.0 (2020-01-29) diff --git a/docs/cmd/kn_service_create.md b/docs/cmd/kn_service_create.md index 74f0fd709a..ce4f4a99ff 100644 --- a/docs/cmd/kn_service_create.md +++ b/docs/cmd/kn_service_create.md @@ -44,7 +44,7 @@ kn service create NAME --image IMAGE [flags] ``` --annotation stringArray Service annotation to set. name=value; you may provide this flag any number of times to set multiple annotations. To unset, specify the annotation name followed by a "-" (e.g., name-). --arg stringArray Add argument to the container command. Example: --arg myArg1 --arg --myArg2 --arg myArg3=3. You can use this flag multiple times. - --async DEPRECATED: please use --no-wait instead. Create service and don't wait for it to become ready. + --async DEPRECATED: please use --no-wait instead. Create service and don't wait for it to be ready. --autoscale-window string Duration to look back for making auto-scaling decisions. The service is scaled to zero if no request was received in during that time. (eg: 10s) --cmd string Specify command to be used as entrypoint instead of default one. Example: --cmd /app/start or --cmd /app/start --arg myArg to pass aditional arguments. --concurrency-limit int Hard Limit of concurrent requests to be processed by a single replica. @@ -63,7 +63,7 @@ kn service create NAME --image IMAGE [flags] --mount stringArray Mount a ConfigMap (prefix cm: or config-map:), a Secret (prefix secret: or sc:), or an existing Volume (without any prefix) on the specified directory. Example: --mount /mydir=cm:myconfigmap, --mount /mydir=secret:mysecret, or --mount /mydir=myvolume. When a configmap or a secret is specified, a corresponding volume is automatically generated. You can use this flag multiple times. For unmounting a directory, append "-", e.g. --mount /mydir-, which also removes any auto-generated volume. -n, --namespace string Specify the namespace to operate in. --no-lock-to-digest Do not keep the running image for the service constant when not explicitly specifying the image. (--no-lock-to-digest pulls the image tag afresh with each new revision) - --no-wait Create service and don't wait for it to become ready. + --no-wait Create service and don't wait for it to be ready. -p, --port int32 The port where application listens on. --pull-secret string Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace. --requests-cpu string The requested CPU (e.g., 250m). diff --git a/docs/cmd/kn_service_delete.md b/docs/cmd/kn_service_delete.md index c778fe4d4f..eef536e552 100644 --- a/docs/cmd/kn_service_delete.md +++ b/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 be deleted. -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 be deleted. + --wait-timeout int Seconds to wait before giving up on waiting for service to be deleted. (default 600) ``` ### Options inherited from parent commands diff --git a/docs/cmd/kn_service_update.md b/docs/cmd/kn_service_update.md index d854f9b26d..0792fa9708 100644 --- a/docs/cmd/kn_service_update.md +++ b/docs/cmd/kn_service_update.md @@ -40,7 +40,7 @@ kn service update NAME [flags] ``` --annotation stringArray Service annotation to set. name=value; you may provide this flag any number of times to set multiple annotations. To unset, specify the annotation name followed by a "-" (e.g., name-). --arg stringArray Add argument to the container command. Example: --arg myArg1 --arg --myArg2 --arg myArg3=3. You can use this flag multiple times. - --async DEPRECATED: please use --no-wait instead. Update service and don't wait for it to become ready. + --async DEPRECATED: please use --no-wait instead. Update service and don't wait for it to be ready. --autoscale-window string Duration to look back for making auto-scaling decisions. The service is scaled to zero if no request was received in during that time. (eg: 10s) --cmd string Specify command to be used as entrypoint instead of default one. Example: --cmd /app/start or --cmd /app/start --arg myArg to pass aditional arguments. --concurrency-limit int Hard Limit of concurrent requests to be processed by a single replica. @@ -58,7 +58,7 @@ kn service update NAME [flags] --mount stringArray Mount a ConfigMap (prefix cm: or config-map:), a Secret (prefix secret: or sc:), or an existing Volume (without any prefix) on the specified directory. Example: --mount /mydir=cm:myconfigmap, --mount /mydir=secret:mysecret, or --mount /mydir=myvolume. When a configmap or a secret is specified, a corresponding volume is automatically generated. You can use this flag multiple times. For unmounting a directory, append "-", e.g. --mount /mydir-, which also removes any auto-generated volume. -n, --namespace string Specify the namespace to operate in. --no-lock-to-digest Do not keep the running image for the service constant when not explicitly specifying the image. (--no-lock-to-digest pulls the image tag afresh with each new revision) - --no-wait Update service and don't wait for it to become ready. + --no-wait Update service and don't wait for it to be ready. -p, --port int32 The port where application listens on. --pull-secret string Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace. --requests-cpu string The requested CPU (e.g., 250m). diff --git a/pkg/kn/commands/service/create.go b/pkg/kn/commands/service/create.go index 2fd79d6dfa..6667cd95f7 100644 --- a/pkg/kn/commands/service/create.go +++ b/pkg/kn/commands/service/create.go @@ -114,7 +114,7 @@ func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command { } commands.AddNamespaceFlags(serviceCreateCommand.Flags(), false) editFlags.AddCreateFlags(serviceCreateCommand) - waitFlags.AddConditionWaitFlags(serviceCreateCommand, commands.WaitDefaultTimeout, "Create", "service") + waitFlags.AddConditionWaitFlags(serviceCreateCommand, commands.WaitDefaultTimeout, "Create", "service", "ready") return serviceCreateCommand } diff --git a/pkg/kn/commands/service/delete.go b/pkg/kn/commands/service/delete.go index 7fd0de1366..81dc8f9612 100644 --- a/pkg/kn/commands/service/delete.go +++ b/pkg/kn/commands/service/delete.go @@ -17,6 +17,7 @@ package service import ( "errors" "fmt" + "time" "github.com/spf13/cobra" @@ -25,6 +26,8 @@ import ( // 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.", @@ -48,9 +51,12 @@ func NewServiceDeleteCommand(p *commands.KnParams) *cobra.Command { if err != nil { return err } - for _, name := range args { - err = client.DeleteService(name) + timeout := time.Duration(0) + if !waitFlags.NoWait { + timeout = time.Duration(waitFlags.TimeoutInSeconds) * time.Second + } + err = client.DeleteService(name, timeout) if err != nil { fmt.Fprintf(cmd.OutOrStdout(), "%s.\n", err) } else { @@ -61,5 +67,6 @@ func NewServiceDeleteCommand(p *commands.KnParams) *cobra.Command { }, } commands.AddNamespaceFlags(serviceDeleteCommand.Flags(), false) + waitFlags.AddConditionWaitFlags(serviceDeleteCommand, commands.WaitDefaultTimeout, "Delete", "service", "deleted") return serviceDeleteCommand } diff --git a/pkg/kn/commands/service/delete_mock_test.go b/pkg/kn/commands/service/delete_mock_test.go index 4e9ac9d960..f901b9ddb1 100644 --- a/pkg/kn/commands/service/delete_mock_test.go +++ b/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) { @@ -30,7 +31,7 @@ func TestServiceDeleteMock(t *testing.T) { // Recording: r := client.Recorder() - r.DeleteService("foo", nil) + r.DeleteService("foo", mock.Any(), nil) output, err := executeServiceCommand(client, "delete", "foo") assert.NilError(t, err) @@ -40,16 +41,34 @@ func TestServiceDeleteMock(t *testing.T) { } +func TestServiceDeleteMockNoWait(t *testing.T) { + // New mock client + client := clientservingv1.NewMockKnServiceClient(t) + + // Recording: + r := client.Recorder() + + r.DeleteService("foo", mock.Any(), nil) + + output, err := executeServiceCommand(client, "delete", "foo", "--no-wait") + assert.NilError(t, err) + assert.Assert(t, util.ContainsAll(output, "deleted", "foo", "default")) + + r.Validate() + +} + func TestMultipleServiceDeleteMock(t *testing.T) { // New mock client client := clientservingv1.NewMockKnServiceClient(t) // Recording: r := client.Recorder() + // Wait for delete event - r.DeleteService("foo", nil) - r.DeleteService("bar", nil) - r.DeleteService("baz", nil) + r.DeleteService("foo", mock.Any(), nil) + r.DeleteService("bar", mock.Any(), nil) + r.DeleteService("baz", mock.Any(), nil) output, err := executeServiceCommand(client, "delete", "foo", "bar", "baz") assert.NilError(t, err) diff --git a/pkg/kn/commands/service/delete_test.go b/pkg/kn/commands/service/delete_test.go index 3668ba41a0..3dba0e0f8b 100644 --- a/pkg/kn/commands/service/delete_test.go +++ b/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, "", "")}, + } +} diff --git a/pkg/kn/commands/service/update.go b/pkg/kn/commands/service/update.go index c5d419e92f..b0589e0a1d 100644 --- a/pkg/kn/commands/service/update.go +++ b/pkg/kn/commands/service/update.go @@ -136,7 +136,7 @@ func NewServiceUpdateCommand(p *commands.KnParams) *cobra.Command { commands.AddNamespaceFlags(serviceUpdateCommand.Flags(), false) editFlags.AddUpdateFlags(serviceUpdateCommand) - waitFlags.AddConditionWaitFlags(serviceUpdateCommand, commands.WaitDefaultTimeout, "Update", "service") + waitFlags.AddConditionWaitFlags(serviceUpdateCommand, commands.WaitDefaultTimeout, "Update", "service", "ready") trafficFlags.Add(serviceUpdateCommand) return serviceUpdateCommand } diff --git a/pkg/kn/commands/wait_flags.go b/pkg/kn/commands/wait_flags.go index bb459157d0..ca78368568 100644 --- a/pkg/kn/commands/wait_flags.go +++ b/pkg/kn/commands/wait_flags.go @@ -39,12 +39,12 @@ type WaitFlags struct { // Add flags which influence the sync/async behaviour when creating or updating // resources. Set `waitDefault` argument if the default behaviour is synchronous. // Use `what` for describing what is waited for. -func (p *WaitFlags) AddConditionWaitFlags(command *cobra.Command, waitTimeoutDefault int, action string, what string) { - waitUsage := fmt.Sprintf("%s %s and don't wait for it to become ready.", action, what) +func (p *WaitFlags) AddConditionWaitFlags(command *cobra.Command, waitTimeoutDefault int, action, what, until string) { + waitUsage := fmt.Sprintf("%s %s and don't wait for it to be %s.", action, what, until) //TODO: deprecated flag should be removed in next release command.Flags().BoolVar(&p.Async, "async", false, "DEPRECATED: please use --no-wait instead. "+waitUsage) command.Flags().BoolVar(&p.NoWait, "no-wait", false, waitUsage) - timeoutUsage := fmt.Sprintf("Seconds to wait before giving up on waiting for %s to be ready.", what) + timeoutUsage := fmt.Sprintf("Seconds to wait before giving up on waiting for %s to be %s.", what, until) command.Flags().IntVar(&p.TimeoutInSeconds, "wait-timeout", waitTimeoutDefault, timeoutUsage) } diff --git a/pkg/kn/commands/wait_flags_test.go b/pkg/kn/commands/wait_flags_test.go index a98f45a94d..9c711d2c78 100644 --- a/pkg/kn/commands/wait_flags_test.go +++ b/pkg/kn/commands/wait_flags_test.go @@ -42,7 +42,7 @@ func TestAddWaitForReadyDeprecatedFlags(t *testing.T) { flags := &WaitFlags{} cmd := cobra.Command{} - flags.AddConditionWaitFlags(&cmd, 60, "Create", "service") + flags.AddConditionWaitFlags(&cmd, 60, "Create", "service", "ready") err := cmd.ParseFlags(tc.args) if err != nil && !tc.isParseErrorExpected { @@ -76,7 +76,7 @@ func TestAddWaitForReadyFlags(t *testing.T) { flags := &WaitFlags{} cmd := cobra.Command{} - flags.AddConditionWaitFlags(&cmd, 60, "Create", "service") + flags.AddConditionWaitFlags(&cmd, 60, "Create", "service", "ready") err := cmd.ParseFlags(tc.args) if err != nil && !tc.isParseErrorExpected { @@ -101,7 +101,7 @@ func TestAddWaitUsageMessage(t *testing.T) { flags := &WaitFlags{} cmd := cobra.Command{} - flags.AddConditionWaitFlags(&cmd, 60, "bla", "blub") + flags.AddConditionWaitFlags(&cmd, 60, "bla", "blub", "deleted") if !strings.Contains(cmd.UsageString(), "blub") { t.Error("no type returned in usage") } @@ -111,4 +111,7 @@ func TestAddWaitUsageMessage(t *testing.T) { if !strings.Contains(cmd.UsageString(), "60") { t.Error("default timeout not contained") } + if !strings.Contains(cmd.UsageString(), "deleted") { + t.Error("wrong until message") + } } diff --git a/pkg/serving/v1/client.go b/pkg/serving/v1/client.go index 5cf196608a..1a6c1defea 100644 --- a/pkg/serving/v1/client.go +++ b/pkg/serving/v1/client.go @@ -69,7 +69,7 @@ type KnServingClient interface { UpdateServiceWithRetry(name string, updateFunc serviceUpdateFunc, nrRetries int) error // Delete a service by name - DeleteService(name string) error + DeleteService(name string, timeout time.Duration) error // Wait for a service to become ready, but not longer than provided timeout. // Return error and how long has been waited @@ -253,10 +253,29 @@ func updateServiceWithRetry(cl KnServingClient, name string, updateFunc serviceU } // Delete a service by name -func (cl *knServingClient) DeleteService(serviceName string) error { +// 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(serviceName string, timeout time.Duration) error { + if timeout == 0 { + return cl.deleteService(serviceName, v1.DeletePropagationBackground) + } + waitC := make(chan error) + go func() { + waitForEvent := wait.NewWaitForEvent("service", cl.WatchService, func(evt *watch.Event) bool { return evt.Type == watch.Deleted }) + err, _ := waitForEvent.Wait(serviceName, timeout, wait.NoopMessageCallback()) + waitC <- err + }() + err := cl.deleteService(serviceName, v1.DeletePropagationForeground) + if err != nil { + return err + } + return <-waitC +} + +func (cl *knServingClient) deleteService(serviceName string, propagationPolicy v1.DeletionPropagation) error { err := cl.client.Services(cl.namespace).Delete( serviceName, - &v1.DeleteOptions{}, + &v1.DeleteOptions{PropagationPolicy: &propagationPolicy}, ) if err != nil { return clienterrors.GetError(err) diff --git a/pkg/serving/v1/client_mock.go b/pkg/serving/v1/client_mock.go index b0000d7665..9381ed34ca 100644 --- a/pkg/serving/v1/client_mock.go +++ b/pkg/serving/v1/client_mock.go @@ -106,12 +106,12 @@ func (c *MockKnServingClient) UpdateServiceWithRetry(name string, updateFunc ser } // Delete a service by name -func (sr *ServingRecorder) DeleteService(name interface{}, err error) { - sr.r.Add("DeleteService", []interface{}{name}, []interface{}{err}) +func (sr *ServingRecorder) DeleteService(name, timeout interface{}, err error) { + sr.r.Add("DeleteService", []interface{}{name, timeout}, []interface{}{err}) } -func (c *MockKnServingClient) DeleteService(name string) error { - call := c.recorder.r.VerifyCall("DeleteService", name) +func (c *MockKnServingClient) DeleteService(name string, timeout time.Duration) error { + call := c.recorder.r.VerifyCall("DeleteService", name, timeout) return mock.ErrorOrNil(call.Result[0]) } diff --git a/pkg/serving/v1/client_mock_test.go b/pkg/serving/v1/client_mock_test.go index a2ee9dc0ae..4bf80e5240 100644 --- a/pkg/serving/v1/client_mock_test.go +++ b/pkg/serving/v1/client_mock_test.go @@ -36,7 +36,7 @@ func TestMockKnClient(t *testing.T) { recorder.ListServices(mock.Any(), nil, nil) recorder.CreateService(&servingv1.Service{}, nil) recorder.UpdateService(&servingv1.Service{}, nil) - recorder.DeleteService("hello", nil) + recorder.DeleteService("hello", time.Duration(10)*time.Second, nil) recorder.WaitForService("hello", time.Duration(10)*time.Second, wait.NoopMessageCallback(), nil, 10*time.Second) recorder.GetRevision("hello", nil, nil) recorder.ListRevisions(mock.Any(), nil, nil) @@ -50,7 +50,7 @@ func TestMockKnClient(t *testing.T) { client.ListServices(WithName("blub")) client.CreateService(&servingv1.Service{}) client.UpdateService(&servingv1.Service{}) - client.DeleteService("hello") + client.DeleteService("hello", time.Duration(10)*time.Second) client.WaitForService("hello", time.Duration(10)*time.Second, wait.NoopMessageCallback()) client.GetRevision("hello") client.ListRevisions(WithName("blub")) diff --git a/pkg/serving/v1/client_test.go b/pkg/serving/v1/client_test.go index 36821c6630..2b9e0192d2 100644 --- a/pkg/serving/v1/client_test.go +++ b/pkg/serving/v1/client_test.go @@ -179,19 +179,38 @@ func TestDeleteService(t *testing.T) { } return true, nil, errors.NewNotFound(servingv1.Resource("service"), name) }) + serving.AddWatchReactor("services", + func(a clienttesting.Action) (bool, watch.Interface, error) { + watchAction := a.(clienttesting.WatchAction) + name, found := watchAction.GetWatchRestrictions().Fields.RequiresExactMatch("metadata.name") + if !found { + return true, nil, errors.NewNotFound(servingv1.Resource("service"), name) + } + w := wait.NewFakeWatch(getServiceDeleteEvents("test-service")) + w.Start() + return true, w, nil + }) t.Run("delete existing service returns no error", func(t *testing.T) { - err := client.DeleteService(serviceName) + err := client.DeleteService(serviceName, time.Duration(10)*time.Second) assert.NilError(t, err) }) t.Run("trying to delete non-existing service returns error", func(t *testing.T) { - err := client.DeleteService(nonExistingServiceName) + err := client.DeleteService(nonExistingServiceName, time.Duration(10)*time.Second) assert.ErrorContains(t, err, "not found") assert.ErrorContains(t, err, nonExistingServiceName) }) } +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, "", "")}, + } +} + func TestGetRevision(t *testing.T) { serving, client := setup() diff --git a/pkg/wait/wait_for_ready.go b/pkg/wait/wait_for_ready.go index e4c51776ad..479b543903 100644 --- a/pkg/wait/wait_for_ready.go +++ b/pkg/wait/wait_for_ready.go @@ -36,9 +36,19 @@ type waitForReadyConfig struct { kind string } +// Callbacks and configuration used while waiting for event +type waitForEvent struct { + watchMaker WatchMaker + eventDone EventDone + kind string +} + +// EventDone is a marker to stop actual waiting on given event state +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,7 +66,7 @@ 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, @@ -64,6 +74,14 @@ func NewWaitForReady(kind string, watchMaker WatchMaker, extractor ConditionsExt } } +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 := "" @@ -119,6 +137,7 @@ func (w *waitForReadyConfig) waitForReadyCondition(start time.Time, name string, // channel used to transport the error errChan := make(chan error) + var errorTimer *time.Timer // Stop error timer if it has been started because of // a ConditionReady has been set to false @@ -178,9 +197,32 @@ func (w *waitForReadyConfig) waitForReadyCondition(start time.Time, name string, } } -// 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) +// Wait until the expected EventDone is satisfied +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) + timer := time.NewTimer(timeout) + defer timer.Stop() + for { + select { + case <-timer.C: + return fmt.Errorf("timeout: %s '%s' not ready after %d seconds", w.kind, name, int(timeout/time.Second)), time.Since(start) + case err = <-errChan: + return err, time.Since(start) + case event := <-watcher.ResultChan(): + if w.eventDone(&event) { + return nil, time.Since(start) + } + } + } +} + func isGivenEqualsObservedGeneration(object runtime.Object) (bool, error) { unstructured, err := runtime.DefaultUnstructuredConverter.ToUnstructured(object) if err != nil { diff --git a/pkg/wait/wait_for_ready_test.go b/pkg/wait/wait_for_ready_test.go index 6ff89d8cd5..cf1ebdde31 100644 --- a/pkg/wait/wait_for_ready_test.go +++ b/pkg/wait/wait_for_ready_test.go @@ -76,6 +76,40 @@ func TestAddWaitForReady(t *testing.T) { } } +func TestAddWaitForDelete(t *testing.T) { + for i, tc := range prepareDeleteTestCases("test-service") { + fakeWatchAPI := NewFakeWatch(tc.events) + + waitForEvent := NewWaitForEvent( + "blub", + func(name 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("foobar", tc.timeout, NoopMessageCallback()) + 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) + } + } + + if fakeWatchAPI.StopCalled != 1 { + t.Errorf("%d: Exactly one 'stop' should be called, but got %d", i, fakeWatchAPI.StopCalled) + } + + } +} + // Test cases which consists of a series of events to send and the expected behaviour. func prepareTestCases(name string) []waitForReadyTestCase { return []waitForReadyTestCase{ @@ -87,6 +121,13 @@ func prepareTestCases(name string) []waitForReadyTestCase { } } +func prepareDeleteTestCases(name string) []waitForReadyTestCase { + return []waitForReadyTestCase{ + tc(deNormal, name, time.Second, ""), + tc(peTimeout, name, 10*time.Second, "timeout"), + } +} + func errorTest(name string) waitForReadyTestCase { events := []watch.Event{ {watch.Added, CreateTestServiceWithConditions(name, corev1.ConditionUnknown, corev1.ConditionUnknown, "", "msg1")}, @@ -150,3 +191,12 @@ func peReadyFalseWithinErrorWindow(name string) ([]watch.Event, int) { {watch.Modified, CreateTestServiceWithConditions(name, corev1.ConditionTrue, corev1.ConditionTrue, "Route ready", "")}, }, len(messages) } + +func deNormal(name string) ([]watch.Event, int) { + messages := pMessages(2) + return []watch.Event{ + {watch.Added, CreateTestServiceWithConditions(name, corev1.ConditionUnknown, corev1.ConditionUnknown, "", messages[0])}, + {watch.Modified, CreateTestServiceWithConditions(name, corev1.ConditionUnknown, corev1.ConditionTrue, "", messages[1])}, + {watch.Deleted, CreateTestServiceWithConditions(name, corev1.ConditionTrue, corev1.ConditionTrue, "", "")}, + }, len(messages) +}