Only discover catalog for an operator when it has been requested in O…

…perandRequest

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

controllers/operandbindinfo/operandbindinfo_controller.go

@@ -159,11 +159,11 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re
 		return ctrl.Result{}, nil
 	}
 	// Get the operand namespace
-	operandOperator := registryInstance.GetOperator(bindInfoInstance.Spec.Operand)
-	if operandOperator == nil {
-		klog.Errorf("failed to find operator %s in the OperandRegistry %s", bindInfoInstance.Spec.Operand, registryInstance.Name)
+	operandOperator, err := r.GetOperandFromRegistry(ctx, registryInstance, bindInfoInstance.Spec.Operand)
+	if err != nil || operandOperator == nil {
+		klog.Errorf("failed to find operator %s in the OperandRegistry %s: %v", bindInfoInstance.Spec.Operand, registryInstance.Name, err)
 		r.Recorder.Eventf(bindInfoInstance, corev1.EventTypeWarning, "NotFound", "NotFound operator %s in the OperandRegistry %s", bindInfoInstance.Spec.Operand, registryInstance.Name)
-		return ctrl.Result{}, nil
+		return ctrl.Result{}, err
 	}
 	operandNamespace := registryInstance.Namespace
 

controllers/operandrequest/operandrequest_controller.go

@@ -354,18 +354,27 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 	ReferencePredicates := predicate.Funcs{
 		CreateFunc: func(e event.CreateEvent) bool {
 			labels := e.Object.GetLabels()
-			for labelKey, labelValue := range labels {
-				if labelKey == constant.ODLMWatchedLabel {
-					return labelValue == "true"
+			// only return true when both conditions are met at the same time:
+			// 1. label contain key "constant.ODLMWatchedLabel" and value is true
+			// 2. label does not contain key "constant.OpbiTypeLabel" with value "copy"
+			if labels != nil {
+				if labelValue, ok := labels[constant.ODLMWatchedLabel]; ok && labelValue == "true" {
+					if labelValue, ok := labels[constant.OpbiTypeLabel]; ok && labelValue == "copy" {
+						return false
+					}
+					return true
 				}
 			}
 			return false
 		},
 		UpdateFunc: func(e event.UpdateEvent) bool {
 			labels := e.ObjectNew.GetLabels()
-			for labelKey, labelValue := range labels {
-				if labelKey == constant.ODLMWatchedLabel {
-					return labelValue == "true"
+			if labels != nil {
+				if labelValue, ok := labels[constant.ODLMWatchedLabel]; ok && labelValue == "true" {
+					if labelValue, ok := labels[constant.OpbiTypeLabel]; ok && labelValue == "copy" {
+						return false
+					}
+					return true
 				}
 			}
 			return false

controllers/operandrequest/reconcile_operand.go

@@ -22,17 +22,16 @@ import (
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
-	"reflect"
 	"regexp"
 	"strconv"
 	"strings"
 	"sync"
 
-	"github.com/barkimedes/go-deepcopy"
 	olmv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
 	"github.com/pkg/errors"
 	authorizationv1 "k8s.io/api/authorization/v1"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/equality"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -73,8 +72,12 @@ func (r *Reconciler) reconcileOperand(ctx context.Context, requestInstance *oper
 
 		for i, operand := range req.Operands {
 
-			opdRegistry := registryInstance.GetOperator(operand.Name)
-			if opdRegistry == nil {
+			opdRegistry, err := r.GetOperandFromRegistry(ctx, registryInstance, operand.Name)
+			if err != nil {
+				klog.Warningf("Failed to get the Operand %s from the OperandRegistry %s", operand.Name, registryKey.String())
+				merr.Add(errors.Wrapf(err, "failed to get the Operand %s from the OperandRegistry %s", operand.Name, registryKey.String()))
+				continue
+			} else if opdRegistry == nil {
 				klog.Warningf("Cannot find %s in the OperandRegistry instance %s in the namespace %s ", operand.Name, req.Registry, req.RegistryNamespace)
 				requestInstance.SetMemberStatus(operand.Name, operatorv1alpha1.OperatorNotFound, operatorv1alpha1.ServiceNotFound, &r.Mutex)
 				continue
@@ -772,7 +775,7 @@ func (r *Reconciler) updateCustomResource(ctx context.Context, existingCR unstru
 		// Merge spec from update existing CR and OperandConfig spec
 		updatedCRSpec := util.MergeCR(updatedExistingCRRaw, crConfig)
 
-		if reflect.DeepEqual(existingCR.Object["spec"], updatedCRSpec) && !forceUpdate {
+		if equality.Semantic.DeepEqual(existingCR.Object["spec"], updatedCRSpec) && !forceUpdate {
 			return true, nil
 		}
 
@@ -1069,13 +1072,6 @@ func (r *Reconciler) updateK8sResource(ctx context.Context, existingK8sRes unstr
 
 			// Merge the existing CR and the CR from the OperandConfig
 			updatedExistingK8sRes := util.MergeCR(existingK8sResRaw, k8sResConfig.Raw)
-
-			// Deep copy the existing k8s resource
-			originalK8sRes, err := deepcopy.Anything(existingK8sRes.Object)
-			if err != nil {
-				return false, errors.Wrapf(err, "failed to deep copy k8s resource -- Kind: %s, NamespacedName: %s/%s", kind, namespace, name)
-			}
-
 			// Update the existing k8s resource with the merged CR
 			existingK8sRes.Object = updatedExistingK8sRes
 
@@ -1085,14 +1081,21 @@ func (r *Reconciler) updateK8sResource(ctx context.Context, existingK8sRes unstr
 				return false, errors.Wrapf(err, "failed to set ownerReferences for k8s resource -- Kind: %s, NamespacedName: %s/%s", kind, namespace, name)
 			}
 
-			if reflect.DeepEqual(originalK8sRes, existingK8sRes.Object) {
+			resourceVersion := existingK8sRes.GetResourceVersion()
+			CRgeneration := existingK8sRes.GetGeneration()
+			err = r.Update(ctx, &existingK8sRes, client.DryRunAll)
+			if err != nil {
+				return false, errors.Wrapf(err, "failed to update k8s resource -- Kind: %s, NamespacedName: %s/%s", kind, namespace, name)
+			}
+
+			if newResourceVersion := existingK8sRes.GetResourceVersion(); resourceVersion == newResourceVersion {
+				// if the resourceVersion is the same, the update is not performed
+				klog.Infof("The k8s resource with apiversion: %s, kind: %s, %s/%s is not updated", apiversion, kind, namespace, name)
 				return true, nil
 			}
 
 			klog.Infof("updating k8s resource with apiversion: %s, kind: %s, %s/%s", apiversion, kind, namespace, name)
 
-			CRgeneration := existingK8sRes.GetGeneration()
-
 			err = r.Update(ctx, &existingK8sRes)
 
 			if err != nil {

controllers/operandrequest/reconcile_operator.go

@@ -137,7 +137,11 @@ func (r *Reconciler) reconcileSubscription(ctx context.Context, requestInstance
 	// Check the requested Operand if exist in specific OperandRegistry
 	var opt *operatorv1alpha1.Operator
 	if registryInstance != nil {
-		opt = registryInstance.GetOperator(operand.Name)
+		var err error
+		opt, err = r.GetOperandFromRegistry(ctx, registryInstance, operand.Name)
+		if err != nil {
+			return err
+		}
 	}
 	if opt == nil {
 		if registryInstance != nil {
@@ -381,7 +385,8 @@ func (r *Reconciler) deleteSubscription(ctx context.Context, cr *operatorv1alpha
 }
 
 func (r *Reconciler) uninstallOperatorsAndOperands(ctx context.Context, operandName string, requestInstance *operatorv1alpha1.OperandRequest, registryInstance *operatorv1alpha1.OperandRegistry, configInstance *operatorv1alpha1.OperandConfig) error {
-	op := registryInstance.GetOperator(operandName)
+	// No error handling for un-installation step in case Catalog has been deleted
+	op, _ := r.GetOperandFromRegistry(ctx, registryInstance, operandName)
 	if op == nil {
 		klog.Warningf("Operand %s not found", operandName)
 		return nil

controllers/operator/manager.go

@@ -60,7 +60,7 @@ func NewODLMOperator(mgr manager.Manager, name string) *ODLMOperator {
 		Config:                  mgr.GetConfig(),
 		Recorder:                mgr.GetEventRecorderFor(name),
 		Scheme:                  mgr.GetScheme(),
-		MaxConcurrentReconciles: 3,
+		MaxConcurrentReconciles: 5,
 	}
 }
 
@@ -70,26 +70,6 @@ func (m *ODLMOperator) GetOperandRegistry(ctx context.Context, key types.Namespa
 	if err := m.Client.Get(ctx, key, reg); err != nil {
 		return nil, err
 	}
-	// Get excluded CatalogSource from annotation
-	// excluded-catalogsource: catalogsource1, catalogsource2
-	var excludedCatalogSources []string
-	if reg.Annotations != nil && reg.Annotations["excluded-catalogsource"] != "" {
-		excludedCatalogSources = strings.Split(reg.Annotations["excluded-catalogsource"], ",")
-	}
-	// Get catalog used by ODLM itself by check its own subscription
-	opts := []client.ListOption{
-		client.MatchingLabels{fmt.Sprintf("operators.coreos.com/ibm-odlm.%s", util.GetOperatorNamespace()): ""},
-		client.InNamespace(util.GetOperatorNamespace()),
-	}
-	odlmCatalog := ""
-	odlmCatalogNs := ""
-	odlmSubList := &olmv1alpha1.SubscriptionList{}
-	if err := m.Reader.List(ctx, odlmSubList, opts...); err != nil || len(odlmSubList.Items) == 0 {
-		klog.Warningf("No Subscription found for ibm-odlm in the namespace %s", util.GetOperatorNamespace())
-	} else {
-		odlmCatalog = odlmSubList.Items[0].Spec.CatalogSource
-		odlmCatalogNs = odlmSubList.Items[0].Spec.CatalogSourceNamespace
-	}
 
 	for i, o := range reg.Spec.Operators {
 		if o.Scope == "" {
@@ -104,18 +84,6 @@ func (m *ODLMOperator) GetOperandRegistry(ctx context.Context, key types.Namespa
 		if o.Namespace == "" {
 			reg.Spec.Operators[i].Namespace = key.Namespace
 		}
-		if o.SourceName == "" || o.SourceNamespace == "" {
-			catalogSourceName, catalogSourceNs, err := m.GetCatalogSourceFromPackage(ctx, o.PackageName, reg.Spec.Operators[i].Namespace, o.Channel, key.Namespace, odlmCatalog, odlmCatalogNs, excludedCatalogSources)
-			if err != nil {
-				return nil, err
-			}
-
-			if catalogSourceName == "" || catalogSourceNs == "" {
-				klog.V(2).Infof("no catalogsource found for %v", o.PackageName)
-			}
-
-			reg.Spec.Operators[i].SourceName, reg.Spec.Operators[i].SourceNamespace = catalogSourceName, catalogSourceNs
-		}
 	}
 	return reg, nil
 }
@@ -431,6 +399,46 @@ func (m *ODLMOperator) GetOperatorNamespace(installMode, namespace string) strin
 	return namespace
 }
 
+// GetOperandFromRegistry gets the Operand from the OperandRegistry
+func (m *ODLMOperator) GetOperandFromRegistry(ctx context.Context, reg *apiv1alpha1.OperandRegistry, operandName string) (*apiv1alpha1.Operator, error) {
+	opt := reg.GetOperator(operandName)
+	// Get excluded CatalogSource from annotation
+	// excluded-catalogsource: catalogsource1, catalogsource2
+	var excludedCatalogSources []string
+	if reg.Annotations != nil && reg.Annotations["excluded-catalogsource"] != "" {
+		excludedCatalogSources = strings.Split(reg.Annotations["excluded-catalogsource"], ",")
+	}
+	// Get catalog used by ODLM itself by check its own subscription
+	opts := []client.ListOption{
+		client.MatchingLabels{fmt.Sprintf("operators.coreos.com/ibm-odlm.%s", util.GetOperatorNamespace()): ""},
+		client.InNamespace(util.GetOperatorNamespace()),
+	}
+	odlmCatalog := ""
+	odlmCatalogNs := ""
+	odlmSubList := &olmv1alpha1.SubscriptionList{}
+	if err := m.Reader.List(ctx, odlmSubList, opts...); err != nil || len(odlmSubList.Items) == 0 {
+		klog.Warningf("No Subscription found for ibm-odlm in the namespace %s", util.GetOperatorNamespace())
+	} else {
+		odlmCatalog = odlmSubList.Items[0].Spec.CatalogSource
+		odlmCatalogNs = odlmSubList.Items[0].Spec.CatalogSourceNamespace
+	}
+
+	if opt.SourceName == "" || opt.SourceNamespace == "" {
+		catalogSourceName, catalogSourceNs, err := m.GetCatalogSourceFromPackage(ctx, opt.PackageName, opt.Namespace, opt.Channel, reg.Namespace, odlmCatalog, odlmCatalogNs, excludedCatalogSources)
+		if err != nil {
+			return nil, err
+		}
+
+		if catalogSourceName == "" || catalogSourceNs == "" {
+			klog.V(2).Infof("no catalogsource found for %v", opt.PackageName)
+		}
+
+		opt.SourceName, opt.SourceNamespace = catalogSourceName, catalogSourceNs
+	}
+
+	return opt, nil
+}
+
 func (m *ODLMOperator) CheckLabel(unstruct unstructured.Unstructured, labels map[string]string) bool {
 	for k, v := range labels {
 		if !m.HasLabel(unstruct, k) {

controllers/operatorconfig/operatorconfig_controller.go

@@ -72,8 +72,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		}
 		for _, u := range reqBlock.Operands {
 			operand := u
-			operator := registry.GetOperator(operand.Name)
-			if operator == nil || operator.OperatorConfig == "" {
+			operator, err := r.GetOperandFromRegistry(ctx, registry, operand.Name)
+			if err != nil {
+				return ctrl.Result{}, err
+			} else if operator == nil || operator.OperatorConfig == "" {
 				continue
 			}
 

controllers/util/merge.go

@@ -18,8 +18,8 @@ package util
 
 import (
 	"encoding/json"
-	"reflect"
 
+	"k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/klog"
 )
 
@@ -59,7 +59,7 @@ func MergeCR(defaultCR, changedCR []byte) map[string]interface{} {
 }
 
 func checkKeyBeforeMerging(key string, defaultMap interface{}, changedMap interface{}, finalMap map[string]interface{}) {
-	if !reflect.DeepEqual(defaultMap, changedMap) {
+	if !equality.Semantic.DeepEqual(defaultMap, changedMap) {
 		switch defaultMap := defaultMap.(type) {
 		case map[string]interface{}:
 			//Check that the changed map value doesn't contain this map at all and is nil