Don't generate kubeclient at runtime in testing (#2790)

- Updates tests to use common Kube and Runtime clients generated at
startup time rather than having them re-generated in each test at
runtime.
- Closes #2570

Signed-off-by: Noah Sapse <[email protected]>

Author: nsapse

pkg/controller/operators/adoption_controller_test.go

@@ -180,7 +180,6 @@ var _ = Describe("Adoption Controller", func() {
 				})
 
 				Context("that has an existing installed csv", func() {
-
 					var (
 						providedCRD *apiextensionsv1.CustomResourceDefinition
 					)
@@ -227,6 +226,7 @@ var _ = Describe("Adoption Controller", func() {
 					})
 
 					Context("with an existing provided CRD", func() {
+
 						BeforeEach(func() {
 							Eventually(func() error {
 								return k8sClient.Create(ctx, providedCRD)
@@ -242,6 +242,7 @@ var _ = Describe("Adoption Controller", func() {
 						})
 
 						Context("when its component label is removed", func() {
+
 							BeforeEach(func() {
 								Eventually(func() error {
 									latest := &apiextensionsv1.CustomResourceDefinition{}

pkg/controller/operators/operator_controller_test.go

@@ -46,10 +46,12 @@ var _ = Describe("Operator Controller", func() {
 
 	Describe("operator deletion", func() {
 		var originalUID types.UID
+
 		JustBeforeEach(func() {
 			originalUID = operator.GetUID()
 			Expect(k8sClient.Delete(ctx, operator)).To(Succeed())
 		})
+
 		Context("with components bearing its label", func() {
 			var (
 				objs      []runtime.Object
@@ -105,6 +107,7 @@ var _ = Describe("Operator Controller", func() {
 	})
 
 	Describe("component selection", func() {
+
 		BeforeEach(func() {
 			Eventually(func() (*operatorsv1.Components, error) {
 				err := k8sClient.Get(ctx, name, operator)
@@ -164,6 +167,7 @@ var _ = Describe("Operator Controller", func() {
 			})
 
 			Context("when new components are labelled", func() {
+
 				BeforeEach(func() {
 					saName := &types.NamespacedName{Namespace: namespace, Name: genName("sa-")}
 					newObjs := testobj.WithLabel(expectedKey, "",
@@ -201,6 +205,7 @@ var _ = Describe("Operator Controller", func() {
 			})
 
 			Context("when component labels are removed", func() {
+
 				BeforeEach(func() {
 					for _, obj := range testobj.StripLabel(expectedKey, objs...) {
 						Expect(k8sClient.Update(ctx, obj.(client.Object))).To(Succeed())

pkg/controller/operators/operatorcondition_controller_test.go

@@ -51,6 +51,7 @@ var _ = Describe("OperatorCondition", func() {
 			namespace         *corev1.Namespace
 			namespacedName    types.NamespacedName
 		)
+
 		BeforeEach(func() {
 			ctx = context.Background()
 			namespace = &corev1.Namespace{
@@ -64,6 +65,7 @@ var _ = Describe("OperatorCondition", func() {
 		})
 
 		When("an operatorCondition is created that specifies an array of ServiceAccounts", func() {
+
 			BeforeEach(func() {
 				operatorCondition = &operatorsv2.OperatorCondition{
 					ObjectMeta: metav1.ObjectMeta{
@@ -150,6 +152,7 @@ var _ = Describe("OperatorCondition", func() {
 
 		When("a CSV exists that owns a deployment", func() {
 			var csv *operatorsv1alpha1.ClusterServiceVersion
+
 			BeforeEach(func() {
 				// Create a coppied csv used as an owner in the following tests.
 				// Copied CSVs are ignored by the OperatorConditionGenerator Reconciler, which we don't want to intervine in this test.
@@ -231,6 +234,7 @@ var _ = Describe("OperatorCondition", func() {
 			})
 
 			Context("and an OperatorCondition with a different name than the CSV includes that deployment in its spec.Deployments array", func() {
+
 				BeforeEach(func() {
 					operatorCondition = &operatorsv2.OperatorCondition{
 						ObjectMeta: metav1.ObjectMeta{
@@ -266,6 +270,7 @@ var _ = Describe("OperatorCondition", func() {
 			})
 
 			Context("and an OperatorCondition with the same name as the CSV includes that deployment in its spec.Deployments array", func() {
+
 				BeforeEach(func() {
 					operatorCondition = &operatorsv2.OperatorCondition{
 						ObjectMeta: metav1.ObjectMeta{

test/e2e/catalog_e2e_test.go

@@ -47,13 +47,12 @@ const (
 
 var _ = Describe("Starting CatalogSource e2e tests", func() {
 	var (
+		ns  corev1.Namespace
 		c   operatorclient.ClientInterface
 		crc versioned.Interface
-		ns  corev1.Namespace
 	)
+
 	BeforeEach(func() {
-		c = newKubeClient()
-		crc = newCRClient()
 		namespaceName := genName("catsrc-e2e-")
 		og := operatorsv1.OperatorGroup{
 			ObjectMeta: metav1.ObjectMeta{
@@ -62,6 +61,8 @@ var _ = Describe("Starting CatalogSource e2e tests", func() {
 			},
 		}
 		ns = SetupGeneratedTestNamespaceWithOperatorGroup(namespaceName, og)
+		c = ctx.Ctx().KubeClient()
+		crc = ctx.Ctx().OperatorClient()
 	})
 
 	AfterEach(func() {
@@ -1105,7 +1106,6 @@ var _ = Describe("Starting CatalogSource e2e tests", func() {
 		Expect(csv.Spec.Replaces).To(Equal("busybox-dependency.v1.0.0"))
 	})
 	When("A catalogSource is created with correct polling interval", func() {
-
 		var source *v1alpha1.CatalogSource
 		singlePod := podCount(1)
 		sourceName := genName("catalog-")
@@ -1177,15 +1177,16 @@ var _ = Describe("Starting CatalogSource e2e tests", func() {
 	})
 
 	When("A catalogSource is created with incorrect polling interval", func() {
-
 		var (
 			source     *v1alpha1.CatalogSource
 			sourceName string
 		)
+
 		const (
 			incorrectInterval = "45mError.code"
 			correctInterval   = "45m"
 		)
+
 		BeforeEach(func() {
 			sourceName = genName("catalog-")
 			source = &v1alpha1.CatalogSource{
@@ -1230,6 +1231,7 @@ var _ = Describe("Starting CatalogSource e2e tests", func() {
 			}).Should(BeTrue())
 		})
 		When("the catalogsource is updated with a valid polling interval", func() {
+
 			BeforeEach(func() {
 				Eventually(func() error {
 					catsrc, err := crc.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Get(context.Background(), source.GetName(), metav1.GetOptions{})
@@ -1241,6 +1243,7 @@ var _ = Describe("Starting CatalogSource e2e tests", func() {
 					return err
 				}).Should(Succeed())
 			})
+
 			It("the catalogsource spec shows the updated polling interval, and the error message in the status is cleared", func() {
 				Eventually(func() error {
 					catsrc, err := crc.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Get(context.Background(), source.GetName(), metav1.GetOptions{})
@@ -1345,7 +1348,6 @@ var _ = Describe("Starting CatalogSource e2e tests", func() {
 	})
 
 	When("A CatalogSource is created with an operator that has a CSV with missing metadata.ApiVersion", func() {
-
 		var (
 			magicCatalog      MagicCatalog
 			catalogSourceName string
@@ -1370,7 +1372,8 @@ var _ = Describe("Starting CatalogSource e2e tests", func() {
 		})
 
 		When("A Subscription is created catalogSource built with the malformed CSV", func() {
-			BeforeEach(func ()  {
+
+			BeforeEach(func() {
 				subscription = &operatorsv1alpha1.Subscription{
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      fmt.Sprintf("%s-sub", catalogSourceName),

test/e2e/catsrc_pod_config_e2e_test.go

@@ -19,6 +19,7 @@ const catalogSourceLabel = "olm.catalogSource"
 var _ = By
 
 var _ = Describe("CatalogSource Grpc Pod Config", func() {
+
 	var (
 		generatedNamespace corev1.Namespace
 	)

test/e2e/crd_e2e_test.go

@@ -9,7 +9,9 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry"
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient"
 	"github.com/operator-framework/operator-lifecycle-manager/test/e2e/ctx"
 	corev1 "k8s.io/api/core/v1"
 
@@ -20,12 +22,15 @@ import (
 )
 
 var _ = Describe("CRD Versions", func() {
-
 	var (
-		ns corev1.Namespace
+		ns  corev1.Namespace
+		c   operatorclient.ClientInterface
+		crc versioned.Interface
 	)
 
 	BeforeEach(func() {
+		c = ctx.Ctx().KubeClient()
+		crc = ctx.Ctx().OperatorClient()
 		ns = SetupGeneratedTestNamespace(genName("crd-e2e-"))
 	})
 
@@ -36,8 +41,6 @@ var _ = Describe("CRD Versions", func() {
 	// issue: https://github.com/operator-framework/operator-lifecycle-manager/issues/2640
 	It("[FLAKE] creates v1 CRDs with a v1 schema successfully", func() {
 		By("v1 crds with a valid openapiv3 schema should be created successfully by OLM")
-		c := newKubeClient()
-		crc := newCRClient()
 
 		mainPackageName := genName("nginx-update2-")
 		mainPackageStable := fmt.Sprintf("%s-stable", mainPackageName)
@@ -115,9 +118,6 @@ var _ = Describe("CRD Versions", func() {
 	It("[FLAKE] blocks a CRD upgrade that could cause data loss", func() {
 		By("checking the storage versions in the existing CRD status and the spec of the new CRD")
 
-		c := newKubeClient()
-		crc := newCRClient()
-
 		mainPackageName := genName("nginx-update2-")
 		mainPackageStable := fmt.Sprintf("%s-stable", mainPackageName)
 		stableChannel := "stable"
@@ -301,9 +301,6 @@ var _ = Describe("CRD Versions", func() {
 	It("allows a CRD upgrade that doesn't cause data loss", func() {
 		By("manually editing the storage versions in the existing CRD status")
 
-		c := newKubeClient()
-		crc := newCRClient()
-
 		crdPlural := genName("ins-v1-")
 		crdName := crdPlural + ".cluster.com"
 		crdGroup := "cluster.com"

test/e2e/csv_e2e_test.go

@@ -36,23 +36,23 @@ import (
 )
 
 var _ = Describe("ClusterServiceVersion", func() {
-
 	var (
 		ns  corev1.Namespace
 		c   operatorclient.ClientInterface
 		crc versioned.Interface
 	)
 
 	BeforeEach(func() {
-		c = newKubeClient()
-		crc = newCRClient()
+		c = ctx.Ctx().KubeClient()
+		crc = ctx.Ctx().OperatorClient()
 	})
 
 	AfterEach(func() {
 		TeardownNamespace(ns.GetName())
 	})
 
 	Context("OwnNamespace OperatorGroup", func() {
+
 		BeforeEach(func() {
 			nsName := genName("csv-e2e-")
 			ns = SetupGeneratedTestNamespace(nsName, nsName)
@@ -280,7 +280,6 @@ var _ = Describe("ClusterServiceVersion", func() {
 		})
 
 		When("an unassociated ClusterServiceVersion in different namespace owns the same CRD", func() {
-
 			var (
 				crd         apiextensionsv1.CustomResourceDefinition
 				apiname     string
@@ -401,6 +400,7 @@ var _ = Describe("ClusterServiceVersion", func() {
 	})
 
 	Context("AllNamespaces OperatorGroup", func() {
+
 		BeforeEach(func() {
 			ns = SetupGeneratedTestNamespace(genName("csv-e2e-"))
 		})
@@ -3966,7 +3966,7 @@ var _ = Describe("ClusterServiceVersion", func() {
 			csv.SetName("csv-hat-1")
 			csv.SetNamespace(ns.GetName())
 
-			createLegacyAPIResources(ns.GetName(), &csv, owned[0])
+			createLegacyAPIResources(ns.GetName(), &csv, owned[0], c)
 
 			// Create the APIService CSV
 			cleanupCSV, err := createCSV(c, crc, csv, ns.GetName(), false, false)
@@ -3976,7 +3976,7 @@ var _ = Describe("ClusterServiceVersion", func() {
 			_, err = fetchCSV(crc, csv.Name, ns.GetName(), csvSucceededChecker)
 			Expect(err).ShouldNot(HaveOccurred())
 
-			checkLegacyAPIResources(ns.GetName(), owned[0], true)
+			checkLegacyAPIResources(ns.GetName(), owned[0], true, c)
 		})
 
 		It("API service resource not migrated if not adoptable", func() {
@@ -4046,7 +4046,7 @@ var _ = Describe("ClusterServiceVersion", func() {
 			csv.SetName("csv-hat-1")
 			csv.SetNamespace(ns.GetName())
 
-			createLegacyAPIResources(ns.GetName(), nil, owned[0])
+			createLegacyAPIResources(ns.GetName(), nil, owned[0], c)
 
 			// Create the APIService CSV
 			cleanupCSV, err := createCSV(c, crc, csv, ns.GetName(), false, false)
@@ -4056,10 +4056,10 @@ var _ = Describe("ClusterServiceVersion", func() {
 			_, err = fetchCSV(crc, csv.Name, ns.GetName(), csvSucceededChecker)
 			Expect(err).ShouldNot(HaveOccurred())
 
-			checkLegacyAPIResources(ns.GetName(), owned[0], false)
+			checkLegacyAPIResources(ns.GetName(), owned[0], false, c)
 
 			// Cleanup the resources created for this test that were not cleaned up.
-			deleteLegacyAPIResources(ns.GetName(), owned[0])
+			deleteLegacyAPIResources(ns.GetName(), owned[0], c)
 		})
 
 		It("multiple API services on a single pod", func() {
@@ -4510,9 +4510,7 @@ func csvExists(namespace string, c versioned.Interface, name string) bool {
 	return true
 }
 
-func deleteLegacyAPIResources(namespace string, desc operatorsv1alpha1.APIServiceDescription) {
-	c := newKubeClient()
-
+func deleteLegacyAPIResources(namespace string, desc operatorsv1alpha1.APIServiceDescription, c operatorclient.ClientInterface) {
 	apiServiceName := fmt.Sprintf("%s.%s", desc.Version, desc.Group)
 
 	err := c.DeleteService(namespace, strings.Replace(apiServiceName, ".", "-", -1), &metav1.DeleteOptions{})
@@ -4534,8 +4532,7 @@ func deleteLegacyAPIResources(namespace string, desc operatorsv1alpha1.APIServic
 	Expect(err).ShouldNot(HaveOccurred())
 }
 
-func createLegacyAPIResources(namespace string, csv *operatorsv1alpha1.ClusterServiceVersion, desc operatorsv1alpha1.APIServiceDescription) {
-	c := newKubeClient()
+func createLegacyAPIResources(namespace string, csv *operatorsv1alpha1.ClusterServiceVersion, desc operatorsv1alpha1.APIServiceDescription, c operatorclient.ClientInterface) {
 
 	apiServiceName := fmt.Sprintf("%s.%s", desc.Version, desc.Group)
 
@@ -4621,8 +4618,7 @@ func createLegacyAPIResources(namespace string, csv *operatorsv1alpha1.ClusterSe
 	Expect(err).ShouldNot(HaveOccurred())
 }
 
-func checkLegacyAPIResources(namespace string, desc operatorsv1alpha1.APIServiceDescription, expectedIsNotFound bool) {
-	c := newKubeClient()
+func checkLegacyAPIResources(namespace string, desc operatorsv1alpha1.APIServiceDescription, expectedIsNotFound bool, c operatorclient.ClientInterface) {
 	apiServiceName := fmt.Sprintf("%s.%s", desc.Version, desc.Group)
 
 	// Attempt to create the legacy service

test/e2e/deprecated_e2e_test.go

@@ -19,7 +19,6 @@ import (
 var missingAPI = `{"apiVersion":"verticalpodautoscalers.autoscaling.k8s.io/v1","kind":"VerticalPodAutoscaler","metadata":{"name":"my.thing","namespace":"foo"}}`
 
 var _ = Describe("Not found APIs", func() {
-
 	var ns corev1.Namespace
 
 	BeforeEach(func() {

test/e2e/disabling_copied_csv_e2e_test.go

@@ -95,6 +95,7 @@ var _ = Describe("Disabling copied CSVs", func() {
 	})
 
 	When("Copied CSVs are disabled", func() {
+
 		BeforeEach(func() {
 			Eventually(func() error {
 				var olmConfig operatorsv1.OLMConfig
@@ -175,6 +176,7 @@ var _ = Describe("Disabling copied CSVs", func() {
 	})
 
 	When("Copied CSVs are toggled back on", func() {
+
 		BeforeEach(func() {
 			Eventually(func() error {
 				var olmConfig operatorsv1.OLMConfig