Skip to content

Commit

Permalink
Added empty FinalizeKind definition to prevent delete sources from ha…
Browse files Browse the repository at this point in the history 
…nging (#3113) (#3116)

* Added empty FinalizeKind definition to prevent delete sources from hanging

* Updated tests to handle reintroduction of finalizer patch events
  • Loading branch information
@Cali0707
Cali0707 authored May 30, 2023
1 parent f47a52f commit 6e2c394
Show file tree
Hide file tree
Showing 2 changed files with 140 additions and 0 deletions.
5 changes: 5 additions & 0 deletions control-plane/pkg/reconciler/source/source.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
135 changes: 135 additions & 0 deletions control-plane/pkg/reconciler/source/source_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -137,6 +149,12 @@ func TestReconcileKind(t *testing.T) {
),
},
},
WantPatches: []clientgotesting.PatchActionImpl{
patchFinalizers(),
},
WantEvents: []string{
finalizerUpdatedEvent,
},
},
{
Name: "Reconciled normal, offset earliest",
Expand Down Expand Up @@ -184,6 +202,12 @@ func TestReconcileKind(t *testing.T) {
),
},
},
WantPatches: []clientgotesting.PatchActionImpl{
patchFinalizers(),
},
WantEvents: []string{
finalizerUpdatedEvent,
},
},
{
Name: "Reconciled normal, offset latest",
Expand Down Expand Up @@ -231,6 +255,12 @@ func TestReconcileKind(t *testing.T) {
),
},
},
WantPatches: []clientgotesting.PatchActionImpl{
patchFinalizers(),
},
WantEvents: []string{
finalizerUpdatedEvent,
},
},
{
Name: "Reconciled normal with delivery spec",
Expand Down Expand Up @@ -283,6 +313,12 @@ func TestReconcileKind(t *testing.T) {
),
},
},
WantPatches: []clientgotesting.PatchActionImpl{
patchFinalizers(),
},
WantEvents: []string{
finalizerUpdatedEvent,
},
},
{
Name: "Reconciled normal, key type label",
Expand Down Expand Up @@ -331,6 +367,12 @@ func TestReconcileKind(t *testing.T) {
),
},
},
WantPatches: []clientgotesting.PatchActionImpl{
patchFinalizers(),
},
WantEvents: []string{
finalizerUpdatedEvent,
},
},
{
Name: "Reconciled normal with SASL with type",
Expand Down Expand Up @@ -460,6 +502,12 @@ func TestReconcileKind(t *testing.T) {
),
},
},
WantPatches: []clientgotesting.PatchActionImpl{
patchFinalizers(),
},
WantEvents: []string{
finalizerUpdatedEvent,
},
},
{
Name: "Reconciled normal with SASL without type",
Expand Down Expand Up @@ -573,6 +621,12 @@ func TestReconcileKind(t *testing.T) {
),
},
},
WantPatches: []clientgotesting.PatchActionImpl{
patchFinalizers(),
},
WantEvents: []string{
finalizerUpdatedEvent,
},
},
{
Name: "Reconciled normal - ce overrides",
Expand Down Expand Up @@ -628,6 +682,12 @@ func TestReconcileKind(t *testing.T) {
),
},
},
WantPatches: []clientgotesting.PatchActionImpl{
patchFinalizers(),
},
WantEvents: []string{
finalizerUpdatedEvent,
},
},
{
Name: "Reconciled normal - with autoscaling annotations",
Expand Down Expand Up @@ -676,6 +736,12 @@ func TestReconcileKind(t *testing.T) {
),
},
},
WantPatches: []clientgotesting.PatchActionImpl{
patchFinalizers(),
},
WantEvents: []string{
finalizerUpdatedEvent,
},
},
{
Name: "Reconciled normal - existing cg with sink update",
Expand Down Expand Up @@ -759,6 +825,12 @@ func TestReconcileKind(t *testing.T) {
),
},
},
WantPatches: []clientgotesting.PatchActionImpl{
patchFinalizers(),
},
WantEvents: []string{
finalizerUpdatedEvent,
},
},
{
Name: "Reconciled normal - existing cg with update",
Expand Down Expand Up @@ -818,6 +890,12 @@ func TestReconcileKind(t *testing.T) {
),
},
},
WantPatches: []clientgotesting.PatchActionImpl{
patchFinalizers(),
},
WantEvents: []string{
finalizerUpdatedEvent,
},
},
{
Name: "Reconciled normal - existing cg with annotations update",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -936,6 +1020,12 @@ func TestReconcileKind(t *testing.T) {
),
},
},
WantPatches: []clientgotesting.PatchActionImpl{
patchFinalizers(),
},
WantEvents: []string{
finalizerUpdatedEvent,
},
},
{
Name: "Reconciled normal - existing cg without update",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -1028,6 +1124,12 @@ func TestReconcileKind(t *testing.T) {
),
},
},
WantPatches: []clientgotesting.PatchActionImpl{
patchFinalizers(),
},
WantEvents: []string{
finalizerUpdatedEvent,
},
},
{
Name: "Reconciled normal - existing cg but failed",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -1122,6 +1230,12 @@ func TestReconcileKind(t *testing.T) {
),
},
},
WantPatches: []clientgotesting.PatchActionImpl{
patchFinalizers(),
},
WantEvents: []string{
finalizerUpdatedEvent,
},
},
{
Name: "Reconciled normal - update cg replicas",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -1255,6 +1375,12 @@ func TestReconcileKind(t *testing.T) {
),
},
},
WantPatches: []clientgotesting.PatchActionImpl{
patchFinalizers(),
},
WantEvents: []string{
finalizerUpdatedEvent,
},
},
}

Expand Down Expand Up @@ -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
}

0 comments on commit 6e2c394

Please sign in to comment.