From fcb04acd70cbc5b4dd3f14d5d8ca38a461861b1c Mon Sep 17 00:00:00 2001 From: Caleb Woodbine Date: Mon, 18 May 2020 16:32:38 +1200 Subject: [PATCH 01/22] Add WIP watch event tooling --- test/e2e/framework/util.go | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index 0660e10e2ea..b26fe9542b3 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -51,6 +51,7 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apimachinery/pkg/watch" utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/client-go/dynamic" clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" restclient "k8s.io/client-go/rest" @@ -1284,3 +1285,36 @@ func taintExists(taints []v1.Taint, taintToFind *v1.Taint) bool { } return false } + +// WatchEventEnsurerAndManager +// manages a watch for a given resource, ensures that events take place in a given order, retries the test on failure +func WatchEventEnsurerAndManager(testContext context.Context, dc dynamic.Interface, resourceType schema.GroupVersionResource, namespace string, resourceName string, listOptions metav1.ListOptions, expectedWatchEvents []watch.Event, scenario func(*watch.Interface) []watch.Event) { + retries := 3 +retriesLoop: + for try := 1; try <= retries; try++ { + var watchEvents []watch.Event + resourceWatch := &watch.Interface{} + // TODO consider watch deaths + &resourceWatch, err = dc.Resource(resourceType).Namespace(namespace).Watch(testContext, listOptions) + framework.ExpectNoError(err, "Failed to set a resource watch up for resourceType %v in namespace %s", resourceType, namespace) + + actualWatchEvents := scenario(resourceWatch) + framework.ExpectEqual(len(expectedWatchEvents) <= len(actualWatchEvents), true, "Amount of actual watch events to be equal to or greater than the amount of expected watch events") + + expectedWatchEventsLoop: + for expectedWatchEventIndex, expectedWatchEvent := range expectedWatchEvents { + for actualWatchEventIndex, _ := range actualWatchEvents { + if actualWatchEvents[expectedWatchEventIndex].Type == expectedWatchEvents[actualWatchEventIndex].Type { + watchEvents = append(watchEvents, expectedWatchEvent) + continue expectedWatchEventsLoop + } + } + } + + // TODO clean up resources, based on watchEvent.Object, using DynamicClient + + if len(watchEvents) != len(actualWatchEvents) { + continue retriesLoop + } + } +} From a3248a7ba8fc780b8e17991ea3681a6ec24d0aee Mon Sep 17 00:00:00 2001 From: Caleb Woodbine Date: Wed, 20 May 2020 16:28:01 +1200 Subject: [PATCH 02/22] Add documentation; Update base from invarients --- test/e2e/framework/util.go | 42 +++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index b26fe9542b3..07b43c6f049 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -1288,33 +1288,37 @@ func taintExists(taints []v1.Taint, taintToFind *v1.Taint) bool { // WatchEventEnsurerAndManager // manages a watch for a given resource, ensures that events take place in a given order, retries the test on failure +// testContext cancelation signal across API boundries, e.g: context.TODO() +// dc sets up a client to the API +// resourceType specify the type of resource +// namespace select a namespace +// resourceName the name of the given resource +// listOptions options used to find the resource, recommended to use listOptions.labelSelector +// expectedWatchEvents array of events which are expected to occur +// scenario the function to run func WatchEventEnsurerAndManager(testContext context.Context, dc dynamic.Interface, resourceType schema.GroupVersionResource, namespace string, resourceName string, listOptions metav1.ListOptions, expectedWatchEvents []watch.Event, scenario func(*watch.Interface) []watch.Event) { + // TODO add client-go/tools/watch/retrywatcher to manage watch restarts retries := 3 retriesLoop: for try := 1; try <= retries; try++ { - var watchEvents []watch.Event - resourceWatch := &watch.Interface{} - // TODO consider watch deaths - &resourceWatch, err = dc.Resource(resourceType).Namespace(namespace).Watch(testContext, listOptions) - framework.ExpectNoError(err, "Failed to set a resource watch up for resourceType %v in namespace %s", resourceType, namespace) - - actualWatchEvents := scenario(resourceWatch) - framework.ExpectEqual(len(expectedWatchEvents) <= len(actualWatchEvents), true, "Amount of actual watch events to be equal to or greater than the amount of expected watch events") - - expectedWatchEventsLoop: - for expectedWatchEventIndex, expectedWatchEvent := range expectedWatchEvents { - for actualWatchEventIndex, _ := range actualWatchEvents { - if actualWatchEvents[expectedWatchEventIndex].Type == expectedWatchEvents[actualWatchEventIndex].Type { - watchEvents = append(watchEvents, expectedWatchEvent) - continue expectedWatchEventsLoop + errs := sets.NewString() + watchEventsLoop: + for watchEventIndex, _ := range expectedWatchEvents { + watchEventNextIndex := watchEventIndex + 1 + if watchEventNextIndex >= len(expectedWatchEvents) { + continue watchEventsLoop + } + // TODO validate watchEvent.Type, not Object + for _, fn := range fns { + if err := fn(expectedWatchEvents[watchEventIndex].Object, expectedWatchEvents[watchEventNextIndex].Object); err != nil { + errs.Insert(err.Error()) } } } - - // TODO clean up resources, based on watchEvent.Object, using DynamicClient - - if len(watchEvents) != len(actualWatchEvents) { + if errs.Len() > 0 && try < retries { + fmt.Errorf("invariants violated:\n* %s", strings.Join(errs.List(), "\n* ")) continue retriesLoop } + ExpectEqual(errs.Len() > 0, false, fmt.Errorf("invariants violated:\n* %s", strings.Join(errs.List(), "\n* "))) } } From 82d76e2d6d3c2e87c9518a0ef6041ebd091bb969 Mon Sep 17 00:00:00 2001 From: Caleb Woodbine Date: Thu, 21 May 2020 10:00:17 +1200 Subject: [PATCH 03/22] Add dynamic client resource watch function --- test/e2e/framework/util.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index 07b43c6f049..8f16a9a3bfc 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -1286,6 +1286,14 @@ func taintExists(taints []v1.Taint, taintToFind *v1.Taint) bool { return false } +func getDynamicResourceWatch(testContext context.Context, dc dynamic.Interface, resourceType schema.GroupVersionResource, namespace string, resourceName string, listOptions metav1.ListOptions) func() (watch.Interface, error) { + return func() (watch.Interface, error) { + res, err := dc.Resource(resourceType).Namespace(namespace).Watch(context.TODO(), listOptions) + ExpectNoError(err, "Failed to create a watch for %v", resourceType.Resource) + return res, err + } +} + // WatchEventEnsurerAndManager // manages a watch for a given resource, ensures that events take place in a given order, retries the test on failure // testContext cancelation signal across API boundries, e.g: context.TODO() From 32e0da39fc8978aa712cef75dc635abf9b70b7a8 Mon Sep 17 00:00:00 2001 From: Caleb Woodbine Date: Thu, 21 May 2020 15:58:55 +1200 Subject: [PATCH 04/22] Update to use watcher --- test/e2e/framework/util.go | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index 8f16a9a3bfc..7e957769ae2 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -1286,12 +1286,9 @@ func taintExists(taints []v1.Taint, taintToFind *v1.Taint) bool { return false } -func getDynamicResourceWatch(testContext context.Context, dc dynamic.Interface, resourceType schema.GroupVersionResource, namespace string, resourceName string, listOptions metav1.ListOptions) func() (watch.Interface, error) { - return func() (watch.Interface, error) { - res, err := dc.Resource(resourceType).Namespace(namespace).Watch(context.TODO(), listOptions) - ExpectNoError(err, "Failed to create a watch for %v", resourceType.Resource) - return res, err - } +func getDynamicResourceWatch(testContext context.Context, dc dynamic.Interface, resourceType schema.GroupVersionResource, namespace string, listOptions metav1.ListOptions) (watch.Interface, error) { + res, err := dc.Resource(resourceType).Namespace(namespace).Watch(context.TODO(), listOptions) + return res, err } // WatchEventEnsurerAndManager @@ -1305,10 +1302,20 @@ func getDynamicResourceWatch(testContext context.Context, dc dynamic.Interface, // expectedWatchEvents array of events which are expected to occur // scenario the function to run func WatchEventEnsurerAndManager(testContext context.Context, dc dynamic.Interface, resourceType schema.GroupVersionResource, namespace string, resourceName string, listOptions metav1.ListOptions, expectedWatchEvents []watch.Event, scenario func(*watch.Interface) []watch.Event) { - // TODO add client-go/tools/watch/retrywatcher to manage watch restarts + listWatcher := &cache.ListWatch{ + WatchFunc: func(listOptions metav1.ListOptions) (watch.Interface, error) { + return getDynamicResourceWatch(testContext, dc, resourceType, namespace, listOptions) + }, + } + resourceWatch, err := watchtools.NewRetryWatcher("", listWatcher) + ExpectNoError(err, "Failed to create a resource watch of %v in namespace %v", resourceType.Resource, namespace) + + ExpectNoError(err, "Failed to create a watch for %v", resourceType.Resource) retries := 3 retriesLoop: for try := 1; try <= retries; try++ { + // TODO pass resourceWatch to the test function, or just collect events separately and parse them after + scenario() errs := sets.NewString() watchEventsLoop: for watchEventIndex, _ := range expectedWatchEvents { From b63b147d126fd88fffb9dbe631df2e50af9cbe85 Mon Sep 17 00:00:00 2001 From: Caleb Woodbine Date: Mon, 25 May 2020 11:49:57 +1200 Subject: [PATCH 05/22] Update name, watch event occurence checking --- test/e2e/framework/util.go | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index 7e957769ae2..2fbc016b4f3 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -1286,12 +1286,7 @@ func taintExists(taints []v1.Taint, taintToFind *v1.Taint) bool { return false } -func getDynamicResourceWatch(testContext context.Context, dc dynamic.Interface, resourceType schema.GroupVersionResource, namespace string, listOptions metav1.ListOptions) (watch.Interface, error) { - res, err := dc.Resource(resourceType).Namespace(namespace).Watch(context.TODO(), listOptions) - return res, err -} - -// WatchEventEnsurerAndManager +// WatchEventSequenceVerifier // manages a watch for a given resource, ensures that events take place in a given order, retries the test on failure // testContext cancelation signal across API boundries, e.g: context.TODO() // dc sets up a client to the API @@ -1301,39 +1296,45 @@ func getDynamicResourceWatch(testContext context.Context, dc dynamic.Interface, // listOptions options used to find the resource, recommended to use listOptions.labelSelector // expectedWatchEvents array of events which are expected to occur // scenario the function to run -func WatchEventEnsurerAndManager(testContext context.Context, dc dynamic.Interface, resourceType schema.GroupVersionResource, namespace string, resourceName string, listOptions metav1.ListOptions, expectedWatchEvents []watch.Event, scenario func(*watch.Interface) []watch.Event) { +func WatchEventSequenceVerifier(testContext context.Context, dc dynamic.Interface, resourceType schema.GroupVersionResource, namespace string, resourceName string, listOptions metav1.ListOptions, expectedWatchEvents []watch.Event, scenario func(*watchtools.RetryWatcher) []watch.Event) { listWatcher := &cache.ListWatch{ WatchFunc: func(listOptions metav1.ListOptions) (watch.Interface, error) { - return getDynamicResourceWatch(testContext, dc, resourceType, namespace, listOptions) + return dc.Resource(resourceType).Namespace(namespace).Watch(testContext, listOptions) }, } resourceWatch, err := watchtools.NewRetryWatcher("", listWatcher) ExpectNoError(err, "Failed to create a resource watch of %v in namespace %v", resourceType.Resource, namespace) - ExpectNoError(err, "Failed to create a watch for %v", resourceType.Resource) + // NOTE value of 3 retries seems to make sense retries := 3 retriesLoop: for try := 1; try <= retries; try++ { - // TODO pass resourceWatch to the test function, or just collect events separately and parse them after - scenario() + // NOTE the test may need access to the events to see what's going on, such as a change in status + actualWatchEvents := scenario(resourceWatch) errs := sets.NewString() watchEventsLoop: for watchEventIndex, _ := range expectedWatchEvents { + foundExpectedWatchEvent := false watchEventNextIndex := watchEventIndex + 1 if watchEventNextIndex >= len(expectedWatchEvents) { continue watchEventsLoop } - // TODO validate watchEvent.Type, not Object - for _, fn := range fns { - if err := fn(expectedWatchEvents[watchEventIndex].Object, expectedWatchEvents[watchEventNextIndex].Object); err != nil { - errs.Insert(err.Error()) + for actualWatchEventIndex, _ := range actualWatchEvents { + if actualWatchEvents[watchEventIndex] == expectedWatchEvents[actualWatchEventIndex] { + foundExpectedWatchEvent = true } } + if foundExpectedWatchEvent == false { + errs.Insert(fmt.Sprintf("Watch event %v not found", expectedWatchEvents[watchEventIndex])) + } } + // TODO restructure failures handling if errs.Len() > 0 && try < retries { fmt.Errorf("invariants violated:\n* %s", strings.Join(errs.List(), "\n* ")) + // TODO delete resources via DynamicClient (on failure) continue retriesLoop } ExpectEqual(errs.Len() > 0, false, fmt.Errorf("invariants violated:\n* %s", strings.Join(errs.List(), "\n* "))) + // TODO delete resources via DynamicClient (on failure) } } From 9f0c24ddcffc7d857a3725b96f52c306186062f7 Mon Sep 17 00:00:00 2001 From: Caleb Woodbine Date: Mon, 25 May 2020 13:33:48 +1200 Subject: [PATCH 06/22] Fix checks, amount checking --- test/e2e/framework/util.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index 2fbc016b4f3..b4b8049bd44 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -1312,22 +1312,22 @@ retriesLoop: // NOTE the test may need access to the events to see what's going on, such as a change in status actualWatchEvents := scenario(resourceWatch) errs := sets.NewString() - watchEventsLoop: + // watchEventsLoop: + totalValidWatchEvents := 0 for watchEventIndex, _ := range expectedWatchEvents { foundExpectedWatchEvent := false - watchEventNextIndex := watchEventIndex + 1 - if watchEventNextIndex >= len(expectedWatchEvents) { - continue watchEventsLoop - } + ExpectEqual(len(expectedWatchEvents) <= len(actualWatchEvents), true, "Error: actual watch events amount must be greater than or equal to expected watch events amount") for actualWatchEventIndex, _ := range actualWatchEvents { - if actualWatchEvents[watchEventIndex] == expectedWatchEvents[actualWatchEventIndex] { + if actualWatchEvents[watchEventIndex].Type == expectedWatchEvents[actualWatchEventIndex].Type { foundExpectedWatchEvent = true } } if foundExpectedWatchEvent == false { - errs.Insert(fmt.Sprintf("Watch event %v not found", expectedWatchEvents[watchEventIndex])) + errs.Insert(fmt.Sprintf("Watch event %v not found", expectedWatchEvents[watchEventIndex].Type)) } + totalValidWatchEvents ++ } + ExpectEqual(totalValidWatchEvents, len(expectedWatchEvents), "Error: there must be an equal amount of total valid watch events (%v) and expected watch events (%v)", totalValidWatchEvents, len(expectedWatchEvents)) // TODO restructure failures handling if errs.Len() > 0 && try < retries { fmt.Errorf("invariants violated:\n* %s", strings.Join(errs.List(), "\n* ")) From 1bdb854e7ebaecb7311381cb068349824b3c94ae Mon Sep 17 00:00:00 2001 From: Caleb Woodbine Date: Mon, 25 May 2020 14:59:25 +1200 Subject: [PATCH 07/22] Add resource deleting if there wasn't a delete watch event found --- test/e2e/framework/util.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index b4b8049bd44..8ad49cb5346 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -1314,6 +1314,7 @@ retriesLoop: errs := sets.NewString() // watchEventsLoop: totalValidWatchEvents := 0 + actualWatchEventsHasDelete := false for watchEventIndex, _ := range expectedWatchEvents { foundExpectedWatchEvent := false ExpectEqual(len(expectedWatchEvents) <= len(actualWatchEvents), true, "Error: actual watch events amount must be greater than or equal to expected watch events amount") @@ -1321,20 +1322,24 @@ retriesLoop: if actualWatchEvents[watchEventIndex].Type == expectedWatchEvents[actualWatchEventIndex].Type { foundExpectedWatchEvent = true } + if actualWatchEvents[actualWatchEventIndex].Type == watch.Deleted && actualWatchEventsHasDelete != true { + actualWatchEventsHasDelete = true + } } if foundExpectedWatchEvent == false { errs.Insert(fmt.Sprintf("Watch event %v not found", expectedWatchEvents[watchEventIndex].Type)) } totalValidWatchEvents ++ } - ExpectEqual(totalValidWatchEvents, len(expectedWatchEvents), "Error: there must be an equal amount of total valid watch events (%v) and expected watch events (%v)", totalValidWatchEvents, len(expectedWatchEvents)) + if actualWatchEventsHasDelete == false { + _ = dc.Resource(resourceType).Namespace(namespace).DeleteCollection(testContext, metav1.DeleteOptions{}, listOptions) + } // TODO restructure failures handling if errs.Len() > 0 && try < retries { - fmt.Errorf("invariants violated:\n* %s", strings.Join(errs.List(), "\n* ")) - // TODO delete resources via DynamicClient (on failure) + fmt.Println(fmt.Errorf("invariants violated:\n* %s", strings.Join(errs.List(), "\n* "))) continue retriesLoop } ExpectEqual(errs.Len() > 0, false, fmt.Errorf("invariants violated:\n* %s", strings.Join(errs.List(), "\n* "))) - // TODO delete resources via DynamicClient (on failure) + ExpectEqual(totalValidWatchEvents, len(expectedWatchEvents), "Error: there must be an equal amount of total valid watch events (%v) and expected watch events (%v)", totalValidWatchEvents, len(expectedWatchEvents)) } } From b9c934102b3a355fcd8af1603fcf40ed422cd269 Mon Sep 17 00:00:00 2001 From: Caleb Woodbine Date: Mon, 25 May 2020 15:27:32 +1200 Subject: [PATCH 08/22] Update default retrywatcher resource version --- test/e2e/framework/util.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index 8ad49cb5346..d1f3887d9c8 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -1302,7 +1302,7 @@ func WatchEventSequenceVerifier(testContext context.Context, dc dynamic.Interfac return dc.Resource(resourceType).Namespace(namespace).Watch(testContext, listOptions) }, } - resourceWatch, err := watchtools.NewRetryWatcher("", listWatcher) + resourceWatch, err := watchtools.NewRetryWatcher("1", listWatcher) ExpectNoError(err, "Failed to create a resource watch of %v in namespace %v", resourceType.Resource, namespace) // NOTE value of 3 retries seems to make sense From 9a77a00c7c85aeb506327ba0969123d2f0fc6ae1 Mon Sep 17 00:00:00 2001 From: Caleb Woodbine Date: Wed, 27 May 2020 08:34:03 +1200 Subject: [PATCH 09/22] Fix formatting --- test/e2e/framework/util.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index d1f3887d9c8..dc341dd97e3 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -1315,10 +1315,10 @@ retriesLoop: // watchEventsLoop: totalValidWatchEvents := 0 actualWatchEventsHasDelete := false - for watchEventIndex, _ := range expectedWatchEvents { + for watchEventIndex := range expectedWatchEvents { foundExpectedWatchEvent := false ExpectEqual(len(expectedWatchEvents) <= len(actualWatchEvents), true, "Error: actual watch events amount must be greater than or equal to expected watch events amount") - for actualWatchEventIndex, _ := range actualWatchEvents { + for actualWatchEventIndex := range actualWatchEvents { if actualWatchEvents[watchEventIndex].Type == expectedWatchEvents[actualWatchEventIndex].Type { foundExpectedWatchEvent = true } @@ -1329,7 +1329,7 @@ retriesLoop: if foundExpectedWatchEvent == false { errs.Insert(fmt.Sprintf("Watch event %v not found", expectedWatchEvents[watchEventIndex].Type)) } - totalValidWatchEvents ++ + totalValidWatchEvents++ } if actualWatchEventsHasDelete == false { _ = dc.Resource(resourceType).Namespace(namespace).DeleteCollection(testContext, metav1.DeleteOptions{}, listOptions) From 250bb35041ef885927e4d930a575946a522cdf22 Mon Sep 17 00:00:00 2001 From: Caleb Woodbine Date: Wed, 27 May 2020 12:09:02 +1200 Subject: [PATCH 10/22] Update documentation --- test/e2e/framework/util.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index dc341dd97e3..3ce564189ff 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -1286,7 +1286,7 @@ func taintExists(taints []v1.Taint, taintToFind *v1.Taint) bool { return false } -// WatchEventSequenceVerifier +// WatchEventSequenceVerifier ... // manages a watch for a given resource, ensures that events take place in a given order, retries the test on failure // testContext cancelation signal across API boundries, e.g: context.TODO() // dc sets up a client to the API From 1db0ca74a931edc10f82a856fe205359a888031a Mon Sep 17 00:00:00 2001 From: Caleb Woodbine Date: Wed, 27 May 2020 16:11:41 +1200 Subject: [PATCH 11/22] Update correct resource version used, watch retry function to not close --- test/e2e/framework/util.go | 49 ++++++++++++++++++++++++++++++++++---- 1 file changed, 45 insertions(+), 4 deletions(-) diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index 3ce564189ff..133b42273f5 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -1297,12 +1297,14 @@ func taintExists(taints []v1.Taint, taintToFind *v1.Taint) bool { // expectedWatchEvents array of events which are expected to occur // scenario the function to run func WatchEventSequenceVerifier(testContext context.Context, dc dynamic.Interface, resourceType schema.GroupVersionResource, namespace string, resourceName string, listOptions metav1.ListOptions, expectedWatchEvents []watch.Event, scenario func(*watchtools.RetryWatcher) []watch.Event) { + initResource, err := dc.Resource(resourceType).Namespace(namespace).List(testContext, listOptions) + ExpectNoError(err, "Failed to fetch inital resource") listWatcher := &cache.ListWatch{ WatchFunc: func(listOptions metav1.ListOptions) (watch.Interface, error) { return dc.Resource(resourceType).Namespace(namespace).Watch(testContext, listOptions) }, } - resourceWatch, err := watchtools.NewRetryWatcher("1", listWatcher) + resourceWatch, err := watchtools.NewRetryWatcher(initResource.GetResourceVersion(), listWatcher) ExpectNoError(err, "Failed to create a resource watch of %v in namespace %v", resourceType.Resource, namespace) // NOTE value of 3 retries seems to make sense @@ -1334,12 +1336,51 @@ retriesLoop: if actualWatchEventsHasDelete == false { _ = dc.Resource(resourceType).Namespace(namespace).DeleteCollection(testContext, metav1.DeleteOptions{}, listOptions) } - // TODO restructure failures handling if errs.Len() > 0 && try < retries { - fmt.Println(fmt.Errorf("invariants violated:\n* %s", strings.Join(errs.List(), "\n* "))) + fmt.Println("invariants violated:\n", strings.Join(errs.List(), "\n - ")) continue retriesLoop } - ExpectEqual(errs.Len() > 0, false, fmt.Errorf("invariants violated:\n* %s", strings.Join(errs.List(), "\n* "))) + ExpectEqual(errs.Len() > 0, false, strings.Join(errs.List(), "\n - ")) ExpectEqual(totalValidWatchEvents, len(expectedWatchEvents), "Error: there must be an equal amount of total valid watch events (%v) and expected watch events (%v)", totalValidWatchEvents, len(expectedWatchEvents)) + break retriesLoop } } + +func WatchUntilWithoutRetry(ctx context.Context, watcher watch.Interface, conditions ...watchtools.ConditionFunc) (*watch.Event, error) { + ch := watcher.ResultChan() + var lastEvent *watch.Event + for _, condition := range conditions { + // check the next condition against the previous event and short circuit waiting for the next watch + if lastEvent != nil { + done, err := condition(*lastEvent) + if err != nil { + return lastEvent, err + } + if done { + continue + } + } + ConditionSucceeded: + for { + select { + case event, ok := <-ch: + if !ok { + return lastEvent, watchtools.ErrWatchClosed + } + lastEvent = &event + + done, err := condition(event) + if err != nil { + return lastEvent, err + } + if done { + break ConditionSucceeded + } + + case <-ctx.Done(): + return lastEvent, wait.ErrWaitTimeout + } + } + } + return lastEvent, nil +} From cd314c193c114efd9380e25989c51fb3c89e30d0 Mon Sep 17 00:00:00 2001 From: Caleb Woodbine Date: Thu, 28 May 2020 08:36:34 +1200 Subject: [PATCH 12/22] Fix linting --- test/e2e/framework/util.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index 133b42273f5..83ab8dd318b 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -1298,7 +1298,7 @@ func taintExists(taints []v1.Taint, taintToFind *v1.Taint) bool { // scenario the function to run func WatchEventSequenceVerifier(testContext context.Context, dc dynamic.Interface, resourceType schema.GroupVersionResource, namespace string, resourceName string, listOptions metav1.ListOptions, expectedWatchEvents []watch.Event, scenario func(*watchtools.RetryWatcher) []watch.Event) { initResource, err := dc.Resource(resourceType).Namespace(namespace).List(testContext, listOptions) - ExpectNoError(err, "Failed to fetch inital resource") + ExpectNoError(err, "Failed to fetch initial resource") listWatcher := &cache.ListWatch{ WatchFunc: func(listOptions metav1.ListOptions) (watch.Interface, error) { return dc.Resource(resourceType).Namespace(namespace).Watch(testContext, listOptions) @@ -1346,6 +1346,14 @@ retriesLoop: } } +// WatchUntilWithoutRetry ... +// reads items from the watch until each provided condition succeeds, and then returns the last watch +// encountered. The first condition that returns an error terminates the watch (and the event is also returned). +// If no event has been received, the returned event will be nil. +// Conditions are satisfied sequentially so as to provide a useful primitive for higher level composition. +// Waits until context deadline or until context is canceled. +// +// the same as watchtools.UntilWithoutRetry, just without the closing of the watch func WatchUntilWithoutRetry(ctx context.Context, watcher watch.Interface, conditions ...watchtools.ConditionFunc) (*watch.Event, error) { ch := watcher.ResultChan() var lastEvent *watch.Event From 8945d98805bb9c5c5282b4bc88b0d6ae7e709f8f Mon Sep 17 00:00:00 2001 From: Caleb Woodbine Date: Wed, 3 Jun 2020 11:36:21 +1200 Subject: [PATCH 13/22] Update desc, naming, cleanup handling --- test/e2e/framework/util.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index 83ab8dd318b..7073ddd2f48 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -1295,13 +1295,14 @@ func taintExists(taints []v1.Taint, taintToFind *v1.Taint) bool { // resourceName the name of the given resource // listOptions options used to find the resource, recommended to use listOptions.labelSelector // expectedWatchEvents array of events which are expected to occur -// scenario the function to run -func WatchEventSequenceVerifier(testContext context.Context, dc dynamic.Interface, resourceType schema.GroupVersionResource, namespace string, resourceName string, listOptions metav1.ListOptions, expectedWatchEvents []watch.Event, scenario func(*watchtools.RetryWatcher) []watch.Event) { - initResource, err := dc.Resource(resourceType).Namespace(namespace).List(testContext, listOptions) +// scenario the test itself +// retryCleanup a function to run which ensures that there are no dangling resources upon test failure +func WatchEventSequenceVerifier(ctx context.Context, dc dynamic.Interface, resourceType schema.GroupVersionResource, namespace string, resourceName string, listOptions metav1.ListOptions, expectedWatchEvents []watch.Event, scenario func(*watchtools.RetryWatcher) []watch.Event, retryCleanup func() error) { + initResource, err := dc.Resource(resourceType).Namespace(namespace).List(ctx, listOptions) ExpectNoError(err, "Failed to fetch initial resource") listWatcher := &cache.ListWatch{ WatchFunc: func(listOptions metav1.ListOptions) (watch.Interface, error) { - return dc.Resource(resourceType).Namespace(namespace).Watch(testContext, listOptions) + return dc.Resource(resourceType).Namespace(namespace).Watch(ctx, listOptions) }, } resourceWatch, err := watchtools.NewRetryWatcher(initResource.GetResourceVersion(), listWatcher) @@ -1319,7 +1320,7 @@ retriesLoop: actualWatchEventsHasDelete := false for watchEventIndex := range expectedWatchEvents { foundExpectedWatchEvent := false - ExpectEqual(len(expectedWatchEvents) <= len(actualWatchEvents), true, "Error: actual watch events amount must be greater than or equal to expected watch events amount") + ExpectEqual(len(expectedWatchEvents) <= len(actualWatchEvents), true, "Error: actual watch events amount (%d) must be greater than or equal to expected watch events amount (%d)", len(actualWatchEvents), len(expectedWatchEvents)) for actualWatchEventIndex := range actualWatchEvents { if actualWatchEvents[watchEventIndex].Type == expectedWatchEvents[actualWatchEventIndex].Type { foundExpectedWatchEvent = true @@ -1333,15 +1334,14 @@ retriesLoop: } totalValidWatchEvents++ } - if actualWatchEventsHasDelete == false { - _ = dc.Resource(resourceType).Namespace(namespace).DeleteCollection(testContext, metav1.DeleteOptions{}, listOptions) - } + err := retryCleanup() + ExpectNoError(err, "Error occured when cleaning up resources") if errs.Len() > 0 && try < retries { fmt.Println("invariants violated:\n", strings.Join(errs.List(), "\n - ")) continue retriesLoop } ExpectEqual(errs.Len() > 0, false, strings.Join(errs.List(), "\n - ")) - ExpectEqual(totalValidWatchEvents, len(expectedWatchEvents), "Error: there must be an equal amount of total valid watch events (%v) and expected watch events (%v)", totalValidWatchEvents, len(expectedWatchEvents)) + ExpectEqual(totalValidWatchEvents, len(expectedWatchEvents), "Error: there must be an equal amount of total valid watch events (%d) and expected watch events (%d)", totalValidWatchEvents, len(expectedWatchEvents)) break retriesLoop } } @@ -1353,7 +1353,7 @@ retriesLoop: // Conditions are satisfied sequentially so as to provide a useful primitive for higher level composition. // Waits until context deadline or until context is canceled. // -// the same as watchtools.UntilWithoutRetry, just without the closing of the watch +// the same as watchtools.UntilWithoutRetry, just without the closing of the watch - as for the purpose of being paired with WatchEventSequenceVerifier, the watch is needed for continual watch event collection func WatchUntilWithoutRetry(ctx context.Context, watcher watch.Interface, conditions ...watchtools.ConditionFunc) (*watch.Event, error) { ch := watcher.ResultChan() var lastEvent *watch.Event From 63c694acdf674fa6e078570f80f4a2d933fc5c59 Mon Sep 17 00:00:00 2001 From: Caleb Woodbine Date: Thu, 4 Jun 2020 08:57:28 +1200 Subject: [PATCH 14/22] Fix verify --- test/e2e/framework/util.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index 7073ddd2f48..3b82aaea797 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -1296,7 +1296,7 @@ func taintExists(taints []v1.Taint, taintToFind *v1.Taint) bool { // listOptions options used to find the resource, recommended to use listOptions.labelSelector // expectedWatchEvents array of events which are expected to occur // scenario the test itself -// retryCleanup a function to run which ensures that there are no dangling resources upon test failure +// retryCleanup a function to run which ensures that there are no dangling resources upon test failure func WatchEventSequenceVerifier(ctx context.Context, dc dynamic.Interface, resourceType schema.GroupVersionResource, namespace string, resourceName string, listOptions metav1.ListOptions, expectedWatchEvents []watch.Event, scenario func(*watchtools.RetryWatcher) []watch.Event, retryCleanup func() error) { initResource, err := dc.Resource(resourceType).Namespace(namespace).List(ctx, listOptions) ExpectNoError(err, "Failed to fetch initial resource") @@ -1335,7 +1335,7 @@ retriesLoop: totalValidWatchEvents++ } err := retryCleanup() - ExpectNoError(err, "Error occured when cleaning up resources") + ExpectNoError(err, "Error occurred when cleaning up resources") if errs.Len() > 0 && try < retries { fmt.Println("invariants violated:\n", strings.Join(errs.List(), "\n - ")) continue retriesLoop From 39fd803140c6dbec731b5ed28bf8e203f062d4a1 Mon Sep 17 00:00:00 2001 From: Caleb Woodbine Date: Mon, 8 Jun 2020 09:29:28 +1200 Subject: [PATCH 15/22] Move watch creating into the retry loop --- test/e2e/framework/util.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index 3b82aaea797..e7a56d68e31 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -1298,20 +1298,22 @@ func taintExists(taints []v1.Taint, taintToFind *v1.Taint) bool { // scenario the test itself // retryCleanup a function to run which ensures that there are no dangling resources upon test failure func WatchEventSequenceVerifier(ctx context.Context, dc dynamic.Interface, resourceType schema.GroupVersionResource, namespace string, resourceName string, listOptions metav1.ListOptions, expectedWatchEvents []watch.Event, scenario func(*watchtools.RetryWatcher) []watch.Event, retryCleanup func() error) { - initResource, err := dc.Resource(resourceType).Namespace(namespace).List(ctx, listOptions) - ExpectNoError(err, "Failed to fetch initial resource") listWatcher := &cache.ListWatch{ WatchFunc: func(listOptions metav1.ListOptions) (watch.Interface, error) { return dc.Resource(resourceType).Namespace(namespace).Watch(ctx, listOptions) }, } - resourceWatch, err := watchtools.NewRetryWatcher(initResource.GetResourceVersion(), listWatcher) - ExpectNoError(err, "Failed to create a resource watch of %v in namespace %v", resourceType.Resource, namespace) // NOTE value of 3 retries seems to make sense retries := 3 retriesLoop: for try := 1; try <= retries; try++ { + initResource, err := dc.Resource(resourceType).Namespace(namespace).List(ctx, listOptions) + ExpectNoError(err, "Failed to fetch initial resource") + + resourceWatch, err := watchtools.NewRetryWatcher(initResource.GetResourceVersion(), listWatcher) + ExpectNoError(err, "Failed to create a resource watch of %v in namespace %v", resourceType.Resource, namespace) + // NOTE the test may need access to the events to see what's going on, such as a change in status actualWatchEvents := scenario(resourceWatch) errs := sets.NewString() @@ -1334,7 +1336,7 @@ retriesLoop: } totalValidWatchEvents++ } - err := retryCleanup() + err = retryCleanup() ExpectNoError(err, "Error occurred when cleaning up resources") if errs.Len() > 0 && try < retries { fmt.Println("invariants violated:\n", strings.Join(errs.List(), "\n - ")) From 5268ba5488c5221e5d9d5cacce19cfcf7d932081 Mon Sep 17 00:00:00 2001 From: Caleb Woodbine Date: Wed, 10 Jun 2020 10:51:39 +1200 Subject: [PATCH 16/22] Ensure events occur regardless of the events in between --- test/e2e/framework/util.go | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index e7a56d68e31..ec6271c2454 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -1317,22 +1317,26 @@ retriesLoop: // NOTE the test may need access to the events to see what's going on, such as a change in status actualWatchEvents := scenario(resourceWatch) errs := sets.NewString() - // watchEventsLoop: + ExpectEqual(len(expectedWatchEvents) <= len(actualWatchEvents), true, "Error: actual watch events amount (%d) must be greater than or equal to expected watch events amount (%d)", len(actualWatchEvents), len(expectedWatchEvents)) + totalValidWatchEvents := 0 - actualWatchEventsHasDelete := false - for watchEventIndex := range expectedWatchEvents { + foundEventIndexes := map[int]*int{} + + for watchEventIndex, expectedWatchEvent := range expectedWatchEvents { foundExpectedWatchEvent := false - ExpectEqual(len(expectedWatchEvents) <= len(actualWatchEvents), true, "Error: actual watch events amount (%d) must be greater than or equal to expected watch events amount (%d)", len(actualWatchEvents), len(expectedWatchEvents)) - for actualWatchEventIndex := range actualWatchEvents { - if actualWatchEvents[watchEventIndex].Type == expectedWatchEvents[actualWatchEventIndex].Type { - foundExpectedWatchEvent = true + actualWatchEventsLoop: + for actualWatchEventIndex, actualWatchEvent := range actualWatchEvents { + if foundEventIndexes[actualWatchEventIndex] != nil { + continue actualWatchEventsLoop } - if actualWatchEvents[actualWatchEventIndex].Type == watch.Deleted && actualWatchEventsHasDelete != true { - actualWatchEventsHasDelete = true + if actualWatchEvent.Type == expectedWatchEvent.Type { + foundExpectedWatchEvent = true + foundEventIndexes[actualWatchEventIndex] = &watchEventIndex + break actualWatchEventsLoop } } if foundExpectedWatchEvent == false { - errs.Insert(fmt.Sprintf("Watch event %v not found", expectedWatchEvents[watchEventIndex].Type)) + errs.Insert(fmt.Sprintf("Watch event %v not found", expectedWatchEvent.Type)) } totalValidWatchEvents++ } From e8b70ce0da1bf3e91b843f7e8f5d4841d27cab0d Mon Sep 17 00:00:00 2001 From: Caleb Woodbine Date: Thu, 11 Jun 2020 11:08:24 +1200 Subject: [PATCH 17/22] Update docs --- test/e2e/framework/util.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index ec6271c2454..54722dd2a78 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -1297,6 +1297,10 @@ func taintExists(taints []v1.Taint, taintToFind *v1.Taint) bool { // expectedWatchEvents array of events which are expected to occur // scenario the test itself // retryCleanup a function to run which ensures that there are no dangling resources upon test failure +// this tooling relies on the test to return the events as they occur +// the entire scenario must be run to ensure that the desired watch events arrive in order (allowing for interweaving of watch events) +// if an expected watch event is missing we elect to clean up and run the entire scenario again +// we try the scenario three times to allow the sequencing to fail a couple of times func WatchEventSequenceVerifier(ctx context.Context, dc dynamic.Interface, resourceType schema.GroupVersionResource, namespace string, resourceName string, listOptions metav1.ListOptions, expectedWatchEvents []watch.Event, scenario func(*watchtools.RetryWatcher) []watch.Event, retryCleanup func() error) { listWatcher := &cache.ListWatch{ WatchFunc: func(listOptions metav1.ListOptions) (watch.Interface, error) { @@ -1304,7 +1308,6 @@ func WatchEventSequenceVerifier(ctx context.Context, dc dynamic.Interface, resou }, } - // NOTE value of 3 retries seems to make sense retries := 3 retriesLoop: for try := 1; try <= retries; try++ { From 0c8ae2abf3d60a43b1987760a8fbfd254ca3f3d5 Mon Sep 17 00:00:00 2001 From: Caleb Woodbine Date: Wed, 1 Apr 2020 12:12:45 +1300 Subject: [PATCH 18/22] Update and improve ConfigMap resource lifecycle test --- test/e2e/common/configmap.go | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/test/e2e/common/configmap.go b/test/e2e/common/configmap.go index 106bd4fc40d..c13fafe07b0 100644 --- a/test/e2e/common/configmap.go +++ b/test/e2e/common/configmap.go @@ -25,6 +25,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/uuid" + watch "k8s.io/apimachinery/pkg/watch" "k8s.io/kubernetes/test/e2e/framework" imageutils "k8s.io/kubernetes/test/utils/image" @@ -161,6 +162,7 @@ var _ = ginkgo.Describe("[sig-node] ConfigMap", func() { testNamespaceName := f.Namespace.Name testConfigMapName := "test-configmap" + string(uuid.NewUUID()) + ginkgo.By("creating a ConfigMap") _, err := f.ClientSet.CoreV1().ConfigMaps(testNamespaceName).Create(context.TODO(), &v1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Name: testConfigMapName, @@ -174,6 +176,20 @@ var _ = ginkgo.Describe("[sig-node] ConfigMap", func() { }, metav1.CreateOptions{}) framework.ExpectNoError(err, "failed to create ConfigMap") + ginkgo.By("setting a watch for the ConfigMap") + // setup a watch for the ConfigMap + resourceWatchTimeoutSeconds := int64(60) + resourceWatch, err := f.ClientSet.CoreV1().ConfigMaps(testNamespaceName).Watch(context.TODO(), metav1.ListOptions{LabelSelector: "test-configmap-static=true", TimeoutSeconds: &resourceWatchTimeoutSeconds}) + framework.ExpectNoError(err, "Failed to setup watch on newly created ConfigMap") + + resourceWatchChan := resourceWatch.ResultChan() + ginkgo.By("waiting for the ConfigMap to be added") + for watchEvent := range resourceWatchChan { + if watchEvent.Type == watch.Added { + break + } + } + configMapPatchPayload, err := json.Marshal(v1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ @@ -186,15 +202,23 @@ var _ = ginkgo.Describe("[sig-node] ConfigMap", func() { }) framework.ExpectNoError(err, "failed to marshal patch data") + ginkgo.By("patching the ConfigMap") _, err = f.ClientSet.CoreV1().ConfigMaps(testNamespaceName).Patch(context.TODO(), testConfigMapName, types.StrategicMergePatchType, []byte(configMapPatchPayload), metav1.PatchOptions{}) framework.ExpectNoError(err, "failed to patch ConfigMap") + ginkgo.By("waiting for the ConfigMap to be modified") + for watchEvent := range resourceWatchChan { + if watchEvent.Type == watch.Modified { + break + } + } + ginkgo.By("fetching the ConfigMap") configMap, err := f.ClientSet.CoreV1().ConfigMaps(testNamespaceName).Get(context.TODO(), testConfigMapName, metav1.GetOptions{}) framework.ExpectNoError(err, "failed to get ConfigMap") framework.ExpectEqual(configMap.Data["valueName"], "value1", "failed to patch ConfigMap") framework.ExpectEqual(configMap.Labels["test-configmap"], "patched", "failed to patch ConfigMap") - // listing in all namespaces to hit the endpoint + ginkgo.By("listing all ConfigMaps in all namespaces") configMapList, err := f.ClientSet.CoreV1().ConfigMaps("").List(context.TODO(), metav1.ListOptions{ LabelSelector: "test-configmap-static=true", }) @@ -212,10 +236,17 @@ var _ = ginkgo.Describe("[sig-node] ConfigMap", func() { } framework.ExpectEqual(testConfigMapFound, true, "failed to find ConfigMap in list") + ginkgo.By("deleting the ConfigMap by a collection") err = f.ClientSet.CoreV1().ConfigMaps(testNamespaceName).DeleteCollection(context.TODO(), metav1.DeleteOptions{}, metav1.ListOptions{ LabelSelector: "test-configmap-static=true", }) framework.ExpectNoError(err, "failed to delete ConfigMap collection with LabelSelector") + ginkgo.By("waiting for the ConfigMap to be deleted") + for watchEvent := range resourceWatchChan { + if watchEvent.Type == watch.Deleted { + break + } + } }) }) From f0757ecd773e8c657e2e06cddbfd03e9e9fd9c59 Mon Sep 17 00:00:00 2001 From: Caleb Woodbine Date: Wed, 1 Apr 2020 16:19:34 +1300 Subject: [PATCH 19/22] Move watchTimeout value to the top and add comments --- test/e2e/common/configmap.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/e2e/common/configmap.go b/test/e2e/common/configmap.go index c13fafe07b0..155ef5f5269 100644 --- a/test/e2e/common/configmap.go +++ b/test/e2e/common/configmap.go @@ -32,6 +32,11 @@ import ( "github.com/onsi/ginkgo" ) +var ( + // tests which use this appear to all pass within the given time + generalWatchTimeout = int64(60) +) + var _ = ginkgo.Describe("[sig-node] ConfigMap", func() { f := framework.NewDefaultFramework("configmap") @@ -178,8 +183,7 @@ var _ = ginkgo.Describe("[sig-node] ConfigMap", func() { ginkgo.By("setting a watch for the ConfigMap") // setup a watch for the ConfigMap - resourceWatchTimeoutSeconds := int64(60) - resourceWatch, err := f.ClientSet.CoreV1().ConfigMaps(testNamespaceName).Watch(context.TODO(), metav1.ListOptions{LabelSelector: "test-configmap-static=true", TimeoutSeconds: &resourceWatchTimeoutSeconds}) + resourceWatch, err := f.ClientSet.CoreV1().ConfigMaps(testNamespaceName).Watch(context.TODO(), metav1.ListOptions{LabelSelector: "test-configmap-static=true", TimeoutSeconds: &generalWatchTimeout}) framework.ExpectNoError(err, "Failed to setup watch on newly created ConfigMap") resourceWatchChan := resourceWatch.ResultChan() @@ -246,6 +250,7 @@ var _ = ginkgo.Describe("[sig-node] ConfigMap", func() { if watchEvent.Type == watch.Deleted { break } + fmt.Println("failed to find Deleted watchEvent") } }) }) From b52740c44f63d5aa62e2ae324102fed2a5ef81e2 Mon Sep 17 00:00:00 2001 From: Caleb Woodbine Date: Wed, 6 May 2020 11:04:04 +1200 Subject: [PATCH 20/22] Add watch event checks --- test/e2e/common/configmap.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/e2e/common/configmap.go b/test/e2e/common/configmap.go index 155ef5f5269..59bc8e9eef6 100644 --- a/test/e2e/common/configmap.go +++ b/test/e2e/common/configmap.go @@ -188,11 +188,14 @@ var _ = ginkgo.Describe("[sig-node] ConfigMap", func() { resourceWatchChan := resourceWatch.ResultChan() ginkgo.By("waiting for the ConfigMap to be added") + foundWatchEvent := false for watchEvent := range resourceWatchChan { if watchEvent.Type == watch.Added { + foundWatchEvent = true break } } + framework.ExpectEqual(true, foundWatchEvent, "expected to find a watch.Delete event configmap %s", testConfigMapName) configMapPatchPayload, err := json.Marshal(v1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ @@ -210,11 +213,14 @@ var _ = ginkgo.Describe("[sig-node] ConfigMap", func() { _, err = f.ClientSet.CoreV1().ConfigMaps(testNamespaceName).Patch(context.TODO(), testConfigMapName, types.StrategicMergePatchType, []byte(configMapPatchPayload), metav1.PatchOptions{}) framework.ExpectNoError(err, "failed to patch ConfigMap") ginkgo.By("waiting for the ConfigMap to be modified") + foundWatchEvent = false for watchEvent := range resourceWatchChan { if watchEvent.Type == watch.Modified { + foundWatchEvent = true break } } + framework.ExpectEqual(true, foundWatchEvent, "expected to find a watch.Modified event configmap %s", testConfigMapName) ginkgo.By("fetching the ConfigMap") configMap, err := f.ClientSet.CoreV1().ConfigMaps(testNamespaceName).Get(context.TODO(), testConfigMapName, metav1.GetOptions{}) @@ -246,12 +252,14 @@ var _ = ginkgo.Describe("[sig-node] ConfigMap", func() { }) framework.ExpectNoError(err, "failed to delete ConfigMap collection with LabelSelector") ginkgo.By("waiting for the ConfigMap to be deleted") + foundWatchEvent = false for watchEvent := range resourceWatchChan { if watchEvent.Type == watch.Deleted { + foundWatchEvent = true break } - fmt.Println("failed to find Deleted watchEvent") } + framework.ExpectEqual(true, foundWatchEvent, "expected to find a watch.Deleted event configmap %s", testConfigMapName) }) }) From 34988cfe0e41b4e2efbeaa82b5a53b7508103aa6 Mon Sep 17 00:00:00 2001 From: Caleb Woodbine Date: Thu, 28 May 2020 09:46:31 +1200 Subject: [PATCH 21/22] Update to include watch tooling --- test/e2e/common/configmap.go | 181 ++++++++++++++++++++--------------- 1 file changed, 105 insertions(+), 76 deletions(-) diff --git a/test/e2e/common/configmap.go b/test/e2e/common/configmap.go index 59bc8e9eef6..e35e743dab5 100644 --- a/test/e2e/common/configmap.go +++ b/test/e2e/common/configmap.go @@ -20,12 +20,16 @@ import ( "context" "encoding/json" "fmt" + "time" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/uuid" watch "k8s.io/apimachinery/pkg/watch" + "k8s.io/client-go/dynamic" + watchtools "k8s.io/client-go/tools/watch" "k8s.io/kubernetes/test/e2e/framework" imageutils "k8s.io/kubernetes/test/utils/image" @@ -40,6 +44,12 @@ var ( var _ = ginkgo.Describe("[sig-node] ConfigMap", func() { f := framework.NewDefaultFramework("configmap") + var dc dynamic.Interface + + ginkgo.BeforeEach(func() { + dc = f.DynamicClient + }) + /* Release : v1.9 Testname: ConfigMap, from environment field @@ -167,8 +177,13 @@ var _ = ginkgo.Describe("[sig-node] ConfigMap", func() { testNamespaceName := f.Namespace.Name testConfigMapName := "test-configmap" + string(uuid.NewUUID()) - ginkgo.By("creating a ConfigMap") - _, err := f.ClientSet.CoreV1().ConfigMaps(testNamespaceName).Create(context.TODO(), &v1.ConfigMap{ + configMapResource := schema.GroupVersionResource{Group: "", Version: "v1", Resource: "configmaps"} + expectedWatchEvents := []watch.Event{ + {Type: watch.Added}, + {Type: watch.Modified}, + {Type: watch.Deleted}, + } + testConfigMap := v1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Name: testConfigMapName, Labels: map[string]string{ @@ -178,88 +193,102 @@ var _ = ginkgo.Describe("[sig-node] ConfigMap", func() { Data: map[string]string{ "valueName": "value", }, - }, metav1.CreateOptions{}) - framework.ExpectNoError(err, "failed to create ConfigMap") - - ginkgo.By("setting a watch for the ConfigMap") - // setup a watch for the ConfigMap - resourceWatch, err := f.ClientSet.CoreV1().ConfigMaps(testNamespaceName).Watch(context.TODO(), metav1.ListOptions{LabelSelector: "test-configmap-static=true", TimeoutSeconds: &generalWatchTimeout}) - framework.ExpectNoError(err, "Failed to setup watch on newly created ConfigMap") - - resourceWatchChan := resourceWatch.ResultChan() - ginkgo.By("waiting for the ConfigMap to be added") - foundWatchEvent := false - for watchEvent := range resourceWatchChan { - if watchEvent.Type == watch.Added { - foundWatchEvent = true - break - } } - framework.ExpectEqual(true, foundWatchEvent, "expected to find a watch.Delete event configmap %s", testConfigMapName) - configMapPatchPayload, err := json.Marshal(v1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - "test-configmap": "patched", + framework.WatchEventSequenceVerifier(context.TODO(), dc, configMapResource, testNamespaceName, testConfigMapName, metav1.ListOptions{LabelSelector: "test-configmap-static=true"}, expectedWatchEvents, func(retryWatcher *watchtools.RetryWatcher) (actualWatchEvents []watch.Event) { + ginkgo.By("creating a ConfigMap") + _, err := f.ClientSet.CoreV1().ConfigMaps(testNamespaceName).Create(context.TODO(), &testConfigMap, metav1.CreateOptions{}) + framework.ExpectNoError(err, "failed to create ConfigMap") + eventFound := false + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + _, err = framework.WatchUntilWithoutRetry(ctx, retryWatcher, func(watchEvent watch.Event) (bool, error) { + if watchEvent.Type != watch.Added { + return false, nil + } + actualWatchEvents = append(actualWatchEvents, watchEvent) + eventFound = true + return true, nil + }) + framework.ExpectNoError(err, "Wait until condition with watch events should not return an error") + framework.ExpectEqual(eventFound, true, "failed to find ConfigMap %v event", watch.Added) + + configMapPatchPayload, err := json.Marshal(v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "test-configmap": "patched", + }, }, - }, - Data: map[string]string{ - "valueName": "value1", - }, - }) - framework.ExpectNoError(err, "failed to marshal patch data") + Data: map[string]string{ + "valueName": "value1", + }, + }) + framework.ExpectNoError(err, "failed to marshal patch data") - ginkgo.By("patching the ConfigMap") - _, err = f.ClientSet.CoreV1().ConfigMaps(testNamespaceName).Patch(context.TODO(), testConfigMapName, types.StrategicMergePatchType, []byte(configMapPatchPayload), metav1.PatchOptions{}) - framework.ExpectNoError(err, "failed to patch ConfigMap") - ginkgo.By("waiting for the ConfigMap to be modified") - foundWatchEvent = false - for watchEvent := range resourceWatchChan { - if watchEvent.Type == watch.Modified { - foundWatchEvent = true - break + ginkgo.By("patching the ConfigMap") + _, err = f.ClientSet.CoreV1().ConfigMaps(testNamespaceName).Patch(context.TODO(), testConfigMapName, types.StrategicMergePatchType, []byte(configMapPatchPayload), metav1.PatchOptions{}) + framework.ExpectNoError(err, "failed to patch ConfigMap") + ginkgo.By("waiting for the ConfigMap to be modified") + eventFound = false + ctx, cancel = context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + _, err = framework.WatchUntilWithoutRetry(ctx, retryWatcher, func(watchEvent watch.Event) (bool, error) { + if watchEvent.Type != watch.Modified { + return false, nil + } + actualWatchEvents = append(actualWatchEvents, watchEvent) + eventFound = true + return true, nil + }) + framework.ExpectNoError(err, "Wait until condition with watch events should not return an error") + framework.ExpectEqual(eventFound, true, "failed to find ConfigMap %v event", watch.Modified) + + ginkgo.By("fetching the ConfigMap") + configMap, err := f.ClientSet.CoreV1().ConfigMaps(testNamespaceName).Get(context.TODO(), testConfigMapName, metav1.GetOptions{}) + framework.ExpectNoError(err, "failed to get ConfigMap") + framework.ExpectEqual(configMap.Data["valueName"], "value1", "failed to patch ConfigMap") + framework.ExpectEqual(configMap.Labels["test-configmap"], "patched", "failed to patch ConfigMap") + + ginkgo.By("listing all ConfigMaps in all namespaces") + configMapList, err := f.ClientSet.CoreV1().ConfigMaps("").List(context.TODO(), metav1.ListOptions{ + LabelSelector: "test-configmap-static=true", + }) + framework.ExpectNoError(err, "failed to list ConfigMaps with LabelSelector") + framework.ExpectNotEqual(len(configMapList.Items), 0, "no ConfigMaps found in ConfigMap list") + testConfigMapFound := false + for _, cm := range configMapList.Items { + if cm.ObjectMeta.Name == testConfigMapName && + cm.ObjectMeta.Namespace == testNamespaceName && + cm.ObjectMeta.Labels["test-configmap-static"] == "true" && + cm.Data["valueName"] == "value1" { + testConfigMapFound = true + break + } } - } - framework.ExpectEqual(true, foundWatchEvent, "expected to find a watch.Modified event configmap %s", testConfigMapName) + framework.ExpectEqual(testConfigMapFound, true, "failed to find ConfigMap in list") - ginkgo.By("fetching the ConfigMap") - configMap, err := f.ClientSet.CoreV1().ConfigMaps(testNamespaceName).Get(context.TODO(), testConfigMapName, metav1.GetOptions{}) - framework.ExpectNoError(err, "failed to get ConfigMap") - framework.ExpectEqual(configMap.Data["valueName"], "value1", "failed to patch ConfigMap") - framework.ExpectEqual(configMap.Labels["test-configmap"], "patched", "failed to patch ConfigMap") + ginkgo.By("deleting the ConfigMap by a collection") + err = f.ClientSet.CoreV1().ConfigMaps(testNamespaceName).DeleteCollection(context.TODO(), metav1.DeleteOptions{}, metav1.ListOptions{ + LabelSelector: "test-configmap-static=true", + }) + framework.ExpectNoError(err, "failed to delete ConfigMap collection with LabelSelector") + ginkgo.By("waiting for the ConfigMap to be deleted") + eventFound = false + ctx, cancel = context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + _, err = framework.WatchUntilWithoutRetry(ctx, retryWatcher, func(watchEvent watch.Event) (bool, error) { + if watchEvent.Type != watch.Deleted { + return false, nil + } + actualWatchEvents = append(actualWatchEvents, watchEvent) + eventFound = true + return true, nil + }) + framework.ExpectNoError(err, "Wait until condition with watch events should not return an error") + framework.ExpectEqual(eventFound, true, "failed to find ConfigMap %v event", watch.Deleted) - ginkgo.By("listing all ConfigMaps in all namespaces") - configMapList, err := f.ClientSet.CoreV1().ConfigMaps("").List(context.TODO(), metav1.ListOptions{ - LabelSelector: "test-configmap-static=true", + return actualWatchEvents }) - framework.ExpectNoError(err, "failed to list ConfigMaps with LabelSelector") - framework.ExpectNotEqual(len(configMapList.Items), 0, "no ConfigMaps found in ConfigMap list") - testConfigMapFound := false - for _, cm := range configMapList.Items { - if cm.ObjectMeta.Name == testConfigMapName && - cm.ObjectMeta.Namespace == testNamespaceName && - cm.ObjectMeta.Labels["test-configmap-static"] == "true" && - cm.Data["valueName"] == "value1" { - testConfigMapFound = true - break - } - } - framework.ExpectEqual(testConfigMapFound, true, "failed to find ConfigMap in list") - - ginkgo.By("deleting the ConfigMap by a collection") - err = f.ClientSet.CoreV1().ConfigMaps(testNamespaceName).DeleteCollection(context.TODO(), metav1.DeleteOptions{}, metav1.ListOptions{ - LabelSelector: "test-configmap-static=true", - }) - framework.ExpectNoError(err, "failed to delete ConfigMap collection with LabelSelector") - ginkgo.By("waiting for the ConfigMap to be deleted") - foundWatchEvent = false - for watchEvent := range resourceWatchChan { - if watchEvent.Type == watch.Deleted { - foundWatchEvent = true - break - } - } - framework.ExpectEqual(true, foundWatchEvent, "expected to find a watch.Deleted event configmap %s", testConfigMapName) }) }) From 34ddb648253854bbe0142a66c3ad71cb807a20c2 Mon Sep 17 00:00:00 2001 From: Caleb Woodbine Date: Thu, 4 Jun 2020 10:47:10 +1200 Subject: [PATCH 22/22] Add cleanup function; Fix build --- test/e2e/common/BUILD | 2 ++ test/e2e/common/configmap.go | 8 +++----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/e2e/common/BUILD b/test/e2e/common/BUILD index 3c642b6e247..5d8f37709a0 100644 --- a/test/e2e/common/BUILD +++ b/test/e2e/common/BUILD @@ -58,6 +58,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/fields:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/intstr:go_default_library", @@ -66,6 +67,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/util/uuid:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library", + "//staging/src/k8s.io/client-go/dynamic:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", "//staging/src/k8s.io/client-go/tools/cache:go_default_library", "//staging/src/k8s.io/client-go/tools/watch:go_default_library", diff --git a/test/e2e/common/configmap.go b/test/e2e/common/configmap.go index e35e743dab5..93311c5ee3d 100644 --- a/test/e2e/common/configmap.go +++ b/test/e2e/common/configmap.go @@ -36,11 +36,6 @@ import ( "github.com/onsi/ginkgo" ) -var ( - // tests which use this appear to all pass within the given time - generalWatchTimeout = int64(60) -) - var _ = ginkgo.Describe("[sig-node] ConfigMap", func() { f := framework.NewDefaultFramework("configmap") @@ -288,6 +283,9 @@ var _ = ginkgo.Describe("[sig-node] ConfigMap", func() { framework.ExpectEqual(eventFound, true, "failed to find ConfigMap %v event", watch.Deleted) return actualWatchEvents + }, func() (err error) { + _ = f.ClientSet.CoreV1().ConfigMaps(testNamespaceName).DeleteCollection(context.TODO(), metav1.DeleteOptions{}, metav1.ListOptions{LabelSelector: "test-configmap-static=true"}) + return err }) }) })