From 2069e35641589189b0a510778e29174a531eda56 Mon Sep 17 00:00:00 2001 From: Varsha Date: Wed, 29 May 2024 12:36:04 -0700 Subject: [PATCH 1/2] Fix: Ensure jobs without unpack label are considered (#3262) Signed-off-by: Varsha Prasad Narsing Upstream-repository: operator-lifecycle-manager Upstream-commit: 01b44e850bfae0d6775bb7379359b5eaf36f2778 --- .../pkg/controller/bundle/bundle_unpacker.go | 13 +++++++++++++ .../pkg/controller/bundle/bundle_unpacker.go | 13 +++++++++++++ 2 files changed, 26 insertions(+) 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 59a86cc44b..2de8e33651 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/bundle/bundle_unpacker.go +++ b/staging/operator-lifecycle-manager/pkg/controller/bundle/bundle_unpacker.go @@ -666,6 +666,19 @@ func (c *ConfigMapUnpacker) ensureJob(cmRef *corev1.ObjectReference, bundlePath if err != nil { return } + + // 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 + } + if jobWithoutLabel != nil { + _, labelExists := jobWithoutLabel.Labels[bundleUnpackRefLabel] + if !labelExists { + jobs = append(jobs, jobWithoutLabel) + } + } + if len(jobs) == 0 { job, err = c.client.BatchV1().Jobs(fresh.GetNamespace()).Create(context.TODO(), fresh, metav1.CreateOptions{}) return diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/bundle/bundle_unpacker.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/bundle/bundle_unpacker.go index 59a86cc44b..2de8e33651 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/bundle/bundle_unpacker.go +++ b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/bundle/bundle_unpacker.go @@ -666,6 +666,19 @@ func (c *ConfigMapUnpacker) ensureJob(cmRef *corev1.ObjectReference, bundlePath if err != nil { return } + + // 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 + } + if jobWithoutLabel != nil { + _, labelExists := jobWithoutLabel.Labels[bundleUnpackRefLabel] + if !labelExists { + jobs = append(jobs, jobWithoutLabel) + } + } + if len(jobs) == 0 { job, err = c.client.BatchV1().Jobs(fresh.GetNamespace()).Create(context.TODO(), fresh, metav1.CreateOptions{}) return From 8c4e751602799cd9473c88ca70bd85cdb4bfb5a0 Mon Sep 17 00:00:00 2001 From: Daniel Franz Date: Fri, 31 May 2024 02:30:58 -0700 Subject: [PATCH 2/2] Unpack Job Creation Failure Test (#3297) Extends an existing unit test to cover the scenario where an old, failed job missing the label added in later OLM versions is not found with the filtered list call, resulting in an 'AlreadyExists' error when the new job is created. Signed-off-by: Daniel Franz Upstream-repository: operator-lifecycle-manager Upstream-commit: 8751ca0212e345a0b7eab452b169d926e7f42437 --- .../pkg/controller/bundle/bundle_unpacker_test.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/staging/operator-lifecycle-manager/pkg/controller/bundle/bundle_unpacker_test.go b/staging/operator-lifecycle-manager/pkg/controller/bundle/bundle_unpacker_test.go index a583eb5412..c79f9beee0 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/bundle/bundle_unpacker_test.go +++ b/staging/operator-lifecycle-manager/pkg/controller/bundle/bundle_unpacker_test.go @@ -1206,14 +1206,15 @@ func TestConfigMapUnpacker(t *testing.T) { }, }, { - description: "CatalogSourcePresent/JobFailed/BundleLookupFailed/WithJobFailReason", + description: "CatalogSourcePresent/JobFailed/BundleLookupFailed/WithJobFailReasonNoLabel", fields: fields{ objs: []runtime.Object{ &batchv1.Job{ ObjectMeta: metav1.ObjectMeta{ Name: pathHash, Namespace: "ns-a", - Labels: map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue, bundleUnpackRefLabel: pathHash}, + //omit the "operatorframework.io/bundle-unpack-ref" label + Labels: map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue}, OwnerReferences: []metav1.OwnerReference{ { APIVersion: "v1", @@ -1427,6 +1428,9 @@ func TestConfigMapUnpacker(t *testing.T) { }, }, expected: expected{ + // If job is not found due to missing "operatorframework.io/bundle-unpack-ref" label, + // we will get an 'AlreadyExists' error in this test when the new job is created + err: nil, res: &BundleUnpackResult{ name: pathHash, BundleLookup: &operatorsv1alpha1.BundleLookup{ @@ -1459,7 +1463,7 @@ func TestConfigMapUnpacker(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: pathHash, Namespace: "ns-a", - Labels: map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue, bundleUnpackRefLabel: pathHash}, + Labels: map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue}, OwnerReferences: []metav1.OwnerReference{ { APIVersion: "v1",