From: Dileep Ranganathan Date: Mon, 9 Sep 2019 22:56:54 +0000 (-0700) Subject: Bug Fix - Daemonset not reloading X-Git-Tag: 1.5.0~7 X-Git-Url: https://gerrit.onap.org/r/gitweb?p=demo.git;a=commitdiff_plain;h=3d29ee771bf6f0f2c2117cc2908561e228c8123f 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 Change-Id: I7ef6d4e640d6190da34cc70d5a7cf80a96c004bd --- 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" }