Bug Fix - Daemonset not reloading

Fixed issue related to Daemonset reloading due to a bug introduced in
the previous patch.
Removed redundant watches across multiple controllers.
Used defer statement instead of calling mutex unlock.

Issue-ID: ONAPARC-461
Signed-off-by: Dileep Ranganathan <dileep.ranganathan@intel.com>
Change-Id: I7ef6d4e640d6190da34cc70d5a7cf80a96c004bd

3 files changed, 41 insertions, 58 deletions

diff --git a/vnfs/DAaaS/microservices/collectd-operator/pkg/controller/collectdglobal/collectdglobal_controller.go b/vnfs/DAaaS/microservices/collectd-operator/pkg/controller/collectdglobal/collectdglobal_controller.go
index a43afdc5..0d3e2bbf 100644
--- a/vnfs/DAaaS/microservices/collectd-operator/pkg/controller/collectdglobal/collectdglobal_controller.go
+++ b/vnfs/DAaaS/microservices/collectd-operator/pkg/controller/collectdglobal/collectdglobal_controller.go
@@ -74,7 +74,17 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {
 					var requests []reconcile.Request
 					cg, err := collectdutils.GetCollectdGlobal(rcp.client, a.Meta.GetNamespace())
 					if err != nil || cg == nil {
-						return nil
+						log.V(1).Info("No CollectdGlobal CR instance Exist")
+						cpList, err := collectdutils.GetCollectdPluginList(rcp.client, a.Meta.GetNamespace())
+						if err != nil || cpList == nil || cpList.Items == nil || len(cpList.Items) == 0 {
+							log.V(1).Info("No CollectdPlugin CR instance Exist")
+							return nil
+						}
+						for _, cp := range cpList.Items {
+							requests = append(requests, reconcile.Request{
+								NamespacedName: client.ObjectKey{Namespace: cp.Namespace, Name: cp.Name}})
+						}
+						return requests
 					}
 					requests = append(requests, reconcile.Request{
 						NamespacedName: client.ObjectKey{Namespace: cg.Namespace, Name: cg.Name}})
@@ -82,7 +92,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {
 				}
 				return nil
 			}),
-		})
+		}, predicate.GenerationChangedPredicate{})
 	if err != nil {
 		return err
 	}
@@ -139,7 +149,7 @@ func (r *ReconcileCollectdGlobal) Reconcile(request reconcile.Request) (reconcil
 		if err := r.addFinalizer(reqLogger, instance); err != nil {
 			return reconcile.Result{}, err
 		}
-		return reconcile.Result{}, nil
+		//return reconcile.Result{}, nil
 	}
 	// Handle the reconciliation for CollectdGlobal.
 	// At this stage the Status of the CollectdGlobal should NOT be ""
