From 00690e8fa6e4010cd55f5d8d44c29e42fd5e9652 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 3fee77fa1b..c9195a9c54 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/bundle/bundle_unpacker.go +++ b/staging/operator-lifecycle-manager/pkg/controller/bundle/bundle_unpacker.go @@ -669,6 +669,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 3fee77fa1b..c9195a9c54 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 @@ -669,6 +669,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 ea03cd9a6a4892b73269059e162d6c3b55fa7eac 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 cca9371c3a..4e3468b210 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 @@ -1218,14 +1218,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", @@ -1442,6 +1443,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{ @@ -1474,7 +1478,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",