Test

staging/operator-lifecycle-manager/pkg/controller/bundle/bundle_unpacker.go

@@ -662,18 +662,18 @@ func (c *ConfigMapUnpacker) ensureConfigmap(csRef *corev1.ObjectReference, name
 	return
 }
 
-func (c *ConfigMapUnpacker) ensureJob(cmRef *corev1.ObjectReference, bundlePath string, secrets []corev1.LocalObjectReference, timeout time.Duration, unpackRetryInterval time.Duration) (job *batchv1.Job, err error) {
+func (c *ConfigMapUnpacker) ensureJob(cmRef *corev1.ObjectReference, bundlePath string, secrets []corev1.LocalObjectReference, timeout time.Duration, unpackRetryInterval time.Duration) (*batchv1.Job, error) {
 	fresh := c.job(cmRef, bundlePath, secrets, timeout)
 	var jobs, toDelete []*batchv1.Job
-	jobs, err = c.jobLister.Jobs(fresh.GetNamespace()).List(k8slabels.ValidatedSetSelector{bundleUnpackRefLabel: cmRef.Name})
+	jobs, err := c.jobLister.Jobs(fresh.GetNamespace()).List(k8slabels.ValidatedSetSelector{bundleUnpackRefLabel: cmRef.Name})
 	if err != nil {
-		return
+		return nil, err
 	}
 
 	// This is to ensure that we account for any existing unpack jobs that may be missing the label
 	jobWithoutLabel, err := c.jobLister.Jobs(fresh.GetNamespace()).Get(cmRef.Name)
 	if err != nil && !apierrors.IsNotFound(err) {
-		return
+		return nil, err
 	}
 	if jobWithoutLabel != nil {
 		_, labelExists := jobWithoutLabel.Labels[bundleUnpackRefLabel]
@@ -683,12 +683,11 @@ func (c *ConfigMapUnpacker) ensureJob(cmRef *corev1.ObjectReference, bundlePath
 	}
 
 	if len(jobs) == 0 {
-		job, err = c.client.BatchV1().Jobs(fresh.GetNamespace()).Create(context.TODO(), fresh, metav1.CreateOptions{})
-		return
+		return c.client.BatchV1().Jobs(fresh.GetNamespace()).Create(context.TODO(), fresh, metav1.CreateOptions{})
 	}
 
-	maxRetainedJobs := 5                                  // TODO: make this configurable
-	job, toDelete = sortUnpackJobs(jobs, maxRetainedJobs) // choose latest or on-failed job attempt
+	maxRetainedJobs := 5                                   // TODO: make this configurable
+	job, toDelete := sortUnpackJobs(jobs, maxRetainedJobs) // choose latest or on-failed job attempt
 
 	// only check for retries if an unpackRetryInterval is specified
 	if unpackRetryInterval > 0 {
@@ -697,26 +696,23 @@ func (c *ConfigMapUnpacker) ensureJob(cmRef *corev1.ObjectReference, bundlePath
 			if cond, failed := getCondition(job, batchv1.JobFailed); failed {
 				if time.Now().After(cond.LastTransitionTime.Time.Add(unpackRetryInterval)) {
 					fresh.SetName(names.SimpleNameGenerator.GenerateName(fresh.GetName()))
-					job, err = c.client.BatchV1().Jobs(fresh.GetNamespace()).Create(context.TODO(), fresh, metav1.CreateOptions{})
+					return c.client.BatchV1().Jobs(fresh.GetNamespace()).Create(context.TODO(), fresh, metav1.CreateOptions{})
 				}
 			}
 
 			// cleanup old failed jobs, but don't clean up successful jobs to avoid repeat unpacking
 			for _, j := range toDelete {
 				_ = c.client.BatchV1().Jobs(j.GetNamespace()).Delete(context.TODO(), j.GetName(), metav1.DeleteOptions{})
 			}
-			return
 		}
 	}
 
 	if equality.Semantic.DeepDerivative(fresh.GetOwnerReferences(), job.GetOwnerReferences()) && equality.Semantic.DeepDerivative(fresh.Spec, job.Spec) {
-		return
+		return job, nil
 	}
 
 	// TODO: Decide when to fail-out instead of deleting the job
-	err = c.client.BatchV1().Jobs(job.GetNamespace()).Delete(context.TODO(), job.GetName(), metav1.DeleteOptions{})
-	job = nil
-	return
+	return nil, c.client.BatchV1().Jobs(job.GetNamespace()).Delete(context.TODO(), job.GetName(), metav1.DeleteOptions{})
 }
 
 func (c *ConfigMapUnpacker) ensureRole(cmRef *corev1.ObjectReference) (role *rbacv1.Role, err error) {

staging/operator-lifecycle-manager/test/e2e/catalog_e2e_test.go

@@ -1470,29 +1470,19 @@ var _ = Describe("Starting CatalogSource e2e tests", func() {
 			})
 		})
 	})
