add event to remind no update although CR changed (#377)

* refactor comparable resource assignment

* add event to commonservice CR

Author: Daniel-Fan

controllers/commonservice_controller.go

@@ -18,10 +18,13 @@ package controllers
 
 import (
 	"context"
+	"fmt"
 
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/tools/record"
 	"k8s.io/klog"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
@@ -40,7 +43,8 @@ type CommonServiceReconciler struct {
 	client.Reader
 	*deploy.Manager
 	*bootstrap.Bootstrap
-	Scheme *runtime.Scheme
+	Scheme   *runtime.Scheme
+	Recorder record.EventRecorder
 }
 
 const (
@@ -64,6 +68,7 @@ func (r *CommonServiceReconciler) Reconcile(req ctrl.Request) (ctrl.Result, erro
 			if err := r.handleDelete(); err != nil {
 				return ctrl.Result{}, err
 			}
+			klog.Info("Deleted reconciling CommonService CR successfully")
 		}
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
@@ -117,14 +122,20 @@ func (r *CommonServiceReconciler) ReconcileMasterCR(instance *apiv3.CommonServic
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
-	if err := r.updateOperandConfig(newConfigs); err != nil {
+	isEqual, err := r.updateOperandConfig(newConfigs)
+	if err != nil {
 		if err := r.updatePhase(instance, CRFailed); err != nil {
 			klog.Error(err)
 		}
 		klog.Errorf("Fail to reconcile %s/%s: %v", instance.Namespace, instance.Name, err)
 		return ctrl.Result{}, err
 	}
 
+	// Create Event if there is no update in OperandConfig after applying current CR
+	if isEqual {
+		r.Recorder.Event(instance, corev1.EventTypeNormal, "Noeffect", fmt.Sprintf("No update, resource sizings in the OperandConfig %s/%s are larger than the profile from CommonService CR %s/%s", r.Bootstrap.MasterNamespace, "common-service", instance.Namespace, instance.Name))
+	}
+
 	if err := r.updatePhase(instance, CRSucceeded); err != nil {
 		klog.Error(err)
 		return ctrl.Result{}, err
@@ -178,14 +189,20 @@ func (r *CommonServiceReconciler) ReconcileGeneralCR(instance *apiv3.CommonServi
 		return ctrl.Result{}, err
 	}
 
-	if err = r.updateOperandConfig(newConfigs); err != nil {
+	isEqual, err := r.updateOperandConfig(newConfigs)
+	if err != nil {
 		if err := r.updatePhase(instance, CRFailed); err != nil {
 			klog.Error(err)
 		}
 		klog.Errorf("Fail to reconcile %s/%s: %v", instance.Namespace, instance.Name, err)
 		return ctrl.Result{}, err
 	}
 
+	// Create Event if there is no update in OperandConfig after applying current CR
+	if isEqual {
+		r.Recorder.Event(instance, corev1.EventTypeNormal, "Noeffect", fmt.Sprintf("No update, resource sizings in the OperandConfig %s/%s are larger than the profile from CommonService CR %s/%s", r.Bootstrap.MasterNamespace, "common-service", instance.Namespace, instance.Name))
+	}
+
 	if err := r.updatePhase(instance, CRSucceeded); err != nil {
 		klog.Errorf("Fail to reconcile %s/%s: %v", instance.Namespace, instance.Name, err)
 		return ctrl.Result{}, err

controllers/operandconfig.go

@@ -23,6 +23,7 @@ import (
 	"strings"
 
 	utilyaml "github.com/ghodss/yaml"
+	"github.com/mohae/deepcopy"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/klog"
 
@@ -142,13 +143,13 @@ func mergeChangedMap(key string, defaultMap interface{}, changedMap interface{},
 			if changedMap == nil {
 				finalMap[key] = defaultMap
 			} else {
-				comparableKeys := []string{"replicas", "cpu", "memory"}
-				for _, comparable := range comparableKeys {
-					if key == comparable {
-						finalMap[key], _ = rules.ResourceComparison(defaultMap, changedMap)
-					} else {
-						finalMap[key] = changedMap
-					}
+				var comparableKeys = map[string]bool{
+					"replicas": true,
+					"cpu":      true,
+					"memory":   true,
+				}
+				if _, ok := comparableKeys[key]; ok {
+					finalMap[key], _ = rules.ResourceComparison(defaultMap, changedMap)
 				}
 			}
 		}
@@ -209,23 +210,25 @@ func deepMergeTwoMaps(key string, defaultMap interface{}, changedMap interface{}
 	}
 }
 
-func (r *CommonServiceReconciler) updateOperandConfig(newConfigs []interface{}) error {
+func (r *CommonServiceReconciler) updateOperandConfig(newConfigs []interface{}) (bool, error) {
 	opcon := util.NewUnstructured("operator.ibm.com", "OperandConfig", "v1alpha1")
 	opconKey := types.NamespacedName{
 		Name:      "common-service",
 		Namespace: r.Bootstrap.MasterNamespace,
 	}
 	if err := r.Reader.Get(ctx, opconKey, opcon); err != nil {
 		klog.Errorf("failed to get OperandConfig %s: %v", opconKey.String(), err)
-		return err
+		return true, err
 	}
 
+	// Keep a version of exsiting config for comparasion later
 	opconServices := opcon.Object["spec"].(map[string]interface{})["services"].([]interface{})
+	existingOpconServices := deepcopy.Copy(opconServices)
 
 	// Convert rules string to slice
 	ruleSlice, err := convertStringToSlice(rules.ConfigurationRules)
 	if err != nil {
-		return err
+		return true, err
 	}
 
 	for _, opService := range opconServices {
@@ -260,17 +263,32 @@ func (r *CommonServiceReconciler) updateOperandConfig(newConfigs []interface{})
 	// Checking all the common service CRs to get the minimal size
 	opconServices, err = r.getMinimalSizes(opconServices, ruleSlice)
 	if err != nil {
-		return err
+		return true, err
+	}
+
+	// Compare to see whether new resource sizing is introduced into opconServices
+	isEqual := true
+	for _, opService := range opconServices {
+		existingOpService := getItemByName(existingOpconServices.([]interface{}), opService.(map[string]interface{})["name"].(string))
+		for cr, spec := range opService.(map[string]interface{})["spec"].(map[string]interface{}) {
+			existingCrSpec := existingOpService.(map[string]interface{})["spec"].(map[string]interface{})[cr].(map[string]interface{})
+			if isEqual = rules.ResourceEqualComparison(existingCrSpec, spec); !isEqual {
+				break
+			}
+		}
+		if !isEqual {
+			break
+		}
 	}
 
 	opcon.Object["spec"].(map[string]interface{})["services"] = opconServices
 
 	if err := r.Update(ctx, opcon); err != nil {
 		klog.Errorf("failed to update OperandConfig %s: %v", opconKey.String(), err)
-		return err
+		return true, err
 	}
 
-	return nil
+	return isEqual, nil
 }
 
 func checkCRFromOperandConfig(serviceStatus map[string]interface{}, operatorName, crName string) bool {

controllers/rules/resource_comparison.go

@@ -67,3 +67,46 @@ func ResourceComparison(resourceA, resourceB interface{}) (interface{}, interfac
 		return resourceA, resourceA
 	}
 }
+
+func ResourceEqualComparison(resourceA interface{}, resourceB interface{}) bool {
+
+	klog.V(2).Infof("Kind of A %s", reflect.TypeOf(resourceA).Kind())
+	klog.V(2).Infof("Kind of B %s", reflect.TypeOf(resourceB).Kind())
+
+	isEqual := true
+	switch resourceA := resourceA.(type) {
+	case map[string]interface{}:
+		if resourceB == nil {
+			isEqual = false
+		} else if _, ok := resourceB.(map[string]interface{}); ok { //Check that the changed map value is also a map[string]interface
+			resourceARef := resourceA
+			resourceBRef := resourceB.(map[string]interface{})
+			for newKey := range resourceARef {
+				isEqual = ResourceEqualComparison(resourceARef[newKey], resourceBRef[newKey])
+				if !isEqual {
+					break
+				}
+			}
+		}
+		return isEqual
+	case int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64, float32, float64:
+		strA := fmt.Sprintf("%v", resourceA)
+		strB := fmt.Sprintf("%v", resourceB)
+
+		floatA, _ := strconv.ParseFloat(strA, 64)
+		floatB, _ := strconv.ParseFloat(strB, 64)
+		if floatA == floatB {
+			isEqual = true
+		} else {
+			isEqual = false
+		}
+		return isEqual
+	default:
+		if resourceA == resourceB {
+			isEqual = true
+		} else {
+			isEqual = false
+		}
+		return isEqual
+	}
+}

go.mod

@@ -5,6 +5,7 @@ go 1.13
 require (
 	github.com/IBM/ibm-namespace-scope-operator v1.0.1
 	github.com/ghodss/yaml v1.0.0
+	github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 // indirect
 	github.com/onsi/ginkgo v1.12.1
 	github.com/onsi/gomega v1.10.1
 	github.com/operator-framework/api v0.3.10

go.sum

@@ -253,6 +253,8 @@ github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJ
 github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=
 github.com/modern-go/reflect2 v1.0.1 h1:9f412s+6RmYXLWZSEzVVgPGK7C2PphHj5RJrvfx9AWI=
 github.com/modern-go/reflect2 v1.0.1/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=
+github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 h1:RWengNIwukTxcDr9M+97sNutRR1RKhG96O6jWumTTnw=
+github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826/go.mod h1:TaXosZuwdSHYgviHp1DAtfrULt5eUgsSMsZf+YrPgl8=
 github.com/munnerz/goautoneg v0.0.0-20120707110453-a547fc61f48d/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
 github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
 github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=

main.go

@@ -123,6 +123,7 @@ func main() {
 			Manager:   deploy.NewDeployManager(mgr),
 			Bootstrap: bootstrap.NewBootstrap(mgr),
 			Scheme:    mgr.GetScheme(),
+			Recorder:  mgr.GetEventRecorderFor("commonservice-controller"),
 		}).SetupWithManager(mgr); err != nil {
 			klog.Errorf("Unable to create controller CommonService: %v", err)
 			os.Exit(1)