reserve the status for operators which are still in deletion stage (#867)

* reserve the status for operators
which are still in deletion stage

Signed-off-by: Daniel Fan <[email protected]>

* reserve the status for operators
which are still in deletion stage

Signed-off-by: Daniel Fan <[email protected]>

Signed-off-by: Daniel Fan <[email protected]>

Author: Daniel-Fan

api/v1alpha1/operandrequest_types.go

@@ -21,6 +21,7 @@ import (
 	"sync"
 	"time"
 
+	gset "github.com/deckarep/golang-set"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	runtime "k8s.io/apimachinery/pkg/runtime"
@@ -388,10 +389,10 @@ func (r *OperandRequest) setOperandReadyCondition(operandPhase ServicePhase, nam
 }
 
 // FreshMemberStatus cleanup Member status from the Member status list.
-func (r *OperandRequest) FreshMemberStatus() {
+func (r *OperandRequest) FreshMemberStatus(failedDeletedOperands *gset.Set) {
 	newMembers := []MemberStatus{}
 	for index, m := range r.Status.Members {
-		if foundOperand(r.Spec.Requests, m.Name) {
+		if foundOperand(r.Spec.Requests, m.Name) || (*failedDeletedOperands).Contains(m.Name) {
 			newMembers = append(newMembers, r.Status.Members[index])
 		}
 	}
@@ -458,6 +459,8 @@ func (r *OperandRequest) UpdateClusterPhase() {
 			clusterStatusStat.runningNum++
 		case OperatorInstalling:
 			clusterStatusStat.installingNum++
+		case OperatorUpdating:
+			clusterStatusStat.installingNum++
 		default:
 		}
 

controllers/operandrequest/operandrequest_controller.go

@@ -25,6 +25,7 @@ import (
 	"sync"
 	"time"
 
+	gset "github.com/deckarep/golang-set"
 	olmv1 "github.com/operator-framework/api/pkg/operators/v1"
 	olmv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
 	"github.com/pkg/errors"
@@ -207,6 +208,7 @@ func (r *Reconciler) addFinalizer(ctx context.Context, cr *operatorv1alpha1.Oper
 
 func (r *Reconciler) checkFinalizer(ctx context.Context, requestInstance *operatorv1alpha1.OperandRequest) error {
 	klog.V(1).Infof("Deleting OperandRequest %s in the namespace %s", requestInstance.Name, requestInstance.Namespace)
+	failedDeletedOperands := gset.NewSet()
 	existingSub := &olmv1alpha1.SubscriptionList{}
 
 	opts := []client.ListOption{
@@ -220,7 +222,7 @@ func (r *Reconciler) checkFinalizer(ctx context.Context, requestInstance *operat
 		return nil
 	}
 	// Delete all the subscriptions that created by current request
-	if err := r.absentOperatorsAndOperands(ctx, requestInstance); err != nil {
+	if err := r.absentOperatorsAndOperands(ctx, requestInstance, &failedDeletedOperands); err != nil {
 		return err
 	}
 	return nil

controllers/operandrequest/reconcile_operand.go

@@ -100,21 +100,6 @@ func (r *Reconciler) reconcileOperand(ctx context.Context, requestInstance *oper
 				klog.Warningf("Subscription %s in the namespace %s isn't created by ODLM", sub.Name, sub.Namespace)
 			}
 
-			// check config annotation in subscription, identify the first ODLM has the priority to reconcile
-			var firstMatch string
-			reg, _ := regexp.Compile(`^(.*)\.(.*)\/config`)
-			for anno := range sub.Annotations {
-				if reg.MatchString(anno) {
-					firstMatch = anno
-					break
-				}
-			}
-
-			if firstMatch != "" && firstMatch != regNs+"."+regName+"/config" {
-				klog.V(2).Infof("Subscription %s in the namespace %s is currently managed by %s", sub.Name, sub.Namespace, firstMatch)
-				continue
-			}
-
 			// It the installplan is not created yet, ODLM will try later
 			if sub.Status.Install == nil || sub.Status.InstallPlanRef.Name == "" {
 				klog.Warningf("The Installplan for Subscription %s is not ready. Will check it again", sub.Name)
@@ -158,6 +143,22 @@ func (r *Reconciler) reconcileOperand(ctx context.Context, requestInstance *oper
 			klog.V(3).Info("Generating customresource base on ClusterServiceVersion: ", csv.GetName())
 			requestInstance.SetMemberStatus(operand.Name, operatorv1alpha1.OperatorRunning, "", &r.Mutex)
 
+			// check config annotation in subscription, identify the first ODLM has the priority to reconcile
+			var firstMatch string
+			reg, _ := regexp.Compile(`^(.*)\.(.*)\/config`)
+			for anno := range sub.Annotations {
+				if reg.MatchString(anno) {
+					firstMatch = anno
+					break
+				}
+			}
+
+			if firstMatch != "" && firstMatch != regNs+"."+regName+"/config" {
+				klog.V(2).Infof("Subscription %s in the namespace %s is currently managed by %s, Skip creating CR for it", sub.Name, sub.Namespace, firstMatch)
+				requestInstance.SetMemberStatus(operand.Name, "", operatorv1alpha1.ServiceRunning, &r.Mutex)
+				continue
+			}
+
 			// Merge and Generate CR
 			if operand.Kind == "" {
 				configInstance, err := r.GetOperandConfig(ctx, registryKey)

controllers/operandrequest/reconcile_operator.go

@@ -46,10 +46,12 @@ import (
 
 func (r *Reconciler) reconcileOperator(ctx context.Context, requestInstance *operatorv1alpha1.OperandRequest) error {
 	klog.V(1).Infof("Reconciling Operators for OperandRequest: %s/%s", requestInstance.GetNamespace(), requestInstance.GetName())
-
+	// It is important to NOT pass the set directly into defer functions.
+	// The arguments to the deferred function are evaluated when the defer executes
+	failedDeletedOperands := gset.NewSet()
 	// Update request status
 	defer func() {
-		requestInstance.FreshMemberStatus()
+		requestInstance.FreshMemberStatus(&failedDeletedOperands)
 		requestInstance.UpdateClusterPhase()
 	}()
 
@@ -112,8 +114,17 @@ func (r *Reconciler) reconcileOperator(ctx context.Context, requestInstance *ope
 			}
 			wg.Wait()
 		}
+
+		if len(merr.Errors) != 0 {
+			return merr
+		}
 	}
 
+	// TODO: update OperandRequest status before patching. Otherwise, the status will be lost.
+	// It is really corner case which would not cause any issue so far since the Status will finally be Running.
+	// Need to consider a more elegant way to preserve status instead of calling Client API like below
+	// requestInstance.FreshMemberStatus(&failedDeletedOperands)
+
 	mergePatch, _ := json.Marshal(map[string]interface{}{
 		"metadata": map[string]interface{}{
 			"annotations": map[string]interface{}{
@@ -126,7 +137,7 @@ func (r *Reconciler) reconcileOperator(ctx context.Context, requestInstance *ope
 	}
 
 	// Delete specific operators
-	if err := r.absentOperatorsAndOperands(ctx, requestInstance); err != nil {
+	if err := r.absentOperatorsAndOperands(ctx, requestInstance, &failedDeletedOperands); err != nil {
 		return err
 	}
 	klog.V(1).Infof("Finished reconciling Operators for OperandRequest: %s/%s", requestInstance.GetNamespace(), requestInstance.GetName())
@@ -359,7 +370,7 @@ func (r *Reconciler) deleteSubscription(ctx context.Context, operandName string,
 	return nil
 }
 
-func (r *Reconciler) absentOperatorsAndOperands(ctx context.Context, requestInstance *operatorv1alpha1.OperandRequest) error {
+func (r *Reconciler) absentOperatorsAndOperands(ctx context.Context, requestInstance *operatorv1alpha1.OperandRequest, failedDeletedOperands *gset.Set) error {
 	needDeletedOperands, err := r.getNeedDeletedOperands(ctx, requestInstance)
 	if err != nil {
 		return err
@@ -396,6 +407,7 @@ func (r *Reconciler) absentOperatorsAndOperands(ctx context.Context, requestInst
 					r.Mutex.Lock()
 					defer r.Mutex.Unlock()
 					merr.Add(err)
+					(*failedDeletedOperands).Add(o)
 				}
 				remainingOp.Remove(o)
 			}()