diff --git a/controllers/operandbindinfo/operandbindinfo_controller.go b/controllers/operandbindinfo/operandbindinfo_controller.go index 41e54892..10f2d964 100644 --- a/controllers/operandbindinfo/operandbindinfo_controller.go +++ b/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 diff --git a/controllers/operandrequest/operandrequest_controller.go b/controllers/operandrequest/operandrequest_controller.go index b9b2c69c..9d1c264f 100644 --- a/controllers/operandrequest/operandrequest_controller.go +++ b/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 diff --git a/controllers/operandrequest/reconcile_operand.go b/controllers/operandrequest/reconcile_operand.go index 7249f2b8..69cbae26 100644 --- a/controllers/operandrequest/reconcile_operand.go +++ b/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 { diff --git a/controllers/operandrequest/reconcile_operator.go b/controllers/operandrequest/reconcile_operator.go index 722ea8b8..5c323c86 100644 --- a/controllers/operandrequest/reconcile_operator.go +++ b/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 diff --git a/controllers/operator/manager.go b/controllers/operator/manager.go index fd2bde11..3bc13d69 100644 --- a/controllers/operator/manager.go +++ b/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) { diff --git a/controllers/operatorconfig/operatorconfig_controller.go b/controllers/operatorconfig/operatorconfig_controller.go index 3ee98ef7..ae71f229 100644 --- a/controllers/operatorconfig/operatorconfig_controller.go +++ b/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 } diff --git a/controllers/util/merge.go b/controllers/util/merge.go index fc3b83c3..8d0743ff 100644 --- a/controllers/util/merge.go +++ b/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