From 6d59fd271ea0267a97ce18f465b15395bd47ac0d Mon Sep 17 00:00:00 2001 From: Pierangelo Di Pilato Date: Tue, 22 Sep 2020 09:59:09 +0200 Subject: [PATCH 1/3] Use WaitForPodRunning instead of WaitForResourceReady on pods Signed-off-by: Pierangelo Di Pilato --- .../helpers/broker_data_plane_test_helper.go | 63 +++++++++---------- test/lib/await.go | 10 +++ 2 files changed, 39 insertions(+), 34 deletions(-) diff --git a/test/conformance/helpers/broker_data_plane_test_helper.go b/test/conformance/helpers/broker_data_plane_test_helper.go index 9facba44571..ab62875c2e0 100644 --- a/test/conformance/helpers/broker_data_plane_test_helper.go +++ b/test/conformance/helpers/broker_data_plane_test_helper.go @@ -33,11 +33,6 @@ import ( cetest "github.com/cloudevents/sdk-go/v2/test" ) -var podMeta = metav1.TypeMeta{ - Kind: "pod", - APIVersion: "v1", -} - func BrokerDataPlaneSetupHelper(ctx context.Context, client *testlib.Client, brokerClass string, brokerTestRunner testlib.ComponentsTestRunner) *eventingv1beta1.Broker { var broker *eventingv1beta1.Broker brokerName := brokerTestRunner.ComponentName @@ -79,12 +74,12 @@ func BrokerDataPlaneNamespaceSetupOption(ctx context.Context, namespace string) } } -//At ingress -//Supports CE 0.3 or CE 1.0 via HTTP -//Supports structured or Binary mode -//Respond with 2xx on good CE -//Respond with 400 on bad CE -//Reject non-POST requests to publish URI +// At ingress +// Supports CE 0.3 or CE 1.0 via HTTP +// Supports structured or Binary mode +// Respond with 2xx on good CE +// Respond with 400 on bad CE +// Reject non-POST requests to publish URI func BrokerV1Beta1IngressDataPlaneTestHelper( ctx context.Context, t *testing.T, @@ -128,7 +123,7 @@ func BrokerV1Beta1IngressDataPlaneTestHelper( cetest.HasId(eventID), cetest.HasSpecVersion("0.3"), )) - client.WaitForResourceReadyOrFail(senderName, &podMeta) + testlib.WaitForPodRunningOrFail(t, ctx, client.Kube, senderName, client.Namespace) eventTracker.AssertAtLeast(1, originalEventMatcher) }) @@ -149,7 +144,7 @@ func BrokerV1Beta1IngressDataPlaneTestHelper( cetest.HasId(eventID), cetest.HasSpecVersion("1.0"), )) - client.WaitForResourceReadyOrFail(senderName, &podMeta) + testlib.WaitForPodRunningOrFail(t, ctx, client.Kube, senderName, client.Namespace) eventTracker.AssertAtLeast(1, originalEventMatcher) }) @@ -169,7 +164,7 @@ func BrokerV1Beta1IngressDataPlaneTestHelper( originalEventMatcher := recordevents.MatchEvent(cetest.AllOf( cetest.HasId(eventID), )) - client.WaitForResourceReadyOrFail(senderName, &podMeta) + testlib.WaitForPodRunningOrFail(t, ctx, client.Kube, senderName, client.Namespace) eventTracker.AssertAtLeast(1, originalEventMatcher) }) @@ -188,7 +183,7 @@ func BrokerV1Beta1IngressDataPlaneTestHelper( originalEventMatcher := recordevents.MatchEvent(cetest.AllOf( cetest.HasId(eventID), )) - client.WaitForResourceReadyOrFail(senderName, &podMeta) + testlib.WaitForPodRunningOrFail(t, ctx, client.Kube, senderName, client.Namespace) eventTracker.AssertAtLeast(1, originalEventMatcher) }) @@ -208,11 +203,11 @@ func BrokerV1Beta1IngressDataPlaneTestHelper( body, sender.WithResponseSink(responseSink), ) - client.WaitForResourceReadyOrFail(senderName, &podMeta) + testlib.WaitForPodRunningOrFail(t, ctx, client.Kube, senderName, client.Namespace) eventTracker.AssertExact(1, recordevents.MatchEvent(sender.MatchStatusCode(202))) // should probably be a range }) - //Respond with 400 on bad CE + // Respond with 400 on bad CE st.Run("Respond with 400 on bad CE", func(t *testing.T) { eventID := "four-hundred-on-bad-ce" body := ";la}{kjsdf;oai2095{}{}8234092349807asdfashdf" @@ -220,7 +215,7 @@ func BrokerV1Beta1IngressDataPlaneTestHelper( senderName := "fourhundres-test-sender" client.SendRequestToAddressable(ctx, senderName, broker.Name, testlib.BrokerTypeMeta, map[string]string{ - "ce-specversion": "9000.1", //its over 9,000! + "ce-specversion": "9000.1", // its over 9,000! "ce-type": testlib.DefaultEventType, "ce-source": "400.request.sender.test.knative.dev", "ce-id": eventID, @@ -228,20 +223,20 @@ func BrokerV1Beta1IngressDataPlaneTestHelper( }, body, sender.WithResponseSink(responseSink)) - client.WaitForResourceReadyOrFail(senderName, &podMeta) + testlib.WaitForPodRunningOrFail(t, ctx, client.Kube, senderName, client.Namespace) eventTracker.AssertExact(1, recordevents.MatchEvent(sender.MatchStatusCode(400))) }) }) } -//At consumer -//No upgrade of version -//Attributes received should be the same as produced (attributes may be added) -//Events are filtered -//Events are delivered to multiple subscribers -//Deliveries succeed at least once -//Replies are accepted and delivered -//Replies that are unsuccessfully forwarded cause initial message to be redelivered (Very difficult to test, can be ignored) +// At consumer +// No upgrade of version +// Attributes received should be the same as produced (attributes may be added) +// Events are filtered +// Events are delivered to multiple subscribers +// Deliveries succeed at least once +// Replies are accepted and delivered +// Replies that are unsuccessfully forwarded cause initial message to be redelivered (Very difficult to test, can be ignored) func BrokerV1Beta1ConsumerDataPlaneTestHelper( ctx context.Context, t *testing.T, @@ -327,7 +322,7 @@ func BrokerV1Beta1ConsumerDataPlaneTestHelper( cetest.HasSpecVersion("0.3"), cetest.HasId("no-upgrade"), )) - client.WaitForResourceReadyOrFail(senderName, &podMeta) + testlib.WaitForPodRunningOrFail(t, ctx, client.Kube, senderName, client.Namespace) eventTracker.AssertExact(1, originalEventMatcher) }) @@ -344,7 +339,7 @@ func BrokerV1Beta1ConsumerDataPlaneTestHelper( cetest.HasSource(baseSource), cetest.HasSpecVersion("1.0"), ) - client.WaitForResourceReadyOrFail(senderName, &podMeta) + testlib.WaitForPodRunningOrFail(t, ctx, client.Kube, senderName, client.Namespace) eventTracker.AssertExact(1, originalEventMatcher) }) @@ -363,8 +358,8 @@ func BrokerV1Beta1ConsumerDataPlaneTestHelper( nonEventMatcher := recordevents.MatchEvent( cetest.HasSource(baseSource), ) - client.WaitForResourceReadyOrFail(firstSenderName, &podMeta) - client.WaitForResourceReadyOrFail(secondSenderName, &podMeta) + testlib.WaitForPodRunningOrFail(t, ctx, client.Kube, firstSenderName, client.Namespace) + testlib.WaitForPodRunningOrFail(t, ctx, client.Kube, secondSenderName, client.Namespace) secondTracker.AssertAtLeast(1, filteredEventMatcher) secondTracker.AssertNot(nonEventMatcher) }) @@ -378,7 +373,7 @@ func BrokerV1Beta1ConsumerDataPlaneTestHelper( filteredEventMatcher := recordevents.MatchEvent( cetest.HasSource(source), ) - client.WaitForResourceReadyOrFail(senderName, &podMeta) + testlib.WaitForPodRunningOrFail(t, ctx, client.Kube, senderName, client.Namespace) eventTracker.AssertAtLeast(1, filteredEventMatcher) secondTracker.AssertAtLeast(1, filteredEventMatcher) }) @@ -392,7 +387,7 @@ func BrokerV1Beta1ConsumerDataPlaneTestHelper( originalEventMatcher := recordevents.MatchEvent( cetest.HasSource(source), ) - client.WaitForResourceReadyOrFail(senderName, &podMeta) + testlib.WaitForPodRunningOrFail(t, ctx, client.Kube, senderName, client.Namespace) eventTracker.AssertAtLeast(1, originalEventMatcher) }) @@ -409,7 +404,7 @@ func BrokerV1Beta1ConsumerDataPlaneTestHelper( cetest.HasType("reply-check-type"), cetest.HasData(transformMsg), ) - client.WaitForResourceReadyOrFail(senderName, &podMeta) + testlib.WaitForPodRunningOrFail(t, ctx, client.Kube, senderName, client.Namespace) eventTracker.AssertAtLeast(2, transformedEventMatcher) }) }) diff --git a/test/lib/await.go b/test/lib/await.go index db2752ff50e..d31c497a10d 100644 --- a/test/lib/await.go +++ b/test/lib/await.go @@ -16,14 +16,18 @@ package lib import ( + "context" "fmt" "io/ioutil" "net/http" + "testing" "time" + "github.com/stretchr/testify/assert" "go.uber.org/zap" "golang.org/x/sync/errgroup" "k8s.io/apimachinery/pkg/util/wait" + pkgtest "knative.dev/pkg/test" ) // WaitFor will register a wait routine to be resolved later @@ -80,6 +84,12 @@ func WaitForReadiness(port int, log *zap.SugaredLogger) error { }) } +// WaitForPodRunningOrFail waits until the pod is running and fails if something goes wrong. +func WaitForPodRunningOrFail(t *testing.T, ctx context.Context, client *pkgtest.KubeClient, name, namespace string) { + err := pkgtest.WaitForPodRunning(ctx, client, name, namespace) + assert.Nil(t, err) +} + type AwaitRoutine func() error type namedAwait struct { From d5054bfb3b1a878d06855b60adfdd04584c9a92a Mon Sep 17 00:00:00 2001 From: Pierangelo Di Pilato Date: Wed, 23 Sep 2020 09:12:19 +0200 Subject: [PATCH 2/3] Check if pod is running in isResourceReady Signed-off-by: Pierangelo Di Pilato --- test/lib/duck/resource_checks.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/test/lib/duck/resource_checks.go b/test/lib/duck/resource_checks.go index 32a51cc56fd..43a8755b5b8 100644 --- a/test/lib/duck/resource_checks.go +++ b/test/lib/duck/resource_checks.go @@ -22,6 +22,7 @@ package duck import ( "time" + corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/wait" @@ -60,7 +61,7 @@ func WaitForResourcesReady(dynamicClient dynamic.Interface, objList *resources.M }) } -// isResourceReady leverage duck-type to check if the given MetaResource is in ready state +// isResourceReady leverage duck-type to check if the given obj is in ready state func isResourceReady(obj runtime.Object, err error) (bool, error) { if k8serrors.IsNotFound(err) { // Return false as we are not done yet. @@ -72,7 +73,16 @@ func isResourceReady(obj runtime.Object, err error) (bool, error) { return false, err } + if pod, isPod := obj.(*corev1.Pod); isPod { + return isPodRunning(pod), nil + } + kr := obj.(*duckv1beta1.KResource) ready := kr.Status.GetCondition(apis.ConditionReady) return ready != nil && ready.IsTrue(), nil } + +// isPodRunning will check the status conditions of the pod and return true if it's Running or Succeeded. +func isPodRunning(pod *corev1.Pod) bool { + return pod.Status.Phase == corev1.PodRunning || pod.Status.Phase == corev1.PodSucceeded +} From 425ba87b56ffacef9b36710f89886fa613c89cab Mon Sep 17 00:00:00 2001 From: Pierangelo Di Pilato Date: Wed, 23 Sep 2020 09:13:39 +0200 Subject: [PATCH 3/3] Revert "Use WaitForPodRunning instead of WaitForResourceReady on pods" This reverts commit 6d59fd271ea0267a97ce18f465b15395bd47ac0d. --- .../helpers/broker_data_plane_test_helper.go | 63 ++++++++++--------- test/lib/await.go | 10 --- 2 files changed, 34 insertions(+), 39 deletions(-) diff --git a/test/conformance/helpers/broker_data_plane_test_helper.go b/test/conformance/helpers/broker_data_plane_test_helper.go index ab62875c2e0..9facba44571 100644 --- a/test/conformance/helpers/broker_data_plane_test_helper.go +++ b/test/conformance/helpers/broker_data_plane_test_helper.go @@ -33,6 +33,11 @@ import ( cetest "github.com/cloudevents/sdk-go/v2/test" ) +var podMeta = metav1.TypeMeta{ + Kind: "pod", + APIVersion: "v1", +} + func BrokerDataPlaneSetupHelper(ctx context.Context, client *testlib.Client, brokerClass string, brokerTestRunner testlib.ComponentsTestRunner) *eventingv1beta1.Broker { var broker *eventingv1beta1.Broker brokerName := brokerTestRunner.ComponentName @@ -74,12 +79,12 @@ func BrokerDataPlaneNamespaceSetupOption(ctx context.Context, namespace string) } } -// At ingress -// Supports CE 0.3 or CE 1.0 via HTTP -// Supports structured or Binary mode -// Respond with 2xx on good CE -// Respond with 400 on bad CE -// Reject non-POST requests to publish URI +//At ingress +//Supports CE 0.3 or CE 1.0 via HTTP +//Supports structured or Binary mode +//Respond with 2xx on good CE +//Respond with 400 on bad CE +//Reject non-POST requests to publish URI func BrokerV1Beta1IngressDataPlaneTestHelper( ctx context.Context, t *testing.T, @@ -123,7 +128,7 @@ func BrokerV1Beta1IngressDataPlaneTestHelper( cetest.HasId(eventID), cetest.HasSpecVersion("0.3"), )) - testlib.WaitForPodRunningOrFail(t, ctx, client.Kube, senderName, client.Namespace) + client.WaitForResourceReadyOrFail(senderName, &podMeta) eventTracker.AssertAtLeast(1, originalEventMatcher) }) @@ -144,7 +149,7 @@ func BrokerV1Beta1IngressDataPlaneTestHelper( cetest.HasId(eventID), cetest.HasSpecVersion("1.0"), )) - testlib.WaitForPodRunningOrFail(t, ctx, client.Kube, senderName, client.Namespace) + client.WaitForResourceReadyOrFail(senderName, &podMeta) eventTracker.AssertAtLeast(1, originalEventMatcher) }) @@ -164,7 +169,7 @@ func BrokerV1Beta1IngressDataPlaneTestHelper( originalEventMatcher := recordevents.MatchEvent(cetest.AllOf( cetest.HasId(eventID), )) - testlib.WaitForPodRunningOrFail(t, ctx, client.Kube, senderName, client.Namespace) + client.WaitForResourceReadyOrFail(senderName, &podMeta) eventTracker.AssertAtLeast(1, originalEventMatcher) }) @@ -183,7 +188,7 @@ func BrokerV1Beta1IngressDataPlaneTestHelper( originalEventMatcher := recordevents.MatchEvent(cetest.AllOf( cetest.HasId(eventID), )) - testlib.WaitForPodRunningOrFail(t, ctx, client.Kube, senderName, client.Namespace) + client.WaitForResourceReadyOrFail(senderName, &podMeta) eventTracker.AssertAtLeast(1, originalEventMatcher) }) @@ -203,11 +208,11 @@ func BrokerV1Beta1IngressDataPlaneTestHelper( body, sender.WithResponseSink(responseSink), ) - testlib.WaitForPodRunningOrFail(t, ctx, client.Kube, senderName, client.Namespace) + client.WaitForResourceReadyOrFail(senderName, &podMeta) eventTracker.AssertExact(1, recordevents.MatchEvent(sender.MatchStatusCode(202))) // should probably be a range }) - // Respond with 400 on bad CE + //Respond with 400 on bad CE st.Run("Respond with 400 on bad CE", func(t *testing.T) { eventID := "four-hundred-on-bad-ce" body := ";la}{kjsdf;oai2095{}{}8234092349807asdfashdf" @@ -215,7 +220,7 @@ func BrokerV1Beta1IngressDataPlaneTestHelper( senderName := "fourhundres-test-sender" client.SendRequestToAddressable(ctx, senderName, broker.Name, testlib.BrokerTypeMeta, map[string]string{ - "ce-specversion": "9000.1", // its over 9,000! + "ce-specversion": "9000.1", //its over 9,000! "ce-type": testlib.DefaultEventType, "ce-source": "400.request.sender.test.knative.dev", "ce-id": eventID, @@ -223,20 +228,20 @@ func BrokerV1Beta1IngressDataPlaneTestHelper( }, body, sender.WithResponseSink(responseSink)) - testlib.WaitForPodRunningOrFail(t, ctx, client.Kube, senderName, client.Namespace) + client.WaitForResourceReadyOrFail(senderName, &podMeta) eventTracker.AssertExact(1, recordevents.MatchEvent(sender.MatchStatusCode(400))) }) }) } -// At consumer -// No upgrade of version -// Attributes received should be the same as produced (attributes may be added) -// Events are filtered -// Events are delivered to multiple subscribers -// Deliveries succeed at least once -// Replies are accepted and delivered -// Replies that are unsuccessfully forwarded cause initial message to be redelivered (Very difficult to test, can be ignored) +//At consumer +//No upgrade of version +//Attributes received should be the same as produced (attributes may be added) +//Events are filtered +//Events are delivered to multiple subscribers +//Deliveries succeed at least once +//Replies are accepted and delivered +//Replies that are unsuccessfully forwarded cause initial message to be redelivered (Very difficult to test, can be ignored) func BrokerV1Beta1ConsumerDataPlaneTestHelper( ctx context.Context, t *testing.T, @@ -322,7 +327,7 @@ func BrokerV1Beta1ConsumerDataPlaneTestHelper( cetest.HasSpecVersion("0.3"), cetest.HasId("no-upgrade"), )) - testlib.WaitForPodRunningOrFail(t, ctx, client.Kube, senderName, client.Namespace) + client.WaitForResourceReadyOrFail(senderName, &podMeta) eventTracker.AssertExact(1, originalEventMatcher) }) @@ -339,7 +344,7 @@ func BrokerV1Beta1ConsumerDataPlaneTestHelper( cetest.HasSource(baseSource), cetest.HasSpecVersion("1.0"), ) - testlib.WaitForPodRunningOrFail(t, ctx, client.Kube, senderName, client.Namespace) + client.WaitForResourceReadyOrFail(senderName, &podMeta) eventTracker.AssertExact(1, originalEventMatcher) }) @@ -358,8 +363,8 @@ func BrokerV1Beta1ConsumerDataPlaneTestHelper( nonEventMatcher := recordevents.MatchEvent( cetest.HasSource(baseSource), ) - testlib.WaitForPodRunningOrFail(t, ctx, client.Kube, firstSenderName, client.Namespace) - testlib.WaitForPodRunningOrFail(t, ctx, client.Kube, secondSenderName, client.Namespace) + client.WaitForResourceReadyOrFail(firstSenderName, &podMeta) + client.WaitForResourceReadyOrFail(secondSenderName, &podMeta) secondTracker.AssertAtLeast(1, filteredEventMatcher) secondTracker.AssertNot(nonEventMatcher) }) @@ -373,7 +378,7 @@ func BrokerV1Beta1ConsumerDataPlaneTestHelper( filteredEventMatcher := recordevents.MatchEvent( cetest.HasSource(source), ) - testlib.WaitForPodRunningOrFail(t, ctx, client.Kube, senderName, client.Namespace) + client.WaitForResourceReadyOrFail(senderName, &podMeta) eventTracker.AssertAtLeast(1, filteredEventMatcher) secondTracker.AssertAtLeast(1, filteredEventMatcher) }) @@ -387,7 +392,7 @@ func BrokerV1Beta1ConsumerDataPlaneTestHelper( originalEventMatcher := recordevents.MatchEvent( cetest.HasSource(source), ) - testlib.WaitForPodRunningOrFail(t, ctx, client.Kube, senderName, client.Namespace) + client.WaitForResourceReadyOrFail(senderName, &podMeta) eventTracker.AssertAtLeast(1, originalEventMatcher) }) @@ -404,7 +409,7 @@ func BrokerV1Beta1ConsumerDataPlaneTestHelper( cetest.HasType("reply-check-type"), cetest.HasData(transformMsg), ) - testlib.WaitForPodRunningOrFail(t, ctx, client.Kube, senderName, client.Namespace) + client.WaitForResourceReadyOrFail(senderName, &podMeta) eventTracker.AssertAtLeast(2, transformedEventMatcher) }) }) diff --git a/test/lib/await.go b/test/lib/await.go index d31c497a10d..db2752ff50e 100644 --- a/test/lib/await.go +++ b/test/lib/await.go @@ -16,18 +16,14 @@ package lib import ( - "context" "fmt" "io/ioutil" "net/http" - "testing" "time" - "github.com/stretchr/testify/assert" "go.uber.org/zap" "golang.org/x/sync/errgroup" "k8s.io/apimachinery/pkg/util/wait" - pkgtest "knative.dev/pkg/test" ) // WaitFor will register a wait routine to be resolved later @@ -84,12 +80,6 @@ func WaitForReadiness(port int, log *zap.SugaredLogger) error { }) } -// WaitForPodRunningOrFail waits until the pod is running and fails if something goes wrong. -func WaitForPodRunningOrFail(t *testing.T, ctx context.Context, client *pkgtest.KubeClient, name, namespace string) { - err := pkgtest.WaitForPodRunning(ctx, client, name, namespace) - assert.Nil(t, err) -} - type AwaitRoutine func() error type namedAwait struct {