From 87e940d2b83bda7480b60494c4eeb6a44e539ba4 Mon Sep 17 00:00:00 2001 From: Stefan Bueringer Date: Thu, 30 Nov 2023 15:57:51 +0100 Subject: [PATCH] Improve log k/v pairs and a improve/drop a few log lines --- .../controllers/kubeadmconfig_controller.go | 23 ++++++++++--------- bootstrap/kubeadm/main.go | 3 +-- controllers/remote/cluster_cache_tracker.go | 3 +++ controlplane/kubeadm/main.go | 3 +-- .../runtime-sdk/implement-extensions.md | 2 +- internal/controllers/cluster/suite_test.go | 3 +-- internal/controllers/machine/suite_test.go | 3 +-- .../machinedeployment_controller.go | 1 - .../machinedeployment/suite_test.go | 3 +-- .../machinehealthcheck/suite_test.go | 3 +-- .../machineset/machineset_controller.go | 2 +- internal/controllers/machineset/suite_test.go | 3 +-- .../topology/cluster/suite_test.go | 3 +-- main.go | 3 +-- test/infrastructure/docker/main.go | 3 +-- 15 files changed, 27 insertions(+), 34 deletions(-) diff --git a/bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go b/bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go index 80a4da1475df..d55d45a1abb6 100644 --- a/bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go +++ b/bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go @@ -147,14 +147,6 @@ func (r *KubeadmConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques if apierrors.IsNotFound(err) { return ctrl.Result{}, nil } - log.Error(err, "Failed to get config") - return ctrl.Result{}, err - } - - // AddOwners adds the owners of KubeadmConfig as k/v pairs to the logger. - // Specifically, it will add KubeadmControlPlane, MachineSet and MachineDeployment. - ctx, log, err := clog.AddOwners(ctx, r.Client, config) - if err != nil { return ctrl.Result{}, err } @@ -165,13 +157,22 @@ func (r *KubeadmConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques return ctrl.Result{}, nil } if err != nil { - log.Error(err, "Failed to get owner") - return ctrl.Result{}, err + return ctrl.Result{}, errors.Wrapf(err, "failed to get owner") } if configOwner == nil { return ctrl.Result{}, nil } log = log.WithValues(configOwner.GetKind(), klog.KRef(configOwner.GetNamespace(), configOwner.GetName()), "resourceVersion", configOwner.GetResourceVersion()) + ctx = ctrl.LoggerInto(ctx, log) + + if configOwner.GetKind() == "Machine" { + // AddOwners adds the owners of Machine as k/v pairs to the logger. + // Specifically, it will add KubeadmControlPlane, MachineSet and MachineDeployment. + ctx, log, err = clog.AddOwners(ctx, r.Client, configOwner) + if err != nil { + return ctrl.Result{}, err + } + } log = log.WithValues("Cluster", klog.KRef(configOwner.GetNamespace(), configOwner.ClusterName())) ctx = ctrl.LoggerInto(ctx, log) @@ -1043,7 +1044,7 @@ func (r *KubeadmConfigReconciler) storeBootstrapData(ctx context.Context, scope if !apierrors.IsAlreadyExists(err) { return errors.Wrapf(err, "failed to create bootstrap data secret for KubeadmConfig %s/%s", scope.Config.Namespace, scope.Config.Name) } - log.Info("bootstrap data secret for KubeadmConfig already exists, updating", "Secret", klog.KObj(secret)) + log.Info("Bootstrap data secret for KubeadmConfig already exists, updating", "Secret", klog.KObj(secret)) if err := r.Client.Update(ctx, secret); err != nil { return errors.Wrapf(err, "failed to update bootstrap data secret for KubeadmConfig %s/%s", scope.Config.Namespace, scope.Config.Name) } diff --git a/bootstrap/kubeadm/main.go b/bootstrap/kubeadm/main.go index b7658f41e504..5ae2aeff4aff 100644 --- a/bootstrap/kubeadm/main.go +++ b/bootstrap/kubeadm/main.go @@ -294,13 +294,12 @@ func setupReconcilers(ctx context.Context, mgr ctrl.Manager) { // Set up a ClusterCacheTracker and ClusterCacheReconciler to provide to controllers // requiring a connection to a remote cluster - log := ctrl.Log.WithName("remote").WithName("ClusterCacheTracker") tracker, err := remote.NewClusterCacheTracker( mgr, remote.ClusterCacheTrackerOptions{ SecretCachingClient: secretCachingClient, ControllerName: controllerName, - Log: &log, + Log: &ctrl.Log, }, ) if err != nil { diff --git a/controllers/remote/cluster_cache_tracker.go b/controllers/remote/cluster_cache_tracker.go index 9bd40ceab13f..be21cff54665 100644 --- a/controllers/remote/cluster_cache_tracker.go +++ b/controllers/remote/cluster_cache_tracker.go @@ -129,6 +129,9 @@ func setDefaultOptions(opts *ClusterCacheTrackerOptions) { opts.Log = &l } + l := opts.Log.WithValues("component", "remote/clustercachetracker") + opts.Log = &l + if len(opts.ClientUncachedObjects) == 0 { opts.ClientUncachedObjects = []client.Object{ &corev1.ConfigMap{}, diff --git a/controlplane/kubeadm/main.go b/controlplane/kubeadm/main.go index 32428b16a042..2311d494379e 100644 --- a/controlplane/kubeadm/main.go +++ b/controlplane/kubeadm/main.go @@ -302,11 +302,10 @@ func setupReconcilers(ctx context.Context, mgr ctrl.Manager) { // Set up a ClusterCacheTracker to provide to controllers // requiring a connection to a remote cluster - log := ctrl.Log.WithName("remote").WithName("ClusterCacheTracker") tracker, err := remote.NewClusterCacheTracker(mgr, remote.ClusterCacheTrackerOptions{ SecretCachingClient: secretCachingClient, ControllerName: controllerName, - Log: &log, + Log: &ctrl.Log, ClientUncachedObjects: []client.Object{ &corev1.ConfigMap{}, &corev1.Secret{}, diff --git a/docs/book/src/tasks/experimental-features/runtime-sdk/implement-extensions.md b/docs/book/src/tasks/experimental-features/runtime-sdk/implement-extensions.md index cf81e06304ce..790fc1ab002d 100644 --- a/docs/book/src/tasks/experimental-features/runtime-sdk/implement-extensions.md +++ b/docs/book/src/tasks/experimental-features/runtime-sdk/implement-extensions.md @@ -89,7 +89,7 @@ func InitFlags(fs *pflag.FlagSet) { func main() { // Creates a logger to be used during the main func. - setupLog := ctrl.Log.WithName("main") + setupLog := ctrl.Log.WithName("setup") // Initialize and parse command line flags. InitFlags(pflag.CommandLine) diff --git a/internal/controllers/cluster/suite_test.go b/internal/controllers/cluster/suite_test.go index e9370946fff2..fa85378ed486 100644 --- a/internal/controllers/cluster/suite_test.go +++ b/internal/controllers/cluster/suite_test.go @@ -64,11 +64,10 @@ func TestMain(m *testing.M) { setupReconcilers := func(ctx context.Context, mgr ctrl.Manager) { // Set up a ClusterCacheTracker and ClusterCacheReconciler to provide to controllers // requiring a connection to a remote cluster - log := ctrl.Log.WithName("remote").WithName("ClusterCacheTracker") tracker, err := remote.NewClusterCacheTracker( mgr, remote.ClusterCacheTrackerOptions{ - Log: &log, + Log: &ctrl.Log, Indexes: []remote.Index{remote.NodeProviderIDIndex}, }, ) diff --git a/internal/controllers/machine/suite_test.go b/internal/controllers/machine/suite_test.go index 4893c5d653b9..0881d273c323 100644 --- a/internal/controllers/machine/suite_test.go +++ b/internal/controllers/machine/suite_test.go @@ -69,11 +69,10 @@ func TestMain(m *testing.M) { setupReconcilers := func(ctx context.Context, mgr ctrl.Manager) { // Set up a ClusterCacheTracker and ClusterCacheReconciler to provide to controllers // requiring a connection to a remote cluster - log := ctrl.Log.WithName("remote").WithName("ClusterCacheTracker") tracker, err := remote.NewClusterCacheTracker( mgr, remote.ClusterCacheTrackerOptions{ - Log: &log, + Log: &ctrl.Log, Indexes: []remote.Index{remote.NodeProviderIDIndex}, }, ) diff --git a/internal/controllers/machinedeployment/machinedeployment_controller.go b/internal/controllers/machinedeployment/machinedeployment_controller.go index 84c8095ef976..a5d5fdbe733e 100644 --- a/internal/controllers/machinedeployment/machinedeployment_controller.go +++ b/internal/controllers/machinedeployment/machinedeployment_controller.go @@ -164,7 +164,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re err = r.reconcile(ctx, cluster, deployment) if err != nil { - log.Error(err, "Failed to reconcile MachineDeployment") r.recorder.Eventf(deployment, corev1.EventTypeWarning, "ReconcileError", "%v", err) } return ctrl.Result{}, err diff --git a/internal/controllers/machinedeployment/suite_test.go b/internal/controllers/machinedeployment/suite_test.go index 8a49c8cb0847..61e247883df4 100644 --- a/internal/controllers/machinedeployment/suite_test.go +++ b/internal/controllers/machinedeployment/suite_test.go @@ -71,11 +71,10 @@ func TestMain(m *testing.M) { setupReconcilers := func(ctx context.Context, mgr ctrl.Manager) { // Set up a ClusterCacheTracker and ClusterCacheReconciler to provide to controllers // requiring a connection to a remote cluster - log := ctrl.Log.WithName("remote").WithName("ClusterCacheTracker") tracker, err := remote.NewClusterCacheTracker( mgr, remote.ClusterCacheTrackerOptions{ - Log: &log, + Log: &ctrl.Log, Indexes: []remote.Index{remote.NodeProviderIDIndex}, }, ) diff --git a/internal/controllers/machinehealthcheck/suite_test.go b/internal/controllers/machinehealthcheck/suite_test.go index 1975241296c3..009612377918 100644 --- a/internal/controllers/machinehealthcheck/suite_test.go +++ b/internal/controllers/machinehealthcheck/suite_test.go @@ -67,11 +67,10 @@ func TestMain(m *testing.M) { setupReconcilers := func(ctx context.Context, mgr ctrl.Manager) { // Set up a ClusterCacheTracker and ClusterCacheReconciler to provide to controllers // requiring a connection to a remote cluster - log := ctrl.Log.WithName("remote").WithName("ClusterCacheTracker") tracker, err := remote.NewClusterCacheTracker( mgr, remote.ClusterCacheTrackerOptions{ - Log: &log, + Log: &ctrl.Log, Indexes: []remote.Index{remote.NodeProviderIDIndex}, }, ) diff --git a/internal/controllers/machineset/machineset_controller.go b/internal/controllers/machineset/machineset_controller.go index 1b93130f7c81..f8f95d00a24f 100644 --- a/internal/controllers/machineset/machineset_controller.go +++ b/internal/controllers/machineset/machineset_controller.go @@ -890,7 +890,7 @@ func (r *Reconciler) updateStatus(ctx context.Context, cluster *clusterv1.Cluste availableReplicasCount++ } } else if machine.GetDeletionTimestamp().IsZero() { - log.Info("Waiting for the Kubernetes node on the machine to report ready state") + log.V(4).Info("Waiting for the Kubernetes node on the machine to report ready state") } } diff --git a/internal/controllers/machineset/suite_test.go b/internal/controllers/machineset/suite_test.go index c7fdde445ddf..b71eb949c154 100644 --- a/internal/controllers/machineset/suite_test.go +++ b/internal/controllers/machineset/suite_test.go @@ -70,11 +70,10 @@ func TestMain(m *testing.M) { setupReconcilers := func(ctx context.Context, mgr ctrl.Manager) { // Set up a ClusterCacheTracker and ClusterCacheReconciler to provide to controllers // requiring a connection to a remote cluster - log := ctrl.Log.WithName("remote").WithName("ClusterCacheTracker") tracker, err := remote.NewClusterCacheTracker( mgr, remote.ClusterCacheTrackerOptions{ - Log: &log, + Log: &ctrl.Log, Indexes: []remote.Index{remote.NodeProviderIDIndex}, }, ) diff --git a/internal/controllers/topology/cluster/suite_test.go b/internal/controllers/topology/cluster/suite_test.go index 2c595a67dc87..338094256ca4 100644 --- a/internal/controllers/topology/cluster/suite_test.go +++ b/internal/controllers/topology/cluster/suite_test.go @@ -76,7 +76,6 @@ func TestMain(m *testing.M) { } // Set up a ClusterCacheTracker and ClusterCacheReconciler to provide to controllers // requiring a connection to a remote cluster - log := ctrl.Log.WithName("remote").WithName("ClusterCacheTracker") secretCachingClient, err := client.New(mgr.GetConfig(), client.Options{ HTTPClient: mgr.GetHTTPClient(), Cache: &client.CacheOptions{ @@ -89,7 +88,7 @@ func TestMain(m *testing.M) { tracker, err := remote.NewClusterCacheTracker( mgr, remote.ClusterCacheTrackerOptions{ - Log: &log, + Log: &ctrl.Log, SecretCachingClient: secretCachingClient, }, ) diff --git a/main.go b/main.go index 3b0d8c3c94a7..52a661f4f54d 100644 --- a/main.go +++ b/main.go @@ -380,13 +380,12 @@ func setupReconcilers(ctx context.Context, mgr ctrl.Manager) { // Set up a ClusterCacheTracker and ClusterCacheReconciler to provide to controllers // requiring a connection to a remote cluster - log := ctrl.Log.WithName("remote").WithName("ClusterCacheTracker") tracker, err := remote.NewClusterCacheTracker( mgr, remote.ClusterCacheTrackerOptions{ SecretCachingClient: secretCachingClient, ControllerName: controllerName, - Log: &log, + Log: &ctrl.Log, Indexes: []remote.Index{remote.NodeProviderIDIndex}, }, ) diff --git a/test/infrastructure/docker/main.go b/test/infrastructure/docker/main.go index 061661d43f60..8a4c8e27932e 100644 --- a/test/infrastructure/docker/main.go +++ b/test/infrastructure/docker/main.go @@ -303,13 +303,12 @@ func setupReconcilers(ctx context.Context, mgr ctrl.Manager) { os.Exit(1) } - log := ctrl.Log.WithName("remote").WithName("ClusterCacheTracker") tracker, err := remote.NewClusterCacheTracker( mgr, remote.ClusterCacheTrackerOptions{ SecretCachingClient: secretCachingClient, ControllerName: controllerName, - Log: &log, + Log: &ctrl.Log, }, ) if err != nil {