From 6155af20f825259a5b8610d1a98871cf13757be5 Mon Sep 17 00:00:00 2001 From: Ahmed Abdalla Date: Wed, 22 Jul 2020 14:03:14 +0200 Subject: [PATCH 1/4] Add PingSource conformance tests initialization --- test/conformance/main_test.go | 13 ++++++++--- test/lib/config.go | 4 ++-- test/lib/resources/constants.go | 13 ++++++----- test/lib/setupclientoptions/sources.go | 32 ++++++++++++++++++++++++++ test/lib/test_runner.go | 7 +++++- 5 files changed, 57 insertions(+), 12 deletions(-) diff --git a/test/conformance/main_test.go b/test/conformance/main_test.go index b12ed719016..92a3459ffac 100644 --- a/test/conformance/main_test.go +++ b/test/conformance/main_test.go @@ -70,12 +70,19 @@ func TestMain(m *testing.M) { } func addSourcesInitializers() { - name := strings.ToLower(fmt.Sprintf("%s", + apiSrcName := strings.ToLower(fmt.Sprintf("%s", testlib.ApiServerSourceTypeMeta.Kind)) + pingSrcName := strings.ToLower(fmt.Sprintf("%s", + testlib.PingSourceTypeMeta.Kind)) sourcesTestRunner.AddComponentSetupClientOption( testlib.ApiServerSourceTypeMeta, - setupclientoptions.ApiServerSourceClientSetupOption(name, + setupclientoptions.ApiServerSourceClientSetupOption(apiSrcName, "Reference", recordEventsPodName, roleName, serviceAccountName), - ) + ) + sourcesTestRunner.AddComponentSetupClientOption( + testlib.PingSourceTypeMeta, + setupclientoptions.PingSourceClientSetupOption(pingSrcName, + recordEventsPodName), + ) } diff --git a/test/lib/config.go b/test/lib/config.go index 98bba5c477d..1017a4ed5fc 100644 --- a/test/lib/config.go +++ b/test/lib/config.go @@ -47,12 +47,12 @@ var ChannelFeatureMap = map[metav1.TypeMeta][]Feature{ } var ApiServerSourceTypeMeta = metav1.TypeMeta{ - APIVersion: resources.SourcesAPIVersion, + APIVersion: resources.SourcesV1B1APIVersion, Kind: resources.ApiServerSourceKind, } var PingSourceTypeMeta = metav1.TypeMeta{ - APIVersion: resources.SourcesAPIVersion, + APIVersion: resources.SourcesV1A2APIVersion, Kind: resources.PingSourceKind, } diff --git a/test/lib/resources/constants.go b/test/lib/resources/constants.go index fd635e7893c..96d1f1abfea 100644 --- a/test/lib/resources/constants.go +++ b/test/lib/resources/constants.go @@ -23,12 +23,13 @@ import ( // API versions for the resources. const ( - CoreAPIVersion = "v1" - EventingAPIVersion = "eventing.knative.dev/v1beta1" - MessagingAPIVersion = "messaging.knative.dev/v1beta1" - FlowsAPIVersion = "flows.knative.dev/v1beta1" - ServingAPIVersion = "serving.knative.dev/v1" - SourcesAPIVersion = "sources.knative.dev/v1beta1" + CoreAPIVersion = "v1" + EventingAPIVersion = "eventing.knative.dev/v1beta1" + MessagingAPIVersion = "messaging.knative.dev/v1beta1" + FlowsAPIVersion = "flows.knative.dev/v1beta1" + ServingAPIVersion = "serving.knative.dev/v1" + SourcesV1A2APIVersion = "sources.knative.dev/v1alpha2" + SourcesV1B1APIVersion = "sources.knative.dev/v1beta1" ) // Kind for Knative resources. diff --git a/test/lib/setupclientoptions/sources.go b/test/lib/setupclientoptions/sources.go index 9c420477866..b02696492a2 100644 --- a/test/lib/setupclientoptions/sources.go +++ b/test/lib/setupclientoptions/sources.go @@ -19,6 +19,8 @@ package setupclientoptions import ( "fmt" rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/util/uuid" + sourcesv1alpha2 "knative.dev/eventing/pkg/apis/sources/v1alpha2" sourcesv1beta1 "knative.dev/eventing/pkg/apis/sources/v1beta1" eventingtesting "knative.dev/eventing/pkg/reconciler/testing" testlib "knative.dev/eventing/test/lib" @@ -64,6 +66,36 @@ func ApiServerSourceClientSetupOption(name string, mode string, recordEventsPodN } } +// PingSourceClientSetupOption returns a ClientSetupOption that can be used +// to create a new PingSource. It creates a RecordEvents pod and a +// PingSource object with the RecordEvent pod as its sink. +func PingSourceClientSetupOption(name string, recordEventsPodName string,) testlib.SetupClientOption { + return func(client *testlib.Client) { + + // create event logger pod and service + recordevents.StartEventRecordOrFail(client, recordEventsPodName) + + // create cron job source + data := fmt.Sprintf(`{"msg":"TestPingSource %s"}`, uuid.NewUUID()) + source := eventingtesting.NewPingSourceV1Alpha2( + name, + client.Namespace, + eventingtesting.WithPingSourceV1A2Spec(sourcesv1alpha2.PingSourceSpec{ + JsonData: data, + SourceSpec: duckv1.SourceSpec{ + Sink: duckv1.Destination{ + Ref: resources.KnativeRefForService(recordEventsPodName, client.Namespace), + }, + }, + }), + ) + client.CreatePingSourceV1Alpha2OrFail(source) + + // wait for all test resources to be ready + client.WaitForAllTestResourcesReadyOrFail() + } +} + func createRbacObjects(client *testlib.Client, roleName string, serviceAccountName string) { // creates ServiceAccount and RoleBinding with a role for reading pods diff --git a/test/lib/test_runner.go b/test/lib/test_runner.go index e63ef2fd0e0..b4fe9dd4166 100644 --- a/test/lib/test_runner.go +++ b/test/lib/test_runner.go @@ -92,10 +92,15 @@ func (tr *ComponentsTestRunner) RunTestsWithComponentOptions( // If in strict mode and a component is not present in the map, then // don't run the tests features, present := tr.ComponentFeatureMap[component] + subTestName := fmt.Sprintf("%s-%s", component.Kind, component.APIVersion) if !strict || ( present && contains(features, feature)) { - t.Run(fmt.Sprintf("%s-%s", component.Kind, component.APIVersion), func(st *testing.T) { + t.Run(subTestName, func(st *testing.T) { testFunc(st, component, tr.componentOptions[component]...) }) + } else { + t.Skipf("Skipping %s for component %s since it did not " + + "match the feature %s and we are in strict mode", t.Name(), + subTestName, feature) } } } From 7f879cb0e2ff7890ae156f1846a6b991eb8e1979 Mon Sep 17 00:00:00 2001 From: Ahmed Abdalla Date: Mon, 27 Jul 2020 11:01:05 +0200 Subject: [PATCH 2/4] Format code as needed --- .../sources/source_status_test_helper.go | 57 +++++++++---------- test/conformance/main_test.go | 14 ++--- test/conformance/source_status_test.go | 3 +- test/lib/setupclientoptions/sources.go | 6 +- test/lib/test_runner.go | 12 ++-- 5 files changed, 47 insertions(+), 45 deletions(-) diff --git a/test/conformance/helpers/sources/source_status_test_helper.go b/test/conformance/helpers/sources/source_status_test_helper.go index e2746f62a4e..69066dcfd99 100644 --- a/test/conformance/helpers/sources/source_status_test_helper.go +++ b/test/conformance/helpers/sources/source_status_test_helper.go @@ -5,7 +5,7 @@ Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at - http://www.apache.org/licenses/LICENSE-2.0 + http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, @@ -18,23 +18,23 @@ package sources import ( "fmt" - "github.com/pkg/errors" - "knative.dev/eventing/test/lib/duck" - "knative.dev/eventing/test/lib/resources" - "knative.dev/pkg/apis" - duckv1beta1 "knative.dev/pkg/apis/duck/v1beta1" "strings" "testing" + "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" testlib "knative.dev/eventing/test/lib" + "knative.dev/eventing/test/lib/duck" + "knative.dev/eventing/test/lib/resources" + "knative.dev/pkg/apis" + duckv1beta1 "knative.dev/pkg/apis/duck/v1beta1" ) // SourceStatusTestHelperWithComponentsTestRunner runs the Source status // conformance tests for all sources in the ComponentsTestRunner. This test -// needs an instance created of each source which should be initialized via -// ComponentsTestRunner.AddComponentSetupClientOption. +// needs an already created instance of each source which should be initialized +//via ComponentsTestRunner.AddComponentSetupClientOption. // // Note: The source object name must be the lower case Kind name (e.g. // apiserversource for the Kind: ApiServerSource source) @@ -43,24 +43,24 @@ func SourceStatusTestHelperWithComponentsTestRunner( componentsTestRunner testlib.ComponentsTestRunner, options ...testlib.SetupClientOption, ) { - table := [] struct { + table := []struct { name string feature testlib.Feature // Sources report success via either a Ready condition or a Succeeded condition - want apis.ConditionType - } { + want apis.ConditionType + }{ { - "Long living sources have Ready status condition", - testlib.FeatureLongLiving, - apis.ConditionReady, + name: "Long living sources have Ready status condition", + feature: testlib.FeatureLongLiving, + want: apis.ConditionReady, }, { - "Batch sources have Succeeded status condition", - testlib.FeatureBatch, - apis.ConditionSucceeded, + name: "Batch sources have Succeeded status condition", + feature: testlib.FeatureBatch, + want: apis.ConditionSucceeded, }, - } + for _, tc := range table { n := tc.name f := tc.feature @@ -73,16 +73,15 @@ func SourceStatusTestHelperWithComponentsTestRunner( options = append(options, componentOptions...) client := testlib.Setup(st, true, options...) defer testlib.TearDown(client) - validateSourceStatus(st, client, source, w, options...) - }) + validateSourceStatus(st, client, source, w) + }) }) } } -func validateSourceStatus(st *testing.T, client *testlib.Client, source metav1.TypeMeta, successCondition apis.ConditionType, options ...testlib.SetupClientOption) { - const( - sourceName = "source-req-status" - ) +func validateSourceStatus(st *testing.T, client *testlib.Client, + source metav1.TypeMeta, + successCondition apis.ConditionType) { st.Logf("Running source status conformance test with source %q", source) @@ -92,7 +91,7 @@ func validateSourceStatus(st *testing.T, client *testlib.Client, source metav1.T } // SPEC: Sources MUST implement conditions with a Ready condition for long lived sources, and Succeeded for batch style sources. - if ! hasCondition(v1beta1Src, successCondition){ + if !hasCondition(v1beta1Src, successCondition) { st.Fatalf("%q does not have %q", source, successCondition) } @@ -103,7 +102,7 @@ func validateSourceStatus(st *testing.T, client *testlib.Client, source metav1.T } func getSourceAsV1Beta1Source(client *testlib.Client, - source metav1.TypeMeta) (*duckv1beta1.Source, error){ + source metav1.TypeMeta) (*duckv1beta1.Source, error) { srcName := strings.ToLower(fmt.Sprintf("%s", source.Kind)) metaResource := resources.NewMetaResource(srcName, client.Namespace, &source) @@ -114,14 +113,14 @@ func getSourceAsV1Beta1Source(client *testlib.Client, } srcObj, ok := obj.(*duckv1beta1.Source) if !ok { - return nil, errors.Errorf("unable to cast source %q to v1beta1 Source" + + return nil, errors.Errorf("unable to cast source %q to v1beta1 Source"+ " duck type", source) } return srcObj, nil } -func hasCondition(src *duckv1beta1.Source, t apis.ConditionType) bool{ - if src.Status.GetCondition(t) == nil{ +func hasCondition(src *duckv1beta1.Source, t apis.ConditionType) bool { + if src.Status.GetCondition(t) == nil { return false } return true diff --git a/test/conformance/main_test.go b/test/conformance/main_test.go index 92a3459ffac..35349e64ed9 100644 --- a/test/conformance/main_test.go +++ b/test/conformance/main_test.go @@ -24,17 +24,17 @@ import ( "strings" "testing" - "knative.dev/pkg/test/zipkin" - "knative.dev/eventing/test" testlib "knative.dev/eventing/test/lib" "knative.dev/eventing/test/lib/resources" "knative.dev/eventing/test/lib/setupclientoptions" + "knative.dev/pkg/test/zipkin" ) + const ( - roleName = "event-watcher-r" - serviceAccountName = "event-watcher-sa" - recordEventsPodName = "api-server-source-logger-pod" + roleName = "event-watcher-r" + serviceAccountName = "event-watcher-sa" + recordEventsPodName = "api-server-source-logger-pod" ) var channelTestRunner testlib.ComponentsTestRunner @@ -79,10 +79,10 @@ func addSourcesInitializers() { setupclientoptions.ApiServerSourceClientSetupOption(apiSrcName, "Reference", recordEventsPodName, roleName, serviceAccountName), - ) + ) sourcesTestRunner.AddComponentSetupClientOption( testlib.PingSourceTypeMeta, setupclientoptions.PingSourceClientSetupOption(pingSrcName, recordEventsPodName), - ) + ) } diff --git a/test/conformance/source_status_test.go b/test/conformance/source_status_test.go index 2fb34d0ed59..d379e9d1fa0 100644 --- a/test/conformance/source_status_test.go +++ b/test/conformance/source_status_test.go @@ -19,9 +19,10 @@ limitations under the License. package conformance import ( + "testing" + srchelpers "knative.dev/eventing/test/conformance/helpers/sources" testlib "knative.dev/eventing/test/lib" - "testing" ) func TestSourceStatus(t *testing.T) { diff --git a/test/lib/setupclientoptions/sources.go b/test/lib/setupclientoptions/sources.go index b02696492a2..f2bf7f9aa83 100644 --- a/test/lib/setupclientoptions/sources.go +++ b/test/lib/setupclientoptions/sources.go @@ -18,8 +18,10 @@ package setupclientoptions import ( "fmt" + rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/util/uuid" + sourcesv1alpha2 "knative.dev/eventing/pkg/apis/sources/v1alpha2" sourcesv1beta1 "knative.dev/eventing/pkg/apis/sources/v1beta1" eventingtesting "knative.dev/eventing/pkg/reconciler/testing" @@ -34,7 +36,7 @@ import ( // RoleBinding, a RecordEvents pod and an ApiServerSource object with the event // mode and RecordEvent pod as its sink. func ApiServerSourceClientSetupOption(name string, mode string, recordEventsPodName string, - roleName string, serviceAccountName string) testlib.SetupClientOption{ + roleName string, serviceAccountName string) testlib.SetupClientOption { return func(client *testlib.Client) { // create needed RBAC SA, Role & RoleBinding createRbacObjects(client, roleName, serviceAccountName) @@ -69,7 +71,7 @@ func ApiServerSourceClientSetupOption(name string, mode string, recordEventsPodN // PingSourceClientSetupOption returns a ClientSetupOption that can be used // to create a new PingSource. It creates a RecordEvents pod and a // PingSource object with the RecordEvent pod as its sink. -func PingSourceClientSetupOption(name string, recordEventsPodName string,) testlib.SetupClientOption { +func PingSourceClientSetupOption(name string, recordEventsPodName string) testlib.SetupClientOption { return func(client *testlib.Client) { // create event logger pod and service diff --git a/test/lib/test_runner.go b/test/lib/test_runner.go index b4fe9dd4166..361b35ef63b 100644 --- a/test/lib/test_runner.go +++ b/test/lib/test_runner.go @@ -22,13 +22,13 @@ import ( "testing" "time" - "knative.dev/eventing/pkg/utils" - corev1 "k8s.io/api/core/v1" apierrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/storage/names" + + "knative.dev/eventing/pkg/utils" pkgTest "knative.dev/pkg/test" "knative.dev/pkg/test/helpers" "knative.dev/pkg/test/prow" @@ -49,7 +49,7 @@ const ( type ComponentsTestRunner struct { ComponentFeatureMap map[metav1.TypeMeta][]Feature ComponentsToTest []metav1.TypeMeta - componentOptions map[metav1.TypeMeta][]SetupClientOption + componentOptions map[metav1.TypeMeta][]SetupClientOption } // RunTests will use all components that support the given feature, to run @@ -93,12 +93,12 @@ func (tr *ComponentsTestRunner) RunTestsWithComponentOptions( // don't run the tests features, present := tr.ComponentFeatureMap[component] subTestName := fmt.Sprintf("%s-%s", component.Kind, component.APIVersion) - if !strict || ( present && contains(features, feature)) { + if !strict || (present && contains(features, feature)) { t.Run(subTestName, func(st *testing.T) { testFunc(st, component, tr.componentOptions[component]...) }) } else { - t.Skipf("Skipping %s for component %s since it did not " + + t.Skipf("Skipping %s for component %s since it did not "+ "match the feature %s and we are in strict mode", t.Name(), subTestName, feature) } @@ -111,7 +111,7 @@ func (tr *ComponentsTestRunner) RunTestsWithComponentOptions( // of a source or a channel) as opposed to other cheap initialization code that // is safe to be called in all cases (e.g. installation of a CRD) func (tr *ComponentsTestRunner) AddComponentSetupClientOption(component metav1.TypeMeta, - options ...SetupClientOption){ + options ...SetupClientOption) { if tr.componentOptions == nil { tr.componentOptions = make(map[metav1.TypeMeta][]SetupClientOption) } From ec8069944c34741217eb14bd672582bb00433a96 Mon Sep 17 00:00:00 2001 From: Ahmed Abdalla Date: Mon, 27 Jul 2020 11:54:35 +0200 Subject: [PATCH 3/4] Fix formatting and use fmt %w instead of errors package --- .../helpers/sources/source_status_test_helper.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/conformance/helpers/sources/source_status_test_helper.go b/test/conformance/helpers/sources/source_status_test_helper.go index 69066dcfd99..dd7945f0e85 100644 --- a/test/conformance/helpers/sources/source_status_test_helper.go +++ b/test/conformance/helpers/sources/source_status_test_helper.go @@ -21,7 +21,6 @@ import ( "strings" "testing" - "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" testlib "knative.dev/eventing/test/lib" @@ -87,12 +86,12 @@ func validateSourceStatus(st *testing.T, client *testlib.Client, v1beta1Src, err := getSourceAsV1Beta1Source(client, source) if err != nil { - st.Fatalf("unable to get source %q with v1beta1 duck type: %v", source, err) + st.Fatalf("Unable to get source %q with v1beta1 duck type: %v", source, err) } // SPEC: Sources MUST implement conditions with a Ready condition for long lived sources, and Succeeded for batch style sources. if !hasCondition(v1beta1Src, successCondition) { - st.Fatalf("%q does not have %q", source, successCondition) + st.Fatalf("Source %q does not have %q", source, successCondition) } // SPEC: Sources MUST propagate the sinkUri to their status to signal to the cluster where their events are being sent. @@ -109,12 +108,13 @@ func getSourceAsV1Beta1Source(client *testlib.Client, obj, err := duck.GetGenericObject(client.Dynamic, metaResource, &duckv1beta1.Source{}) if err != nil { - return nil, errors.Wrapf(err, "unable to get the source as v1beta1 Source duck type: %q", source) + return nil, fmt.Errorf("unable to get the source as v1beta1 "+ + "Source duck type: %q: %w", source, err) } srcObj, ok := obj.(*duckv1beta1.Source) if !ok { - return nil, errors.Errorf("unable to cast source %q to v1beta1 Source"+ - " duck type", source) + return nil, fmt.Errorf("unable to cast source %q to v1beta1 "+ + "Source duck type", source) } return srcObj, nil } From c57571517eb47764972deb35770a931ce170a3a6 Mon Sep 17 00:00:00 2001 From: Ahmed Abdalla Abdelrehim Date: Mon, 27 Jul 2020 12:22:00 +0200 Subject: [PATCH 4/4] Add a clearer log message Co-authored-by: Antoine Cotten --- test/conformance/helpers/sources/source_status_test_helper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/conformance/helpers/sources/source_status_test_helper.go b/test/conformance/helpers/sources/source_status_test_helper.go index dd7945f0e85..b9a250aafbf 100644 --- a/test/conformance/helpers/sources/source_status_test_helper.go +++ b/test/conformance/helpers/sources/source_status_test_helper.go @@ -91,7 +91,7 @@ func validateSourceStatus(st *testing.T, client *testlib.Client, // SPEC: Sources MUST implement conditions with a Ready condition for long lived sources, and Succeeded for batch style sources. if !hasCondition(v1beta1Src, successCondition) { - st.Fatalf("Source %q does not have %q", source, successCondition) + st.Fatalf("Source %q does not have condition %q", source, successCondition) } // SPEC: Sources MUST propagate the sinkUri to their status to signal to the cluster where their events are being sent.