From bd1e167c162d4d9b0306b39a760915791e27f218 Mon Sep 17 00:00:00 2001 From: Yauheni Kaliuta Date: Thu, 19 Sep 2024 22:24:59 +0300 Subject: [PATCH] components: wrap logger into context Wrap the logger into Context like controller-runtime does [1][2]. Move former ConfigComponentLogger into DSC and rename it to configLoggerForComponent since it's a local function to prepare logger before it is passed to components. Components themself add their names. So, it moves the logic out of components into common place. It changes logging a bit since now component name is a separate level after "DSC.Components". Ammend parameters of ReconcileComponent methods, it does not take logger anymore. [1] https://github.com/kubernetes-sigs/controller-runtime/blob/38546806f2faf5973e3321a7bd5bb3afdbb5767d/pkg/internal/controller/controller.go#L297 [2] https://github.com/kubernetes-sigs/controller-runtime/blob/38546806f2faf5973e3321a7bd5bb3afdbb5767d/pkg/internal/controller/controller.go#L111 Credits-To: Bartosz Majsak Signed-off-by: Yauheni Kaliuta --- components/codeflare/codeflare.go | 4 +--- components/component.go | 18 ++++++------------ components/dashboard/dashboard.go | 10 +--------- .../datasciencepipelines.go | 4 +--- components/kserve/kserve.go | 5 ++--- components/kueue/kueue.go | 5 ++--- .../modelmeshserving/modelmeshserving.go | 4 +--- components/modelregistry/modelregistry.go | 5 ++--- components/ray/ray.go | 5 ++--- .../trainingoperator/trainingoperator.go | 5 ++--- components/trustyai/trustyai.go | 5 ++--- components/workbenches/workbenches.go | 5 ++--- .../datasciencecluster_controller.go | 13 ++++++++++++- 13 files changed, 36 insertions(+), 52 deletions(-) diff --git a/components/codeflare/codeflare.go b/components/codeflare/codeflare.go index ad3d15b168f..59c1f61456f 100644 --- a/components/codeflare/codeflare.go +++ b/components/codeflare/codeflare.go @@ -8,7 +8,6 @@ import ( "fmt" "path/filepath" - "github.com/go-logr/logr" operatorv1 "github.com/openshift/api/operator/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -74,12 +73,11 @@ func (c *CodeFlare) GetComponentName() string { func (c *CodeFlare) ReconcileComponent(ctx context.Context, cli client.Client, - logger logr.Logger, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error { - l := c.ConfigComponentLogger(logger, ComponentName, dscispec) + l := logf.FromContext(ctx).WithName(ComponentName) enabled := c.GetManagementState() == operatorv1.Managed monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed diff --git a/components/component.go b/components/component.go index b1e849c5b45..2caea1232da 100644 --- a/components/component.go +++ b/components/component.go @@ -15,7 +15,6 @@ import ( dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" - ctrlogger "github.com/opendatahub-io/opendatahub-operator/v2/pkg/logger" ) // Component struct defines the basis for each OpenDataHub component configuration. @@ -83,22 +82,17 @@ type ManifestsConfig struct { type ComponentInterface interface { Init(ctx context.Context, platform cluster.Platform) error - ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger, - owner metav1.Object, DSCISpec *dsciv1.DSCInitializationSpec, platform cluster.Platform, currentComponentStatus bool) error + ReconcileComponent(ctx context.Context, + cli client.Client, + owner metav1.Object, + DSCISpec *dsciv1.DSCInitializationSpec, + platform cluster.Platform, + currentComponentStatus bool) error Cleanup(ctx context.Context, cli client.Client, owner metav1.Object, DSCISpec *dsciv1.DSCInitializationSpec) error GetComponentName() string GetManagementState() operatorv1.ManagementState OverrideManifests(ctx context.Context, platform cluster.Platform) error UpdatePrometheusConfig(cli client.Client, logger logr.Logger, enable bool, component string) error - ConfigComponentLogger(logger logr.Logger, component string, dscispec *dsciv1.DSCInitializationSpec) logr.Logger -} - -// extend origal ConfigLoggers to include component name. -func (c *Component) ConfigComponentLogger(logger logr.Logger, component string, dscispec *dsciv1.DSCInitializationSpec) logr.Logger { - if dscispec.DevFlags != nil { - return ctrlogger.ConfigLoggers(dscispec.DevFlags.LogMode).WithName("DSC.Components." + component) - } - return logger.WithName("DSC.Components." + component) } // UpdatePrometheusConfig update prometheus-configs.yaml to include/exclude .rules diff --git a/components/dashboard/dashboard.go b/components/dashboard/dashboard.go index bf07c398c37..39b5fafbc12 100644 --- a/components/dashboard/dashboard.go +++ b/components/dashboard/dashboard.go @@ -96,20 +96,12 @@ func (d *Dashboard) GetComponentName() string { func (d *Dashboard) ReconcileComponent(ctx context.Context, cli client.Client, - logger logr.Logger, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, currentComponentExist bool, ) error { - var l logr.Logger - - if platform == cluster.SelfManagedRhods || platform == cluster.ManagedRhods { - l = d.ConfigComponentLogger(logger, ComponentNameDownstream, dscispec) - } else { - l = d.ConfigComponentLogger(logger, ComponentNameUpstream, dscispec) - } - + l := logf.FromContext(ctx).WithName(componentName(platform)) entryPath := DefaultPath enabled := d.GetManagementState() == operatorv1.Managed monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed diff --git a/components/datasciencepipelines/datasciencepipelines.go b/components/datasciencepipelines/datasciencepipelines.go index 3cece8a45d0..18494caedaa 100644 --- a/components/datasciencepipelines/datasciencepipelines.go +++ b/components/datasciencepipelines/datasciencepipelines.go @@ -8,7 +8,6 @@ import ( "fmt" "path/filepath" - "github.com/go-logr/logr" operatorv1 "github.com/openshift/api/operator/v1" conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" corev1 "k8s.io/api/core/v1" @@ -95,13 +94,12 @@ func (d *DataSciencePipelines) GetComponentName() string { func (d *DataSciencePipelines) ReconcileComponent(ctx context.Context, cli client.Client, - logger logr.Logger, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool, ) error { - l := d.ConfigComponentLogger(logger, ComponentName, dscispec) + l := logf.FromContext(ctx).WithName(ComponentName) enabled := d.GetManagementState() == operatorv1.Managed monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed diff --git a/components/kserve/kserve.go b/components/kserve/kserve.go index 53b44164b40..955f7909901 100644 --- a/components/kserve/kserve.go +++ b/components/kserve/kserve.go @@ -8,7 +8,6 @@ import ( "path/filepath" "strings" - "github.com/go-logr/logr" operatorv1 "github.com/openshift/api/operator/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -112,8 +111,8 @@ func (k *Kserve) GetComponentName() string { } func (k *Kserve) ReconcileComponent(ctx context.Context, cli client.Client, - logger logr.Logger, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error { - l := k.ConfigComponentLogger(logger, ComponentName, dscispec) + owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error { + l := logf.FromContext(ctx).WithName(ComponentName) enabled := k.GetManagementState() == operatorv1.Managed monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed diff --git a/components/kueue/kueue.go b/components/kueue/kueue.go index 2cc126220aa..528205e2759 100644 --- a/components/kueue/kueue.go +++ b/components/kueue/kueue.go @@ -6,7 +6,6 @@ import ( "fmt" "path/filepath" - "github.com/go-logr/logr" operatorv1 "github.com/openshift/api/operator/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -68,9 +67,9 @@ func (k *Kueue) GetComponentName() string { return ComponentName } -func (k *Kueue) ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger, +func (k *Kueue) ReconcileComponent(ctx context.Context, cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error { - l := k.ConfigComponentLogger(logger, ComponentName, dscispec) + l := logf.FromContext(ctx).WithName(ComponentName) enabled := k.GetManagementState() == operatorv1.Managed monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed diff --git a/components/modelmeshserving/modelmeshserving.go b/components/modelmeshserving/modelmeshserving.go index 583e11964a2..0594c00fd82 100644 --- a/components/modelmeshserving/modelmeshserving.go +++ b/components/modelmeshserving/modelmeshserving.go @@ -8,7 +8,6 @@ import ( "path/filepath" "strings" - "github.com/go-logr/logr" operatorv1 "github.com/openshift/api/operator/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -103,13 +102,12 @@ func (m *ModelMeshServing) GetComponentName() string { func (m *ModelMeshServing) ReconcileComponent(ctx context.Context, cli client.Client, - logger logr.Logger, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool, ) error { - l := m.ConfigComponentLogger(logger, ComponentName, dscispec) + l := logf.FromContext(ctx).WithName(ComponentName) enabled := m.GetManagementState() == operatorv1.Managed monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed diff --git a/components/modelregistry/modelregistry.go b/components/modelregistry/modelregistry.go index bdd4dc5ffce..5306c2e838a 100644 --- a/components/modelregistry/modelregistry.go +++ b/components/modelregistry/modelregistry.go @@ -10,7 +10,6 @@ import ( "strings" "text/template" - "github.com/go-logr/logr" operatorv1 "github.com/openshift/api/operator/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -97,9 +96,9 @@ func (m *ModelRegistry) GetComponentName() string { return ComponentName } -func (m *ModelRegistry) ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger, +func (m *ModelRegistry) ReconcileComponent(ctx context.Context, cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error { - l := m.ConfigComponentLogger(logger, ComponentName, dscispec) + l := logf.FromContext(ctx).WithName(ComponentName) enabled := m.GetManagementState() == operatorv1.Managed monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed diff --git a/components/ray/ray.go b/components/ray/ray.go index 654ebeb960f..1d08970eb68 100644 --- a/components/ray/ray.go +++ b/components/ray/ray.go @@ -8,7 +8,6 @@ import ( "fmt" "path/filepath" - "github.com/go-logr/logr" operatorv1 "github.com/openshift/api/operator/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -69,9 +68,9 @@ func (r *Ray) GetComponentName() string { return ComponentName } -func (r *Ray) ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger, +func (r *Ray) ReconcileComponent(ctx context.Context, cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error { - l := r.ConfigComponentLogger(logger, ComponentName, dscispec) + l := logf.FromContext(ctx).WithName(ComponentName) enabled := r.GetManagementState() == operatorv1.Managed monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed diff --git a/components/trainingoperator/trainingoperator.go b/components/trainingoperator/trainingoperator.go index a8623d89190..66ef414acc2 100644 --- a/components/trainingoperator/trainingoperator.go +++ b/components/trainingoperator/trainingoperator.go @@ -8,7 +8,6 @@ import ( "fmt" "path/filepath" - "github.com/go-logr/logr" operatorv1 "github.com/openshift/api/operator/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -70,9 +69,9 @@ func (r *TrainingOperator) GetComponentName() string { return ComponentName } -func (r *TrainingOperator) ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger, +func (r *TrainingOperator) ReconcileComponent(ctx context.Context, cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error { - l := r.ConfigComponentLogger(logger, ComponentName, dscispec) + l := logf.FromContext(ctx).WithName(ComponentName) enabled := r.GetManagementState() == operatorv1.Managed monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed diff --git a/components/trustyai/trustyai.go b/components/trustyai/trustyai.go index 28d10eb7cb4..bd32a133add 100644 --- a/components/trustyai/trustyai.go +++ b/components/trustyai/trustyai.go @@ -7,7 +7,6 @@ import ( "fmt" "path/filepath" - "github.com/go-logr/logr" operatorv1 "github.com/openshift/api/operator/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -79,9 +78,9 @@ func (t *TrustyAI) GetComponentName() string { return ComponentName } -func (t *TrustyAI) ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger, +func (t *TrustyAI) ReconcileComponent(ctx context.Context, cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error { - l := t.ConfigComponentLogger(logger, ComponentName, dscispec) + l := logf.FromContext(ctx).WithName(ComponentName) enabled := t.GetManagementState() == operatorv1.Managed monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed diff --git a/components/workbenches/workbenches.go b/components/workbenches/workbenches.go index 44898b8c81e..8a5e489b6a4 100644 --- a/components/workbenches/workbenches.go +++ b/components/workbenches/workbenches.go @@ -8,7 +8,6 @@ import ( "path/filepath" "strings" - "github.com/go-logr/logr" operatorv1 "github.com/openshift/api/operator/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -111,9 +110,9 @@ func (w *Workbenches) GetComponentName() string { return ComponentName } -func (w *Workbenches) ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger, +func (w *Workbenches) ReconcileComponent(ctx context.Context, cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error { - l := w.ConfigComponentLogger(logger, ComponentName, dscispec) + l := logf.FromContext(ctx).WithName(ComponentName) // Set default notebooks namespace // Create rhods-notebooks namespace in managed platforms diff --git a/controllers/datasciencecluster/datasciencecluster_controller.go b/controllers/datasciencecluster/datasciencecluster_controller.go index 6463853bd72..28a0ef57057 100644 --- a/controllers/datasciencecluster/datasciencecluster_controller.go +++ b/controllers/datasciencecluster/datasciencecluster_controller.go @@ -46,6 +46,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" @@ -56,6 +57,7 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/components/modelregistry" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" + ctrlogger "github.com/opendatahub-io/opendatahub-operator/v2/pkg/logger" annotations "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/labels" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/upgrade" @@ -80,6 +82,13 @@ const ( finalizerName = "datasciencecluster.opendatahub.io/finalizer" ) +func configLoggerForComponent(logger logr.Logger, dscispec *dsciv1.DSCInitializationSpec) logr.Logger { + if dscispec.DevFlags != nil { + logger = ctrlogger.ConfigLoggers(dscispec.DevFlags.LogMode) + } + return logger.WithName("DSC.Components") +} + // 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 @@ -314,7 +323,9 @@ func (r *DataScienceClusterReconciler) reconcileSubComponent(ctx context.Context r.Log.Error(err, "Failed to determine platform") return instance, err } - err = component.ReconcileComponent(ctx, r.Client, r.Log, instance, r.DataScienceCluster.DSCISpec, platform, installedComponentValue) + + ctxComponent := logf.IntoContext(ctx, configLoggerForComponent(r.Log, r.DataScienceCluster.DSCISpec)) + err = component.ReconcileComponent(ctxComponent, r.Client, instance, r.DataScienceCluster.DSCISpec, platform, installedComponentValue) // TODO: replace this hack with a full refactor of component status in the future