controllers: switch to k8s contextual logger

Remove own logger from controllers' reconcilers and switch to k8s contextual logger instead [1]. Use contextual logger for SecretGeneratorReconciler and CertConfigmapGeneratorReconciler setups as well. [1] https://www.kubernetes.dev/blog/2022/05/25/contextual-logging/ Signed-off-by: Yauheni Kaliuta <[email protected]>
opendatahub-io · Oct 9, 2024 · 5cfa13b · 5cfa13b
1 parent 83d9e37
commit 5cfa13b
Show file tree

Hide file tree

Showing 9 changed files with 44 additions and 53 deletions.
diff --git a/controllers/certconfigmapgenerator/certconfigmapgenerator_controller.go b/controllers/certconfigmapgenerator/certconfigmapgenerator_controller.go
@@ -5,7 +5,6 @@ import (
 	"context"
 	"reflect"
 
-	"github.com/go-logr/logr"
 	operatorv1 "github.com/openshift/api/operator/v1"
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
@@ -16,6 +15,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
@@ -28,13 +28,11 @@ import (
 type CertConfigmapGeneratorReconciler struct {
 	Client client.Client
 	Scheme *runtime.Scheme
-	Log    logr.Logger
 }
 
 // SetupWithManager sets up the controller with the Manager.
-func (r *CertConfigmapGeneratorReconciler) SetupWithManager(mgr ctrl.Manager) error {
-	log := r.Log
-	log.Info("Adding controller for Configmap Generation.")
+func (r *CertConfigmapGeneratorReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error {
+	logf.FromContext(ctx).Info("Adding controller for Configmap Generation.")
 	return ctrl.NewControllerManagedBy(mgr).
 		Named("cert-configmap-generator-controller").
 		Watches(&corev1.ConfigMap{}, handler.EnqueueRequestsFromMapFunc(r.watchTrustedCABundleConfigMapResource), builder.WithPredicates(ConfigMapChangedPredicate)).
@@ -45,7 +43,7 @@ func (r *CertConfigmapGeneratorReconciler) SetupWithManager(mgr ctrl.Manager) er
 // Reconcile will generate new configmap, odh-trusted-ca-bundle, that includes cluster-wide trusted-ca bundle and custom
 // ca bundle in every new namespace created.
 func (r *CertConfigmapGeneratorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
-	log := r.Log
+	log := logf.FromContext(ctx)
 	// Request includes namespace that is newly created or where odh-trusted-ca-bundle configmap is updated.
 	log.Info("Reconciling CertConfigMapGenerator.", " Request.Namespace", req.NamespacedName)
 	// Get namespace instance
@@ -109,8 +107,8 @@ func (r *CertConfigmapGeneratorReconciler) watchNamespaceResource(_ context.Cont
 	return nil
 }
 
-func (r *CertConfigmapGeneratorReconciler) watchTrustedCABundleConfigMapResource(_ context.Context, a client.Object) []reconcile.Request {
-	log := r.Log
+func (r *CertConfigmapGeneratorReconciler) watchTrustedCABundleConfigMapResource(ctx context.Context, a client.Object) []reconcile.Request {
+	log := logf.FromContext(ctx)
 	if a.GetName() == trustedcabundle.CAConfigMapName {
 		log.Info("Cert configmap has been updated, start reconcile")
 		return []reconcile.Request{{NamespacedName: types.NamespacedName{Name: a.GetName(), Namespace: a.GetNamespace()}}}

diff --git a/controllers/datasciencecluster/datasciencecluster_controller.go b/controllers/datasciencecluster/datasciencecluster_controller.go
@@ -66,7 +66,6 @@ import (
 type DataScienceClusterReconciler struct {
 	client.Client
 	Scheme *runtime.Scheme
-	Log    logr.Logger
 	// Recorder to generate events
 	Recorder           record.EventRecorder
 	DataScienceCluster *DataScienceClusterConfig
@@ -84,7 +83,7 @@ const (
 // 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 *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { //nolint:maintidx,gocyclo
-	log := r.Log
+	log := logf.FromContext(ctx)
 	log.Info("Reconciling DataScienceCluster resources", "Request.Name", req.Name)
 
 	// Get information on version and platform
@@ -168,7 +167,7 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R
 			saved.Status.Phase = status.PhaseError
 		})
 		if err != nil {
-			r.reportError(err, instance, "failed to update DataScienceCluster condition")
+			r.reportError(ctx, err, instance, "failed to update DataScienceCluster condition")
 
 			return ctrl.Result{}, err
 		}
@@ -232,7 +231,7 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R
 			saved.Status.Release = currentOperatorRelease
 		})
 		if err != nil {
-			_ = r.reportError(err, instance, fmt.Sprintf("failed to add conditions to status of DataScienceCluster resource name %s", req.Name))
+			_ = r.reportError(ctx, err, instance, fmt.Sprintf("failed to add conditions to status of DataScienceCluster resource name %s", req.Name))
 
 			return ctrl.Result{}, err
 		}
@@ -288,7 +287,7 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R
 func (r *DataScienceClusterReconciler) reconcileSubComponent(ctx context.Context, instance *dscv1.DataScienceCluster,
 	platform cluster.Platform, component components.ComponentInterface,
 ) (*dscv1.DataScienceCluster, error) {
-	log := r.Log
+	log := logf.FromContext(ctx)
 	componentName := component.GetComponentName()
 
 	enabled := component.GetManagementState() == operatorv1.Managed
@@ -305,7 +304,7 @@ func (r *DataScienceClusterReconciler) reconcileSubComponent(ctx context.Context
 			status.SetComponentCondition(&saved.Status.Conditions, componentName, status.ReconcileInit, message, corev1.ConditionUnknown)
 		})
 		if err != nil {
-			_ = r.reportError(err, instance, "failed to update DataScienceCluster conditions before first time reconciling "+componentName)
+			_ = r.reportError(ctx, err, instance, "failed to update DataScienceCluster conditions before first time reconciling "+componentName)
 			// try to continue with reconciliation, as further updates can fix the status
 		}
 	}
@@ -318,7 +317,7 @@ func (r *DataScienceClusterReconciler) reconcileSubComponent(ctx context.Context
 
 	if err != nil {
 		// reconciliation failed: log errors, raise event and update status accordingly
-		instance = r.reportError(err, instance, "failed to reconcile "+componentName+" on DataScienceCluster")
+		instance = r.reportError(ctx, err, instance, "failed to reconcile "+componentName+" on DataScienceCluster")
 		instance, _ = status.UpdateWithRetry(ctx, r.Client, instance, func(saved *dscv1.DataScienceCluster) {
 			if enabled {
 				if strings.Contains(err.Error(), datasciencepipelines.ArgoWorkflowCRD+" CRD already exists") {
@@ -354,7 +353,7 @@ func (r *DataScienceClusterReconciler) reconcileSubComponent(ctx context.Context
 		}
 	})
 	if err != nil {
-		instance = r.reportError(err, instance, "failed to update DataScienceCluster status after reconciling "+componentName)
+		instance = r.reportError(ctx, err, instance, "failed to update DataScienceCluster status after reconciling "+componentName)
 
 		return instance, err
 	}
@@ -367,9 +366,8 @@ func newComponentLogger(logger logr.Logger, componentName string) logr.Logger {
 	return logger.WithName("DSC.Components."+componentName).WithValues("component", componentName)
 }
 
-func (r *DataScienceClusterReconciler) reportError(err error, instance *dscv1.DataScienceCluster, message string) *dscv1.DataScienceCluster {
-	log := r.Log
-	log.Error(err, message, "instance.Name", instance.Name)
+func (r *DataScienceClusterReconciler) reportError(ctx context.Context, err error, instance *dscv1.DataScienceCluster, message string) *dscv1.DataScienceCluster {
+	logf.FromContext(ctx).Error(err, message, "instance.Name", instance.Name)
 	r.Recorder.Eventf(instance, corev1.EventTypeWarning, "DataScienceClusterReconcileError",
 		"%s for instance %s", message, instance.Name)
 	return instance

diff --git a/controllers/dscinitialization/dscinitialization_controller.go b/controllers/dscinitialization/dscinitialization_controller.go
@@ -22,7 +22,6 @@ import (
 	"path/filepath"
 	"reflect"
 
-	"github.com/go-logr/logr"
 	operatorv1 "github.com/openshift/api/operator/v1"
 	routev1 "github.com/openshift/api/route/v1"
 	appsv1 "k8s.io/api/apps/v1"
@@ -39,6 +38,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
@@ -64,7 +64,6 @@ var managementStateChangeTrustedCA = false
 type DSCInitializationReconciler struct {
 	client.Client
 	Scheme                *runtime.Scheme
-	Log                   logr.Logger
 	Recorder              record.EventRecorder
 	ApplicationsNamespace string
 }
@@ -77,7 +76,7 @@ type DSCInitializationReconciler struct {
 
 // Reconcile contains controller logic specific to DSCInitialization instance updates.
 func (r *DSCInitializationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { //nolint:funlen,gocyclo,maintidx
-	log := r.Log
+	log := logf.FromContext(ctx)
 	log.Info("Reconciling DSCInitialization.", "DSCInitialization Request.Name", req.Name)
 
 	currentOperatorRelease := cluster.GetRelease()
@@ -371,8 +370,8 @@ var dsciPredicateStateChangeTrustedCA = predicate.Funcs{
 	},
 }
 
-func (r *DSCInitializationReconciler) watchMonitoringConfigMapResource(_ context.Context, a client.Object) []reconcile.Request {
-	log := r.Log
+func (r *DSCInitializationReconciler) watchMonitoringConfigMapResource(ctx context.Context, a client.Object) []reconcile.Request {
+	log := logf.FromContext(ctx)
 	if a.GetName() == "prometheus" && a.GetNamespace() == "redhat-ods-monitoring" {
 		log.Info("Found monitoring configmap has updated, start reconcile")
 
@@ -381,8 +380,8 @@ func (r *DSCInitializationReconciler) watchMonitoringConfigMapResource(_ context
 	return nil
 }
 
-func (r *DSCInitializationReconciler) watchMonitoringSecretResource(_ context.Context, a client.Object) []reconcile.Request {
-	log := r.Log
+func (r *DSCInitializationReconciler) watchMonitoringSecretResource(ctx context.Context, a client.Object) []reconcile.Request {
+	log := logf.FromContext(ctx)
 	operatorNs, err := cluster.GetOperatorNamespace()
 	if err != nil {
 		return nil
@@ -397,7 +396,7 @@ func (r *DSCInitializationReconciler) watchMonitoringSecretResource(_ context.Co
 }
 
 func (r *DSCInitializationReconciler) watchDSCResource(ctx context.Context) []reconcile.Request {
-	log := r.Log
+	log := logf.FromContext(ctx)
 	instanceList := &dscv1.DataScienceClusterList{}
 	if err := r.Client.List(ctx, instanceList); err != nil {
 		// do not handle if cannot get list

diff --git a/controllers/dscinitialization/monitoring.go b/controllers/dscinitialization/monitoring.go
@@ -15,6 +15,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
 	dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
@@ -39,7 +40,7 @@ var (
 // only when reconcile on DSCI CR, initial set to true
 // if reconcile from monitoring, initial set to false, skip blackbox and rolebinding.
 func (r *DSCInitializationReconciler) configureManagedMonitoring(ctx context.Context, dscInit *dsciv1.DSCInitialization, initial string) error {
-	log := r.Log
+	log := logf.FromContext(ctx)
 	if initial == "init" {
 		// configure Blackbox exporter
 		if err := configureBlackboxExporter(ctx, dscInit, r); err != nil {
@@ -89,7 +90,7 @@ func (r *DSCInitializationReconciler) configureManagedMonitoring(ctx context.Con
 }
 
 func configureAlertManager(ctx context.Context, dsciInit *dsciv1.DSCInitialization, r *DSCInitializationReconciler) error {
-	log := r.Log
+	log := logf.FromContext(ctx)
 	// Get Deadmansnitch secret
 	deadmansnitchSecret, err := r.waitForManagedSecret(ctx, "redhat-rhods-deadmanssnitch", dsciInit.Spec.Monitoring.Namespace)
 	if err != nil {
@@ -200,7 +201,7 @@ func configureAlertManager(ctx context.Context, dsciInit *dsciv1.DSCInitializati
 }
 
 func configurePrometheus(ctx context.Context, dsciInit *dsciv1.DSCInitialization, r *DSCInitializationReconciler) error {
-	log := r.Log
+	log := logf.FromContext(ctx)
 	// Update rolebinding-viewer
 	err := common.ReplaceStringsInFile(filepath.Join(prometheusManifestsPath, "prometheus-rolebinding-viewer.yaml"),
 		map[string]string{
@@ -348,7 +349,7 @@ func configurePrometheus(ctx context.Context, dsciInit *dsciv1.DSCInitialization
 }
 
 func configureBlackboxExporter(ctx context.Context, dsciInit *dsciv1.DSCInitialization, r *DSCInitializationReconciler) error {
-	log := r.Log
+	log := logf.FromContext(ctx)
 	consoleRoute := &routev1.Route{}
 	err := r.Client.Get(ctx, client.ObjectKey{Name: "console", Namespace: "openshift-console"}, consoleRoute)
 	if err != nil {
@@ -438,7 +439,7 @@ func createMonitoringProxySecret(ctx context.Context, cli client.Client, name st
 }
 
 func (r *DSCInitializationReconciler) configureSegmentIO(ctx context.Context, dsciInit *dsciv1.DSCInitialization) error {
-	log := r.Log
+	log := logf.FromContext(ctx)
 	// create segment.io only when configmap does not exist in the cluster
 	segmentioConfigMap := &corev1.ConfigMap{}
 	if err := r.Client.Get(ctx, client.ObjectKey{
@@ -467,7 +468,7 @@ func (r *DSCInitializationReconciler) configureSegmentIO(ctx context.Context, ds
 }
 
 func (r *DSCInitializationReconciler) configureCommonMonitoring(ctx context.Context, dsciInit *dsciv1.DSCInitialization) error {
-	log := r.Log
+	log := logf.FromContext(ctx)
 	if err := r.configureSegmentIO(ctx, dsciInit); err != nil {
 		return err
 	}

diff --git a/controllers/dscinitialization/servicemesh_setup.go b/controllers/dscinitialization/servicemesh_setup.go
@@ -9,6 +9,7 @@ import (
 	conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1"
 	corev1 "k8s.io/api/core/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
 	dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
 	"github.com/opendatahub-io/opendatahub-operator/v2/controllers/status"
@@ -19,7 +20,7 @@ import (
 )
 
 func (r *DSCInitializationReconciler) configureServiceMesh(ctx context.Context, instance *dsciv1.DSCInitialization) error {
-	log := r.Log
+	log := logf.FromContext(ctx)
 	serviceMeshManagementState := operatorv1.Removed
 	if instance.Spec.ServiceMesh != nil {
 		serviceMeshManagementState = instance.Spec.ServiceMesh.ManagementState
@@ -62,7 +63,7 @@ func (r *DSCInitializationReconciler) configureServiceMesh(ctx context.Context,
 }
 
 func (r *DSCInitializationReconciler) removeServiceMesh(ctx context.Context, instance *dsciv1.DSCInitialization) error {
-	log := r.Log
+	log := logf.FromContext(ctx)
 	// on condition of Managed, do not handle Removed when set to Removed it trigger DSCI reconcile to clean up
 	if instance.Spec.ServiceMesh == nil {
 		return nil

diff --git a/controllers/dscinitialization/suite_test.go b/controllers/dscinitialization/suite_test.go
@@ -137,7 +137,6 @@ var _ = BeforeSuite(func() {
 	err = (&dscictrl.DSCInitializationReconciler{
 		Client:   k8sClient,
 		Scheme:   testScheme,
-		Log:      ctrl.Log.WithName("controllers").WithName("DSCInitialization"),
 		Recorder: mgr.GetEventRecorderFor("dscinitialization-controller"),
 	}).SetupWithManager(gCtx, mgr)
 

diff --git a/controllers/dscinitialization/utils.go b/controllers/dscinitialization/utils.go
@@ -18,6 +18,7 @@ import (
 	"k8s.io/client-go/util/retry"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
 	dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
@@ -37,7 +38,7 @@ var (
 // - Network Policies 'opendatahub' that allow traffic between the ODH namespaces
 // - RoleBinding 'opendatahub'.
 func (r *DSCInitializationReconciler) createOdhNamespace(ctx context.Context, dscInit *dsciv1.DSCInitialization, name string, platform cluster.Platform) error {
-	log := r.Log
+	log := logf.FromContext(ctx)
 	// Expected application namespace for the given name
 	desiredNamespace := &corev1.Namespace{
 		ObjectMeta: metav1.ObjectMeta{
@@ -142,7 +143,7 @@ func (r *DSCInitializationReconciler) createOdhNamespace(ctx context.Context, ds
 }
 
 func (r *DSCInitializationReconciler) createDefaultRoleBinding(ctx context.Context, name string, dscInit *dsciv1.DSCInitialization) error {
-	log := r.Log
+	log := logf.FromContext(ctx)
 	// Expected namespace for the given name
 	desiredRoleBinding := &rbacv1.RoleBinding{
 		TypeMeta: metav1.TypeMeta{
@@ -193,7 +194,7 @@ func (r *DSCInitializationReconciler) createDefaultRoleBinding(ctx context.Conte
 }
 
 func (r *DSCInitializationReconciler) reconcileDefaultNetworkPolicy(ctx context.Context, name string, dscInit *dsciv1.DSCInitialization, platform cluster.Platform) error {
-	log := r.Log
+	log := logf.FromContext(ctx)
 	if platform == cluster.ManagedRhods || platform == cluster.SelfManagedRhods {
 		// Deploy networkpolicy for operator namespace
 		err := deploy.DeployManifestsFromPath(ctx, r.Client, dscInit, networkpolicyPath+"/operator", "redhat-ods-operator", "networkpolicy", true)
@@ -375,7 +376,7 @@ func GenerateRandomHex(length int) ([]byte, error) {
 }
 
 func (r *DSCInitializationReconciler) createOdhCommonConfigMap(ctx context.Context, name string, dscInit *dsciv1.DSCInitialization) error {
-	log := r.Log
+	log := logf.FromContext(ctx)
 	// Expected configmap for the given namespace
 	desiredConfigMap := &corev1.ConfigMap{
 		TypeMeta: metav1.TypeMeta{

diff --git a/controllers/secretgenerator/secretgenerator_controller.go b/controllers/secretgenerator/secretgenerator_controller.go
@@ -23,7 +23,6 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/go-logr/logr"
 	oauthv1 "github.com/openshift/api/oauth/v1"
 	routev1 "github.com/openshift/api/route/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -37,6 +36,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
@@ -52,13 +52,11 @@ const (
 type SecretGeneratorReconciler struct {
 	Client client.Client
 	Scheme *runtime.Scheme
-	Log    logr.Logger
 }
 
 // SetupWithManager sets up the controller with the Manager.
-func (r *SecretGeneratorReconciler) SetupWithManager(mgr ctrl.Manager) error {
-	log := r.Log
-	log.Info("Adding controller for Secret Generation.")
+func (r *SecretGeneratorReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error {
+	logf.FromContext(ctx).Info("Adding controller for Secret Generation.")
 
 	// Watch only new secrets with the corresponding annotation
 	predicates := predicate.Funcs{
@@ -106,7 +104,7 @@ func (r *SecretGeneratorReconciler) SetupWithManager(mgr ctrl.Manager) error {
 // based on the specified type and complexity. This will avoid possible race
 // conditions when a deployment mounts the secret before it is reconciled.
 func (r *SecretGeneratorReconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) {
-	log := r.Log
+	log := logf.FromContext(ctx)
 	foundSecret := &corev1.Secret{}
 	err := r.Client.Get(ctx, request.NamespacedName, foundSecret)
 	if err != nil {
@@ -209,7 +207,7 @@ func (r *SecretGeneratorReconciler) getRoute(ctx context.Context, name string, n
 }
 
 func (r *SecretGeneratorReconciler) createOAuthClient(ctx context.Context, name string, secretName string, uri string) error {
-	log := r.Log
+	log := logf.FromContext(ctx)
 	// Create OAuthClient resource
 	oauthClient := &oauthv1.OAuthClient{
 		TypeMeta: metav1.TypeMeta{