From 6085ee8d7d50abcb03043259a287af87f80113e0 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 | 9 ++ config/base/rbac/role.yaml | 1 + .../controllers/clustercatalog_controller.go | 76 +++++++++++++++ .../clustercatalog_controller_test.go | 95 +++++++++++++++++++ 4 files changed, 181 insertions(+) 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..55951ca83 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -278,6 +278,15 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "ClusterExtension") os.Exit(1) } + + if err = (&controllers.ClusterCatalogReconciler{ + Client: cl, + Cache: cacheFetcher, + }).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..ee0a59833 100644 --- a/config/base/rbac/role.yaml +++ b/config/base/rbac/role.yaml @@ -21,6 +21,7 @@ rules: resources: - clustercatalogs verbs: + - get - list - watch - apiGroups: diff --git a/internal/controllers/clustercatalog_controller.go b/internal/controllers/clustercatalog_controller.go new file mode 100644 index 000000000..0f7a26a6c --- /dev/null +++ b/internal/controllers/clustercatalog_controller.go @@ -0,0 +1,76 @@ +/* +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" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" + + catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1" +) + +type CatalogCacheRemover interface { + Remove(catalogName string) error +} + +// ClusterCatalogReconciler reconciles a ClusterCatalog object +type ClusterCatalogReconciler struct { + client.Client + Cache CatalogCacheRemover +} + +//+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clustercatalogs,verbs=get;list;watch + +func (r *ClusterCatalogReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + existingCatalog := &catalogd.ClusterCatalog{} + err := r.Client.Get(ctx, req.NamespacedName, existingCatalog) + if apierrors.IsNotFound(err) { + return ctrl.Result{}, r.Cache.Remove(req.Name) + } + if err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil +} + +// SetupWithManager sets up the controller with the Manager. +func (r *ClusterCatalogReconciler) SetupWithManager(mgr ctrl.Manager) error { + _, err := ctrl.NewControllerManagedBy(mgr). + For(&catalogd.ClusterCatalog{}, builder.WithPredicates(predicate.Funcs{ + CreateFunc: func(e event.CreateEvent) bool { + return false + }, + UpdateFunc: func(e event.UpdateEvent) bool { + return false + }, + DeleteFunc: func(e event.DeleteEvent) bool { + return true + }, + GenericFunc: func(e event.GenericEvent) bool { + return false + }, + })). + 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..762fa15ec --- /dev/null +++ b/internal/controllers/clustercatalog_controller_test.go @@ -0,0 +1,95 @@ +package controllers_test + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + 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/fake" + + 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) { + catalogKey := types.NamespacedName{Name: "test-catalog"} + + for _, tt := range []struct { + name string + catalog *catalogd.ClusterCatalog + cacheRemoveFunc func(catalogName string) error + wantCacheRemoveCalled bool + wantErr string + }{ + { + name: "catalog exists", + catalog: &catalogd.ClusterCatalog{ + ObjectMeta: metav1.ObjectMeta{ + Name: catalogKey.Name, + }, + }, + }, + { + name: "catalog does not exist", + cacheRemoveFunc: func(catalogName string) error { + assert.Equal(t, catalogKey.Name, catalogName) + return nil + }, + wantCacheRemoveCalled: true, + }, + { + name: "catalog does not exist - error on removal", + cacheRemoveFunc: func(catalogName string) error { + return errors.New("fake error from remove") + }, + wantCacheRemoveCalled: true, + wantErr: "fake error from remove", + }, + } { + 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() + + cacheRemover := &mockCatalogCacheRemover{ + removeFunc: tt.cacheRemoveFunc, + } + + reconciler := &controllers.ClusterCatalogReconciler{ + Client: cl, + Cache: cacheRemover, + } + + 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) + + assert.Equal(t, tt.wantCacheRemoveCalled, cacheRemover.called) + }) + } +} + +type mockCatalogCacheRemover struct { + called bool + removeFunc func(catalogName string) error +} + +func (m *mockCatalogCacheRemover) Remove(catalogName string) error { + m.called = true + return m.removeFunc(catalogName) +}