@@ -149,7 +159,8 @@ func (r *ReconcileCollectdGlobal) Reconcile(request reconcile.Request) (reconcil
 
 // handleCollectdGlobal regenerates the collectd conf on CR Create, Update, Delete events
 func (r *ReconcileCollectdGlobal) handleCollectdGlobal(reqLogger logr.Logger, cr *onapv1alpha1.CollectdGlobal, isDelete bool) error {
-	var collectdConf string
+	collectdutils.ReconcileLock.Lock()
+	defer collectdutils.ReconcileLock.Unlock()
 
 	retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
 		cm, err := collectdutils.GetConfigMap(r.client, reqLogger, cr.Namespace)
@@ -174,13 +185,11 @@ func (r *ReconcileCollectdGlobal) handleCollectdGlobal(reqLogger logr.Logger, cr
 		// Update the ConfigMap with new Spec and reload DaemonSets
 		reqLogger.Info("Updating the ConfigMap", "ConfigMap.Namespace", cm.Namespace, "ConfigMap.Name", cm.Name)
 		updateErr := r.client.Update(context.TODO(), cm)
-		return updateErr
-	})
-	if retryErr != nil {
-		panic(fmt.Errorf("Update failed: %v", retryErr))
-	}
+		if updateErr != nil {
+			reqLogger.Error(updateErr, "Update ConfigMap failed")
+			return updateErr
+		}
 
-	retryErr = retry.RetryOnConflict(retry.DefaultRetry, func() error {
 		// Retrieve the latest version of Daemonset before attempting update
 		// RetryOnConflict uses exponential backoff to avoid exhausting the apiserver
 		// Select DaemonSets with label
@@ -208,7 +217,7 @@ func (r *ReconcileCollectdGlobal) handleCollectdGlobal(reqLogger logr.Logger, cr
 			"daaas-random": collectdutils.ComputeSHA256([]byte(collectdConf)),
 		})
 		r.handleTypesDB(reqLogger, cr, ds, isDelete)
-		updateErr := r.client.Update(context.TODO(), ds)
+		updateErr = r.client.Update(context.TODO(), ds)
 		return updateErr
 	})
 	if retryErr != nil {
diff --git a/vnfs/DAaaS/microservices/collectd-operator/pkg/controller/collectdplugin/collectdplugin_controller.go b/vnfs/DAaaS/microservices/collectd-operator/pkg/controller/collectdplugin/collectdplugin_controller.go
index 644a6bb3..32775c5c 100644
--- a/vnfs/DAaaS/microservices/collectd-operator/pkg/controller/collectdplugin/collectdplugin_controller.go
+++ b/vnfs/DAaaS/microservices/collectd-operator/pkg/controller/collectdplugin/collectdplugin_controller.go
@@ -4,8 +4,6 @@ import (
 	"context"
 	"fmt"
 	"reflect"
-	"strings"
-	"sync"
 
 	"github.com/go-logr/logr"
 	"github.com/operator-framework/operator-sdk/pkg/predicate"
@@ -29,8 +27,6 @@ import (
 
 var log = logf.Log.WithName("controller_collectdplugin")
 
-var reconcileLock sync.Mutex
-
 // Add creates a new CollectdPlugin Controller and adds it to the Manager. The Manager will set fields on the Controller
 // and Start it when the Manager is Started.
 func Add(mgr manager.Manager) error {
@@ -58,37 +54,6 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {
 		return err
 	}
 
-	log.V(1).Info("Add watcher for secondary resource Collectd Daemonset")
-	err = c.Watch(
-		&source.Kind{Type: &appsv1.DaemonSet{}},
-		&handler.EnqueueRequestsFromMapFunc{
-			ToRequests: handler.ToRequestsFunc(func(a handler.MapObject) []reconcile.Request {
-				labelSelector, err := collectdutils.GetWatchLabels()
-				labels := strings.Split(labelSelector, "=")
-				if err != nil {
-					log.Error(err, "Failed to get watch labels, continuing with default label")
-				}
-				rcp := r.(*ReconcileCollectdPlugin)
-				// Select the Daemonset with labelSelector (Defautl  is app=collectd)
-				if a.Meta.GetLabels()[labels[0]] == labels[1] {
-					var requests []reconcile.Request
-					cpList, err := collectdutils.GetCollectdPluginList(rcp.client, a.Meta.GetNamespace())
-					if err != nil {
-						return nil
-					}
-					for _, cp := range cpList.Items {
-						requests = append(requests, reconcile.Request{
-							NamespacedName: client.ObjectKey{Namespace: cp.Namespace, Name: cp.Name}})
-					}
-					return requests
-				}
-				return nil
-			}),
-		})
-	if err != nil {
-		return err
-	}
-
 	return nil
 }
 
@@ -141,19 +106,18 @@ func (r *ReconcileCollectdPlugin) Reconcile(request reconcile.Request) (reconcil
 		if err := r.addFinalizer(reqLogger, instance); err != nil {
 			return reconcile.Result{}, err
 		}
-		return reconcile.Result{}, nil
+		//return reconcile.Result{}, nil
 	}
 	// Handle the reconciliation for CollectdPlugin.
 	// At this stage the Status of the CollectdPlugin should NOT be ""
-	reconcileLock.Lock()
 	err = r.handleCollectdPlugin(reqLogger, instance, false)
-	reconcileLock.Unlock()
 	return reconcile.Result{}, err
 }
 
 // handleCollectdPlugin regenerates the collectd conf on CR Create, Update, Delete events
 func (r *ReconcileCollectdPlugin) handleCollectdPlugin(reqLogger logr.Logger, cr *onapv1alpha1.CollectdPlugin, isDelete bool) error {
-	var collectdConf string
+	collectdutils.ReconcileLock.Lock()
+	defer collectdutils.ReconcileLock.Unlock()
 	retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
 		cm, err := collectdutils.GetConfigMap(r.client, reqLogger, cr.Namespace)
 		if err != nil {
@@ -176,12 +140,11 @@ func (r *ReconcileCollectdPlugin) handleCollectdPlugin(reqLogger logr.Logger, cr
 		// Update the ConfigMap with new Spec and reload DaemonSets
 		reqLogger.Info("Updating the ConfigMap", "ConfigMap.Namespace", cm.Namespace, "ConfigMap.Name", cm.Name)
 		updateErr := r.client.Update(context.TODO(), cm)
-		return updateErr
-	})
-	if retryErr != nil {
-		panic(fmt.Errorf("Update failed: %v", retryErr))
-	}
-	retryErr = retry.RetryOnConflict(retry.DefaultRetry, func() error {
+		if updateErr != nil {
+			reqLogger.Error(updateErr, "Update ConfigMap failed")
+			return updateErr
+		}
+
 		// Retrieve the latest version of Daemonset before attempting update
 		// RetryOnConflict uses exponential backoff to avoid exhausting the apiserver
 		// Select DaemonSets with label
@@ -208,7 +171,7 @@ func (r *ReconcileCollectdPlugin) handleCollectdPlugin(reqLogger logr.Logger, cr
 		ds.Spec.Template.SetAnnotations(map[string]string{
 			"daaas-random": collectdutils.ComputeSHA256([]byte(collectdConf)),
 		})
-		updateErr := r.client.Update(context.TODO(), ds)
+		updateErr = r.client.Update(context.TODO(), ds)
 		return updateErr
 	})
 	if retryErr != nil {
diff --git a/vnfs/DAaaS/microservices/collectd-operator/pkg/controller/utils/collectdutils.go b/vnfs/DAaaS/microservices/collectd-operator/pkg/controller/utils/collectdutils.go
index 6a85103c..17cad0e5 100644
--- a/vnfs/DAaaS/microservices/collectd-operator/pkg/controller/utils/collectdutils.go
+++ b/vnfs/DAaaS/microservices/collectd-operator/pkg/controller/utils/collectdutils.go
@@ -5,6 +5,7 @@ import (
 	"crypto/sha256"
 	"fmt"
 	"os"
+	"sort"
 	"sync"
 
 	"github.com/go-logr/logr"
@@ -30,6 +31,9 @@ const (
 
 var lock sync.Mutex
 
+// ReconcileLock - Used to sync between global and plugin controller
+var ReconcileLock sync.Mutex
+
 // ResourceMap to hold objects to update/reload
 type ResourceMap struct {
 	ConfigMap       *corev1.ConfigMap
@@ -195,7 +199,14 @@ func RebuildCollectdConf(rc client.Client, ns string, isDelete bool, delPlugin s
 		collectdConf += collectdGlobalConf
 	}
 
-	for cpName, cpConf := range loadPlugin {
+	pluginKeys := make([]string, 0, len(loadPlugin))
+	for k := range loadPlugin {
+		pluginKeys = append(pluginKeys, k)
+	}
+	sort.Strings(pluginKeys)
+
+	for _, cpName := range pluginKeys {
+		cpConf := loadPlugin[cpName]
 		collectdConf += "LoadPlugin" + " " + cpName + "\n"
 		collectdConf += cpConf + "\n"
 	}