07-simplify-contentmanager

Author: joelanford

cmd/operator-controller/main.go

@@ -40,7 +40,6 @@ import (
 	"k8s.io/client-go/discovery/cached/memory"
 	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
-	"k8s.io/client-go/rest"
 	"k8s.io/klog/v2"
 	"k8s.io/utils/ptr"
 	"pkg.package-operator.run/boxcutter/managedcache"
@@ -705,13 +704,14 @@ func (c *helmReconcilerConfigurator) Configure(ceReconciler *controllers.Cluster
 		preAuth = authorization.NewRBACPreAuthorizer(c.mgr.GetClient())
 	}
 
-	cm := contentmanager.NewManager(func(_ context.Context, _ client.Object, cfg *rest.Config) (*rest.Config, error) {
-		return cfg, nil
-	}, c.mgr.GetConfig(), c.mgr.GetRESTMapper())
+	cm, err := contentmanager.NewManager(c.mgr.GetConfig(), c.mgr.GetRESTMapper())
+	if err != nil {
+		setupLog.Error(err, "unable to create content manager")
+		return err
+	}
 	err = c.finalizers.Register(controllers.ClusterExtensionCleanupContentManagerCacheFinalizer, finalizers.FinalizerFunc(func(ctx context.Context, obj client.Object) (crfinalizer.Result, error) {
-		ext := obj.(*ocv1.ClusterExtension)
-		err := cm.Delete(ext)
-		return crfinalizer.Result{}, err
+		cm.Delete(ctx, obj.GetName())
+		return crfinalizer.Result{}, nil
 	}))
 	if err != nil {
 		setupLog.Error(err, "unable to register content manager cleanup finalizer")

internal/operator-controller/applier/helm.go

@@ -29,7 +29,7 @@ import (
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/authorization"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/contentmanager"
-	"github.com/operator-framework/operator-controller/internal/operator-controller/contentmanager/cache"
+	cmcache "github.com/operator-framework/operator-controller/internal/operator-controller/contentmanager/cache"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/features"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util"
 	imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image"
@@ -67,7 +67,7 @@ type Helm struct {
 	HelmReleaseToObjectsConverter HelmReleaseToObjectsConverterInterface
 
 	Manager contentmanager.Manager
-	Watcher cache.Watcher
+	Watcher cmcache.Watcher
 }
 
 // runPreAuthorizationChecks performs pre-authorization checks for a Helm release
@@ -176,12 +176,7 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte
 		return true, "", err
 	}
 	klog.FromContext(ctx).Info("watching managed objects")
-	cache, err := h.Manager.Get(ctx, ext)
-	if err != nil {
-		return true, "", err
-	}
-
-	if err := cache.Watch(ctx, h.Watcher, relObjects...); err != nil {
+	if err := h.Manager.Watch(ctx, ext.Name, h.Watcher, relObjects...); err != nil {
 		return true, "", err
 	}
 
@@ -226,17 +221,11 @@ func (h *Helm) reconcileExistingRelease(ctx context.Context, ac helmclient.Actio
 		logger.Error(fmt.Errorf("manager is nil"), "Manager not initialized, cannot set up drift detection watches (resources are applied but drift detection disabled)")
 		return true, "", nil
 	}
-	cache, err := h.Manager.Get(ctx, ext)
-	if err != nil {
-		logger.Error(err, "failed to get managed content cache, cannot set up drift detection watches (resources are applied but drift detection disabled)")
-		return true, "", nil
-	}
-
 	if h.Watcher == nil {
 		logger.Error(fmt.Errorf("watcher is nil"), "Watcher not initialized, cannot set up drift detection watches (resources are applied but drift detection disabled)")
 		return true, "", nil
 	}
-	if err := cache.Watch(ctx, h.Watcher, relObjects...); err != nil {
+	if err := h.Manager.Watch(ctx, ext.Name, h.Watcher, relObjects...); err != nil {
 		logger.Error(err, "failed to set up drift detection watches (resources are applied but drift detection disabled)")
 		return true, "", nil
 	}

internal/operator-controller/applier/helm_test.go

@@ -32,39 +32,17 @@ import (
 var _ contentmanager.Manager = (*mockManagedContentCacheManager)(nil)
 
 type mockManagedContentCacheManager struct {
-	err   error
-	cache cmcache.Cache
-}
-
-func (m *mockManagedContentCacheManager) Get(_ context.Context, _ *ocv1.ClusterExtension) (cmcache.Cache, error) {
-	if m.err != nil {
-		return nil, m.err
-	}
-	return m.cache, nil
+	err error
 }
 
-func (m *mockManagedContentCacheManager) Delete(_ *ocv1.ClusterExtension) error {
+func (m *mockManagedContentCacheManager) Watch(_ context.Context, _ string, _ cmcache.Watcher, _ ...client.Object) error {
 	return m.err
 }
 
-type mockManagedContentCache struct {
-	err error
-}
-
-var _ cmcache.Cache = (*mockManagedContentCache)(nil)
+func (m *mockManagedContentCacheManager) Delete(_ context.Context, _ string) {}
 
-func (m *mockManagedContentCache) Close() error {
-	if m.err != nil {
-		return m.err
-	}
-	return nil
-}
-
-func (m *mockManagedContentCache) Watch(_ context.Context, _ cmcache.Watcher, _ ...client.Object) error {
-	if m.err != nil {
-		return m.err
-	}
-	return nil
+func (m *mockManagedContentCacheManager) Close() error {
+	return m.err
 }
 
 type mockPreflight struct {
@@ -339,9 +317,7 @@ func TestApply_Installation(t *testing.T) {
 			ActionClientGetter:            mockAcg,
 			HelmChartProvider:             DummyHelmChartProvider,
 			HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{},
-			Manager: &mockManagedContentCacheManager{
-				cache: &mockManagedContentCache{},
-			},
+			Manager:                       &mockManagedContentCacheManager{},
 		}
 
 		installSucceeded, installStatus, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -521,9 +497,7 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
 			},
 			HelmChartProvider:             DummyHelmChartProvider,
 			HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{},
-			Manager: &mockManagedContentCacheManager{
-				cache: &mockManagedContentCache{},
-			},
+			Manager:                       &mockManagedContentCacheManager{},
 		}
 
 		// Use a ClusterExtension with valid Spec fields.
@@ -644,9 +618,7 @@ func TestApply_Upgrade(t *testing.T) {
 			ActionClientGetter:            mockAcg,
 			HelmChartProvider:             DummyHelmChartProvider,
 			HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{},
-			Manager: &mockManagedContentCacheManager{
-				cache: &mockManagedContentCache{},
-			},
+			Manager:                       &mockManagedContentCacheManager{},
 		}
 
 		installSucceeded, installStatus, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -673,9 +645,7 @@ func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) {
 				},
 			},
 			HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{},
-			Manager: &mockManagedContentCacheManager{
-				cache: &mockManagedContentCache{},
-			},
+			Manager:                       &mockManagedContentCacheManager{},
 		}
 
 		_, _, _ = helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -695,9 +665,7 @@ func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) {
 					return nil, errors.New("some error")
 				},
 			},
-			Manager: &mockManagedContentCacheManager{
-				cache: &mockManagedContentCache{},
-			},
+			Manager: &mockManagedContentCacheManager{},
 		}
 
 		_, _, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)