Merge pull request #61 from alexander-demicev/logging

🌱 Make logging more consistent

Author: k8s-ci-robot

internal/controllers/genericprovider_controller.go

@@ -51,6 +51,10 @@ func (r *GenericProviderReconciler) SetupWithManager(mgr ctrl.Manager, options c
 }
 
 func (r *GenericProviderReconciler) Reconcile(ctx context.Context, req reconcile.Request) (_ reconcile.Result, reterr error) {
+	log := ctrl.LoggerFrom(ctx)
+
+	log.Info("Reconciling provider")
+
 	typedProvider, err := r.newGenericProvider()
 	if err != nil {
 		return ctrl.Result{}, err
@@ -120,12 +124,6 @@ func patchProvider(ctx context.Context, provider genericprovider.GenericProvider
 }
 
 func (r *GenericProviderReconciler) reconcile(ctx context.Context, provider genericprovider.GenericProvider, genericProviderList genericprovider.GenericProviderList) (ctrl.Result, error) {
-	log := ctrl.LoggerFrom(ctx)
-
-	log.V(1).Info("starting reconciliation",
-		"Generation", provider.GetGeneration(),
-		"ObservedGeneration", provider.GetStatus().ObservedGeneration)
-
 	reconciler := newPhaseReconciler(*r, provider, genericProviderList)
 	phases := []reconcilePhaseFn{
 		reconciler.preflightChecks,
@@ -155,7 +153,8 @@ func (r *GenericProviderReconciler) reconcile(ctx context.Context, provider gene
 
 func (r *GenericProviderReconciler) reconcileDelete(ctx context.Context, provider genericprovider.GenericProvider) (ctrl.Result, error) {
 	log := ctrl.LoggerFrom(ctx)
-	log.V(2).Info("deleting provider resources")
+
+	log.Info("Deleting provider resources")
 
 	reconciler := newPhaseReconciler(*r, provider, nil)
 	phases := []reconcilePhaseFn{

internal/controllers/phases.go

@@ -111,15 +111,15 @@ func (p *phaseReconciler) preflightChecks(ctx context.Context) (reconcile.Result
 func (p *phaseReconciler) load(ctx context.Context) (reconcile.Result, error) {
 	log := ctrl.LoggerFrom(ctx)
 
-	log.V(2).Info("Loading provider", "name", p.provider.GetName())
+	log.Info("Loading provider")
 
 	// Load provider's secret and config url.
 	reader, err := p.secretReader(ctx)
 	if err != nil {
 		return reconcile.Result{}, wrapPhaseError(err, "failed to load the secret reader", operatorv1.PreflightCheckCondition)
 	}
 
-	// Initialize the a client for interacting with the clusterctl configuration.
+	// Initialize a client for interacting with the clusterctl configuration.
 	p.configClient, err = configclient.New("", configclient.InjectReader(reader))
 	if err != nil {
 		return reconcile.Result{}, err
@@ -137,7 +137,7 @@ func (p *phaseReconciler) load(ctx context.Context) (reconcile.Result, error) {
 	// If a configmap selector was specified, use it to find the configmap with provider configuraion. This is
 	// a case for "air-gapped" environments. If no selector was specified, use GitHub repository.
 	if spec.FetchConfig != nil && spec.FetchConfig.Selector != nil {
-		log.V(2).Info("Custom ConfigMap was provided for fetching manifests", "provider", p.provider.GetName())
+		log.V(5).Info("Custom ConfigMap was provided for fetching manifests")
 		p.repo, err = p.configmapRepository(ctx)
 	} else {
 		p.repo, err = repository.NewGitHubRepository(p.providerConfig, p.configClient.Variables())
@@ -182,12 +182,12 @@ func (p *phaseReconciler) secretReader(ctx context.Context) (configclient.Reader
 			mr.Set(k, string(v))
 		}
 	} else {
-		log.V(2).Info("No configuration secret was specified", "provider", p.provider.GetName())
+		log.Info("No configuration secret was specified")
 	}
 
 	// If provided store fetch config url in memory reader.
 	if p.provider.GetSpec().FetchConfig != nil && p.provider.GetSpec().FetchConfig.URL != "" {
-		log.V(2).Info("Custom fetch configuration url was provided", "provider", p.provider.GetName())
+		log.Info("Custom fetch configuration url was provided")
 		return mr.AddProvider(p.provider.GetName(), util.ClusterctlProviderType(p.provider), p.provider.GetSpec().FetchConfig.URL)
 	}
 
@@ -280,7 +280,7 @@ func (p *phaseReconciler) validateRepoCAPIVersion() error {
 // fetch fetches the provider components from the repository and processes all yaml manifests.
 func (p *phaseReconciler) fetch(ctx context.Context) (reconcile.Result, error) {
 	log := ctrl.LoggerFrom(ctx)
-	log.V(1).Info("Fetching provider", "name", p.provider.GetName())
+	log.Info("Fetching provider")
 
 	// Fetch the provider components yaml file from the provided repository Github/ConfigMap.
 	componentsFile, err := p.repo.GetFile(p.options.Version, p.repo.ComponentsPath())
@@ -321,7 +321,8 @@ func (p *phaseReconciler) preInstall(ctx context.Context) (reconcile.Result, err
 
 	clusterClient := p.newClusterClient()
 
-	log.V(2).Info("Ensuring clustectl CRDs are installed", "name", p.provider.GetName())
+	log.Info("Ensuring clustectl CRDs are installed")
+
 	err := clusterClient.ProviderInventory().EnsureCustomResourceDefinitions()
 	if err != nil {
 		return reconcile.Result{}, wrapPhaseError(err, "failed installing clusterctl CRDs", operatorv1.ProviderInstalledCondition)
@@ -332,7 +333,7 @@ func (p *phaseReconciler) preInstall(ctx context.Context) (reconcile.Result, err
 		return reconcile.Result{}, wrapPhaseError(err, "failed getting clusterctl Provider", operatorv1.ProviderInstalledCondition)
 	}
 
-	log.V(1).Info("Upgrade detected, deleting existing components", "name", p.provider.GetName())
+	log.Info("Upgrade detected, deleting existing components")
 	return p.delete(ctx)
 }
 
@@ -364,7 +365,7 @@ func (p *phaseReconciler) install(ctx context.Context) (reconcile.Result, error)
 	installer := clusterClient.ProviderInstaller()
 	installer.Add(p.components)
 
-	log.V(1).Info("Installing provider", "name", p.provider.GetName())
+	log.Info("Installing provider")
 
 	if _, err := installer.Install(cluster.InstallOptions{}); err != nil {
 		reason := "Install failed"
@@ -380,15 +381,15 @@ func (p *phaseReconciler) install(ctx context.Context) (reconcile.Result, error)
 	status.InstalledVersion = &installedVersion
 	p.provider.SetStatus(status)
 
-	log.V(1).Info("Provider successfully installed", "name", p.provider.GetName())
+	log.Info("Provider successfully installed")
 	conditions.Set(p.provider, conditions.TrueCondition(operatorv1.ProviderInstalledCondition))
 	return reconcile.Result{}, nil
 }
 
 // delete deletes the provider components using clusterctl library.
 func (p *phaseReconciler) delete(ctx context.Context) (reconcile.Result, error) {
 	log := ctrl.LoggerFrom(ctx)
-	log.V(1).Info("Deleting provider", "name", p.provider.GetName())
+	log.Info("Deleting provider")
 
 	clusterClient := p.newClusterClient()
 

internal/controllers/preflight_checks.go

@@ -48,13 +48,13 @@ var (
 func preflightChecks(ctx context.Context, c client.Client, provider genericprovider.GenericProvider, providerList genericprovider.GenericProviderList) (ctrl.Result, error) {
 	log := ctrl.LoggerFrom(ctx)
 
-	log.V(1).Info("Performing preflight checks.", "provider", provider.GetName())
+	log.Info("Performing preflight checks")
 
 	spec := provider.GetSpec()
 
 	// Check that provider version is not empty.
 	if spec.Version == "" {
-		log.V(2).Info("Version can't be empty", "provider", provider.GetName())
+		log.Info("Version can't be empty")
 		conditions.Set(provider, conditions.FalseCondition(
 			operatorv1.PreflightCheckCondition,
 			operatorv1.EmptyVersionReason,
@@ -66,7 +66,7 @@ func preflightChecks(ctx context.Context, c client.Client, provider genericprovi
 
 	// Check that provider version contains a valid value.
 	if _, err := version.ParseSemantic(spec.Version); err != nil {
-		log.V(2).Info("Version contains invalid value", "provider", provider.GetName())
+		log.Info("Version contains invalid value")
 		conditions.Set(provider, conditions.FalseCondition(
 			operatorv1.PreflightCheckCondition,
 			operatorv1.IncorrectVersionFormatReason,
@@ -108,7 +108,7 @@ func preflightChecks(ctx context.Context, c client.Client, provider genericprovi
 
 		// CoreProvider is a singleton resource, more than one instances should not exist
 		if util.IsCoreProvider(p) {
-			log.V(4).Info(moreThanOneCoreProviderInstanceExistsMessage)
+			log.Info(moreThanOneCoreProviderInstanceExistsMessage)
 			preflightFalseCondition.Message = moreThanOneCoreProviderInstanceExistsMessage
 			conditions.Set(provider, preflightFalseCondition)
 			return ctrl.Result{RequeueAfter: preflightFailedRequeueAfter}, fmt.Errorf("only one instance of CoreProvider is allowed")
@@ -117,7 +117,7 @@ func preflightChecks(ctx context.Context, c client.Client, provider genericprovi
 		// For any other provider we should check that instances with similar name exist in any namespace
 		if p.GetObjectKind().GroupVersionKind().Kind != coreProvider && p.GetName() == provider.GetName() {
 			preflightFalseCondition.Message = fmt.Sprintf(moreThanOneProviderInstanceExistsMessage, p.GetName(), p.GetNamespace())
-			log.V(2).Info(preflightFalseCondition.Message)
+			log.Info(preflightFalseCondition.Message)
 			conditions.Set(provider, preflightFalseCondition)
 			return ctrl.Result{RequeueAfter: preflightFailedRequeueAfter}, fmt.Errorf("only one %s provider is allowed in the cluster", p.GetName())
 		}
@@ -130,7 +130,7 @@ func preflightChecks(ctx context.Context, c client.Client, provider genericprovi
 			return ctrl.Result{}, errors.Wrap(err, "failed to get coreProvider ready condition")
 		}
 		if !ready {
-			log.V(2).Info(waitingForCoreProviderReadyMessage)
+			log.Info(waitingForCoreProviderReadyMessage)
 			conditions.Set(provider, conditions.FalseCondition(
 				operatorv1.PreflightCheckCondition,
 				operatorv1.WaitingForCoreProviderReadyReason,
@@ -142,6 +142,8 @@ func preflightChecks(ctx context.Context, c client.Client, provider genericprovi
 	}
 
 	conditions.Set(provider, conditions.TrueCondition(operatorv1.PreflightCheckCondition))
+
+	log.Info("Preflight checks passed")
 	return ctrl.Result{}, nil
 }