From 6e2c39406893d344e2079310a3175ef757a19c35 Mon Sep 17 00:00:00 2001 From: Calum Murray Date: Tue, 30 May 2023 11:17:35 -0400 Subject: [PATCH] Added empty FinalizeKind definition to prevent delete sources from hanging (#3113) (#3116) * Added empty FinalizeKind definition to prevent delete sources from hanging * Updated tests to handle reintroduction of finalizer patch events --- control-plane/pkg/reconciler/source/source.go | 5 + .../pkg/reconciler/source/source_test.go | 135 ++++++++++++++++++ 2 files changed, 140 insertions(+) diff --git a/control-plane/pkg/reconciler/source/source.go b/control-plane/pkg/reconciler/source/source.go index fe4aa453bb..97441ffd2e 100644 --- a/control-plane/pkg/reconciler/source/source.go +++ b/control-plane/pkg/reconciler/source/source.go @@ -98,6 +98,11 @@ func GetLabels(name string) map[string]string { } } +// Need to have an empty definition here to ensure that we can delete older sources which had a finalizer +func (r Reconciler) FinalizeKind(ctx context.Context, ks *sources.KafkaSource) reconciler.Event { + return nil +} + func (r Reconciler) reconcileConsumerGroup(ctx context.Context, ks *sources.KafkaSource) (*internalscg.ConsumerGroup, error) { var deliverySpec *internalscg.DeliverySpec if ks.Spec.Delivery != nil { diff --git a/control-plane/pkg/reconciler/source/source_test.go b/control-plane/pkg/reconciler/source/source_test.go index 9d9a5d5645..3fe6a9cb9a 100644 --- a/control-plane/pkg/reconciler/source/source_test.go +++ b/control-plane/pkg/reconciler/source/source_test.go @@ -55,6 +55,18 @@ import ( kedaclient "knative.dev/eventing-kafka-broker/third_party/pkg/client/injection/client/fake" ) +const ( + finalizerName = "kafkasources.sources.knative.dev" +) + +var ( + finalizerUpdatedEvent = Eventf( + corev1.EventTypeNormal, + "FinalizerUpdate", + fmt.Sprintf(`Updated %q finalizers`, SourceName), + ) +) + func TestGetLabels(t *testing.T) { testLabels := GetLabels("testSourceName") @@ -137,6 +149,12 @@ func TestReconcileKind(t *testing.T) { ), }, }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchFinalizers(), + }, + WantEvents: []string{ + finalizerUpdatedEvent, + }, }, { Name: "Reconciled normal, offset earliest", @@ -184,6 +202,12 @@ func TestReconcileKind(t *testing.T) { ), }, }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchFinalizers(), + }, + WantEvents: []string{ + finalizerUpdatedEvent, + }, }, { Name: "Reconciled normal, offset latest", @@ -231,6 +255,12 @@ func TestReconcileKind(t *testing.T) { ), }, }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchFinalizers(), + }, + WantEvents: []string{ + finalizerUpdatedEvent, + }, }, { Name: "Reconciled normal with delivery spec", @@ -283,6 +313,12 @@ func TestReconcileKind(t *testing.T) { ), }, }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchFinalizers(), + }, + WantEvents: []string{ + finalizerUpdatedEvent, + }, }, { Name: "Reconciled normal, key type label", @@ -331,6 +367,12 @@ func TestReconcileKind(t *testing.T) { ), }, }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchFinalizers(), + }, + WantEvents: []string{ + finalizerUpdatedEvent, + }, }, { Name: "Reconciled normal with SASL with type", @@ -460,6 +502,12 @@ func TestReconcileKind(t *testing.T) { ), }, }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchFinalizers(), + }, + WantEvents: []string{ + finalizerUpdatedEvent, + }, }, { Name: "Reconciled normal with SASL without type", @@ -573,6 +621,12 @@ func TestReconcileKind(t *testing.T) { ), }, }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchFinalizers(), + }, + WantEvents: []string{ + finalizerUpdatedEvent, + }, }, { Name: "Reconciled normal - ce overrides", @@ -628,6 +682,12 @@ func TestReconcileKind(t *testing.T) { ), }, }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchFinalizers(), + }, + WantEvents: []string{ + finalizerUpdatedEvent, + }, }, { Name: "Reconciled normal - with autoscaling annotations", @@ -676,6 +736,12 @@ func TestReconcileKind(t *testing.T) { ), }, }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchFinalizers(), + }, + WantEvents: []string{ + finalizerUpdatedEvent, + }, }, { Name: "Reconciled normal - existing cg with sink update", @@ -759,6 +825,12 @@ func TestReconcileKind(t *testing.T) { ), }, }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchFinalizers(), + }, + WantEvents: []string{ + finalizerUpdatedEvent, + }, }, { Name: "Reconciled normal - existing cg with update", @@ -818,6 +890,12 @@ func TestReconcileKind(t *testing.T) { ), }, }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchFinalizers(), + }, + WantEvents: []string{ + finalizerUpdatedEvent, + }, }, { Name: "Reconciled normal - existing cg with annotations update", @@ -878,6 +956,12 @@ func TestReconcileKind(t *testing.T) { ), }, }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchFinalizers(), + }, + WantEvents: []string{ + finalizerUpdatedEvent, + }, }, { Name: "Reconciled normal - existing cg with update but not ready", @@ -936,6 +1020,12 @@ func TestReconcileKind(t *testing.T) { ), }, }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchFinalizers(), + }, + WantEvents: []string{ + finalizerUpdatedEvent, + }, }, { Name: "Reconciled normal - existing cg without update", @@ -983,6 +1073,12 @@ func TestReconcileKind(t *testing.T) { ), }, }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchFinalizers(), + }, + WantEvents: []string{ + finalizerUpdatedEvent, + }, }, { Name: "Reconciled normal - existing cg without update but not ready", @@ -1028,6 +1124,12 @@ func TestReconcileKind(t *testing.T) { ), }, }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchFinalizers(), + }, + WantEvents: []string{ + finalizerUpdatedEvent, + }, }, { Name: "Reconciled normal - existing cg but failed", @@ -1074,6 +1176,12 @@ func TestReconcileKind(t *testing.T) { ), }, }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchFinalizers(), + }, + WantEvents: []string{ + finalizerUpdatedEvent, + }, }, { Name: "Reconciled normal - existing cg with replicas set in status", @@ -1122,6 +1230,12 @@ func TestReconcileKind(t *testing.T) { ), }, }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchFinalizers(), + }, + WantEvents: []string{ + finalizerUpdatedEvent, + }, }, { Name: "Reconciled normal - update cg replicas", @@ -1204,6 +1318,12 @@ func TestReconcileKind(t *testing.T) { ), }, }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchFinalizers(), + }, + WantEvents: []string{ + finalizerUpdatedEvent, + }, }, { Name: "Reconciled normal - ignore source replicas when KEDA is enabled", @@ -1255,6 +1375,12 @@ func TestReconcileKind(t *testing.T) { ), }, }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchFinalizers(), + }, + WantEvents: []string{ + finalizerUpdatedEvent, + }, }, } @@ -1366,3 +1492,12 @@ func SourceNetSaslTls(withType bool) KRShapedOption { } } } + +func patchFinalizers() clientgotesting.PatchActionImpl { + action := clientgotesting.PatchActionImpl{} + action.Name = SourceName + action.Namespace = SourceNamespace + patch := `{"metadata":{"finalizers":["` + finalizerName + `"],"resourceVersion":""}}` + action.Patch = []byte(patch) + return action +}