Using subscription reconciliation to clean installplan status instead

Signed-off-by: Vu Dinh <[email protected]>

Author: dinhxuanvu

pkg/controller/operators/catalog/operator.go

@@ -831,12 +831,6 @@ func (o *Operator) syncResolvingNamespace(obj interface{}) error {
 		return err
 	}
 
-	ips, err := o.listInstallPlansMap(namespace)
-	if err != nil {
-		logger.WithError(err).Debug("couldn't list installplan")
-		return err
-	}
-
 	// TODO: parallel
 	maxGeneration := 0
 	subscriptionUpdated := false
@@ -853,7 +847,7 @@ func (o *Operator) syncResolvingNamespace(obj interface{}) error {
 		}
 
 		// ensure the installplan reference is correct
-		sub, changedIP, err := o.ensureSubscriptionInstallPlanState(logger, sub, ips)
+		sub, changedIP, err := o.ensureSubscriptionInstallPlanState(logger, sub)
 		if err != nil {
 			logger.Debugf("error ensuring installplan state: %v", err)
 			return err
@@ -958,29 +952,9 @@ func (o *Operator) nothingToUpdate(logger *logrus.Entry, sub *v1alpha1.Subscript
 	return false
 }
 
-// checkMissingInstallPlan checks if the installplan is missing or not when
-// the subscription is in pending upgrade state
-func (o *Operator) checkMissingInstallPlan(sub *v1alpha1.Subscription, ips map[string]struct{}) (*v1alpha1.Subscription, bool, error) {
-	_, ok := ips[sub.Status.InstallPlanRef.Name]
-	if !ok && sub.Status.State == v1alpha1.SubscriptionStateUpgradePending {
-		out := sub.DeepCopy()
-		out.Status.InstallPlanRef = nil
-		out.Status.Install = nil
-		out.Status.CurrentCSV = ""
-		out.Status.State = v1alpha1.SubscriptionStateNone
-		out.Status.LastUpdated = o.now()
-		updated, err := o.client.OperatorsV1alpha1().Subscriptions(sub.GetNamespace()).UpdateStatus(context.TODO(), out, metav1.UpdateOptions{})
-		if err != nil {
-			return out, false, nil
-		}
-		return updated, true, nil
-	}
-	return sub, false, nil
-}
-
-func (o *Operator) ensureSubscriptionInstallPlanState(logger *logrus.Entry, sub *v1alpha1.Subscription, ips map[string]struct{}) (*v1alpha1.Subscription, bool, error) {
+func (o *Operator) ensureSubscriptionInstallPlanState(logger *logrus.Entry, sub *v1alpha1.Subscription) (*v1alpha1.Subscription, bool, error) {
 	if sub.Status.InstallPlanRef != nil || sub.Status.Install != nil {
-		return o.checkMissingInstallPlan(sub, ips)
+		return sub, false, nil
 	}
 
 	logger.Debug("checking for existing installplan")
@@ -2046,20 +2020,6 @@ func (o *Operator) listInstallPlans(namespace string) (ips []*v1alpha1.InstallPl
 	return
 }
 
-func (o *Operator) listInstallPlansMap(namespace string) (ips map[string]struct{}, err error) {
-	list, err := o.client.OperatorsV1alpha1().InstallPlans(namespace).List(context.TODO(), metav1.ListOptions{})
-	if err != nil {
-		return
-	}
-
-	ips = make(map[string]struct{})
-	for i := range list.Items {
-		ips[list.Items[i].GetName()] = struct{}{}
-	}
-
-	return
-}
-
 // competingCRDOwnersExist returns true if there exists a CSV that owns at least one of the given CSVs owned CRDs (that's not the given CSV)
 func competingCRDOwnersExist(namespace string, csv *v1alpha1.ClusterServiceVersion, existingOwners map[string][]string) (bool, error) {
 	// Attempt to find a pre-existing owner in the namespace for any owned crd

pkg/controller/operators/catalog/subscription/state.go

@@ -385,23 +385,33 @@ func (i *installPlanReferencedState) InstallPlanNotFound(now *metav1.Time, clien
 	// Remove pending and failed conditions
 	out.Status.RemoveConditions(v1alpha1.SubscriptionInstallPlanPending, v1alpha1.SubscriptionInstallPlanFailed)
 
-	// Set missing condition to true
-	missingCond := out.Status.GetCondition(v1alpha1.SubscriptionInstallPlanMissing)
-	missingCond.Status = corev1.ConditionTrue
-	missingCond.Reason = v1alpha1.ReferencedInstallPlanNotFound
-	missingCond.LastTransitionTime = now
-	out.Status.SetCondition(missingCond)
+	// If the installplan is missing when subscription is in pending upgrade,
+	// clear the installplan ref so the resolution can happen again
+	if in.Status.State == v1alpha1.SubscriptionStateUpgradePending {
+		out.Status.InstallPlanRef = nil
+		out.Status.Install = nil
+		out.Status.CurrentCSV = ""
+		out.Status.State = v1alpha1.SubscriptionStateNone
+		out.Status.LastUpdated = *now
+	} else {
+		// Set missing condition to true
+		cond := out.Status.GetCondition(v1alpha1.SubscriptionInstallPlanMissing)
+		cond.Status = corev1.ConditionTrue
+		cond.Reason = v1alpha1.ReferencedInstallPlanNotFound
+		cond.LastTransitionTime = now
+		out.Status.SetCondition(cond)
+	}
 
 	// Build missing state
-	missingState := &installPlanMissingState{
+	state := &installPlanMissingState{
 		InstallPlanKnownState: &installPlanKnownState{
 			InstallPlanReferencedState: i,
 		},
 	}
 
 	// Bail out if the conditions haven't changed (using select fields included in a hash)
 	if hashEqual(out.Status.Conditions, in.Status.Conditions) {
-		return missingState, nil
+		return state, nil
 	}
 
 	// Update the Subscription
@@ -412,9 +422,9 @@ func (i *installPlanReferencedState) InstallPlanNotFound(now *metav1.Time, clien
 	}
 
 	// Stuff updated Subscription into state
-	missingState.setSubscription(updated)
+	state.setSubscription(updated)
 
-	return missingState, nil
+	return state, nil
 }
 
 func (i *installPlanReferencedState) CheckInstallPlanStatus(now *metav1.Time, client clientv1alpha1.SubscriptionInterface, status *v1alpha1.InstallPlanStatus) (InstallPlanReferencedState, error) {