From f0e061c0ef033fb41aafe801b48753ad219c6def Mon Sep 17 00:00:00 2001 From: Justin Kulikauskas Date: Fri, 13 Oct 2023 16:32:54 -0400 Subject: [PATCH] Update log calls to be more consistent Signed-off-by: Justin Kulikauskas --- .../gatekeeper_constraint_sync.go | 2 +- controllers/secretsync/secret_sync.go | 24 +++++++++++-------- controllers/specsync/policy_spec_sync.go | 4 +++- controllers/statussync/eventMapper.go | 20 ++++------------ controllers/statussync/policy_status_sync.go | 10 ++++---- controllers/templatesync/template_sync.go | 2 +- 6 files changed, 30 insertions(+), 32 deletions(-) diff --git a/controllers/gatekeepersync/gatekeeper_constraint_sync.go b/controllers/gatekeepersync/gatekeeper_constraint_sync.go index 908f6e99..7f0b2247 100644 --- a/controllers/gatekeepersync/gatekeeper_constraint_sync.go +++ b/controllers/gatekeepersync/gatekeeper_constraint_sync.go @@ -112,7 +112,7 @@ func (r *GatekeeperConstraintReconciler) Reconcile( err := r.Get(ctx, request.NamespacedName, policy) if err != nil { if k8serrors.IsNotFound(err) { - log.V(1).Info("The Policy was deleted. Cleaning up watchers and status message cache.") + log.Info("The Policy was deleted. Cleaning up watchers and status message cache.") r.lastSentMessages.Range(func(key, value any) bool { keyTyped := key.(policyKindName) diff --git a/controllers/secretsync/secret_sync.go b/controllers/secretsync/secret_sync.go index 028bdc35..ff3016a8 100644 --- a/controllers/secretsync/secret_sync.go +++ b/controllers/secretsync/secret_sync.go @@ -63,7 +63,7 @@ func (r *SecretReconciler) Reconcile(ctx context.Context, request reconcile.Requ ) if uninstall.DeploymentIsUninstalling { - log.Info("Skipping reconcile because the deployment is in uninstallation mode") + reqLogger.Info("Skipping reconcile because the deployment is in uninstallation mode") return reconcile.Result{RequeueAfter: 5 * time.Minute}, nil } @@ -71,7 +71,7 @@ func (r *SecretReconciler) Reconcile(ctx context.Context, request reconcile.Requ reqLogger.Info("Reconciling Secret") // The cache configuration of SelectorsByObject should prevent this from happening, but add this as a precaution. if request.Name != SecretName { - log.Info("Got a reconciliation request for an unexpected Secret. This should have been filtered out.") + reqLogger.Info("Got a reconciliation request for an unexpected Secret. This should have been filtered out.") return reconcile.Result{}, nil } @@ -81,12 +81,12 @@ func (r *SecretReconciler) Reconcile(ctx context.Context, request reconcile.Requ err := r.Get(ctx, request.NamespacedName, hubEncryptionSecret) if err != nil { if !errors.IsNotFound(err) { - log.Error(err, "Failed to get the Secret on the Hub. Requeueing the request.") + reqLogger.Error(err, "Failed to get the Secret on the Hub. Requeueing the request.") return reconcile.Result{}, err } - log.Info("The Secret is no longer on the Hub. Deleting the replicated Secret.") + reqLogger.Info("The Secret is no longer on the Hub. Deleting the replicated Secret.") err := r.ManagedClient.Delete( ctx, @@ -98,11 +98,13 @@ func (r *SecretReconciler) Reconcile(ctx context.Context, request reconcile.Requ }, ) if err != nil && !errors.IsNotFound(err) { - log.Error(err, "Failed to delete the replicated Secret. Requeueing the request.") + reqLogger.Error(err, "Failed to delete the replicated Secret. Requeueing the request.") return reconcile.Result{}, err } + reqLogger.Info("Secret deleted, Reconciliation complete.") + return reconcile.Result{}, nil } @@ -113,11 +115,13 @@ func (r *SecretReconciler) Reconcile(ctx context.Context, request reconcile.Requ if err != nil { if !errors.IsNotFound(err) { - log.Error(err, "Failed to get the replicated Secret. Requeueing the request.") + reqLogger.Error(err, "Failed to get the replicated Secret. Requeueing the request.") return reconcile.Result{}, err } + reqLogger.Info("Creating the replicated secret") + // Don't completely copy the Hub secret since it isn't desired to have any annotations related to disaster // recovery copied over. managedEncryptionSecret := &corev1.Secret{ @@ -130,24 +134,24 @@ func (r *SecretReconciler) Reconcile(ctx context.Context, request reconcile.Requ err := r.ManagedClient.Create(ctx, managedEncryptionSecret) if err != nil { - log.Error(err, "Failed to replicate the Secret. Requeueing the request.") + reqLogger.Error(err, "Failed to replicate the Secret. Requeueing the request.") return reconcile.Result{}, err } - reqLogger.Info("Reconciliation complete") + reqLogger.Info("Secret replicated, Reconciliation complete") return reconcile.Result{}, nil } if !equality.Semantic.DeepEqual(hubEncryptionSecret.Data, managedEncryptionSecret.Data) { - log.Info("Updating the replicated secret due to it not matching the source on the Hub") + reqLogger.Info("Updating the replicated secret due to it not matching the source on the Hub") managedEncryptionSecret.Data = hubEncryptionSecret.Data err := r.ManagedClient.Update(ctx, managedEncryptionSecret) if err != nil { - log.Error(err, "Failed to update the replicated Secret. Requeueing the request.") + reqLogger.Error(err, "Failed to update the replicated Secret. Requeueing the request.") return reconcile.Result{}, err } diff --git a/controllers/specsync/policy_spec_sync.go b/controllers/specsync/policy_spec_sync.go index a620fe8a..46dbe43b 100644 --- a/controllers/specsync/policy_spec_sync.go +++ b/controllers/specsync/policy_spec_sync.go @@ -72,7 +72,7 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ ) if uninstall.DeploymentIsUninstalling { - log.Info("Skipping reconcile because the deployment is in uninstallation mode") + reqLogger.Info("Skipping reconcile because the deployment is in uninstallation mode") return reconcile.Result{RequeueAfter: 5 * time.Minute}, nil } @@ -101,6 +101,8 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ if err != nil && !errors.IsNotFound(err) { reqLogger.Error(err, "Failed to remove policy on managed cluster...") + + return reconcile.Result{}, err } reqLogger.Info("Policy has been removed from managed cluster...Reconciliation complete.") diff --git a/controllers/statussync/eventMapper.go b/controllers/statussync/eventMapper.go index d934d8a8..85f7495c 100644 --- a/controllers/statussync/eventMapper.go +++ b/controllers/statussync/eventMapper.go @@ -5,7 +5,6 @@ package statussync import ( "context" - "fmt" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" @@ -17,13 +16,9 @@ func eventMapper(_ context.Context, obj client.Object) []reconcile.Request { //nolint:forcetypeassert event := obj.(*corev1.Event) - log.Info( - fmt.Sprintf( - "Reconcile Request for Event %s in namespace %s", - event.GetName(), - event.GetNamespace(), - ), - ) + log := log.WithValues("eventName", event.GetName(), "eventNamespace", event.GetNamespace()) + + log.V(2).Info("Reconcile Request") var result []reconcile.Request @@ -32,13 +27,8 @@ func eventMapper(_ context.Context, obj client.Object) []reconcile.Request { Namespace: event.InvolvedObject.Namespace, }} - log.Info( - fmt.Sprintf( - "Queue event for Policy %s in namespace %s", - event.InvolvedObject.Name, - event.InvolvedObject.Namespace, - ), - ) + log.V(2).Info("Queueing event", "involvedName", event.InvolvedObject.Name, + "involvedNamespace", event.InvolvedObject.Namespace) return append(result, request) } diff --git a/controllers/statussync/policy_status_sync.go b/controllers/statussync/policy_status_sync.go index 02c62c1c..917a1ff5 100644 --- a/controllers/statussync/policy_status_sync.go +++ b/controllers/statussync/policy_status_sync.go @@ -86,7 +86,7 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ ) if uninstall.DeploymentIsUninstalling { - log.Info("Skipping reconcile because the deployment is in uninstallation mode") + reqLogger.Info("Skipping reconcile because the deployment is in uninstallation mode") return reconcile.Result{RequeueAfter: 5 * time.Minute}, nil } @@ -130,6 +130,8 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ managedInstance.SetOwnerReferences(nil) managedInstance.SetResourceVersion("") + reqLogger.Info("Policy missing from managed cluster, creating it.") + return reconcile.Result{}, r.ManagedClient.Create(ctx, managedInstance) } // Error reading the object - requeue the request. @@ -245,7 +247,7 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ existingDpt = dpt found = true - reqLogger.Info("Found existing status, retrieving it", "PolicyTemplate", tName) + reqLogger.V(1).Info("Found existing status, retrieving it", "PolicyTemplate", tName) break } @@ -360,7 +362,7 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ // append existingDpt to status newStatus.Details = append(newStatus.Details, existingDpt) - reqLogger.Info("Status update complete", "PolicyTemplate", tName) + reqLogger.V(1).Info("Status update complete", "PolicyTemplate", tName) } instance.Status = newStatus @@ -411,7 +413,7 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ // Re-fetch the hub template in case it changed err = r.HubClient.Get(ctx, types.NamespacedName{Namespace: r.ClusterNamespaceOnHub, Name: request.Name}, hubPlc) if err != nil { - log.Error(err, "Failed to refresh the cached policy. Will use existing policy.") + reqLogger.Error(err, "Failed to refresh the cached policy. Will use existing policy.") } if !equality.Semantic.DeepEqual(hubPlc.Status, instance.Status) { diff --git a/controllers/templatesync/template_sync.go b/controllers/templatesync/template_sync.go index f1cc04cd..5cdeaecc 100644 --- a/controllers/templatesync/template_sync.go +++ b/controllers/templatesync/template_sync.go @@ -222,7 +222,7 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ } if uninstall.DeploymentIsUninstalling { - log.Info("Skipping reconcile because the deployment is in uninstallation mode") + reqLogger.Info("Skipping reconcile because the deployment is in uninstallation mode") return reconcile.Result{RequeueAfter: 5 * time.Minute}, nil }