diff --git a/internal/catalogd/controllers/core/clustercatalog_controller.go b/internal/catalogd/controllers/core/clustercatalog_controller.go index d0597d3ee..ce1636266 100644 --- a/internal/catalogd/controllers/core/clustercatalog_controller.go +++ b/internal/catalogd/controllers/core/clustercatalog_controller.go @@ -196,6 +196,14 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *ocv1. return ctrl.Result{}, nil } + if catalog.GetDeletionTimestamp() != nil { + // If we've gotten here, that means the cluster catalog is being deleted, we've handled all of + // _our_ finalizers (above), but the cluster catalog is still present in the cluster, likely + // because there are _other_ finalizers that other controllers need to handle, (e.g. the orphan + // deletion finalizer). + return ctrl.Result{}, nil + } + // TODO: The below algorithm to get the current state based on an in-memory // storedCatalogs map is a hack that helps us keep the ClusterCatalog's // status up-to-date. The fact that we need this setup is indicative of diff --git a/internal/catalogd/controllers/core/clustercatalog_controller_test.go b/internal/catalogd/controllers/core/clustercatalog_controller_test.go index f7b917dc1..95a18733a 100644 --- a/internal/catalogd/controllers/core/clustercatalog_controller_test.go +++ b/internal/catalogd/controllers/core/clustercatalog_controller_test.go @@ -766,6 +766,40 @@ func TestCatalogdControllerReconcile(t *testing.T) { }, }, }, + { + name: "reconcile should be short-circuited if the clustercatalog has a deletion timestamp and all known finalizers have been removed", + catalog: &ocv1.ClusterCatalog{ + ObjectMeta: metav1.ObjectMeta{ + Name: "catalog", + Finalizers: []string{"finalizer"}, + DeletionTimestamp: &metav1.Time{Time: time.Date(2025, 6, 10, 16, 43, 0, 0, time.UTC)}, + }, + Spec: ocv1.ClusterCatalogSpec{ + Source: ocv1.CatalogSource{ + Type: ocv1.SourceTypeImage, + Image: &ocv1.ImageSource{ + Ref: "my.org/someimage:latest", + }, + }, + AvailabilityMode: ocv1.AvailabilityModeAvailable, + }, + }, + expectedCatalog: &ocv1.ClusterCatalog{ + ObjectMeta: metav1.ObjectMeta{ + Name: "catalog", + Finalizers: []string{"finalizer"}, + DeletionTimestamp: &metav1.Time{Time: time.Date(2025, 6, 10, 16, 43, 0, 0, time.UTC)}}, + Spec: ocv1.ClusterCatalogSpec{ + Source: ocv1.CatalogSource{ + Type: ocv1.SourceTypeImage, + Image: &ocv1.ImageSource{ + Ref: "my.org/someimage:latest", + }, + }, + AvailabilityMode: ocv1.AvailabilityModeAvailable, + }, + }, + }, } { t.Run(tt.name, func(t *testing.T) { reconciler := &ClusterCatalogReconciler{ diff --git a/internal/operator-controller/controllers/clusterextension_controller.go b/internal/operator-controller/controllers/clusterextension_controller.go index e571174b0..9a79e8c75 100644 --- a/internal/operator-controller/controllers/clusterextension_controller.go +++ b/internal/operator-controller/controllers/clusterextension_controller.go @@ -206,6 +206,14 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1.Cl return ctrl.Result{}, nil } + if ext.GetDeletionTimestamp() != nil { + // If we've gotten here, that means the cluster extension is being deleted, we've handled all of + // _our_ finalizers (above), but the cluster extension is still present in the cluster, likely + // because there are _other_ finalizers that other controllers need to handle, (e.g. the orphan + // deletion finalizer). + return ctrl.Result{}, nil + } + l.Info("getting installed bundle") installedBundle, err := r.InstalledBundleGetter.GetInstalledBundle(ctx, ext) if err != nil { diff --git a/internal/operator-controller/controllers/clusterextension_controller_test.go b/internal/operator-controller/controllers/clusterextension_controller_test.go index be61891a0..64883c416 100644 --- a/internal/operator-controller/controllers/clusterextension_controller_test.go +++ b/internal/operator-controller/controllers/clusterextension_controller_test.go @@ -48,6 +48,79 @@ func TestClusterExtensionDoesNotExist(t *testing.T) { require.NoError(t, err) } +func TestClusterExtensionShortCircuitsReconcileDuringDeletion(t *testing.T) { + cl, reconciler := newClientAndReconciler(t) + + installedBundleGetterCalledErr := errors.New("installed bundle getter called") + checkInstalledBundleGetterCalled := func(t require.TestingT, err error, args ...interface{}) { + require.Equal(t, installedBundleGetterCalledErr, err) + } + reconciler.InstalledBundleGetter = &MockInstalledBundleGetter{ + err: installedBundleGetterCalledErr, + } + + type testCase struct { + name string + finalizers []string + shouldDelete bool + expectErr require.ErrorAssertionFunc + } + for _, tc := range []testCase{ + { + name: "no finalizers, not deleted", + expectErr: checkInstalledBundleGetterCalled, + }, + { + name: "has finalizers, not deleted", + finalizers: []string{"finalizer"}, + expectErr: checkInstalledBundleGetterCalled, + }, + { + name: "has finalizers, deleted", + finalizers: []string{"finalizer"}, + shouldDelete: true, + expectErr: require.NoError, + }, + } { + t.Run(tc.name, func(t *testing.T) { + pkgName := fmt.Sprintf("test-pkg-%s", rand.String(6)) + + ctx := context.Background() + extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} + + t.Log("When the cluster extension specifies a non-existent package") + t.Log("By initializing cluster state") + clusterExtension := &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{ + Name: extKey.Name, + Finalizers: tc.finalizers, + }, + Spec: ocv1.ClusterExtensionSpec{ + Source: ocv1.SourceConfig{ + SourceType: "Catalog", + Catalog: &ocv1.CatalogFilter{ + PackageName: pkgName, + }, + }, + Namespace: "default", + ServiceAccount: ocv1.ServiceAccountReference{ + Name: "default", + }, + }, + } + require.NoError(t, cl.Create(ctx, clusterExtension)) + if tc.shouldDelete { + require.NoError(t, cl.Delete(ctx, clusterExtension)) + } + + t.Log("By running reconcile") + res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) + require.Equal(t, ctrl.Result{}, res) + tc.expectErr(t, err) + }) + } +} + func TestClusterExtensionResolutionFails(t *testing.T) { pkgName := fmt.Sprintf("non-existent-%s", rand.String(6)) cl, reconciler := newClientAndReconciler(t)