From 458dccc4a8419b84a2cd71fcf95c9c489d732b2c Mon Sep 17 00:00:00 2001 From: Anik Bhattacharjee Date: Fri, 2 Feb 2024 11:40:15 -0500 Subject: [PATCH] OCPBUGS-24587: Clear (existing) error cond from Subscription, once error resolved Upstream fix: https://github.com/operator-framework/operator-lifecycle-manager/pull/3166 Upstream-repository: operator-lifecycle-manager Upstream-commit: 54da66a9996632315827ba37e14823de6405b4d9 Signed-off-by: Anik Bhattacharjee --- .../controller/operators/catalog/operator.go | 18 ++---- .../operators/catalog/operator_test.go | 63 ++++++++++++++++--- .../test/e2e/subscription_e2e_test.go | 2 +- 3 files changed, 62 insertions(+), 21 deletions(-) diff --git a/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go b/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go index 53b76c6272..2d67f4abb2 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go +++ b/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go @@ -1393,16 +1393,14 @@ func (o *Operator) syncResolvingNamespace(obj interface{}) error { } // Make sure that we no longer indicate unpacking progress - subs = o.setSubsCond(subs, v1alpha1.SubscriptionCondition{ - Type: v1alpha1.SubscriptionBundleUnpacking, - Status: corev1.ConditionFalse, - }) + o.removeSubsCond(subs, v1alpha1.SubscriptionBundleUnpacking) // Remove BundleUnpackFailed condition from subscriptions o.removeSubsCond(subs, v1alpha1.SubscriptionBundleUnpackFailed) // Remove resolutionfailed condition from subscriptions o.removeSubsCond(subs, v1alpha1.SubscriptionResolutionFailed) + newSub := true for _, updatedSub := range updatedSubs { updatedSub.Status.RemoveConditions(v1alpha1.SubscriptionResolutionFailed) @@ -1679,13 +1677,9 @@ func (o *Operator) setSubsCond(subs []*v1alpha1.Subscription, cond v1alpha1.Subs return subList } -// removeSubsCond will remove the condition to the subscription if it exists -// Only return the list of updated subscriptions -func (o *Operator) removeSubsCond(subs []*v1alpha1.Subscription, condType v1alpha1.SubscriptionConditionType) []*v1alpha1.Subscription { - var ( - lastUpdated = o.now() - ) - var subList []*v1alpha1.Subscription +// removeSubsCond removes the given condition from all of the subscriptions in the input +func (o *Operator) removeSubsCond(subs []*v1alpha1.Subscription, condType v1alpha1.SubscriptionConditionType) { + lastUpdated := o.now() for _, sub := range subs { cond := sub.Status.GetCondition(condType) // if status is ConditionUnknown, the condition doesn't exist. Just skip @@ -1694,9 +1688,7 @@ func (o *Operator) removeSubsCond(subs []*v1alpha1.Subscription, condType v1alph } sub.Status.LastUpdated = lastUpdated sub.Status.RemoveConditions(condType) - subList = append(subList, sub) } - return subList } func (o *Operator) updateSubscriptionStatuses(subs []*v1alpha1.Subscription) ([]*v1alpha1.Subscription, error) { diff --git a/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator_test.go b/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator_test.go index bd906dcf4a..27535f5c4c 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator_test.go +++ b/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator_test.go @@ -1261,13 +1261,6 @@ func TestSyncResolvingNamespace(t *testing.T) { Status: v1alpha1.SubscriptionStatus{ CurrentCSV: "", State: "", - Conditions: []v1alpha1.SubscriptionCondition{ - { - Type: v1alpha1.SubscriptionBundleUnpacking, - Status: corev1.ConditionFalse, - }, - }, - LastUpdated: now, }, }, }, @@ -1399,6 +1392,62 @@ func TestSyncResolvingNamespace(t *testing.T) { }, wantErr: fmt.Errorf("some error"), }, + { + name: "HadErrorShouldClearError", + fields: fields{ + clientOptions: []clientfake.Option{clientfake.WithSelfLinks(t)}, + existingOLMObjs: []runtime.Object{ + &v1alpha1.Subscription{ + TypeMeta: metav1.TypeMeta{ + Kind: v1alpha1.SubscriptionKind, + APIVersion: v1alpha1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "sub", + Namespace: testNamespace, + }, + Spec: &v1alpha1.SubscriptionSpec{ + CatalogSource: "src", + CatalogSourceNamespace: testNamespace, + }, + Status: v1alpha1.SubscriptionStatus{ + InstalledCSV: "sub-csv", + State: "AtLatestKnown", + Conditions: []v1alpha1.SubscriptionCondition{ + { + Type: v1alpha1.SubscriptionResolutionFailed, + Reason: "ConstraintsNotSatisfiable", + Message: "constraints not satisfiable: no operators found from catalog src in namespace testNamespace referenced by subscrition sub, subscription sub exists", + Status: corev1.ConditionTrue, + }, + }, + }, + }, + }, + resolveErr: nil, + }, + wantSubscriptions: []*v1alpha1.Subscription{ + { + TypeMeta: metav1.TypeMeta{ + Kind: v1alpha1.SubscriptionKind, + APIVersion: v1alpha1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "sub", + Namespace: testNamespace, + }, + Spec: &v1alpha1.SubscriptionSpec{ + CatalogSource: "src", + CatalogSourceNamespace: testNamespace, + }, + Status: v1alpha1.SubscriptionStatus{ + InstalledCSV: "sub-csv", + State: "AtLatestKnown", + LastUpdated: now, + }, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/staging/operator-lifecycle-manager/test/e2e/subscription_e2e_test.go b/staging/operator-lifecycle-manager/test/e2e/subscription_e2e_test.go index 6d499797fd..cfc8369110 100644 --- a/staging/operator-lifecycle-manager/test/e2e/subscription_e2e_test.go +++ b/staging/operator-lifecycle-manager/test/e2e/subscription_e2e_test.go @@ -2445,7 +2445,7 @@ var _ = Describe("Subscription", func() { }, 5*time.Minute, interval, - ).Should(Equal(corev1.ConditionFalse)) + ).Should(Equal(corev1.ConditionUnknown)) By("verifying that the subscription is not reporting unpacking errors") Eventually(