From 93b05e0fb47438c6086db26a15265e77aa6f5840 Mon Sep 17 00:00:00 2001 From: Brett Tofel Date: Thu, 30 May 2024 15:53:11 -0400 Subject: [PATCH] Change global var GetInstalledBundle to interface Signed-off-by: Brett Tofel --- cmd/manager/main.go | 15 +- .../clusterextension_controller.go | 31 ++-- .../clusterextension_controller_test.go | 175 +++++++++--------- ...terextension_registryv1_validation_test.go | 12 +- internal/controllers/suite_test.go | 31 +++- 5 files changed, 149 insertions(+), 115 deletions(-) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 09c7df94..ab677dc3 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -181,13 +181,14 @@ func main() { } if err = (&controllers.ClusterExtensionReconciler{ - Client: cl, - ReleaseNamespace: systemNamespace, - BundleProvider: catalogClient, - ActionClientGetter: acg, - Unpacker: unpacker, - Storage: localStorage, - Handler: handler.HandlerFunc(handler.HandleClusterExtension), + Client: cl, + ReleaseNamespace: systemNamespace, + BundleProvider: catalogClient, + ActionClientGetter: acg, + Unpacker: unpacker, + Storage: localStorage, + Handler: handler.HandlerFunc(handler.HandleClusterExtension), + InstalledBundleGetter: &controllers.DefaultInstalledBundleGetter{}, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "ClusterExtension") os.Exit(1) diff --git a/internal/controllers/clusterextension_controller.go b/internal/controllers/clusterextension_controller.go index f9243000..66c7f29f 100644 --- a/internal/controllers/clusterextension_controller.go +++ b/internal/controllers/clusterextension_controller.go @@ -83,16 +83,21 @@ import ( // ClusterExtensionReconciler reconciles a ClusterExtension object type ClusterExtensionReconciler struct { client.Client - ReleaseNamespace string - BundleProvider BundleProvider - Unpacker rukpaksource.Unpacker - ActionClientGetter helmclient.ActionClientGetter - Storage storage.Storage - Handler handler.Handler - dynamicWatchMutex sync.RWMutex - dynamicWatchGVKs map[schema.GroupVersionKind]struct{} - controller crcontroller.Controller - cache cache.Cache + ReleaseNamespace string + BundleProvider BundleProvider + Unpacker rukpaksource.Unpacker + ActionClientGetter helmclient.ActionClientGetter + Storage storage.Storage + Handler handler.Handler + dynamicWatchMutex sync.RWMutex + dynamicWatchGVKs map[schema.GroupVersionKind]struct{} + controller crcontroller.Controller + cache cache.Cache + InstalledBundleGetter InstalledBundleGetter +} + +type InstalledBundleGetter interface { + GetInstalledBundle(ctx context.Context, acg helmclient.ActionClientGetter, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) } //+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensions,verbs=get;list;watch @@ -416,7 +421,7 @@ func (r *ClusterExtensionReconciler) resolve(ctx context.Context, ext ocv1alpha1 channelName := ext.Spec.Channel versionRange := ext.Spec.Version - installedBundle, err := GetInstalledbundle(ctx, r.ActionClientGetter, allBundles, &ext) + installedBundle, err := r.InstalledBundleGetter.GetInstalledBundle(ctx, r.ActionClientGetter, allBundles, &ext) if err != nil { return nil, err } @@ -709,7 +714,9 @@ func (r *ClusterExtensionReconciler) getReleaseState(cl helmclient.ActionInterfa return currentRelease, stateUnchanged, nil } -var GetInstalledbundle = func(ctx context.Context, acg helmclient.ActionClientGetter, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) { +type DefaultInstalledBundleGetter struct{} + +func (d *DefaultInstalledBundleGetter) GetInstalledBundle(ctx context.Context, acg helmclient.ActionClientGetter, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) { cl, err := acg.ActionClientFor(ctx, ext) if err != nil { return nil, err diff --git a/internal/controllers/clusterextension_controller_test.go b/internal/controllers/clusterextension_controller_test.go index 08fd0da2..5ff9a152 100644 --- a/internal/controllers/clusterextension_controller_test.go +++ b/internal/controllers/clusterextension_controller_test.go @@ -19,7 +19,6 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client" "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/alpha/property" rukpakv1alpha2 "github.com/operator-framework/rukpak/api/v1alpha2" @@ -34,7 +33,7 @@ import ( // Describe: ClusterExtension Controller Test func TestClusterExtensionDoesNotExist(t *testing.T) { - _, reconciler := newClientAndReconciler(t) + _, reconciler := newClientAndReconciler(t, nil) t.Log("When the cluster extension does not exist") t.Log("It returns no error") @@ -44,7 +43,7 @@ func TestClusterExtensionDoesNotExist(t *testing.T) { } func TestClusterExtensionNonExistentPackage(t *testing.T) { - cl, reconciler := newClientAndReconciler(t) + cl, reconciler := newClientAndReconciler(t, nil) ctx := context.Background() extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} @@ -85,7 +84,7 @@ func TestClusterExtensionNonExistentPackage(t *testing.T) { } func TestClusterExtensionNonExistentVersion(t *testing.T) { - cl, reconciler := newClientAndReconciler(t) + cl, reconciler := newClientAndReconciler(t, nil) ctx := context.Background() extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} @@ -130,7 +129,7 @@ func TestClusterExtensionNonExistentVersion(t *testing.T) { } func TestClusterExtensionChannelVersionExists(t *testing.T) { - cl, reconciler := newClientAndReconciler(t) + cl, reconciler := newClientAndReconciler(t, nil) mockUnpacker := unpacker.(*MockUnpacker) // Set up the Unpack method to return a result with StateUnpacked mockUnpacker.On("Unpack", mock.Anything, mock.AnythingOfType("*v1alpha2.BundleDeployment")).Return(&source.Result{ @@ -189,7 +188,7 @@ func TestClusterExtensionChannelVersionExists(t *testing.T) { } func TestClusterExtensionChannelExistsNoVersion(t *testing.T) { - cl, reconciler := newClientAndReconciler(t) + cl, reconciler := newClientAndReconciler(t, nil) mockUnpacker := unpacker.(*MockUnpacker) // Set up the Unpack method to return a result with StateUnpacked mockUnpacker.On("Unpack", mock.Anything, mock.AnythingOfType("*v1alpha2.BundleDeployment")).Return(&source.Result{ @@ -248,7 +247,7 @@ func TestClusterExtensionChannelExistsNoVersion(t *testing.T) { } func TestClusterExtensionVersionNoChannel(t *testing.T) { - cl, reconciler := newClientAndReconciler(t) + cl, reconciler := newClientAndReconciler(t, nil) ctx := context.Background() extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} @@ -298,7 +297,7 @@ func TestClusterExtensionVersionNoChannel(t *testing.T) { } func TestClusterExtensionNoChannel(t *testing.T) { - cl, reconciler := newClientAndReconciler(t) + cl, reconciler := newClientAndReconciler(t, nil) ctx := context.Background() extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} @@ -345,7 +344,7 @@ func TestClusterExtensionNoChannel(t *testing.T) { } func TestClusterExtensionNoVersion(t *testing.T) { - cl, reconciler := newClientAndReconciler(t) + cl, reconciler := newClientAndReconciler(t, nil) ctx := context.Background() extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} @@ -416,18 +415,28 @@ func verifyConditionsInvariants(t *testing.T, ext *ocv1alpha1.ClusterExtension) } func TestClusterExtensionUpgrade(t *testing.T) { - cl, reconciler := newClientAndReconciler(t) + bundle := &catalogmetadata.Bundle{ + Bundle: declcfg.Bundle{ + Name: "operatorhub/prometheus/beta/1.0.0", + Package: "prometheus", + Image: "quay.io/operatorhubio/prometheus@fake1.0.0", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"1.0.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, + }, + }, + CatalogName: "fake-catalog", + InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, + } + + cl, reconciler := newClientAndReconciler(t, bundle) + mockUnpacker := unpacker.(*MockUnpacker) // Set up the Unpack method to return a result with StateUnpackPending mockUnpacker.On("Unpack", mock.Anything, mock.AnythingOfType("*v1alpha2.BundleDeployment")).Return(&source.Result{ State: source.StatePending, }, nil) ctx := context.Background() - defer func() { - controllers.GetInstalledbundle = func(ctx context.Context, acg helmclient.ActionClientGetter, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) { - return nil, nil - } - }() t.Run("semver upgrade constraints enforcement of upgrades within major version", func(t *testing.T) { defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, true)() @@ -477,22 +486,21 @@ func TestClusterExtensionUpgrade(t *testing.T) { err = cl.Update(ctx, clusterExtension) require.NoError(t, err) - controllers.GetInstalledbundle = func(ctx context.Context, acg helmclient.ActionClientGetter, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) { - return &catalogmetadata.Bundle{ - Bundle: declcfg.Bundle{ - Name: "operatorhub/prometheus/beta/1.0.0", - Package: "prometheus", - Image: "quay.io/operatorhubio/prometheus@fake1.0.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"1.0.0"}`)}, - {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, - }, + bundle := &catalogmetadata.Bundle{ + Bundle: declcfg.Bundle{ + Name: "operatorhub/prometheus/beta/1.0.0", + Package: "prometheus", + Image: "quay.io/operatorhubio/prometheus@fake1.0.0", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"1.0.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, }, - CatalogName: "fake-catalog", - InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, - }, nil + }, + CatalogName: "fake-catalog", + InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, } + cl, reconciler := newClientAndReconciler(t, bundle) // Run reconcile again res, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) require.Error(t, err) @@ -586,22 +594,22 @@ func TestClusterExtensionUpgrade(t *testing.T) { err = cl.Update(ctx, clusterExtension) require.NoError(t, err) - controllers.GetInstalledbundle = func(ctx context.Context, acg helmclient.ActionClientGetter, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) { - return &catalogmetadata.Bundle{ - Bundle: declcfg.Bundle{ - Name: "operatorhub/prometheus/beta/1.0.0", - Package: "prometheus", - Image: "quay.io/operatorhubio/prometheus@fake1.0.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"1.0.0"}`)}, - {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, - }, + bundle := &catalogmetadata.Bundle{ + Bundle: declcfg.Bundle{ + Name: "operatorhub/prometheus/beta/1.0.0", + Package: "prometheus", + Image: "quay.io/operatorhubio/prometheus@fake1.0.0", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"1.0.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, }, - CatalogName: "fake-catalog", - InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, - }, nil + }, + CatalogName: "fake-catalog", + InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, } + cl, reconciler := newClientAndReconciler(t, bundle) + // Run reconcile again res, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) require.Error(t, err) @@ -709,22 +717,22 @@ func TestClusterExtensionUpgrade(t *testing.T) { err = cl.Update(ctx, clusterExtension) require.NoError(t, err) - controllers.GetInstalledbundle = func(ctx context.Context, acg helmclient.ActionClientGetter, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) { - return &catalogmetadata.Bundle{ - Bundle: declcfg.Bundle{ - Name: "operatorhub/prometheus/beta/1.0.0", - Package: "prometheus", - Image: "quay.io/operatorhubio/prometheus@fake1.0.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"1.0.0"}`)}, - {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, - }, + bundle := &catalogmetadata.Bundle{ + Bundle: declcfg.Bundle{ + Name: "operatorhub/prometheus/beta/1.0.0", + Package: "prometheus", + Image: "quay.io/operatorhubio/prometheus@fake1.0.0", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"1.0.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, }, - CatalogName: "fake-catalog", - InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, - }, nil + }, + CatalogName: "fake-catalog", + InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, } + cl, reconciler := newClientAndReconciler(t, bundle) + // Run reconcile again res, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) require.NoError(t, err) @@ -749,18 +757,13 @@ func TestClusterExtensionUpgrade(t *testing.T) { } func TestClusterExtensionDowngrade(t *testing.T) { - cl, reconciler := newClientAndReconciler(t) + cl, reconciler := newClientAndReconciler(t, nil) mockUnpacker := unpacker.(*MockUnpacker) // Set up the Unpack method to return a result with StateUnpacked mockUnpacker.On("Unpack", mock.Anything, mock.AnythingOfType("*v1alpha2.BundleDeployment")).Return(&source.Result{ State: source.StatePending, }, nil) ctx := context.Background() - defer func() { - controllers.GetInstalledbundle = func(ctx context.Context, acg helmclient.ActionClientGetter, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) { - return nil, nil - } - }() t.Run("enforce upgrade constraints", func(t *testing.T) { for _, tt := range []struct { @@ -821,22 +824,22 @@ func TestClusterExtensionDowngrade(t *testing.T) { err = cl.Update(ctx, clusterExtension) require.NoError(t, err) - controllers.GetInstalledbundle = func(ctx context.Context, acg helmclient.ActionClientGetter, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) { - return &catalogmetadata.Bundle{ - Bundle: declcfg.Bundle{ - Name: "operatorhub/prometheus/beta/1.0.1", - Package: "prometheus", - Image: "quay.io/operatorhubio/prometheus@fake1.0.1", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"1.0.1"}`)}, - {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, - }, + bundle := &catalogmetadata.Bundle{ + Bundle: declcfg.Bundle{ + Name: "operatorhub/prometheus/beta/1.0.1", + Package: "prometheus", + Image: "quay.io/operatorhubio/prometheus@fake1.0.1", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"1.0.1"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, }, - CatalogName: "fake-catalog", - InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, - }, nil + }, + CatalogName: "fake-catalog", + InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, } + cl, reconciler := newClientAndReconciler(t, bundle) + // Run reconcile again res, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) require.Error(t, err) @@ -920,22 +923,22 @@ func TestClusterExtensionDowngrade(t *testing.T) { err = cl.Update(ctx, clusterExtension) require.NoError(t, err) - controllers.GetInstalledbundle = func(ctx context.Context, acg helmclient.ActionClientGetter, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) { - return &catalogmetadata.Bundle{ - Bundle: declcfg.Bundle{ - Name: "operatorhub/prometheus/beta/2.0.0", - Package: "prometheus", - Image: "quay.io/operatorhubio/prometheus@fake2.0.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"2.0.0"}`)}, - {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, - }, + bundle := &catalogmetadata.Bundle{ + Bundle: declcfg.Bundle{ + Name: "operatorhub/prometheus/beta/2.0.0", + Package: "prometheus", + Image: "quay.io/operatorhubio/prometheus@fake2.0.0", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"2.0.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, }, - CatalogName: "fake-catalog", - InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, - }, nil + }, + CatalogName: "fake-catalog", + InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, } + cl, reconciler := newClientAndReconciler(t, bundle) + // Run reconcile again res, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) require.NoError(t, err) diff --git a/internal/controllers/clusterextension_registryv1_validation_test.go b/internal/controllers/clusterextension_registryv1_validation_test.go index 5588f746..a828a3e0 100644 --- a/internal/controllers/clusterextension_registryv1_validation_test.go +++ b/internal/controllers/clusterextension_registryv1_validation_test.go @@ -107,12 +107,16 @@ func TestClusterExtensionRegistryV1DisallowDependencies(t *testing.T) { mockUnpacker.On("Unpack", mock.Anything, mock.AnythingOfType("*v1alpha2.BundleDeployment")).Return(&source.Result{ State: source.StatePending, }, nil) + // Create and configure the mock InstalledBundleGetter + mockInstalledBundleGetter := &MockInstalledBundleGetter{} + mockInstalledBundleGetter.SetBundle(tt.bundle) reconciler := &controllers.ClusterExtensionReconciler{ - Client: cl, - BundleProvider: &fakeCatalogClient, - ActionClientGetter: helmClientGetter, - Unpacker: unpacker, + Client: cl, + BundleProvider: &fakeCatalogClient, + ActionClientGetter: helmClientGetter, + Unpacker: unpacker, + InstalledBundleGetter: mockInstalledBundleGetter, } installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8)) diff --git a/internal/controllers/suite_test.go b/internal/controllers/suite_test.go index a46e1bc6..6de9560e 100644 --- a/internal/controllers/suite_test.go +++ b/internal/controllers/suite_test.go @@ -38,6 +38,8 @@ import ( "github.com/operator-framework/rukpak/pkg/source" "github.com/operator-framework/rukpak/pkg/storage" + ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" + "github.com/operator-framework/operator-controller/internal/catalogmetadata" "github.com/operator-framework/operator-controller/internal/controllers" "github.com/operator-framework/operator-controller/pkg/scheme" testutil "github.com/operator-framework/operator-controller/test/util" @@ -94,15 +96,32 @@ func newClient(t *testing.T) client.Client { return cl } -func newClientAndReconciler(t *testing.T) (client.Client, *controllers.ClusterExtensionReconciler) { +type MockInstalledBundleGetter struct { + bundle *catalogmetadata.Bundle +} + +func (m *MockInstalledBundleGetter) SetBundle(bundle *catalogmetadata.Bundle) { + m.bundle = bundle +} + +func (m *MockInstalledBundleGetter) GetInstalledBundle(ctx context.Context, acg helmclient.ActionClientGetter, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) { + return m.bundle, nil +} + +func newClientAndReconciler(t *testing.T, bundle *catalogmetadata.Bundle) (client.Client, *controllers.ClusterExtensionReconciler) { cl := newClient(t) fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList) + + mockInstalledBundleGetter := &MockInstalledBundleGetter{} + mockInstalledBundleGetter.SetBundle(bundle) + reconciler := &controllers.ClusterExtensionReconciler{ - Client: cl, - BundleProvider: &fakeCatalogClient, - ActionClientGetter: helmClientGetter, - Unpacker: unpacker, - Storage: store, + Client: cl, + BundleProvider: &fakeCatalogClient, + ActionClientGetter: helmClientGetter, + Unpacker: unpacker, + Storage: store, + InstalledBundleGetter: mockInstalledBundleGetter, } return cl, reconciler }