From 764edf1ad333a87a8b88116bdba5d54546d4c141 Mon Sep 17 00:00:00 2001 From: zhaojizhuang <571130360@qq.com> Date: Wed, 3 Mar 2021 00:35:34 +0800 Subject: [PATCH] fix #4986 remove pingsource adapter --- pkg/adapter/mtping/adapter.go | 2 +- pkg/adapter/mtping/adapter_test.go | 2 +- pkg/adapter/mtping/controller.go | 17 ++++--- pkg/adapter/mtping/controller_test.go | 10 +++- pkg/adapter/mtping/pingsource.go | 25 ++++++---- pkg/adapter/mtping/pingsource_test.go | 70 +++++++-------------------- 6 files changed, 54 insertions(+), 72 deletions(-) diff --git a/pkg/adapter/mtping/adapter.go b/pkg/adapter/mtping/adapter.go index 6dc74439dee..1e0c045bcfe 100644 --- a/pkg/adapter/mtping/adapter.go +++ b/pkg/adapter/mtping/adapter.go @@ -113,7 +113,7 @@ func (a *mtpingAdapter) Update(ctx context.Context, source *v1beta2.PingSource) a.entryidMu.Unlock() } -func (a *mtpingAdapter) Remove(ctx context.Context, source *v1beta2.PingSource) { +func (a *mtpingAdapter) Remove(source *v1beta2.PingSource) { key := fmt.Sprintf("%s/%s", source.Namespace, source.Name) a.entryidMu.RLock() diff --git a/pkg/adapter/mtping/adapter_test.go b/pkg/adapter/mtping/adapter_test.go index f03648ada89..0316b4316ec 100644 --- a/pkg/adapter/mtping/adapter_test.go +++ b/pkg/adapter/mtping/adapter_test.go @@ -81,7 +81,7 @@ func TestUpdateRemoveAdapter(t *testing.T) { t.Error(`Expected cron entries to contain "test-ns/test-name"`) } - adapter.Remove(ctx, &v1beta2.PingSource{ + adapter.Remove(&v1beta2.PingSource{ ObjectMeta: metav1.ObjectMeta{ Name: "test-name", Namespace: "test-ns", diff --git a/pkg/adapter/mtping/controller.go b/pkg/adapter/mtping/controller.go index 707a428f5b6..ef7bace0a56 100644 --- a/pkg/adapter/mtping/controller.go +++ b/pkg/adapter/mtping/controller.go @@ -19,15 +19,15 @@ package mtping import ( "context" - "knative.dev/pkg/reconciler" - - "knative.dev/pkg/controller" - "knative.dev/pkg/logging" + "k8s.io/client-go/tools/cache" "knative.dev/eventing/pkg/adapter/v2" "knative.dev/eventing/pkg/apis/sources/v1beta2" pingsourceinformer "knative.dev/eventing/pkg/client/injection/informers/sources/v1beta2/pingsource" pingsourcereconciler "knative.dev/eventing/pkg/client/injection/reconciler/sources/v1beta2/pingsource" + "knative.dev/pkg/controller" + "knative.dev/pkg/logging" + "knative.dev/pkg/reconciler" ) // TODO: code generation @@ -38,7 +38,7 @@ type MTAdapter interface { Update(ctx context.Context, source *v1beta2.PingSource) // Remove is called when the source has been deleted. - Remove(ctx context.Context, source *v1beta2.PingSource) + Remove(source *v1beta2.PingSource) // RemoveAll is called when the adapter stopped leading RemoveAll(ctx context.Context) @@ -64,6 +64,11 @@ func NewController(ctx context.Context, adapter adapter.Adapter) *controller.Imp }) logging.FromContext(ctx).Info("Setting up event handlers") - pingsourceinformer.Get(ctx).Informer().AddEventHandler(controller.HandleAll(impl.Enqueue)) + pingsourceinformer.Get(ctx).Informer().AddEventHandler( + cache.ResourceEventHandlerFuncs{ + AddFunc: impl.Enqueue, + UpdateFunc: controller.PassNew(impl.Enqueue), + DeleteFunc: r.deleteFunc, + }) return impl } diff --git a/pkg/adapter/mtping/controller_test.go b/pkg/adapter/mtping/controller_test.go index 1a4ae72c259..1187a499588 100644 --- a/pkg/adapter/mtping/controller_test.go +++ b/pkg/adapter/mtping/controller_test.go @@ -18,6 +18,8 @@ package mtping import ( "context" + "fmt" + "testing" . "knative.dev/pkg/reconciler/testing" @@ -29,6 +31,8 @@ import ( "knative.dev/eventing/pkg/apis/sources/v1beta2" ) +var removePingsource map[string]bool + type testAdapter struct { adapter.Adapter } @@ -36,7 +40,11 @@ type testAdapter struct { func (testAdapter) Update(context.Context, *v1beta2.PingSource) { } -func (testAdapter) Remove(context.Context, *v1beta2.PingSource) { +func (testAdapter) Remove(p *v1beta2.PingSource) { + if removePingsource == nil { + removePingsource = make(map[string]bool) + } + removePingsource[fmt.Sprintf("%s/%s", p.Namespace, p.Name)] = true } func (testAdapter) RemoveAll(context.Context) { diff --git a/pkg/adapter/mtping/pingsource.go b/pkg/adapter/mtping/pingsource.go index 4b590e6da69..2894bc0e59e 100644 --- a/pkg/adapter/mtping/pingsource.go +++ b/pkg/adapter/mtping/pingsource.go @@ -20,10 +20,10 @@ import ( "context" "fmt" - "knative.dev/pkg/reconciler" - "knative.dev/eventing/pkg/apis/sources/v1beta2" pingsourcereconciler "knative.dev/eventing/pkg/client/injection/reconciler/sources/v1beta2/pingsource" + "knative.dev/pkg/kmeta" + "knative.dev/pkg/reconciler" ) // TODO: code generation @@ -36,9 +36,6 @@ type Reconciler struct { // Check that our Reconciler implements ReconcileKind. var _ pingsourcereconciler.Interface = (*Reconciler)(nil) -// Check that our Reconciler implements FinalizeKind. -var _ pingsourcereconciler.Finalizer = (*Reconciler)(nil) - func (r *Reconciler) ReconcileKind(ctx context.Context, source *v1beta2.PingSource) reconciler.Event { if !source.Status.IsReady() { return fmt.Errorf("warning: PingSource is not ready") @@ -50,9 +47,17 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, source *v1beta2.PingSour return nil } -func (r *Reconciler) FinalizeKind(ctx context.Context, source *v1beta2.PingSource) reconciler.Event { - // Update the adapter state - r.mtadapter.Remove(ctx, source) - - return nil +func (r *Reconciler) deleteFunc(obj interface{}) { + if obj == nil { + return + } + acc, err := kmeta.DeletionHandlingAccessor(obj) + if err != nil { + return + } + pingSource, ok := acc.(*v1beta2.PingSource) + if !ok || pingSource == nil { + return + } + r.mtadapter.Remove(pingSource) } diff --git a/pkg/adapter/mtping/pingsource_test.go b/pkg/adapter/mtping/pingsource_test.go index 81e52ab2574..64a3ba59778 100644 --- a/pkg/adapter/mtping/pingsource_test.go +++ b/pkg/adapter/mtping/pingsource_test.go @@ -18,14 +18,12 @@ package mtping import ( "context" + "testing" cloudevents "github.com/cloudevents/sdk-go/v2" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - clientgotesting "k8s.io/client-go/testing" "knative.dev/eventing/pkg/apis/sources/v1beta2" fakeeventingclient "knative.dev/eventing/pkg/client/injection/client/fake" "knative.dev/eventing/pkg/client/injection/reconciler/sources/v1beta2/pingsource" @@ -41,14 +39,13 @@ import ( ) const ( - testNS = "test-namespace" - pingSourceName = "test-pingsource" - testSchedule = "*/2 * * * *" - testContentType = cloudevents.TextPlain - testData = "data" - testDataBase64 = "ZGF0YQ==" - sinkName = "mysink" - defaultFinalizerName = "pingsources.sources.knative.dev" + testNS = "test-namespace" + pingSourceName = "test-pingsource" + testSchedule = "*/2 * * * *" + testContentType = cloudevents.TextPlain + testData = "data" + testDataBase64 = "ZGF0YQ==" + sinkName = "mysink" ) var ( @@ -91,12 +88,6 @@ func TestAllCases(t *testing.T) { rttestingv1beta2.WithPingSourceCloudEventAttributes, ), }, - WantEvents: []string{ - Eventf(corev1.EventTypeNormal, "FinalizerUpdate", `Updated "%s" finalizers`, pingSourceName), - }, - WantPatches: []clientgotesting.PatchActionImpl{ - patchFinalizers(testNS, pingSourceName, defaultFinalizerName), - }, WantErr: false, }, { Name: "valid schedule without contentType, data and dataBase64", @@ -116,12 +107,6 @@ func TestAllCases(t *testing.T) { rttestingv1beta2.WithPingSourceCloudEventAttributes, ), }, - WantEvents: []string{ - Eventf(corev1.EventTypeNormal, "FinalizerUpdate", `Updated "%s" finalizers`, pingSourceName), - }, - WantPatches: []clientgotesting.PatchActionImpl{ - patchFinalizers(testNS, pingSourceName, defaultFinalizerName), - }, WantErr: false, }, { Name: "valid schedule with dataBase64", @@ -143,12 +128,6 @@ func TestAllCases(t *testing.T) { rttestingv1beta2.WithPingSourceCloudEventAttributes, ), }, - WantEvents: []string{ - Eventf(corev1.EventTypeNormal, "FinalizerUpdate", `Updated "%s" finalizers`, pingSourceName), - }, - WantPatches: []clientgotesting.PatchActionImpl{ - patchFinalizers(testNS, pingSourceName, defaultFinalizerName), - }, WantErr: false, }, { Name: "valid schedule, with finalizer", @@ -168,7 +147,6 @@ func TestAllCases(t *testing.T) { rttestingv1beta2.WithPingSourceDeployed, rttestingv1beta2.WithPingSourceSink(sinkURI), rttestingv1beta2.WithPingSourceCloudEventAttributes, - rttestingv1beta2.WithPingSourceFinalizers(defaultFinalizerName), ), }, WantErr: false, @@ -190,20 +168,13 @@ func TestAllCases(t *testing.T) { rttestingv1beta2.WithPingSourceDeployed, rttestingv1beta2.WithPingSourceSink(sinkURI), rttestingv1beta2.WithPingSourceCloudEventAttributes, - rttestingv1beta2.WithPingSourceFinalizers(defaultFinalizerName), rttestingv1beta2.WithPingSourceDeleted, ), }, - WantEvents: []string{ - Eventf(corev1.EventTypeNormal, "FinalizerUpdate", `Updated "%s" finalizers`, pingSourceName), - }, - WantPatches: []clientgotesting.PatchActionImpl{ - patchFinalizers(testNS, pingSourceName, ""), - }, WantErr: false, }, { Name: "valid schedule, deleted without finalizer", - Key: pingsourceKey, + Key: "a/a", Objects: []runtime.Object{ rttestingv1beta2.NewPingSource(pingSourceName, testNS, rttestingv1beta2.WithPingSourceSpec(v1beta2.PingSourceSpec{ @@ -237,20 +208,13 @@ func TestAllCases(t *testing.T) { } -func patchFinalizers(namespace, name string, finalizers string) clientgotesting.PatchActionImpl { - fstr := "" - if finalizers != "" { - fstr = `"` + finalizers + `"` - } - return clientgotesting.PatchActionImpl{ - ActionImpl: clientgotesting.ActionImpl{ - Namespace: namespace, - Verb: "patch", - Resource: schema.GroupVersionResource{Group: "sources.knative.dev", Version: "v1beta1", Resource: "pingsources"}, - Subresource: "", - }, - Name: name, - PatchType: "application/merge-patch+json", - Patch: []byte(`{"metadata":{"finalizers":[` + fstr + `],"resourceVersion":""}}`), +func TestReconciler_deleteFunc(t *testing.T) { + pingsourceKey := testNS + "/" + pingSourceName + adapter := &testAdapter{} + p := rttestingv1beta2.NewPingSource(pingSourceName, testNS) + r := &Reconciler{mtadapter: adapter} + r.deleteFunc(p) + if _, ok := removePingsource[pingsourceKey]; !ok { + t.Errorf("Got error when call deleteFunc") } }