diff --git a/staging/operator-lifecycle-manager/pkg/controller/bundle/bundle_unpacker.go b/staging/operator-lifecycle-manager/pkg/controller/bundle/bundle_unpacker.go index ad300b7c88..5042136bc9 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/bundle/bundle_unpacker.go +++ b/staging/operator-lifecycle-manager/pkg/controller/bundle/bundle_unpacker.go @@ -681,18 +681,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] @@ -702,12 +702,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 { @@ -716,7 +715,7 @@ 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{}) } } @@ -724,18 +723,15 @@ func (c *ConfigMapUnpacker) ensureJob(cmRef *corev1.ObjectReference, bundlePath 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) { diff --git a/staging/operator-lifecycle-manager/test/e2e/catalog_e2e_test.go b/staging/operator-lifecycle-manager/test/e2e/catalog_e2e_test.go index 39fa0497d7..847e807f50 100644 --- a/staging/operator-lifecycle-manager/test/e2e/catalog_e2e_test.go +++ b/staging/operator-lifecycle-manager/test/e2e/catalog_e2e_test.go @@ -1467,29 +1467,19 @@ var _ = Describe("Starting CatalogSource e2e tests", Label("CatalogSource"), fun }) }) }) - 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 ( 0 { @@ -716,7 +715,7 @@ 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{}) } } @@ -724,18 +723,15 @@ func (c *ConfigMapUnpacker) ensureJob(cmRef *corev1.ObjectReference, bundlePath 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) {