Skip to content

Commit

Permalink
controllers, monitoring: use local variable for logger
Browse files Browse the repository at this point in the history 
Prepare to switch to dynamic logger. It can be k8s contextual
logging [1] or some internal one. Copy reconciler's logger into
local variables.

Just to avoid polluting the other patch with the replacements.

[1] https://www.kubernetes.dev/blog/2022/05/25/contextual-logging/

Signed-off-by: Yauheni Kaliuta <[email protected]>
  • Loading branch information
@ykaliuta
ykaliuta committed Oct 2, 2024
1 parent 1a441e7 commit 7898344
Show file tree
Hide file tree
Showing 7 changed files with 149 additions and 124 deletions.
19 changes: 11 additions & 8 deletions controllers/certconfigmapgenerator/certconfigmapgenerator_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ type CertConfigmapGeneratorReconciler struct {

// SetupWithManager sets up the controller with the Manager.
func (r *CertConfigmapGeneratorReconciler) SetupWithManager(mgr ctrl.Manager) error {
r.Log.Info("Adding controller for Configmap Generation.")
log := r.Log
log.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)).
Expand All @@ -44,8 +45,9 @@ 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
// Request includes namespace that is newly created or where odh-trusted-ca-bundle configmap is updated.
r.Log.Info("Reconciling CertConfigMapGenerator.", " Request.Namespace", req.NamespacedName)
log.Info("Reconciling CertConfigMapGenerator.", " Request.Namespace", req.NamespacedName)
// Get namespace instance
userNamespace := &corev1.Namespace{}
if err := r.Client.Get(ctx, client.ObjectKey{Name: req.Namespace}, userNamespace); err != nil {
Expand All @@ -55,7 +57,7 @@ func (r *CertConfigmapGeneratorReconciler) Reconcile(ctx context.Context, req ct
// Get DSCI instance
dsciInstances := &dsciv1.DSCInitializationList{}
if err := r.Client.List(ctx, dsciInstances); err != nil {
r.Log.Error(err, "Failed to retrieve DSCInitialization resource for CertConfigMapGenerator ", "Request.Name", req.Name)
log.Error(err, "Failed to retrieve DSCInitialization resource for CertConfigMapGenerator ", "Request.Name", req.Name)
return ctrl.Result{}, err
}

Expand All @@ -73,10 +75,10 @@ func (r *CertConfigmapGeneratorReconciler) Reconcile(ctx context.Context, req ct

// Delete odh-trusted-ca-bundle Configmap if namespace has annotation set to opt-out CA bundle injection
if trustedcabundle.HasCABundleAnnotationDisabled(userNamespace) {
r.Log.Info("Namespace has opted-out of CA bundle injection using annotation", "namespace", userNamespace.Name,
log.Info("Namespace has opted-out of CA bundle injection using annotation", "namespace", userNamespace.Name,
"annotation", annotation.InjectionOfCABundleAnnotatoion)
if err := trustedcabundle.DeleteOdhTrustedCABundleConfigMap(ctx, r.Client, req.Namespace); client.IgnoreNotFound(err) != nil {
r.Log.Error(err, "error deleting existing configmap from namespace", "name", trustedcabundle.CAConfigMapName, "namespace", userNamespace.Name)
log.Error(err, "error deleting existing configmap from namespace", "name", trustedcabundle.CAConfigMapName, "namespace", userNamespace.Name)
return reconcile.Result{}, err
}

Expand All @@ -85,11 +87,11 @@ func (r *CertConfigmapGeneratorReconciler) Reconcile(ctx context.Context, req ct

// Add odh-trusted-ca-bundle Configmap
if trustedcabundle.ShouldInjectTrustedBundle(userNamespace) {
r.Log.Info("Adding trusted CA bundle configmap to the new or existing namespace ", "namespace", userNamespace.Name,
log.Info("Adding trusted CA bundle configmap to the new or existing namespace ", "namespace", userNamespace.Name,
"configmap", trustedcabundle.CAConfigMapName)
trustCAData := dsciInstance.Spec.TrustedCABundle.CustomCABundle
if err := trustedcabundle.CreateOdhTrustedCABundleConfigMap(ctx, r.Client, req.Namespace, trustCAData); err != nil {
r.Log.Error(err, "error adding configmap to namespace", "name", trustedcabundle.CAConfigMapName, "namespace", userNamespace.Name)
log.Error(err, "error adding configmap to namespace", "name", trustedcabundle.CAConfigMapName, "namespace", userNamespace.Name)
return reconcile.Result{}, err
}
}
Expand All @@ -108,8 +110,9 @@ func (r *CertConfigmapGeneratorReconciler) watchNamespaceResource(_ context.Cont
}

func (r *CertConfigmapGeneratorReconciler) watchTrustedCABundleConfigMapResource(_ context.Context, a client.Object) []reconcile.Request {
log := r.Log
if a.GetName() == trustedcabundle.CAConfigMapName {
r.Log.Info("Cert configmap has been updated, start reconcile")
log.Info("Cert configmap has been updated, start reconcile")
return []reconcile.Request{{NamespacedName: types.NamespacedName{Name: a.GetName(), Namespace: a.GetNamespace()}}}
}
return nil
Expand Down
31 changes: 17 additions & 14 deletions controllers/datasciencecluster/datasciencecluster_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,13 @@ 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
r.Log.Info("Reconciling DataScienceCluster resources", "Request.Name", req.Name)
log := r.Log
log.Info("Reconciling DataScienceCluster resources", "Request.Name", req.Name)

// Get information on version and platform
currentOperatorRelease, err := cluster.GetRelease(ctx, r.Client)
if err != nil {
r.Log.Error(err, "failed to get operator release version")
log.Error(err, "failed to get operator release version")
return ctrl.Result{}, err
}
// Set platform
Expand Down Expand Up @@ -129,10 +130,10 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R
if controllerutil.ContainsFinalizer(instance, finalizerName) {
if controllerutil.RemoveFinalizer(instance, finalizerName) {
if err := r.Update(ctx, instance); err != nil {
r.Log.Info("Error to remove DSC finalizer", "error", err)
log.Info("Error to remove DSC finalizer", "error", err)
return ctrl.Result{}, err
}
r.Log.Info("Removed finalizer for DataScienceCluster", "name", instance.Name, "finalizer", finalizerName)
log.Info("Removed finalizer for DataScienceCluster", "name", instance.Name, "finalizer", finalizerName)
}
}
if err := r.Client.Delete(ctx, instance, []client.DeleteOption{}...); err != nil {
Expand All @@ -152,7 +153,7 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R
dsciInstances := &dsciv1.DSCInitializationList{}
err = r.Client.List(ctx, dsciInstances)
if err != nil {
r.Log.Error(err, "Failed to retrieve DSCInitialization resource.", "DSCInitialization Request.Name", req.Name)
log.Error(err, "Failed to retrieve DSCInitialization resource.", "DSCInitialization Request.Name", req.Name)
r.Recorder.Eventf(instance, corev1.EventTypeWarning, "DSCInitializationReconcileError", "Failed to retrieve DSCInitialization instance")

return ctrl.Result{}, err
Expand All @@ -163,7 +164,7 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R
case 0:
reason := status.ReconcileFailed
message := "Failed to get a valid DSCInitialization instance, please create a DSCI instance"
r.Log.Info(message)
log.Info(message)
instance, err = status.UpdateWithRetry(ctx, r.Client, instance, func(saved *dscv1.DataScienceCluster) {
status.SetProgressingCondition(&saved.Status.Conditions, reason, message)
// Patch Degraded with True status
Expand All @@ -183,14 +184,14 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R

if instance.ObjectMeta.DeletionTimestamp.IsZero() {
if !controllerutil.ContainsFinalizer(instance, finalizerName) {
r.Log.Info("Adding finalizer for DataScienceCluster", "name", instance.Name, "finalizer", finalizerName)
log.Info("Adding finalizer for DataScienceCluster", "name", instance.Name, "finalizer", finalizerName)
controllerutil.AddFinalizer(instance, finalizerName)
if err := r.Update(ctx, instance); err != nil {
return ctrl.Result{}, err
}
}
} else {
r.Log.Info("Finalization DataScienceCluster start deleting instance", "name", instance.Name, "finalizer", finalizerName)
log.Info("Finalization DataScienceCluster start deleting instance", "name", instance.Name, "finalizer", finalizerName)
for _, component := range allComponents {
if err := component.Cleanup(ctx, r.Client, instance, r.DataScienceCluster.DSCISpec); err != nil {
return ctrl.Result{}, err
Expand Down Expand Up @@ -252,14 +253,14 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R

// Process errors for components
if componentErrors != nil {
r.Log.Info("DataScienceCluster Deployment Incomplete.")
log.Info("DataScienceCluster Deployment Incomplete.")
instance, err = status.UpdateWithRetry(ctx, r.Client, instance, func(saved *dscv1.DataScienceCluster) {
status.SetCompleteCondition(&saved.Status.Conditions, status.ReconcileCompletedWithComponentErrors,
fmt.Sprintf("DataScienceCluster resource reconciled with component errors: %v", componentErrors))
saved.Status.Phase = status.PhaseReady
})
if err != nil {
r.Log.Error(err, "failed to update DataScienceCluster conditions with incompleted reconciliation")
log.Error(err, "failed to update DataScienceCluster conditions with incompleted reconciliation")

return ctrl.Result{}, err
}
Expand All @@ -276,12 +277,12 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R
})

if err != nil {
r.Log.Error(err, "failed to update DataScienceCluster conditions after successfully completed reconciliation")
log.Error(err, "failed to update DataScienceCluster conditions after successfully completed reconciliation")

return ctrl.Result{}, err
}

r.Log.Info("DataScienceCluster Deployment Completed.")
log.Info("DataScienceCluster Deployment Completed.")
r.Recorder.Eventf(instance, corev1.EventTypeNormal, "DataScienceClusterCreationSuccessful",
"DataScienceCluster instance %s created and deployed successfully", instance.Name)

Expand All @@ -291,6 +292,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
componentName := component.GetComponentName()

enabled := component.GetManagementState() == operatorv1.Managed
Expand All @@ -312,7 +314,7 @@ func (r *DataScienceClusterReconciler) reconcileSubComponent(ctx context.Context
}
}
// Reconcile component
componentLogger := newComponentLogger(r.Log, componentName, r.DataScienceCluster.DSCISpec)
componentLogger := newComponentLogger(log, componentName, r.DataScienceCluster.DSCISpec)
err := component.ReconcileComponent(ctx, r.Client, componentLogger, instance, r.DataScienceCluster.DSCISpec, platform, installedComponentValue)

// TODO: replace this hack with a full refactor of component status in the future
Expand Down Expand Up @@ -373,7 +375,8 @@ func newComponentLogger(logger logr.Logger, componentName string, dscispec *dsci
}

func (r *DataScienceClusterReconciler) reportError(err error, instance *dscv1.DataScienceCluster, message string) *dscv1.DataScienceCluster {
r.Log.Error(err, message, "instance.Name", instance.Name)
log := r.Log
log.Error(err, message, "instance.Name", instance.Name)
r.Recorder.Eventf(instance, corev1.EventTypeWarning, "DataScienceClusterReconcileError",
"%s for instance %s", message, instance.Name)
return instance
Expand Down
Loading

0 comments on commit 7898344

Please sign in to comment.