update logging

update logging update logging update logging update logging fixup lint update logging update logging update logging fixup lint Structured logging
openstack-k8s-operators · Dec 5, 2023 · 8d0daa7 · 8d0daa7
1 parent 7e706e9
commit 8d0daa7
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 35 deletions.
diff --git a/controllers/openstackdataplanedeployment_controller.go b/controllers/openstackdataplanedeployment_controller.go
@@ -44,7 +44,11 @@ type OpenStackDataPlaneDeploymentReconciler struct {
 	client.Client
 	Kclient kubernetes.Interface
 	Scheme  *runtime.Scheme
-	Log     logr.Logger
+}
+
+// GetLogger returns a logger object with a prefix of "controller.name" and additional controller context fields
+func (r *OpenStackDataPlaneDeploymentReconciler) GetLogger(ctx context.Context) logr.Logger {
+	return log.FromContext(ctx).WithName("Controllers").WithName("OpenStackDataPlaneDeployment")
 }
 
 //+kubebuilder:rbac:groups=dataplane.openstack.org,resources=openstackdataplanedeployments,verbs=get;list;watch;create;delete
@@ -58,8 +62,9 @@ type OpenStackDataPlaneDeploymentReconciler struct {
 // Reconcile is part of the main kubernetes reconciliation loop which aims to
 // move the current state of the cluster closer to the desired state.
 func (r *OpenStackDataPlaneDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, _err error) {
-	logger := log.FromContext(ctx)
-	logger.Info("Reconciling Deployment")
+
+	Log := r.GetLogger(ctx)
+	Log.Info("Reconciling Deployment")
 
 	// Fetch the OpenStackDataPlaneDeployment instance
 	instance := &dataplanev1.OpenStackDataPlaneDeployment{}
@@ -80,7 +85,7 @@ func (r *OpenStackDataPlaneDeploymentReconciler) Reconcile(ctx context.Context,
 		r.Client,
 		r.Kclient,
 		r.Scheme,
-		logger,
+		Log,
 	)
 
 	// Always patch the instance status when exiting this function so we can persist any changes.
@@ -98,15 +103,15 @@ func (r *OpenStackDataPlaneDeploymentReconciler) Reconcile(ctx context.Context,
 		}
 		err := helper.PatchInstance(ctx, instance)
 		if err != nil {
-			logger.Error(err, "Error updating instance status conditions")
+			Log.Error(err, "Error updating instance status conditions")
 			_err = err
 			return
 		}
 	}()
 
 	// If the deploy is already done, return immediately.
 	if instance.Status.Deployed {
-		logger.Info("Already deployed", "instance.Status.Deployed", instance.Status.Deployed)
+		Log.Info("Already deployed", "instance.Status.Deployed", instance.Status.Deployed)
 		return ctrl.Result{}, nil
 	}
 
@@ -134,7 +139,7 @@ func (r *OpenStackDataPlaneDeploymentReconciler) Reconcile(ctx context.Context,
 		if err != nil {
 			// NodeSet not found, force a requeue
 			if k8s_errors.IsNotFound(err) {
-				logger.Info("NodeSet not found", "NodeSet", nodeSet)
+				Log.Info("NodeSet not found", "NodeSet", nodeSet)
 				return ctrl.Result{RequeueAfter: time.Second * 15}, nil
 			}
 			// Error reading the object - requeue the request.
@@ -146,7 +151,7 @@ func (r *OpenStackDataPlaneDeploymentReconciler) Reconcile(ctx context.Context,
 	// Check that all nodeSets are SetupReady
 	for _, nodeSet := range nodeSets.Items {
 		if !nodeSet.Status.Conditions.IsTrue(dataplanev1.SetupReadyCondition) {
-			logger.Info("NodeSet SetupReadyCondition is not True", "NodeSet", nodeSet.Name)
+			Log.Info("NodeSet SetupReadyCondition is not True", "NodeSet", nodeSet.Name)
 			return ctrl.Result{RequeueAfter: time.Second * 15}, nil
 		}
 	}
@@ -163,10 +168,10 @@ func (r *OpenStackDataPlaneDeploymentReconciler) Reconcile(ctx context.Context,
 	haveError := false
 	for _, nodeSet := range nodeSets.Items {
 
-		logger.Info(fmt.Sprintf("Deploying NodeSet: %s", nodeSet.Name))
-		logger.Info("Set Status.Deployed to false", "instance", instance)
+		Log.Info(fmt.Sprintf("Deploying NodeSet: %s", nodeSet.Name))
+		Log.Info("Set Status.Deployed to false", "instance", instance)
 		instance.Status.Deployed = false
-		logger.Info("Set DeploymentReadyCondition false", "instance", instance)
+		Log.Info("Set DeploymentReadyCondition false")
 		instance.Status.Conditions.MarkFalse(
 			condition.DeploymentReadyCondition, condition.RequestedReason,
 			condition.SeverityInfo, condition.DeploymentReadyRunningMessage)
@@ -214,8 +219,8 @@ func (r *OpenStackDataPlaneDeploymentReconciler) Reconcile(ctx context.Context,
 		if deployResult != nil {
 			shouldRequeue = true
 		} else {
-			logger.Info("OpenStackDeployment succeeded for NodeSet", "NodeSet", nodeSet.Name)
-			logger.Info("Set NodeSetDeploymentReadyCondition true", "nodeSet", nodeSet.Name)
+			Log.Info("OpenStackDeployment succeeded for NodeSet", "NodeSet", nodeSet.Name)
+			Log.Info("Set NodeSetDeploymentReadyCondition true", "nodeSet", nodeSet.Name)
 			instance.Status.Conditions.MarkTrue(
 				condition.Type(fmt.Sprintf(dataplanev1.NodeSetDeploymentReadyCondition, nodeSet.Name)),
 				condition.DeploymentReadyMessage)
@@ -227,14 +232,14 @@ func (r *OpenStackDataPlaneDeploymentReconciler) Reconcile(ctx context.Context,
 	}
 
 	if shouldRequeue {
-		logger.Info("Not all NodeSets done for OpenStackDeployment")
+		Log.Info("Not all NodeSets done for OpenStackDeployment")
 		return ctrl.Result{}, nil
 	}
 
-	logger.Info("Set DeploymentReadyCondition true", "instance", instance)
+	Log.Info("Set DeploymentReadyCondition true")
 	instance.Status.Conditions.MarkTrue(condition.DeploymentReadyCondition, condition.DeploymentReadyMessage)
 	instance.Status.Deployed = true
-	logger.Info("Set status deploy true", "instance", instance)
+	Log.Info("Set status deploy true", "instance", instance)
 	return ctrl.Result{}, nil
 }
 

diff --git a/controllers/openstackdataplanenodeset_controller.go b/controllers/openstackdataplanenodeset_controller.go
@@ -93,7 +93,11 @@ type OpenStackDataPlaneNodeSetReconciler struct {
 	client.Client
 	Kclient kubernetes.Interface
 	Scheme  *runtime.Scheme
-	Log     logr.Logger
+}
+
+// GetLogger returns a logger object with a prefix of "controller.name" and additional controller context fields
+func (r *OpenStackDataPlaneNodeSetReconciler) GetLogger(ctx context.Context) logr.Logger {
+	return log.FromContext(ctx).WithName("Controllers").WithName("OpenStackDataPlaneNodeSet")
 }
 
 //+kubebuilder:rbac:groups=dataplane.openstack.org,resources=openstackdataplanenodesets,verbs=get;list;watch;create;update;patch;delete
@@ -129,8 +133,9 @@ type OpenStackDataPlaneNodeSetReconciler struct {
 // For more details, check Reconcile and its Result here:
 // - https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/reconcile
 func (r *OpenStackDataPlaneNodeSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, _err error) {
-	logger := log.FromContext(ctx)
-	logger.Info("Reconciling NodeSet")
+
+	Log := r.GetLogger(ctx)
+	Log.Info("Reconciling NodeSet")
 
 	// Fetch the OpenStackDataPlaneNodeSet instance
 	instance := &dataplanev1.OpenStackDataPlaneNodeSet{}
@@ -151,7 +156,7 @@ func (r *OpenStackDataPlaneNodeSetReconciler) Reconcile(ctx context.Context, req
 		r.Client,
 		r.Kclient,
 		r.Scheme,
-		logger,
+		Log,
 	)
 
 	// Always patch the instance status when exiting this function so we can persist any changes.
@@ -169,7 +174,7 @@ func (r *OpenStackDataPlaneNodeSetReconciler) Reconcile(ctx context.Context, req
 		}
 		err := helper.PatchInstance(ctx, instance)
 		if err != nil {
-			logger.Error(err, "Error updating instance status conditions")
+			Log.Error(err, "Error updating instance status conditions")
 			_err = err
 			return
 		}
@@ -281,7 +286,7 @@ func (r *OpenStackDataPlaneNodeSetReconciler) Reconcile(ctx context.Context, req
 	if instance.Status.Deployed && instance.DeletionTimestamp.IsZero() {
 		// The role is already deployed and not being deleted, so reconciliation
 		// is already complete.
-		logger.Info("NodeSet already deployed", "instance", instance)
+		Log.Info("NodeSet already deployed", "instance", instance)
 		return ctrl.Result{}, nil
 	}
 
@@ -299,23 +304,23 @@ func (r *OpenStackDataPlaneNodeSetReconciler) Reconcile(ctx context.Context, req
 	// Set DeploymentReadyCondition to False if it was unknown.
 	// Handles the case where the NodeSet is created, but not yet deployed.
 	if instance.Status.Conditions.IsUnknown(condition.DeploymentReadyCondition) {
-		logger.Info("Set DeploymentReadyCondition false")
+		Log.Info("Set DeploymentReadyCondition false")
 		instance.Status.Conditions.MarkFalse(condition.DeploymentReadyCondition,
 			condition.NotRequestedReason, condition.SeverityInfo,
 			condition.DeploymentReadyInitMessage)
 	}
 
 	deploymentExists, isDeploymentReady, err := checkDeployment(helper, req)
 	if err != nil {
-		logger.Error(err, "Unable to get deployed OpenStackDataPlaneDeployments.")
+		Log.Error(err, "Unable to get deployed OpenStackDataPlaneDeployments.")
 		return ctrl.Result{}, err
 	}
 	if isDeploymentReady {
-		logger.Info("Set NodeSet DeploymentReadyCondition true")
+		Log.Info("Set NodeSet DeploymentReadyCondition true")
 		instance.Status.Conditions.MarkTrue(condition.DeploymentReadyCondition,
 			condition.DeploymentReadyMessage)
 	} else if deploymentExists {
-		logger.Info("Set NodeSet DeploymentReadyCondition false")
+		Log.Info("Set NodeSet DeploymentReadyCondition false")
 		instance.Status.Conditions.MarkFalse(condition.DeploymentReadyCondition,
 			condition.RequestedReason, condition.SeverityInfo,
 			condition.DeploymentReadyRunningMessage)
@@ -349,8 +354,9 @@ func checkDeployment(helper *helper.Helper,
 }
 
 // SetupWithManager sets up the controller with the Manager.
-func (r *OpenStackDataPlaneNodeSetReconciler) SetupWithManager(mgr ctrl.Manager) error {
+func (r *OpenStackDataPlaneNodeSetReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error {
 	reconcileFunction := handler.EnqueueRequestsFromMapFunc(func(o client.Object) []reconcile.Request {
+		Log := r.GetLogger(ctx)
 		result := []reconcile.Request{}
 
 		// For each DNSMasq change event get the list of all
@@ -362,7 +368,7 @@ func (r *OpenStackDataPlaneNodeSetReconciler) SetupWithManager(mgr ctrl.Manager)
 			client.InNamespace(o.GetNamespace()),
 		}
 		if err := r.Client.List(context.Background(), nodeSets, listOpts...); err != nil {
-			r.Log.Error(err, "Unable to retrieve OpenStackDataPlaneNodeSetList %w")
+			Log.Error(err, "Unable to retrieve OpenStackDataPlaneNodeSetList %w")
 			return nil
 		}
 

diff --git a/main.go b/main.go
@@ -17,6 +17,7 @@ limitations under the License.
 package main
 
 import (
+	"context"
 	"crypto/tls"
 	"flag"
 	"os"
@@ -81,7 +82,7 @@ func main() {
 			"Enabling this will ensure there is only one active controller manager.")
 	devMode, err := strconv.ParseBool(os.Getenv("DEV_MODE"))
 	if err != nil {
-		devMode = false
+		devMode = true
 	}
 	opts := zap.Options{
 		Development: devMode,
@@ -138,8 +139,7 @@ func main() {
 		Client:  mgr.GetClient(),
 		Scheme:  mgr.GetScheme(),
 		Kclient: kclient,
-		Log:     ctrl.Log.WithName("controllers").WithName("OpenStackDataPlaneNodeSet"),
-	}).SetupWithManager(mgr); err != nil {
+	}).SetupWithManager(context.Background(), mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "OpenStackDataPlaneNodeSet")
 		os.Exit(1)
 	}
@@ -162,7 +162,6 @@ func main() {
 		Client:  mgr.GetClient(),
 		Scheme:  mgr.GetScheme(),
 		Kclient: kclient,
-		Log:     ctrl.Log.WithName("controllers").WithName("OpenStackDataPlaneDeployment"),
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "OpenStackDataPlaneDeployment")
 		os.Exit(1)

diff --git a/tests/functional/suite_test.go b/tests/functional/suite_test.go
@@ -169,15 +169,13 @@ var _ = BeforeSuite(func() {
 		Client:  k8sManager.GetClient(),
 		Scheme:  k8sManager.GetScheme(),
 		Kclient: kclient,
-		Log:     ctrl.Log.WithName("controllers").WithName("DataplaneNodeSet"),
-	}).SetupWithManager(k8sManager)
+	}).SetupWithManager(context.Background(), k8sManager)
 	Expect(err).ToNot(HaveOccurred())
 
 	err = (&controllers.OpenStackDataPlaneDeploymentReconciler{
 		Client:  k8sManager.GetClient(),
 		Scheme:  k8sManager.GetScheme(),
 		Kclient: kclient,
-		Log:     ctrl.Log.WithName("controllers").WithName("DataplaneDeployment"),
 	}).SetupWithManager(k8sManager)
 	Expect(err).ToNot(HaveOccurred())