-	When("The namespace is labled as Pod Security Admission policy enforce:restricted", func() {
+	When("The namespace is labeled as Pod Security Admission policy enforce:restricted", func() {
 		BeforeEach(func() {
-			var err error
-			testNS := &corev1.Namespace{}
 			Eventually(func() error {
-				testNS, err = c.KubernetesInterface().CoreV1().Namespaces().Get(context.TODO(), generatedNamespace.GetName(), metav1.GetOptions{})
+				testNS, err := c.KubernetesInterface().CoreV1().Namespaces().Get(context.TODO(), generatedNamespace.GetName(), metav1.GetOptions{})
 				if err != nil {
 					return err
 				}
-				return nil
-			}).Should(BeNil())
-
-			testNS.ObjectMeta.Labels = map[string]string{
-				"pod-security.kubernetes.io/enforce":         "restricted",
-				"pod-security.kubernetes.io/enforce-version": "latest",
-			}
-
-			Eventually(func() error {
-				_, err := c.KubernetesInterface().CoreV1().Namespaces().Update(context.TODO(), testNS, metav1.UpdateOptions{})
-				if err != nil {
-					return err
+				testNS.ObjectMeta.Labels = map[string]string{
+					"pod-security.kubernetes.io/enforce":         "restricted",
+					"pod-security.kubernetes.io/enforce-version": "latest",
 				}
-				return nil
+				_, err = c.KubernetesInterface().CoreV1().Namespaces().Update(context.TODO(), testNS, metav1.UpdateOptions{})
+				return err
 			}).Should(BeNil())
 		})
 		When("A CatalogSource built with opm v1.21.0 (<v1.23.2)is created with spec.GrpcPodConfig.SecurityContextConfig set to restricted", func() {
@@ -1543,27 +1533,17 @@ var _ = Describe("Starting CatalogSource e2e tests", func() {
 	})
 	When("The namespace is labled as Pod Security Admission policy enforce:baseline", func() {
 		BeforeEach(func() {
-			var err error
-			testNS := &corev1.Namespace{}
 			Eventually(func() error {
-				testNS, err = c.KubernetesInterface().CoreV1().Namespaces().Get(context.TODO(), generatedNamespace.GetName(), metav1.GetOptions{})
+				testNS, err := c.KubernetesInterface().CoreV1().Namespaces().Get(context.TODO(), generatedNamespace.GetName(), metav1.GetOptions{})
 				if err != nil {
 					return err
 				}
-				return nil
-			}).Should(BeNil())
-
-			testNS.ObjectMeta.Labels = map[string]string{
-				"pod-security.kubernetes.io/enforce":         "baseline",
-				"pod-security.kubernetes.io/enforce-version": "latest",
-			}
-
-			Eventually(func() error {
-				_, err := c.KubernetesInterface().CoreV1().Namespaces().Update(context.TODO(), testNS, metav1.UpdateOptions{})
-				if err != nil {
-					return err
+				testNS.ObjectMeta.Labels = map[string]string{
+					"pod-security.kubernetes.io/enforce":         "baseline",
+					"pod-security.kubernetes.io/enforce-version": "latest",
 				}
-				return nil
+				_, err = c.KubernetesInterface().CoreV1().Namespaces().Update(context.TODO(), testNS, metav1.UpdateOptions{})
+				return err
 			}).Should(BeNil())
 		})
 		When("A CatalogSource built with opm v1.21.0 (<v1.23.2)is created with spec.GrpcPodConfig.SecurityContextConfig set to legacy", func() {

staging/operator-lifecycle-manager/test/e2e/downstream_csv_namespace_labeler_plugin_test.go

@@ -2,7 +2,6 @@ package e2e
 
 import (
 	"context"
-
 	"github.com/blang/semver/v4"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -88,11 +87,18 @@ var _ = Describe("CSV Namespace Labeler Plugin", func() {
 		}).Should(HaveKeyWithValue(plugins.NamespaceLabelSyncerLabelKey, "true"))
 
 		// delete label
-		ns := &v1.Namespace{}
-		Expect(determinedE2eClient.Get(context.Background(), k8scontrollerclient.ObjectKeyFromObject(&testNamespace), ns)).To(Succeed())
-		nsCopy := ns.DeepCopy()
-		delete(nsCopy.Annotations, plugins.NamespaceLabelSyncerLabelKey)
-		Expect(determinedE2eClient.Update(context.Background(), nsCopy)).To(Succeed())
+		// NOTE: not using the determined client here because it shouldn't be used for update operations due to
+		// race conditions (the updated resource could change b/w 'get' and 'update' operations
+		c := ctx.Ctx().E2EClient()
+		Eventually(func() error {
+			ns := &v1.Namespace{}
+			if err := c.Get(context.Background(), k8scontrollerclient.ObjectKeyFromObject(&testNamespace), ns); err != nil {
+				return err
+			}
+			nsCopy := ns.DeepCopy()
+			delete(nsCopy.Annotations, plugins.NamespaceLabelSyncerLabelKey)
+			return c.Update(context.Background(), nsCopy)
+		}).Should(BeNil())
 
 		// namespace should be labeled
 		Eventually(func() (map[string]string, error) {

staging/operator-lifecycle-manager/test/e2e/fail_forward_e2e_test.go

@@ -321,7 +321,7 @@ var _ = Describe("Fail Forward Upgrades", func() {
 			Expect(subscription.Status.InstallPlanRef.Name).To(Equal(failedInstallPlanRef.Name))
 		})
 	})
-	When("a CSV resource is in a failed state", func() {
+	XWhen("a CSV resource is in a failed state (https://github.com/operator-framework/operator-lifecycle-manager/issues/3573)", func() {
 
 		var (
 			catalogSourceName string

staging/operator-lifecycle-manager/test/e2e/util/e2e_determined_client.go

@@ -28,6 +28,11 @@ func (m *DeterminedE2EClient) Create(context context.Context, obj k8scontrollerc
 	return nil
 }
 
+// Update retries update operation until success or timeout
+//
+// Deprecation: do not use this method as it's not resilient to the case where the resource has changed out of band
+// it will conflict until it times out.
+// There's no priority to fix this client implementation - please use regular client instead
 func (m *DeterminedE2EClient) Update(context context.Context, obj k8scontrollerclient.Object, options ...k8scontrollerclient.UpdateOption) error {
 	m.keepTrying(func() error {
 		return m.E2EKubeClient.Update(context, obj, options...)