Refactor members and services status update (#1056)

* Refactor members and services status update

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

* Update typo in log message

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

* Re-construct if condition for better code readability

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

---------

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

Author: Daniel-Fan

api/v1alpha1/operandrequest_types.go

@@ -631,12 +631,16 @@ func (r *OperandRequest) GetRegistryKey(req Request) types.NamespacedName {
 }
 
 // InitRequestStatus OperandConfig status.
-func (r *OperandRequest) InitRequestStatus() bool {
+func (r *OperandRequest) InitRequestStatus(mu sync.Locker) bool {
 	isInitialized := true
 	if r.Status.Phase == "" {
 		isInitialized = false
-		r.Status.Phase = ClusterPhaseNone
 	}
+	for _, member := range r.Status.Members {
+		klog.V(2).Info("Cleaning the member status for Operand: ", member.Name)
+		r.RemoveOperandPhase(member.Name, mu)
+	}
+	r.Status.Phase = ClusterPhaseNone
 	return isInitialized
 }
 
@@ -679,58 +683,6 @@ func (r *OperandRequest) UpdateLabels() bool {
 	return isUpdated
 }
 
-func (r *OperandRequest) CheckServiceStatus() bool {
-	requeue := false
-	monitoredServices := []string{"ibm-iam-operator", "ibm-idp-config-ui-operator", "ibm-mongodb-operator", "ibm-im-operator"}
-	servicesRequested := false
-	for _, serviceName := range monitoredServices {
-		if foundOperand(r.Spec.Requests, serviceName) {
-			servicesRequested = true
-			break
-		}
-	}
-	if servicesRequested {
-		if len(r.Status.Services) == 0 {
-			klog.Info("Waiting for status.services to be instantiated ...")
-			requeue = true
-			return requeue
-		}
-		var IMOrIAM string
-		exists := false
-		if foundOperand(r.Spec.Requests, "ibm-iam-operator") {
-			IMOrIAM = "ibm-iam-operator"
-			exists = true
-		} else if foundOperand(r.Spec.Requests, "ibm-im-operator") {
-			IMOrIAM = "ibm-im-operator"
-			exists = true
-		}
-
-		if exists {
-			var imIndex int
-			found := false
-			for i, s := range r.Status.Services {
-				if IMOrIAM == s.OperatorName { //eventually this should be changed to the variable but the operator name is still listed as iam in practice even when im is requested
-					found = true
-					imIndex = i
-					break
-				}
-			}
-			if found {
-				if r.Status.Services[imIndex].Status != "Ready" {
-					klog.Info("Waiting for IM service to be Ready ...")
-					requeue = true
-					return requeue
-				}
-			} else {
-				klog.Info("Waiting for IM service status ...")
-				requeue = true
-				return requeue
-			}
-		}
-	}
-	return requeue
-}
-
 // GetAllRegistryReconcileRequest gets all the Registry ReconcileRequest.
 func (r *OperandRequest) GetAllRegistryReconcileRequest() []reconcile.Request {
 	rrs := []reconcile.Request{}

controllers/constant/constant.go

@@ -93,4 +93,7 @@ const (
 
 	//DefaultCRRetryNumber is the default maximum number of retry for reconciling a custom resource
 	DefaultCRRetryNumber = 3
+
+	//StatusMonitoredServices is the annotation key for monitored services
+	StatusMonitoredServices = "status-monitored-services"
 )

controllers/operandrequest/operandrequest_controller.go

@@ -144,7 +144,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re
 	}
 
 	// Initialize the status for OperandRequest instance
-	if !requestInstance.InitRequestStatus() {
+	if !requestInstance.InitRequestStatus(&r.Mutex) {
 		return ctrl.Result{Requeue: true}, nil
 	}
 
@@ -156,18 +156,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re
 		return ctrl.Result{Requeue: true}, err
 	}
 
-	// Clean the phase of each operand under spec.request.operand
-	for _, member := range requestInstance.Status.Members {
-		klog.V(2).Info("Cleaning the member status for Operand: ", member.Name)
-		requestInstance.RemoveOperandPhase(member.Name, &r.Mutex)
-	}
-	requestInstance.Status.Phase = operatorv1alpha1.ClusterPhaseNone
-
-	if err := r.Client.Status().Update(ctx, requestInstance); err != nil {
-		klog.Errorf("failed to update the status of the OperandRequest %s: %v", req.NamespacedName.String(), err)
-		return ctrl.Result{}, err
-	}
-
 	// Reconcile Operators
 	if err := r.reconcileOperator(ctx, requestInstance); err != nil {
 		klog.Errorf("failed to reconcile Operators for OperandRequest %s: %v", req.NamespacedName.String(), err)
@@ -186,8 +174,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re
 		return ctrl.Result{RequeueAfter: constant.DefaultRequeueDuration}, nil
 	}
 
-	//check if status.services is present (if a relevant service was requested), requeue again is im/iam is not ready yet
-	if requestInstance.CheckServiceStatus() {
+	//check if status.services is present (if a relevant service was requested), requeue again if service is not ready yet
+	if isReady, err := r.ServiceStatusIsReady(ctx, requestInstance); !isReady || err != nil {
 		return ctrl.Result{RequeueAfter: constant.DefaultRequeueDuration}, nil
 	}
 
@@ -416,7 +404,7 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 			UpdateFunc: func(e event.UpdateEvent) bool {
 				oldObject := e.ObjectOld.(*operatorv1alpha1.OperandRegistry)
 				newObject := e.ObjectNew.(*operatorv1alpha1.OperandRegistry)
-				return !reflect.DeepEqual(oldObject.Spec, newObject.Spec)
+				return !reflect.DeepEqual(oldObject.Spec, newObject.Spec) || !reflect.DeepEqual(oldObject.GetAnnotations(), newObject.GetAnnotations())
 			},
 			DeleteFunc: func(e event.DeleteEvent) bool {
 				// Evaluates to false if the object has been confirmed deleted.

controllers/operandrequest/reconcile_operand.go

@@ -326,17 +326,16 @@ func (r *Reconciler) reconcileCRwithConfig(ctx context.Context, service *operato
 					merr.Add(err)
 					continue
 				}
-				statusSpec, err := r.getOperandStatus(crFromALM)
+				statusFromCR, err := r.getOperandStatus(crFromALM)
 				if err != nil {
 					return err
 				}
 				serviceKind := crFromALM.GetKind()
-				if serviceKind != "OperandRequest" && statusSpec.ObjectName != "" {
+				if serviceKind != "OperandRequest" && statusFromCR.ObjectName != "" {
 					var resources []operatorv1alpha1.OperandStatus
-					resources = append(resources, statusSpec)
-					serviceSpec := newServiceStatus(operandName, operatorNamespace, resources)
-					seterr := requestInstance.SetServiceStatus(ctx, serviceSpec, r.Client, mu)
-					if seterr != nil {
+					resources = append(resources, statusFromCR)
+					serviceStatus := newServiceStatus(operandName, operatorNamespace, resources)
+					if seterr := requestInstance.SetServiceStatus(ctx, serviceStatus, r.Client, mu); seterr != nil {
 						return seterr
 					}
 				}
@@ -410,15 +409,15 @@ func (r *Reconciler) reconcileCRwithRequest(ctx context.Context, requestInstance
 			if err := r.updateCustomResource(ctx, crFromRequest, requestKey.Namespace, operand.Kind, operand.Spec.Raw, map[string]interface{}{}, requestInstance); err != nil {
 				return err
 			}
-			statusSpec, err := r.getOperandStatus(crFromRequest)
+			statusFromCR, err := r.getOperandStatus(crFromRequest)
 			if err != nil {
 				return err
 			}
-			if operand.Kind != "OperandRequest" && statusSpec.ObjectName != "" {
+			if operand.Kind != "OperandRequest" && statusFromCR.ObjectName != "" {
 				var resources []operatorv1alpha1.OperandStatus
-				resources = append(resources, statusSpec)
-				serviceSpec := newServiceStatus(operand.Name, operatorNamespace, resources)
-				seterr := requestInstance.SetServiceStatus(ctx, serviceSpec, r.Client, mu)
+				resources = append(resources, statusFromCR)
+				serviceStatus := newServiceStatus(operand.Name, operatorNamespace, resources)
+				seterr := requestInstance.SetServiceStatus(ctx, serviceStatus, r.Client, mu)
 				if seterr != nil {
 					return seterr
 				}
@@ -481,7 +480,6 @@ func newServiceStatus(operatorName string, namespace string, resources []operato
 		}
 	}
 	serviceSpec.Status = status //TODO logic to determine readiness
-	// serviceSpec.LastUpdateTime = time.Now().Format(time.RFC3339)
 	serviceSpec.Resources = resources
 	return serviceSpec
 }
@@ -1358,3 +1356,49 @@ func (r *Reconciler) setOwnerReferences(ctx context.Context, controlledRes *unst
 	}
 	return nil
 }
+
+func (r *Reconciler) ServiceStatusIsReady(ctx context.Context, requestInstance *operatorv1alpha1.OperandRequest) (bool, error) {
+	requestedServicesSet := make(map[string]struct{})
+	for _, req := range requestInstance.Spec.Requests {
+		registryKey := requestInstance.GetRegistryKey(req)
+		registryInstance, err := r.GetOperandRegistry(ctx, registryKey)
+		if err != nil {
+			klog.Errorf("Failed to get OperandRegistry %s, %v", registryKey, err)
+			return false, err
+		}
+		if registryInstance.Annotations != nil && registryInstance.Annotations[constant.StatusMonitoredServices] != "" {
+			monitoredServices := strings.Split(registryInstance.Annotations[constant.StatusMonitoredServices], ",")
+			for _, operand := range req.Operands {
+				if util.Contains(monitoredServices, operand.Name) {
+					requestedServicesSet[operand.Name] = struct{}{}
+				}
+			}
+		}
+	}
+
+	if len(requestedServicesSet) == 0 {
+		klog.V(2).Infof("No services to be monitored for OperandRequest %s/%s", requestInstance.Namespace, requestInstance.Name)
+		return true, nil
+	}
+
+	if len(requestInstance.Status.Services) == 0 {
+		klog.Infof("Waiting for status.services to be instantiated for OperandRequest %s/%s ...", requestInstance.Namespace, requestInstance.Name)
+		return false, nil
+	}
+	if len(requestedServicesSet) != len(requestInstance.Status.Services) {
+		klog.Infof("Waiting for status of all requested services to be instantiated for OperandRequest %s/%s ...", requestInstance.Namespace, requestInstance.Name)
+		return false, nil
+	}
+
+	serviceStatus := true
+	// wait for the status of the requested services to be ready
+	for _, s := range requestInstance.Status.Services {
+		if _, ok := requestedServicesSet[s.OperatorName]; ok {
+			if s.Status != "Ready" {
+				klog.Infof("Waiting for status of service %s to be Ready for OperandRequest %s/%s ...", s.OperatorName, requestInstance.Namespace, requestInstance.Name)
+				serviceStatus = false
+			}
+		}
+	}
+	return serviceStatus, nil
+}