From bd389d8ac400d0672cb7c6dff0f88ef4323d6cc1 Mon Sep 17 00:00:00 2001 From: Todd Short Date: Wed, 15 Nov 2023 16:33:55 -0500 Subject: [PATCH] Consolidate client/reconciler initialization Signed-off-by: Todd Short --- internal/controllers/admission_test.go | 31 +-- .../controllers/operator_controller_test.go | 186 ++---------------- internal/controllers/suite_test.go | 26 ++- 3 files changed, 51 insertions(+), 192 deletions(-) diff --git a/internal/controllers/admission_test.go b/internal/controllers/admission_test.go index 6e01e6692..6f0c41c93 100644 --- a/internal/controllers/admission_test.go +++ b/internal/controllers/admission_test.go @@ -63,11 +63,8 @@ func TestOperatorSpecs(t *testing.T) { d := od t.Run(d.comment, func(t *testing.T) { t.Parallel() - t.Logf("Running %s", d.comment) - cl, err := newClient() - require.NoError(t, err) - require.NotNil(t, cl) - err = cl.Create(ctx, d.spec) + cl := newClient(t) + err := cl.Create(ctx, d.spec) require.Error(t, err) require.ErrorContains(t, err, d.errMsg) }) @@ -104,10 +101,8 @@ func TestOperatorInvalidSemver(t *testing.T) { d := sm t.Run(d, func(t *testing.T) { t.Parallel() - cl, err := newClient() - require.NoError(t, err) - require.NotNil(t, cl) - err = cl.Create(ctx, operator(operatorsv1alpha1.OperatorSpec{ + cl := newClient(t) + err := cl.Create(ctx, operator(operatorsv1alpha1.OperatorSpec{ PackageName: "package", Version: d, })) @@ -161,14 +156,12 @@ func TestOperatorValidSemver(t *testing.T) { d := smx t.Run(d, func(t *testing.T) { t.Parallel() + cl := newClient(t) op := operator(operatorsv1alpha1.OperatorSpec{ PackageName: "package", Version: d, }) - cl, err := newClient() - require.NoError(t, err) - require.NotNil(t, cl) - err = cl.Create(ctx, op) + err := cl.Create(ctx, op) require.NoErrorf(t, err, "unexpected error for semver range %q: %w", d, err) }) } @@ -193,10 +186,8 @@ func TestOperatorInvalidChannel(t *testing.T) { d := ch t.Run(d, func(t *testing.T) { t.Parallel() - cl, err := newClient() - require.NoError(t, err) - require.NotNil(t, cl) - err = cl.Create(ctx, operator(operatorsv1alpha1.OperatorSpec{ + cl := newClient(t) + err := cl.Create(ctx, operator(operatorsv1alpha1.OperatorSpec{ PackageName: "package", Channel: d, })) @@ -220,14 +211,12 @@ func TestOperatorValidChannel(t *testing.T) { d := ch t.Run(d, func(t *testing.T) { t.Parallel() + cl := newClient(t) op := operator(operatorsv1alpha1.OperatorSpec{ PackageName: "package", Channel: d, }) - cl, err := newClient() - require.NoError(t, err) - require.NotNil(t, cl) - err = cl.Create(ctx, op) + err := cl.Create(ctx, op) require.NoErrorf(t, err, "unexpected error creating valid channel %q: %w", d, err) }) } diff --git a/internal/controllers/operator_controller_test.go b/internal/controllers/operator_controller_test.go index a0fd2215f..bff247b5f 100644 --- a/internal/controllers/operator_controller_test.go +++ b/internal/controllers/operator_controller_test.go @@ -6,7 +6,6 @@ import ( "fmt" "testing" - "github.com/operator-framework/deppy/pkg/deppy/solver" "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/alpha/property" rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1" @@ -25,23 +24,13 @@ import ( operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" "github.com/operator-framework/operator-controller/internal/catalogmetadata" "github.com/operator-framework/operator-controller/internal/conditionsets" - "github.com/operator-framework/operator-controller/internal/controllers" "github.com/operator-framework/operator-controller/pkg/features" - testutil "github.com/operator-framework/operator-controller/test/util" ) // Describe: Operator Controller Test func TestOperatorDoesNotExist(t *testing.T) { - cl, err := newClient() - require.NoError(t, err) - require.NotNil(t, cl) + _, reconciler := newClientAndReconciler(t) - fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList) - reconciler := &controllers.OperatorReconciler{ - Client: cl, - Scheme: sch, - Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)), - } t.Log("When the operator does not exist") t.Log("It returns no error") res, err := reconciler.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Name: "non-existent"}}) @@ -50,16 +39,8 @@ func TestOperatorDoesNotExist(t *testing.T) { } func TestOperatorNonExistantPackage(t *testing.T) { - cl, err := newClient() - require.NoError(t, err) - require.NotNil(t, cl) + cl, reconciler := newClientAndReconciler(t) ctx := context.Background() - fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList) - reconciler := &controllers.OperatorReconciler{ - Client: cl, - Scheme: sch, - Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)), - } opKey := types.NamespacedName{Name: fmt.Sprintf("operator-test-%s", rand.String(8))} t.Log("When the operator specifies a non-existent package") @@ -97,16 +78,8 @@ func TestOperatorNonExistantPackage(t *testing.T) { } func TestOperatorNonExistantVersion(t *testing.T) { - cl, err := newClient() - require.NoError(t, err) - require.NotNil(t, cl) + cl, reconciler := newClientAndReconciler(t) ctx := context.Background() - fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList) - reconciler := &controllers.OperatorReconciler{ - Client: cl, - Scheme: sch, - Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)), - } opKey := types.NamespacedName{Name: fmt.Sprintf("operator-test-%s", rand.String(8))} t.Log("When the operator specifies a version that does not exist") @@ -152,16 +125,8 @@ func TestOperatorNonExistantVersion(t *testing.T) { } func TestOperatorBundleDeploymentDoesNotExist(t *testing.T) { - cl, err := newClient() - require.NoError(t, err) - require.NotNil(t, cl) + cl, reconciler := newClientAndReconciler(t) ctx := context.Background() - fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList) - reconciler := &controllers.OperatorReconciler{ - Client: cl, - Scheme: sch, - Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)), - } opKey := types.NamespacedName{Name: fmt.Sprintf("operator-test-%s", rand.String(8))} const pkgName = "prometheus" @@ -216,16 +181,8 @@ func TestOperatorBundleDeploymentDoesNotExist(t *testing.T) { } func TestOperatorBundleDeploymentOutOfDate(t *testing.T) { - cl, err := newClient() - require.NoError(t, err) - require.NotNil(t, cl) + cl, reconciler := newClientAndReconciler(t) ctx := context.Background() - fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList) - reconciler := &controllers.OperatorReconciler{ - Client: cl, - Scheme: sch, - Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)), - } opKey := types.NamespacedName{Name: fmt.Sprintf("operator-test-%s", rand.String(8))} const pkgName = "prometheus" @@ -315,16 +272,8 @@ func TestOperatorBundleDeploymentOutOfDate(t *testing.T) { } func TestOperatorBundleDeploymentUpToDate(t *testing.T) { - cl, err := newClient() - require.NoError(t, err) - require.NotNil(t, cl) + cl, reconciler := newClientAndReconciler(t) ctx := context.Background() - fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList) - reconciler := &controllers.OperatorReconciler{ - Client: cl, - Scheme: sch, - Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)), - } opKey := types.NamespacedName{Name: fmt.Sprintf("operator-test-%s", rand.String(8))} const pkgName = "prometheus" @@ -598,16 +547,8 @@ func TestOperatorBundleDeploymentUpToDate(t *testing.T) { } func TestOperatorExpectedBundleDeployment(t *testing.T) { - cl, err := newClient() - require.NoError(t, err) - require.NotNil(t, cl) + cl, reconciler := newClientAndReconciler(t) ctx := context.Background() - fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList) - reconciler := &controllers.OperatorReconciler{ - Client: cl, - Scheme: sch, - Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)), - } opKey := types.NamespacedName{Name: fmt.Sprintf("operator-test-%s", rand.String(8))} const pkgName = "prometheus" @@ -681,16 +622,8 @@ func TestOperatorExpectedBundleDeployment(t *testing.T) { } func TestOperatorDuplicatePackage(t *testing.T) { - cl, err := newClient() - require.NoError(t, err) - require.NotNil(t, cl) + cl, reconciler := newClientAndReconciler(t) ctx := context.Background() - fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList) - reconciler := &controllers.OperatorReconciler{ - Client: cl, - Scheme: sch, - Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)), - } opKey := types.NamespacedName{Name: fmt.Sprintf("operator-test-%s", rand.String(8))} const pkgName = "prometheus" @@ -740,16 +673,8 @@ func TestOperatorDuplicatePackage(t *testing.T) { } func TestOperatorChannelVersionExists(t *testing.T) { - cl, err := newClient() - require.NoError(t, err) - require.NotNil(t, cl) + cl, reconciler := newClientAndReconciler(t) ctx := context.Background() - fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList) - reconciler := &controllers.OperatorReconciler{ - Client: cl, - Scheme: sch, - Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)), - } opKey := types.NamespacedName{Name: fmt.Sprintf("operator-test-%s", rand.String(8))} t.Log("When the operator specifies a channel with version that exist") @@ -765,7 +690,7 @@ func TestOperatorChannelVersionExists(t *testing.T) { Channel: pkgChan, }, } - err = cl.Create(ctx, operator) + err := cl.Create(ctx, operator) require.NoError(t, err) t.Log("It sets resolution success status") @@ -808,16 +733,8 @@ func TestOperatorChannelVersionExists(t *testing.T) { } func TestOperatorChannelExistsNoVersion(t *testing.T) { - cl, err := newClient() - require.NoError(t, err) - require.NotNil(t, cl) + cl, reconciler := newClientAndReconciler(t) ctx := context.Background() - fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList) - reconciler := &controllers.OperatorReconciler{ - Client: cl, - Scheme: sch, - Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)), - } opKey := types.NamespacedName{Name: fmt.Sprintf("operator-test-%s", rand.String(8))} t.Log("When the operator specifies a package that exists within a channel but no version specified") @@ -874,16 +791,8 @@ func TestOperatorChannelExistsNoVersion(t *testing.T) { } func TestOperatorVersionNoChannel(t *testing.T) { - cl, err := newClient() - require.NoError(t, err) - require.NotNil(t, cl) + cl, reconciler := newClientAndReconciler(t) ctx := context.Background() - fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList) - reconciler := &controllers.OperatorReconciler{ - Client: cl, - Scheme: sch, - Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)), - } opKey := types.NamespacedName{Name: fmt.Sprintf("operator-test-%s", rand.String(8))} t.Log("When the operator specifies a package version in a channel that does not exist") @@ -933,16 +842,8 @@ func TestOperatorVersionNoChannel(t *testing.T) { } func TestOperatorNoChannel(t *testing.T) { - cl, err := newClient() - require.NoError(t, err) - require.NotNil(t, cl) + cl, reconciler := newClientAndReconciler(t) ctx := context.Background() - fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList) - reconciler := &controllers.OperatorReconciler{ - Client: cl, - Scheme: sch, - Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)), - } opKey := types.NamespacedName{Name: fmt.Sprintf("operator-test-%s", rand.String(8))} t.Log("When the operator specifies a package in a channel that does not exist") @@ -989,16 +890,8 @@ func TestOperatorNoChannel(t *testing.T) { } func TestOperatorNoVersion(t *testing.T) { - cl, err := newClient() - require.NoError(t, err) - require.NotNil(t, cl) + cl, reconciler := newClientAndReconciler(t) ctx := context.Background() - fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList) - reconciler := &controllers.OperatorReconciler{ - Client: cl, - Scheme: sch, - Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)), - } opKey := types.NamespacedName{Name: fmt.Sprintf("operator-test-%s", rand.String(8))} t.Log("When the operator specifies a package version that does not exist in the channel") @@ -1047,16 +940,8 @@ func TestOperatorNoVersion(t *testing.T) { } func TestOperatorPlainV0Bundle(t *testing.T) { - cl, err := newClient() - require.NoError(t, err) - require.NotNil(t, cl) + cl, reconciler := newClientAndReconciler(t) ctx := context.Background() - fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList) - reconciler := &controllers.OperatorReconciler{ - Client: cl, - Scheme: sch, - Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)), - } opKey := types.NamespacedName{Name: fmt.Sprintf("operator-test-%s", rand.String(8))} t.Log("When the operator specifies a package with a plain+v0 bundle") @@ -1113,16 +998,8 @@ func TestOperatorPlainV0Bundle(t *testing.T) { } func TestOperatorBadBundleMediaType(t *testing.T) { - cl, err := newClient() - require.NoError(t, err) - require.NotNil(t, cl) + cl, reconciler := newClientAndReconciler(t) ctx := context.Background() - fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList) - reconciler := &controllers.OperatorReconciler{ - Client: cl, - Scheme: sch, - Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)), - } opKey := types.NamespacedName{Name: fmt.Sprintf("operator-test-%s", rand.String(8))} t.Log("When the operator specifies a package with a bad bundle mediatype") @@ -1172,17 +1049,8 @@ func TestOperatorBadBundleMediaType(t *testing.T) { } func TestOperatorInvalidSemverPastRegex(t *testing.T) { - cl, err := newClient() - require.NoError(t, err) - require.NotNil(t, cl) + cl, reconciler := newClientAndReconciler(t) ctx := context.Background() - fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList) - reconciler := &controllers.OperatorReconciler{ - Client: cl, - Scheme: sch, - Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)), - } - t.Log("When an invalid semver is provided that bypasses the regex validation") opKey := types.NamespacedName{Name: fmt.Sprintf("operator-validation-test-%s", rand.String(8))} @@ -1255,16 +1123,8 @@ func verifyConditionsInvariants(t *testing.T, op *operatorsv1alpha1.Operator) { } func TestOperatorUpgrade(t *testing.T) { - cl, err := newClient() - require.NoError(t, err) - require.NotNil(t, cl) + cl, reconciler := newClientAndReconciler(t) ctx := context.Background() - fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList) - reconciler := &controllers.OperatorReconciler{ - Client: cl, - Scheme: sch, - Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)), - } t.Run("semver upgrade constraints enforcement of upgrades within major version", func(t *testing.T) { defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, true)() @@ -1537,16 +1397,8 @@ func TestOperatorUpgrade(t *testing.T) { } func TestOperatorDowngrade(t *testing.T) { - cl, err := newClient() - require.NoError(t, err) - require.NotNil(t, cl) + cl, reconciler := newClientAndReconciler(t) ctx := context.Background() - fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList) - reconciler := &controllers.OperatorReconciler{ - Client: cl, - Scheme: sch, - Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)), - } t.Run("enforce upgrade constraints", func(t *testing.T) { for _, tt := range []struct { diff --git a/internal/controllers/suite_test.go b/internal/controllers/suite_test.go index 0569cbbf3..3b10a032d 100644 --- a/internal/controllers/suite_test.go +++ b/internal/controllers/suite_test.go @@ -28,20 +28,38 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" + "github.com/operator-framework/deppy/pkg/deppy/solver" rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1" + "github.com/stretchr/testify/require" operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" + "github.com/operator-framework/operator-controller/internal/controllers" + testutil "github.com/operator-framework/operator-controller/test/util" ) +func newClient(t *testing.T) client.Client { + cl, err := client.New(cfg, client.Options{Scheme: sch}) + require.NoError(t, err) + require.NotNil(t, cl) + return cl +} + +func newClientAndReconciler(t *testing.T) (client.Client, *controllers.OperatorReconciler) { + cl := newClient(t) + fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList) + reconciler := &controllers.OperatorReconciler{ + Client: cl, + Scheme: sch, + Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)), + } + return cl, reconciler +} + var ( sch *runtime.Scheme cfg *rest.Config ) -func newClient() (client.Client, error) { - return client.New(cfg, client.Options{Scheme: sch}) -} - func TestMain(m *testing.M) { testEnv := &envtest.Environment{ CRDDirectoryPaths: []string{