improve log legibility

Signed-off-by: Cameron Wall <cwall@redhat.com> improve log legibility Signed-off-by: Cameron Wall <cwall@redhat.com>
stolostron · Dec 5, 2023 · ea88ce0 · ea88ce0
1 parent d7f963e
commit ea88ce0
Show file tree

Hide file tree

Showing 11 changed files with 26 additions and 97 deletions.
diff --git a/api/v1/multiclusterengine_webhook.go b/api/v1/multiclusterengine_webhook.go
@@ -28,10 +28,10 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/discovery"
+	log "k8s.io/klog/v2"
 	ctrl "sigs.k8s.io/controller-runtime"
 	cl "sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/config"
-	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
@@ -40,10 +40,8 @@ const (
 	DefaultTargetNamespace = "multicluster-engine"
 )
 
-// log is for logging in this package.
 var (
-	backplaneconfiglog = logf.Log.WithName("backplaneconfig-resource")
-	Client             cl.Client
+	Client cl.Client
 
 	ErrInvalidComponent    = errors.New("invalid component config")
 	ErrInvalidNamespace    = errors.New("invalid TargetNamespace")
@@ -146,7 +144,7 @@ var _ webhook.Defaulter = &MultiClusterEngine{}
 
 // Default implements webhook.Defaulter so a webhook will be registered for the type
 func (r *MultiClusterEngine) Default() {
-	backplaneconfiglog.Info("default", "name", r.Name)
+	log.Info("default", "name", r.Name)
 	if r.Spec.TargetNamespace == "" {
 		r.Spec.TargetNamespace = DefaultTargetNamespace
 	}
@@ -157,7 +155,7 @@ var _ webhook.Validator = &MultiClusterEngine{}
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type
 func (r *MultiClusterEngine) ValidateCreate() (admission.Warnings, error) {
 	ctx := context.Background()
-	backplaneconfiglog.Info("validate create", "name", r.Name)
+	log.Info("validate create", "name", r.Name)
 
 	if (r.Spec.AvailabilityConfig != HABasic) && (r.Spec.AvailabilityConfig != HAHigh) && (r.Spec.AvailabilityConfig != "") {
 		return nil, ErrInvalidAvailability
@@ -198,10 +196,10 @@ func (r *MultiClusterEngine) ValidateCreate() (admission.Warnings, error) {
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
 func (r *MultiClusterEngine) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
-	backplaneconfiglog.Info("validate update", "name", r.Name)
+	log.Info("validate update", "name", r.Name)
 
 	oldMCE := old.(*MultiClusterEngine)
-	backplaneconfiglog.Info(oldMCE.Spec.TargetNamespace)
+	log.Info(oldMCE.Spec.TargetNamespace)
 	if (r.Spec.TargetNamespace != oldMCE.Spec.TargetNamespace) && (oldMCE.Spec.TargetNamespace != "") {
 		return nil, fmt.Errorf("%w: changes cannot be made to target namespace", ErrInvalidNamespace)
 	}
@@ -269,7 +267,7 @@ func (r *MultiClusterEngine) ValidateUpdate(old runtime.Object) (admission.Warni
 // ValidateDelete implements webhook.Validator so a webhook will be registered for the type
 func (r *MultiClusterEngine) ValidateDelete() (admission.Warnings, error) {
 	// TODO(user): fill in your validation logic upon object deletion.
-	backplaneconfiglog.Info("validate delete", "name", r.Name)
+	log.Info("validate delete", "name", r.Name)
 	ctx := context.Background()
 
 	cfg, err := config.GetConfig()

diff --git a/api/v1/webhook_suite_test.go b/api/v1/webhook_suite_test.go
@@ -35,11 +35,10 @@ import (
 	//+kubebuilder:scaffold:imports
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/rest"
+	log "k8s.io/klog/v2"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
-	logf "sigs.k8s.io/controller-runtime/pkg/log"
-	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 )
 
 // These tests use Ginkgo (BDD-style Go testing framework). Refer to
@@ -57,8 +56,8 @@ func TestAPIs(t *testing.T) {
 }
 
 var _ = BeforeSuite(func() {
-	logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)))
-
+	log.InitFlags(nil)
+	defer log.Flush()
 	ctx, cancel = context.WithCancel(context.TODO())
 
 	By("bootstrapping test environment")

diff --git a/controllers/backplaneconfig_controller.go b/controllers/backplaneconfig_controller.go
@@ -49,13 +49,13 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/client-go/util/workqueue"
+	log "k8s.io/klog/v2"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
-	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
@@ -145,7 +145,6 @@ 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 *MultiClusterEngineReconciler) Reconcile(ctx context.Context, req ctrl.Request) (retRes ctrl.Result, retErr error) {
-	log := log.FromContext(ctx)
 	// Fetch the BackplaneConfig instance
 	backplaneConfig, err := r.getBackplaneConfig(ctx, req)
 	if err != nil && !apierrors.IsNotFound(err) {
@@ -453,9 +452,7 @@ func (r *MultiClusterEngineReconciler) SetupWithManager(mgr ctrl.Manager) error
 
 // createTrustBundleConfigmap creates a configmap that will be injected with the
 // trusted CA bundle for use with the OCP cluster wide proxy
-func (r *MultiClusterEngineReconciler) createTrustBundleConfigmap(ctx context.Context,
-	mce *backplanev1.MultiClusterEngine) (ctrl.Result, error) {
-	log := log.FromContext(ctx)
+func (r *MultiClusterEngineReconciler) createTrustBundleConfigmap(ctx context.Context, mce *backplanev1.MultiClusterEngine) (ctrl.Result, error) {
 
 	// Get Trusted Bundle configmap name
 	trustBundleName := defaultTrustBundleName
@@ -645,7 +642,6 @@ func (r *MultiClusterEngineReconciler) createMetricsServiceMonitor(ctx context.C
 
 // DeployAlwaysSubcomponents ensures all subcomponents exist
 func (r *MultiClusterEngineReconciler) DeployAlwaysSubcomponents(ctx context.Context, backplaneConfig *backplanev1.MultiClusterEngine) (ctrl.Result, error) {
-	log := log.FromContext(ctx)
 
 	chartsDir := renderer.AlwaysChartsDir
 	// Renders all templates from charts
@@ -936,7 +932,6 @@ func (r *MultiClusterEngineReconciler) applyTemplate(ctx context.Context, backpl
 // deleteTemplate return true if resource does not exist and returns an error if a GET or DELETE errors unexpectedly. A false response without error
 // means the resource is in the process of deleting.
 func (r *MultiClusterEngineReconciler) deleteTemplate(ctx context.Context, backplaneConfig *backplanev1.MultiClusterEngine, template *unstructured.Unstructured) (ctrl.Result, error) {
-	log := log.FromContext(ctx)
 	err := r.Client.Get(ctx, types.NamespacedName{Name: template.GetName(), Namespace: template.GetNamespace()}, template)
 
 	if err != nil && (apierrors.IsNotFound(err) || apimeta.IsNoMatchError(err)) {
@@ -960,7 +955,6 @@ func (r *MultiClusterEngineReconciler) deleteTemplate(ctx context.Context, backp
 }
 
 func (r *MultiClusterEngineReconciler) ensureCustomResources(ctx context.Context, backplaneConfig *backplanev1.MultiClusterEngine) (ctrl.Result, error) {
-	log := log.FromContext(ctx)
 
 	if foundation.CanInstallAddons(ctx, r.Client) {
 		addonTemplates, err := foundation.GetAddons()
@@ -989,7 +983,6 @@ func (r *MultiClusterEngineReconciler) ensureCustomResources(ctx context.Context
 func (r *MultiClusterEngineReconciler) ensureOpenShiftNamespaceLabel(ctx context.Context,
 	backplaneConfig *backplanev1.MultiClusterEngine) (ctrl.Result, error) {
 
-	log := log.FromContext(ctx)
 	existingNs := &corev1.Namespace{}
 
 	err := r.Client.Get(ctx, types.NamespacedName{Name: backplaneConfig.Spec.TargetNamespace}, existingNs)
@@ -1020,7 +1013,6 @@ func (r *MultiClusterEngineReconciler) ensureOpenShiftNamespaceLabel(ctx context
 }
 
 func (r *MultiClusterEngineReconciler) finalizeBackplaneConfig(ctx context.Context, backplaneConfig *backplanev1.MultiClusterEngine) error {
-	log := log.FromContext(ctx)
 
 	ocpConsole, err := r.CheckConsole(ctx)
 	if err != nil {
@@ -1159,7 +1151,6 @@ func (r *MultiClusterEngineReconciler) finalizeBackplaneConfig(ctx context.Conte
 }
 
 func (r *MultiClusterEngineReconciler) getBackplaneConfig(ctx context.Context, req ctrl.Request) (*backplanev1.MultiClusterEngine, error) {
-	log := log.FromContext(ctx)
 	backplaneConfig := &backplanev1.MultiClusterEngine{}
 	err := r.Client.Get(ctx, req.NamespacedName, backplaneConfig)
 	if err != nil {
@@ -1178,7 +1169,6 @@ func (r *MultiClusterEngineReconciler) getBackplaneConfig(ctx context.Context, r
 
 // ensureUnstructuredResource ensures that the unstructured resource is applied in the cluster properly
 func (r *MultiClusterEngineReconciler) ensureUnstructuredResource(ctx context.Context, bpc *backplanev1.MultiClusterEngine, u *unstructured.Unstructured) (ctrl.Result, error) {
-	log := log.FromContext(ctx)
 
 	found := &unstructured.Unstructured{}
 	found.SetGroupVersionKind(u.GroupVersionKind())
@@ -1214,7 +1204,6 @@ func (r *MultiClusterEngineReconciler) ensureUnstructuredResource(ctx context.Co
 }
 
 func (r *MultiClusterEngineReconciler) setDefaults(ctx context.Context, m *backplanev1.MultiClusterEngine) (ctrl.Result, error) {
-	log := log.FromContext(ctx)
 
 	updateNecessary := false
 	if !utils.AvailabilityConfigIsValid(m.Spec.AvailabilityConfig) {
@@ -1315,7 +1304,6 @@ func (r *MultiClusterEngineReconciler) setDefaults(ctx context.Context, m *backp
 }
 
 func (r *MultiClusterEngineReconciler) validateNamespace(ctx context.Context, m *backplanev1.MultiClusterEngine) (ctrl.Result, error) {
-	log := log.FromContext(ctx)
 	newNs := &corev1.Namespace{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: m.Spec.TargetNamespace,
@@ -1366,7 +1354,6 @@ func (r *MultiClusterEngineReconciler) validateImagePullSecret(ctx context.Conte
 }
 
 func (r *MultiClusterEngineReconciler) getClusterVersion(ctx context.Context) (string, error) {
-	log := log.FromContext(ctx)
 	// If Unit test
 	if val, ok := os.LookupEnv("UNIT_TEST"); ok && val == "true" {
 		if _, exists := os.LookupEnv("ACM_HUB_OCP_VERSION"); exists {
@@ -1392,7 +1379,6 @@ func (r *MultiClusterEngineReconciler) getClusterVersion(ctx context.Context) (s
 //+kubebuilder:rbac:groups="config.openshift.io",resources="ingresses",verbs=get;list;watch
 
 func (r *MultiClusterEngineReconciler) getClusterIngressDomain(ctx context.Context, mce *backplanev1.MultiClusterEngine) (string, error) {
-	log := log.FromContext(ctx)
 	// If Unit test
 	if val, ok := os.LookupEnv("UNIT_TEST"); ok && val == "true" {
 		return "apps.installer-test-cluster.dev00.red-chesterfield.com", nil

diff --git a/controllers/hosted.go b/controllers/hosted.go
@@ -23,16 +23,15 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/clientcmd"
+	log "k8s.io/klog/v2"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
-	"sigs.k8s.io/controller-runtime/pkg/log"
 )
 
 var ErrBadFormat = errors.New("bad format")
 
 func (r *MultiClusterEngineReconciler) HostedReconcile(ctx context.Context, mce *backplanev1.MultiClusterEngine) (retRes ctrl.Result, retErr error) {
-	log := log.FromContext(ctx)
 
 	defer func() {
 		mce.Status = r.StatusManager.ReportStatus(*mce)
@@ -226,8 +225,6 @@ func parseKubeCreds(secret *corev1.Secret) ([]byte, error) {
 
 func (r *MultiClusterEngineReconciler) ensureHostedImport(ctx context.Context, backplaneConfig *backplanev1.MultiClusterEngine, hostedClient client.Client) (ctrl.Result, error) {
 
-	log := log.FromContext(ctx)
-
 	r.StatusManager.AddComponent(toggle.EnabledStatus(types.NamespacedName{Name: "managedcluster-import-controller-v2", Namespace: backplaneConfig.Spec.TargetNamespace}))
 	r.StatusManager.RemoveComponent(toggle.DisabledStatus(types.NamespacedName{Name: "managedcluster-import-controller-v2", Namespace: backplaneConfig.Spec.TargetNamespace}, []*unstructured.Unstructured{}))
 	templates, errs := renderer.RenderChart(toggle.HostingImportChartDir, backplaneConfig, r.Images)
@@ -267,7 +264,6 @@ func (r *MultiClusterEngineReconciler) ensureHostedImport(ctx context.Context, b
 
 func (r *MultiClusterEngineReconciler) ensureNoHostedImport(ctx context.Context, backplaneConfig *backplanev1.MultiClusterEngine, hostedClient client.Client) (ctrl.Result, error) {
 
-	log := log.FromContext(ctx)
 	r.StatusManager.RemoveComponent(toggle.EnabledStatus(types.NamespacedName{Name: "managedcluster-import-controller-v2", Namespace: backplaneConfig.Spec.TargetNamespace}))
 	r.StatusManager.AddComponent(toggle.DisabledStatus(types.NamespacedName{Name: "managedcluster-import-controller-v2", Namespace: backplaneConfig.Spec.TargetNamespace}, []*unstructured.Unstructured{}))
 	templates, errs := renderer.RenderChart(toggle.HostingImportChartDir, backplaneConfig, r.Images)
@@ -306,7 +302,6 @@ func (r *MultiClusterEngineReconciler) ensureNoHostedImport(ctx context.Context,
 }
 
 func (r *MultiClusterEngineReconciler) ensureHostedClusterManager(ctx context.Context, mce *backplanev1.MultiClusterEngine) (ctrl.Result, error) {
-	log := log.FromContext(ctx)
 	cmName := fmt.Sprintf("%s-cluster-manager", mce.Name)
 
 	r.StatusManager.AddComponent(status.ClusterManagerStatus{
@@ -421,7 +416,6 @@ func (r *MultiClusterEngineReconciler) ensureNoHostedClusterManager(ctx context.
 
 // setHostedDefaults configures the MCE with default values and updates
 func (r *MultiClusterEngineReconciler) setHostedDefaults(ctx context.Context, m *backplanev1.MultiClusterEngine) (ctrl.Result, error) {
-	log := log.FromContext(ctx)
 
 	updateNecessary := false
 	if !utils.AvailabilityConfigIsValid(m.Spec.AvailabilityConfig) {
@@ -495,7 +489,6 @@ func (r *MultiClusterEngineReconciler) hostedApplyTemplate(ctx context.Context,
 // deleteTemplate return true if resource does not exist and returns an error if a GET or DELETE errors unexpectedly. A false response without error
 // means the resource is in the process of deleting.
 func (r *MultiClusterEngineReconciler) hostedDeleteTemplate(ctx context.Context, backplaneConfig *backplanev1.MultiClusterEngine, template *unstructured.Unstructured, hostedClient client.Client) (ctrl.Result, error) {
-	log := log.FromContext(ctx)
 	err := hostedClient.Get(ctx, types.NamespacedName{Name: template.GetName(), Namespace: template.GetNamespace()}, template)
 
 	if err != nil && apierrors.IsNotFound(err) {

diff --git a/controllers/suite_test.go b/controllers/suite_test.go
@@ -47,11 +47,10 @@ import (
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
 
+	log "k8s.io/klog/v2"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
-	logf "sigs.k8s.io/controller-runtime/pkg/log"
-	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 )
 
 // These tests use Ginkgo (BDD-style Go testing framework). Refer to
@@ -70,8 +69,8 @@ var cancel context.CancelFunc
 var reconciler MultiClusterEngineReconciler
 
 var _ = BeforeSuite(func() {
-	logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)))
-
+	log.InitFlags(nil)
+	defer log.Flush()
 	// SetupSignalHandler can only be called once, so we'll save the
 	// context it returns and reuse it each time we start a new
 	// manager.