Skip to content

Commit

Permalink
Add slog structured logging to main controllers
Browse files Browse the repository at this point in the history
    Integrated slog logging into the MirrorPeerSecretReconciler, MirrorPeerReconciler, and DRPolicyReconciler controllers.
    Improved error handling and logging for better observability and debugging.
    Replaced klog with slog for consistency across the codebase.

Changes include:

    Added Logger field to controller structs to pass and use slog.Logger.
    Enhanced logging messages to provide more context, including function names, object names, namespaces, and error details.

Signed-off-by: vbadrina <[email protected]>
  • Loading branch information
vbnrh committed Jun 21, 2024
1 parent d9a23ce commit 15a271e
Show file tree
Hide file tree
Showing 30 changed files with 863 additions and 543 deletions.
5 changes: 5 additions & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ run:
timeout: 5m
modules-download-mode: readonly


linters:
enable:
- sloglint

linters-settings:
errcheck:
# report about not checking of errors in type assertions: `a := b.(MyStruct)`;
Expand Down
224 changes: 143 additions & 81 deletions addons/agent_mirrorpeer_controller.go

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions addons/agent_mirrorpeer_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ func TestMirrorPeerReconcile(t *testing.T) {
SpokeClient: fakeSpokeClient,
Scheme: scheme,
SpokeClusterName: pr.ClusterName,
Logger: utils.GetLogger(true),
}

req := ctrl.Request{
Expand Down Expand Up @@ -209,6 +210,7 @@ func TestDisableMirroring(t *testing.T) {
SpokeClient: fakeSpokeClient,
Scheme: scheme,
SpokeClusterName: pr.ClusterName,
Logger: utils.GetLogger(true),
}
if err := r.disableMirroring(ctx, pr.StorageClusterRef.Name, pr.StorageClusterRef.Namespace, &mirrorpeer1); err != nil {
t.Error("failed to disable mirroring", err)
Expand Down Expand Up @@ -258,6 +260,7 @@ func TestDeleteGreenSecret(t *testing.T) {
SpokeClient: fakeSpokeClient,
Scheme: scheme,
SpokeClusterName: pr.ClusterName,
Logger: utils.GetLogger(true),
}

if err := r.deleteGreenSecret(ctx, pr.ClusterName, pr.StorageClusterRef.Namespace, &mirrorpeer1); err != nil {
Expand Down Expand Up @@ -297,6 +300,7 @@ func TestDeleteS3(t *testing.T) {
SpokeClient: fakeSpokeClient,
Scheme: scheme,
SpokeClusterName: pr.ClusterName,
Logger: utils.GetLogger(true),
}
if err := r.deleteS3(ctx, mirrorpeer1, pr.StorageClusterRef.Namespace); err != nil {
t.Errorf("failed to delete s3 bucket")
Expand Down
15 changes: 11 additions & 4 deletions addons/blue_secret_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package addons

import (
"context"
"log/slog"

ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
Expand All @@ -13,7 +14,6 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/klog/v2"
)

// BlueSecretReconciler reconciles a MirrorPeer object
Expand All @@ -22,6 +22,7 @@ type BlueSecretReconciler struct {
HubClient client.Client
SpokeClient client.Client
SpokeClusterName string
Logger *slog.Logger
}

// SetupWithManager sets up the controller with the Manager.
Expand All @@ -45,6 +46,8 @@ func (r *BlueSecretReconciler) SetupWithManager(mgr ctrl.Manager) error {
},
}

r.Logger.Info("Setting up controller with manager")

return ctrl.NewControllerManagedBy(mgr).
Named("bluesecret_controller").
Watches(&corev1.Secret{}, &handler.EnqueueRequestForObject{},
Expand All @@ -55,22 +58,26 @@ func (r *BlueSecretReconciler) SetupWithManager(mgr ctrl.Manager) error {
func (r *BlueSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
var err error
var secret corev1.Secret
logger := r.Logger.With("secret", req.NamespacedName.String())

klog.Info("Reconciling blue secret", "secret", req.NamespacedName.String())
logger.Info("Starting reconciliation for BlueSecret")
err = r.SpokeClient.Get(ctx, req.NamespacedName, &secret)
if err != nil {
if errors.IsNotFound(err) {
klog.Infof("Could not find secret. Ignoring since it must have been deleted")
logger.Info("BlueSecret not found, possibly deleted")
return ctrl.Result{}, nil
}
klog.Error("Failed to get secret.", err)
logger.Error("Failed to retrieve BlueSecret", "error", err)
return ctrl.Result{}, err
}

logger.Info("Successfully retrieved BlueSecret")
err = r.syncBlueSecretForRook(ctx, secret)
if err != nil {
logger.Error("Failed to synchronize BlueSecret", "error", err)
return ctrl.Result{}, err
}

logger.Info("Reconciliation complete for BlueSecret")
return ctrl.Result{}, nil
}
1 change: 0 additions & 1 deletion addons/blue_secret_controller_test.go

This file was deleted.

17 changes: 12 additions & 5 deletions addons/green_secret_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ package addons
import (
"context"
"fmt"
"log/slog"

"github.com/red-hat-storage/odf-multicluster-orchestrator/controllers/utils"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/klog/v2"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand All @@ -24,6 +24,7 @@ type GreenSecretReconciler struct {
HubClient client.Client
SpokeClient client.Client
SpokeClusterName string
Logger *slog.Logger
}

// SetupWithManager sets up the controller with the Manager.
Expand Down Expand Up @@ -51,6 +52,8 @@ func (r *GreenSecretReconciler) SetupWithManager(mgr ctrl.Manager) error {
},
}

r.Logger.Info("Setting up controller with manager")

return ctrl.NewControllerManagedBy(mgr).
Named("greensecret_controller").
Watches(&corev1.Secret{}, &handler.EnqueueRequestForObject{},
Expand All @@ -61,26 +64,30 @@ func (r *GreenSecretReconciler) SetupWithManager(mgr ctrl.Manager) error {
func (r *GreenSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
var err error
var greenSecret corev1.Secret
logger := r.Logger.With("secret", req.NamespacedName.String())

klog.Info("Reconciling green secret", "secret", req.NamespacedName.String())
logger.Info("Reconciling green secret")
err = r.HubClient.Get(ctx, req.NamespacedName, &greenSecret)
if err != nil {
if errors.IsNotFound(err) {
klog.Infof("Could not find secret. Ignoring since it must have been deleted")
logger.Info("Green secret not found, likely deleted")
return ctrl.Result{}, nil
}
klog.Error("Failed to get secret.", err)
logger.Error("Failed to retrieve green secret", "error", err)
return ctrl.Result{}, err
}

if err = validateGreenSecret(greenSecret); err != nil {
return ctrl.Result{}, fmt.Errorf("failed to validate secret %q", greenSecret.Name)
logger.Error("Validation failed for green secret", "error", err)
return ctrl.Result{}, fmt.Errorf("failed to validate green secret %q: %v", greenSecret.Name, err)
}

err = r.syncGreenSecretForRook(ctx, greenSecret)
if err != nil {
logger.Error("Failed to sync green secret for Rook", "error", err)
return ctrl.Result{}, err
}

logger.Info("Successfully reconciled and synced green secret")
return ctrl.Result{}, nil
}
1 change: 0 additions & 1 deletion addons/green_secret_controller_test.go

This file was deleted.

Loading

0 comments on commit 15a271e

Please sign in to comment.