fix review findings

kubernetes-sigs · Sep 18, 2023 · b08a5e6 · b08a5e6
1 parent ec92d06
commit b08a5e6
Show file tree

Hide file tree

Showing 6 changed files with 7 additions and 8 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -33,6 +33,7 @@ linters:
   - govet
   - importas
   - ineffassign
+  - loggercheck
   - misspell
   - nakedret
   - nilerr

diff --git a/controllers/clustermodule_reconciler.go b/controllers/clustermodule_reconciler.go
@@ -63,7 +63,7 @@ func NewReconciler(controllerManagerCtx *capvcontext.ControllerManagerContext) R
 
 func (r Reconciler) Reconcile(ctx context.Context, clusterCtx *capvcontext.ClusterContext) (reconcile.Result, error) {
 	log := ctrl.LoggerFrom(ctx)
-	log.Info("Reconcile cluster modules")
+	log.Info("Reconciling cluster modules")
 
 	if !clustermodule.IsClusterCompatible(clusterCtx) {
 		conditions.MarkFalse(clusterCtx.VSphereCluster, infrav1.ClusterModulesAvailableCondition, infrav1.VCenterVersionIncompatibleReason, clusterv1.ConditionSeverityInfo,

diff --git a/controllers/serviceaccount_controller.go b/controllers/serviceaccount_controller.go
@@ -458,7 +458,7 @@ func (r *ServiceAccountReconciler) syncServiceAccountSecret(ctx context.Context,
 	if len(svcAccountTokenSecret.Data) == 0 {
 		// Note: We don't have to requeue here because we have a watch on the secret and the cluster should be reconciled
 		// when a secret has token data populated.
-		log.Info("Skipping sync secret for provider service account: secret has no data", "secret", secretName)
+		log.Info("Skipping sync secret for provider service account: secret has no data", "Secret", klog.KRef(pSvcAccount.Namespace, secretName))
 		return nil
 	}
 

diff --git a/controllers/servicediscovery_controller.go b/controllers/servicediscovery_controller.go
@@ -296,9 +296,8 @@ func (r *serviceDiscoveryReconciler) reconcileSupervisorHeadlessService(ctx cont
 		)
 	default:
 		log.Error(
-			fmt.Errorf(
-				"unexpected result during createOrPatch k8s service endpoints",
-			),
+			nil,
+			"unexpected result during createOrPatch k8s service endpoints",
 			"endpointsKey",
 			endpointsKey,
 			"endpointsSubsets",

diff --git a/controllers/vmware/vspherecluster_reconciler.go b/controllers/vmware/vspherecluster_reconciler.go
@@ -195,7 +195,7 @@ func (r *ClusterReconciler) reconcileNormal(ctx context.Context, clusterCtx *vmw
 	}
 
 	clusterCtx.VSphereCluster.Status.Ready = true
-	log.V(2).Info("Reconcile completed, VSphereCluster is ready")
+	log.V(2).Info("Reconciling completed, VSphereCluster is ready")
 	return nil
 }
 
@@ -349,7 +349,7 @@ func (r *ClusterReconciler) VSphereMachineToCluster(ctx context.Context, o clien
 
 	vsphereMachine, ok := o.(*vmwarev1.VSphereMachine)
 	if !ok {
-		log.Error(errors.New("did not get vspheremachine"), "got", fmt.Sprintf("%T", o))
+		log.Error(nil, fmt.Sprintf("expected a VSphereMachine but got a %T", o))
 		return nil
 	}
 	log = log.WithValues("VSphereMachine", klog.KObj(vsphereMachine))

diff --git a/controllers/vspherecluster_reconciler.go b/controllers/vspherecluster_reconciler.go
@@ -555,7 +555,6 @@ func (r *clusterReconciler) setOwnerRefsOnVsphereMachines(ctx context.Context, c
 	return kerrors.NewAggregate(patchErrors)
 }
 
-// TODO: zhanggbj: Add golang context after refactoring ClusterModule controller and reconciler.
 func (r *clusterReconciler) reconcileClusterModules(ctx context.Context, clusterCtx *capvcontext.ClusterContext) (reconcile.Result, error) {
 	if feature.Gates.Enabled(feature.NodeAntiAffinity) {
 		return r.clusterModuleReconciler.Reconcile(ctx, clusterCtx)