Merge pull request #2627 from SD-13/shadow_variable

🌱 Enable govet shadow variables

Author: metal3-io-bot

.golangci.yaml

@@ -145,6 +145,9 @@ linters-settings:
     case:
       rules:
         json: goCamel
+  govet:
+    enable:
+    - shadow
 
 issues:
   exclude-dirs:

baremetal/metal3data_manager.go

@@ -346,7 +346,6 @@ func (m *DataManager) getAddressesFromPool(ctx context.Context,
 
 	for pool, ref := range poolRefs {
 		var rc reconciledClaim
-		var err error
 		if isMetal3IPPoolRef(ref) {
 			rc, err = m.ensureM3IPClaim(ctx, ref)
 		} else {
@@ -365,7 +364,6 @@ func (m *DataManager) getAddressesFromPool(ctx context.Context,
 			continue
 		}
 		if rc.fetchAgain {
-			var err error
 			if rc.m3Claim != nil {
 				rc.m3Claim, err = fetchM3IPClaim(ctx, m.client, m.Log, m.Data.Name+"-"+ref.Name, m.Data.Namespace)
 			} else {
@@ -1489,7 +1487,7 @@ func fetchM3IPClaim(ctx context.Context, cl client.Client, mLog logr.Logger,
 			mLog.Info(errMessage)
 			return nil, WithTransientError(errors.New(errMessage), requeueAfter)
 		}
-		err := errors.Wrap(err, "Failed to get address claim")
+		err = errors.Wrap(err, "Failed to get address claim")
 		return nil, err
 	}
 	return metal3IPClaim, nil

baremetal/metal3datatemplate_manager.go

@@ -198,7 +198,7 @@ func (m *DataTemplateManager) updateData(ctx context.Context,
 	}
 	// Always patch dataClaim exiting this function so we can persist any changes.
 	defer func() {
-		err := helper.Patch(ctx, dataClaim)
+		err = helper.Patch(ctx, dataClaim)
 		if err != nil {
 			m.Log.Info("failed to Patch capm3DataClaim")
 		}

baremetal/metal3machine_manager.go

@@ -286,7 +286,7 @@ func (m *MachineManager) SetPauseAnnotation(ctx context.Context) error {
 		return errors.Wrap(err, "failed to marshall status annotation")
 	}
 	obj := map[string]interface{}{}
-	if err := json.Unmarshal(newAnnotation, &obj); err != nil {
+	if err = json.Unmarshal(newAnnotation, &obj); err != nil {
 		return errors.Wrap(err, "failed to unmarshall status annotation")
 	}
 	delete(obj, "hardware")
@@ -382,7 +382,7 @@ func (m *MachineManager) Associate(ctx context.Context) error {
 	if m.Metal3Machine.Spec.DataTemplate != nil {
 		// Requeue to get the DataTemplate output. We need to requeue to trigger the
 		// wait on the Metal3DataTemplate
-		if err := m.WaitForM3Metadata(ctx); err != nil {
+		if err = m.WaitForM3Metadata(ctx); err != nil {
 			return err
 		}
 
@@ -547,7 +547,7 @@ func (m *MachineManager) Delete(ctx context.Context) error {
 		if bmhUpdated {
 			// Update the BMH object, if the errors are NotFound, do not return the
 			// errors.
-			if err := patchIfFound(ctx, helper, host); err != nil {
+			if err = patchIfFound(ctx, helper, host); err != nil {
 				return err
 			}
 
@@ -599,7 +599,7 @@ func (m *MachineManager) Delete(ctx context.Context) error {
 						Name:      m.Metal3Machine.ObjectMeta.GetAnnotations()[clusterv1.TemplateClonedFromNameAnnotation],
 						Namespace: m.Metal3Machine.Namespace,
 					}
-					if err := m.client.Get(ctx, m3mtKey, m3mt); err != nil {
+					if err = m.client.Get(ctx, m3mtKey, m3mt); err != nil {
 						// we are here, because while normal deprovisioning, Metal3MachineTemplate will be deleted first
 						// and we can't get it even though Metal3Machine has reference to it. We consider it nil and move
 						// forward with normal deprovisioning.
@@ -620,7 +620,8 @@ func (m *MachineManager) Delete(ctx context.Context) error {
 						if m.isControlPlane() {
 							// Fetch ControlPlane name for controlplane machine
 							m.Log.Info("Fetch ControlPlane name while deprovisioning host", "host", host.Name)
-							cpName, err := m.getControlPlaneName(ctx)
+							var cpName string
+							cpName, err = m.getControlPlaneName(ctx)
 							if err != nil {
 								return err
 							}
@@ -630,7 +631,8 @@ func (m *MachineManager) Delete(ctx context.Context) error {
 						} else {
 							// Fetch MachineDeployment name for worker machine
 							m.Log.Info("Fetch MachineDeployment name while deprovisioning host", "host", host.Name)
-							mdName, err := m.getMachineDeploymentName(ctx)
+							var mdName string
+							mdName, err = m.getMachineDeploymentName(ctx)
 							if err != nil {
 								return err
 							}
@@ -698,7 +700,7 @@ func (m *MachineManager) Update(ctx context.Context) error {
 		return WithTransientError(errors.New(errMessage), requeueAfter)
 	}
 
-	if err := m.WaitForM3Metadata(ctx); err != nil {
+	if err = m.WaitForM3Metadata(ctx); err != nil {
 		return err
 	}
 
@@ -812,12 +814,13 @@ func (m *MachineManager) chooseHost(ctx context.Context) (*bmov1alpha1.BareMetal
 	// I think it's because we have a local cache of all BareMetalHosts.
 	labelSelector := labels.NewSelector()
 	var reqs labels.Requirements
+	var r *labels.Requirement
 
 	for labelKey, labelVal := range m.Metal3Machine.Spec.HostSelector.MatchLabels {
 		m.Log.Info("Adding requirement to match label",
 			"label key", labelKey,
 			"label value", labelVal)
-		r, err := labels.NewRequirement(labelKey, selection.Equals, []string{labelVal})
+		r, err = labels.NewRequirement(labelKey, selection.Equals, []string{labelVal})
 		if err != nil {
 			m.Log.Error(err, "Failed to create MatchLabel requirement, not choosing host")
 			return nil, nil, err
@@ -830,7 +833,7 @@ func (m *MachineManager) chooseHost(ctx context.Context) (*bmov1alpha1.BareMetal
 			"label operator", req.Operator,
 			"label value", req.Values)
 		lowercaseOperator := selection.Operator(strings.ToLower(string(req.Operator)))
-		r, err := labels.NewRequirement(req.Key, lowercaseOperator, req.Values)
+		r, err = labels.NewRequirement(req.Key, lowercaseOperator, req.Values)
 		if err != nil {
 			m.Log.Error(err, "Failed to create MatchExpression requirement, not choosing host")
 			return nil, nil, err
@@ -844,8 +847,9 @@ func (m *MachineManager) chooseHost(ctx context.Context) (*bmov1alpha1.BareMetal
 
 	for i, host := range hosts.Items {
 		if host.Spec.ConsumerRef != nil && consumerRefMatches(host.Spec.ConsumerRef, m.Metal3Machine) {
+			var helper *v1beta1patch.Helper
 			m.Log.Info("Found host with existing ConsumerRef", "host", host.Name)
-			helper, err := v1beta1patch.NewHelper(&hosts.Items[i], m.client)
+			helper, err = v1beta1patch.NewHelper(&hosts.Items[i], m.client)
 			return &hosts.Items[i], helper, err
 		}
 		if host.Spec.ConsumerRef != nil ||
@@ -1467,7 +1471,7 @@ func (m *MachineManager) SetProviderIDFromNodeLabel(ctx context.Context, clientF
 			// By default we use the new format, if not set on the node.
 			m.SetProviderID(providerIDNew)
 			m.SetReadyTrue()
-			err := m.setNodeProviderID(ctx, corev1Remote, node, providerIDNew)
+			err = m.setNodeProviderID(ctx, corev1Remote, node, providerIDNew)
 
 			if err != nil {
 				return false, err

baremetal/metal3machine_manager_test.go

@@ -1602,7 +1602,7 @@ var _ = Describe("Metal3Machine manager", func() {
 				}
 				host := bmov1alpha1.BareMetalHost{}
 
-				err := fakeClient.Get(context.TODO(), key, &host)
+				err = fakeClient.Get(context.TODO(), key, &host)
 				Expect(err).NotTo(HaveOccurred())
 
 				name := ""

baremetal/utils.go

@@ -228,7 +228,7 @@ func fetchM3DataTemplate(ctx context.Context,
 			mLog.Info(errMessage)
 			return nil, WithTransientError(errors.New(errMessage), requeueAfter)
 		}
-		err := errors.Wrap(err, "Failed to get Metal3DataTemplate")
+		err = errors.Wrap(err, "Failed to get Metal3DataTemplate")
 		return nil, err
 	}
 
@@ -256,7 +256,7 @@ func fetchM3DataClaim(ctx context.Context, cl client.Client, mLog logr.Logger,
 			mLog.Info(errMessage)
 			return nil, WithTransientError(errors.New(errMessage), requeueAfter)
 		}
-		err := errors.Wrap(err, "Failed to get Metal3DataClaim")
+		err = errors.Wrap(err, "Failed to get Metal3DataClaim")
 		return nil, err
 	}
 	return m3DataClaim, nil
@@ -278,7 +278,7 @@ func fetchM3Data(ctx context.Context, cl client.Client, mLog logr.Logger,
 			mLog.Info(errMessage)
 			return nil, WithTransientError(errors.New(errMessage), requeueAfter)
 		}
-		err := errors.Wrap(err, "Failed to get Metal3Data")
+		err = errors.Wrap(err, "Failed to get Metal3Data")
 		return nil, err
 	}
 	return m3Data, nil
@@ -306,7 +306,7 @@ func getM3Machine(ctx context.Context, cl client.Client, mLog logr.Logger,
 			}
 			return nil, nil //nolint:nilnil
 		}
-		err := errors.Wrap(err, "Failed to get Metal3Machine")
+		err = errors.Wrap(err, "Failed to get Metal3Machine")
 		return nil, err
 	}
 

baremetal/utils_test.go

@@ -237,7 +237,7 @@ var _ = Describe("Metal3 manager utils", func() {
 				Expect(err).NotTo(HaveOccurred())
 				Expect(savedObject.Spec).To(Equal(tc.TestObject.Spec))
 				Expect(savedObject.ResourceVersion).NotTo(Equal(tc.TestObject.ResourceVersion))
-				err := updateObject(context.TODO(), k8sClient, obj)
+				err = updateObject(context.TODO(), k8sClient, obj)
 				Expect(err).To(HaveOccurred())
 				Expect(err).To(BeAssignableToTypeOf(ReconcileError{}))
 			}

controllers/metal3cluster_controller.go

@@ -102,7 +102,7 @@ func (r *Metal3ClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	}
 	// Always patch metal3Cluster when exiting this function so we can persist any metal3Cluster changes.
 	defer func() {
-		if err := patchMetal3Cluster(ctx, patchHelper, metal3Cluster); err != nil {
+		if err = patchMetal3Cluster(ctx, patchHelper, metal3Cluster); err != nil {
 			clusterLog.Error(err, "failed to Patch metal3Cluster")
 			rerr = err
 		}
@@ -143,7 +143,8 @@ func (r *Metal3ClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 
 	// Handle deleted clusters
 	if !metal3Cluster.DeletionTimestamp.IsZero() {
-		res, err := reconcileDelete(ctx, clusterMgr)
+		var res ctrl.Result
+		res, err = reconcileDelete(ctx, clusterMgr)
 		// Requeue if the reconcile failed because the ClusterCache was locked for
 		// the current cluster because of concurrent access.
 		if errors.Is(err, clustercache.ErrClusterNotConnected) {

controllers/metal3data_controller.go

@@ -78,14 +78,14 @@ func (r *Metal3DataReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	defer func() {
 		// Check if the object still exists before attempting to patch
 		var currentObj infrav1.Metal3Data
-		if err := r.Client.Get(ctx, req.NamespacedName, &currentObj); err != nil {
+		if err = r.Client.Get(ctx, req.NamespacedName, &currentObj); err != nil {
 			if apierrors.IsNotFound(err) {
 				metadataLog.Info("Metal3Data no longer exists, skipping patch")
 				return
 			}
 			metadataLog.Info("Failed to check if Metal3Data exists, attempting patch")
 		}
-		err := helper.Patch(ctx, metal3Data)
+		err = helper.Patch(ctx, metal3Data)
 		if err != nil {
 			metadataLog.Info("failed to Patch Metal3Data")
 			rerr = err

controllers/metal3datatemplate_controller.go

@@ -88,7 +88,7 @@ func (r *Metal3DataTemplateReconciler) Reconcile(ctx context.Context, req ctrl.R
 	}
 	// Always patch the Metal3DataTemplate exiting this function so we can persist any changes.
 	defer func() {
-		err := helper.Patch(ctx, metal3DataTemplate)
+		err = helper.Patch(ctx, metal3DataTemplate)
 		if err != nil {
 			log.Info("failed to Patch Metal3DataTemplate")
 			rerr = err