Merge pull request #686 from anik120/release-4.14

OCPBUGS-29192: [release-4.14]: Clear (existing) error cond from Subscription, once error resolved

Author: openshift-merge-bot[bot]

Diff for: staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go

@@ -1158,16 +1158,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)
@@ -1444,13 +1442,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
@@ -1459,9 +1453,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 for: staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator_test.go

@@ -1253,13 +1253,6 @@ func TestSyncResolvingNamespace(t *testing.T) {
 					Status: v1alpha1.SubscriptionStatus{
 						CurrentCSV: "",
 						State:      "",
-						Conditions: []v1alpha1.SubscriptionCondition{
-							{
-								Type:   v1alpha1.SubscriptionBundleUnpacking,
-								Status: corev1.ConditionFalse,
-							},
-						},
-						LastUpdated: now,
 					},
 				},
 			},
@@ -1391,6 +1384,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 for: staging/operator-lifecycle-manager/test/e2e/subscription_e2e_test.go

@@ -2456,7 +2456,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(
@@ -2672,7 +2672,7 @@ properties:
 				if err != nil {
 					return err
 				}
-				if cond := fetched.Status.GetCondition(v1alpha1.SubscriptionBundleUnpacking); cond.Status != corev1.ConditionFalse {
+				if cond := fetched.Status.GetCondition(v1alpha1.SubscriptionBundleUnpacking); cond.Status != corev1.ConditionUnknown {
 					return fmt.Errorf("subscription condition %s has unexpected value %s, expected %s", v1alpha1.SubscriptionBundleUnpacking, cond.Status, corev1.ConditionFalse)
 				}
 				if cond := fetched.Status.GetCondition(v1alpha1.SubscriptionBundleUnpackFailed); cond.Status != corev1.ConditionUnknown {