From 7a2b32f178331aba2ddd428835d5b08f41ff7049 Mon Sep 17 00:00:00 2001 From: Mikalai Radchuk Date: Mon, 16 Sep 2024 17:36:38 +0200 Subject: [PATCH] Add a finaliser to ClusterCatalog New finaliser allows us to remove catalog cache from filesystem on catalog deletion. Signed-off-by: Mikalai Radchuk --- cmd/manager/main.go | 17 +++ config/base/rbac/role.yaml | 16 ++- .../controllers/clustercatalog_controller.go | 79 +++++++++++ .../clustercatalog_controller_test.go | 125 ++++++++++++++++++ 4 files changed, 231 insertions(+), 6 deletions(-) create mode 100644 internal/controllers/clustercatalog_controller.go create mode 100644 internal/controllers/clustercatalog_controller_test.go diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 03de6c1c5..41455966a 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -278,6 +278,23 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "ClusterExtension") os.Exit(1) } + + clusterCatalogFinalizers := crfinalizer.NewFinalizers() + if err := clusterCatalogFinalizers.Register(controllers.ClusterCatalogCacheDeletionFinalizer, finalizerFunc(func(ctx context.Context, obj client.Object) (crfinalizer.Result, error) { + return crfinalizer.Result{}, cacheFetcher.Remove(obj.GetName()) + })); err != nil { + setupLog.Error(err, "unable to register finalizer", "finalizerKey", controllers.ClusterCatalogCacheDeletionFinalizer) + os.Exit(1) + } + + if err = (&controllers.ClusterCatalogReconciler{ + Client: cl, + Finalizers: clusterCatalogFinalizers, + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "ClusterCatalog") + os.Exit(1) + } + //+kubebuilder:scaffold:builder if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { diff --git a/config/base/rbac/role.yaml b/config/base/rbac/role.yaml index 38d394780..9a17f5209 100644 --- a/config/base/rbac/role.yaml +++ b/config/base/rbac/role.yaml @@ -21,24 +21,28 @@ rules: resources: - clustercatalogs verbs: + - get - list + - update - watch - apiGroups: - olm.operatorframework.io resources: - - clusterextensions + - clustercatalogs/finalizers + - clustercatalogs/status + - clusterextensions/finalizers verbs: - - get - - list - - patch - update - - watch - apiGroups: - olm.operatorframework.io resources: - - clusterextensions/finalizers + - clusterextensions verbs: + - get + - list + - patch - update + - watch - apiGroups: - olm.operatorframework.io resources: diff --git a/internal/controllers/clustercatalog_controller.go b/internal/controllers/clustercatalog_controller.go new file mode 100644 index 000000000..7fb7dcc64 --- /dev/null +++ b/internal/controllers/clustercatalog_controller.go @@ -0,0 +1,79 @@ +/* +Copyright 2024. + +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 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controllers + +import ( + "context" + "errors" + "fmt" + + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + crfinalizer "sigs.k8s.io/controller-runtime/pkg/finalizer" + + catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1" +) + +const ( + ClusterCatalogCacheDeletionFinalizer = "olm.operatorframework.io/cluster-catalog-cache-deletion" +) + +// ClusterCatalogReconciler reconciles a ClusterCatalog object +type ClusterCatalogReconciler struct { + client.Client + Finalizers crfinalizer.Finalizers +} + +//+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clustercatalogs,verbs=get;list;watch;update +//+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clustercatalogs/status,verbs=update +//+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clustercatalogs/finalizers,verbs=update + +func (r *ClusterCatalogReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + existingCatalog := &catalogd.ClusterCatalog{} + if err := r.Client.Get(ctx, req.NamespacedName, existingCatalog); err != nil { + return ctrl.Result{}, client.IgnoreNotFound(err) + } + + finalizeResult, err := r.Finalizers.Finalize(ctx, existingCatalog) + if err != nil { + return ctrl.Result{}, err + } + + var updateError error + if finalizeResult.StatusUpdated { + if err := r.Client.Status().Update(ctx, existingCatalog); err != nil { + updateError = errors.Join(updateError, fmt.Errorf("error updating status: %v", err)) + } + } + + if finalizeResult.Updated { + if err := r.Client.Update(ctx, existingCatalog); err != nil { + updateError = errors.Join(updateError, fmt.Errorf("error updating finalizers: %v", err)) + } + } + + return ctrl.Result{}, updateError +} + +// SetupWithManager sets up the controller with the Manager. +func (r *ClusterCatalogReconciler) SetupWithManager(mgr ctrl.Manager) error { + _, err := ctrl.NewControllerManagedBy(mgr). + For(&catalogd.ClusterCatalog{}). + Build(r) + + return err +} diff --git a/internal/controllers/clustercatalog_controller_test.go b/internal/controllers/clustercatalog_controller_test.go new file mode 100644 index 000000000..e5b9ead27 --- /dev/null +++ b/internal/controllers/clustercatalog_controller_test.go @@ -0,0 +1,125 @@ +package controllers_test + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/stretchr/testify/require" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + crfinalizer "sigs.k8s.io/controller-runtime/pkg/finalizer" + + catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1" + + "github.com/operator-framework/operator-controller/internal/controllers" + "github.com/operator-framework/operator-controller/internal/scheme" +) + +func TestClusterCatalogReconcilerFinalizers(t *testing.T) { + const fakeFinalizerKey = "fake-finalizer" + catalogKey := types.NamespacedName{Name: "test-catalog"} + + for _, tt := range []struct { + name string + catalog *catalogd.ClusterCatalog + finalizer mockFinalizer + wantCatalogToNotExist bool + wantErr string + }{ + { + name: "catalog exists with a finalizer", + catalog: &catalogd.ClusterCatalog{ + ObjectMeta: metav1.ObjectMeta{ + Name: catalogKey.Name, + Finalizers: []string{fakeFinalizerKey}, + }, + }, + }, + { + name: "catalog exists without a finalizer", + catalog: &catalogd.ClusterCatalog{ + ObjectMeta: metav1.ObjectMeta{ + Name: catalogKey.Name, + }, + }, + }, + { + name: "catalog exists and is marked for deletion", + catalog: &catalogd.ClusterCatalog{ + ObjectMeta: metav1.ObjectMeta{ + Name: catalogKey.Name, + DeletionTimestamp: &metav1.Time{Time: time.Now().Add(-time.Minute)}, + Finalizers: []string{fakeFinalizerKey}, + }, + }, + wantCatalogToNotExist: true, + }, + { + name: "catalog exists and is marked for deletion - finalizer errors", + catalog: &catalogd.ClusterCatalog{ + ObjectMeta: metav1.ObjectMeta{ + Name: catalogKey.Name, + DeletionTimestamp: &metav1.Time{Time: time.Now().Add(-time.Minute)}, + Finalizers: []string{fakeFinalizerKey}, + }, + }, + finalizer: mockFinalizer{resultErr: errors.New("fake error from finalizer")}, + wantErr: "fake error from finalizer", + }, + { + name: "catalog does not exist", + wantCatalogToNotExist: true, + }, + } { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + + clientBuilder := fake.NewClientBuilder().WithScheme(scheme.Scheme) + if tt.catalog != nil { + clientBuilder = clientBuilder.WithObjects(tt.catalog) + } + cl := clientBuilder.Build() + + clusterCatalogFinalizers := crfinalizer.NewFinalizers() + err := clusterCatalogFinalizers.Register(fakeFinalizerKey, tt.finalizer) + require.NoError(t, err) + + reconciler := &controllers.ClusterCatalogReconciler{ + Client: cl, + Finalizers: clusterCatalogFinalizers, + } + + result, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: catalogKey}) + if tt.wantErr == "" { + require.NoError(t, err) + } else { + require.ErrorContains(t, err, tt.wantErr) + } + require.Equal(t, ctrl.Result{}, result) + + if tt.wantCatalogToNotExist { + reconciledCatalog := &catalogd.ClusterCatalog{} + require.True(t, apierrors.IsNotFound(cl.Get(ctx, catalogKey, reconciledCatalog))) + } else { + reconciledCatalog := &catalogd.ClusterCatalog{} + require.NoError(t, cl.Get(ctx, catalogKey, reconciledCatalog)) + require.Len(t, reconciledCatalog.Finalizers, 1) + require.Equal(t, reconciledCatalog.Finalizers[0], fakeFinalizerKey) + } + }) + } +} + +type mockFinalizer struct { + resultErr error +} + +func (f mockFinalizer) Finalize(ctx context.Context, obj client.Object) (crfinalizer.Result, error) { + return crfinalizer.Result{}, f.resultErr +}