Removed potential nil pointer derefs and fixed linting issues

cmd/machine-controller-manager-cli/main.go

@@ -4,8 +4,8 @@ import (
 	"bytes"
 	"flag"
 	"fmt"
-	"io/ioutil"
 	"log"
+	"os"
 
 	"github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1"
 	"github.com/gardener/machine-controller-manager/pkg/driver"
@@ -119,7 +119,7 @@ func main() {
 
 // Read function decodes the yaml file passed to it
 func Read(fileName string, decodedObj interface{}) error {
-	m, err := ioutil.ReadFile(fileName)
+	m, err := os.ReadFile(fileName)
 	if err != nil {
 		log.Fatalf("Could not read %s: %s", fileName, err)
 	}

cmd/machine-controller-manager/app/controllermanager.go

@@ -330,16 +330,16 @@ func getAvailableResources(clientBuilder corecontroller.ClientBuilder) (map[sche
 		return nil, fmt.Errorf("failed to get api versions from server: %v: %v", healthzContent, err)
 	}
 
-	resourceMap, err := discoveryClient.ServerResources()
+	_, resources, err := discoveryClient.ServerGroupsAndResources()
 	if err != nil {
 		utilruntime.HandleError(fmt.Errorf("unable to get all supported resources from server: %v", err))
 	}
-	if len(resourceMap) == 0 {
+	if len(resources) == 0 {
 		return nil, fmt.Errorf("unable to get any supported resources from server")
 	}
 
 	allResources := map[schema.GroupVersionResource]bool{}
-	for _, apiResourceList := range resourceMap {
+	for _, apiResourceList := range resources {
 		version, err := schema.ParseGroupVersion(apiResourceList.GroupVersion)
 		if err != nil {
 			return nil, err

pkg/apis/machine/register.go

@@ -43,8 +43,7 @@ var (
 	// the code-generation can find it.
 	SchemeBuilder = runtime.NewSchemeBuilder(addKnownTypes)
 	// AddToScheme is exposed for API installation
-	AddToScheme        = SchemeBuilder.AddToScheme
-	localSchemeBuilder = &SchemeBuilder
+	AddToScheme = SchemeBuilder.AddToScheme
 )
 
 func addKnownTypes(scheme *runtime.Scheme) error {

pkg/controller/controller_utils.go

@@ -466,10 +466,10 @@ func validateControllerRef(controllerRef *metav1.OwnerReference) error {
 	if len(controllerRef.Kind) == 0 {
 		return fmt.Errorf("controllerRef has empty Kind")
 	}
-	if controllerRef.Controller == nil || *controllerRef.Controller != true {
+	if controllerRef.Controller == nil || !*controllerRef.Controller {
 		return fmt.Errorf("controllerRef.Controller is not set to true")
 	}
-	if controllerRef.BlockOwnerDeletion == nil || *controllerRef.BlockOwnerDeletion != true {
+	if controllerRef.BlockOwnerDeletion == nil || !*controllerRef.BlockOwnerDeletion {
 		return fmt.Errorf("controllerRef.BlockOwnerDeletion is not set")
 	}
 	return nil
@@ -814,11 +814,7 @@ func IsMachineActive(p *v1alpha1.Machine) bool {
 
 // IsMachineFailed checks if machine has failed
 func IsMachineFailed(p *v1alpha1.Machine) bool {
-	if p.Status.CurrentStatus.Phase == v1alpha1.MachineFailed {
-		return true
-	}
-
-	return false
+	return p.Status.CurrentStatus.Phase == v1alpha1.MachineFailed
 }
 
 // MachineKey is the function used to get the machine name from machine object
@@ -1019,6 +1015,10 @@ func RemoveAnnotationsOffNode(ctx context.Context, c clientset.Interface, nodeNa
 
 		// Remove the annotations from the node.
 		newNode, updated, err = annotationsutils.RemoveAnnotation(oldNodeCopy, annotations)
+		if err != nil {
+			klog.Errorf("Failed to remove annotations from node Node %s. Err: %v", nodeName, err)
+			return err
+		}
 
 		if !updated {
 			return nil
@@ -1030,7 +1030,7 @@ func RemoveAnnotationsOffNode(ctx context.Context, c clientset.Interface, nodeNa
 // GetAnnotationsFromNode returns all the annotations of the provided node.
 func GetAnnotationsFromNode(ctx context.Context, c clientset.Interface, nodeName string) (map[string]string, error) {
 
-	// Short circuit if annotation doesnt exist for limiting API calls.
+	// Short circuit if annotation doesn't exist for limiting API calls.
 	if nodeName == "" {
 		return nil, nil
 	}

pkg/controller/deployment_machineset_util.go

@@ -217,11 +217,6 @@ func isMachineAvailable(machine *v1alpha1.Machine) bool {
 }
 
 func isMachineReady(machine *v1alpha1.Machine) bool {
-
 	// TODO add more conditions
-	if machine.Status.CurrentStatus.Phase == v1alpha1.MachineRunning {
-		return true
-	}
-
-	return false
+	return machine.Status.CurrentStatus.Phase == v1alpha1.MachineRunning
 }

pkg/controller/deployment_sync.go

@@ -105,7 +105,7 @@ func (dc *controller) checkPausedConditions(ctx context.Context, d *v1alpha1.Mac
 	}
 
 	var err error
-	d, err = dc.controlMachineClient.MachineDeployments(d.Namespace).UpdateStatus(ctx, d, metav1.UpdateOptions{})
+	_, err = dc.controlMachineClient.MachineDeployments(d.Namespace).UpdateStatus(ctx, d, metav1.UpdateOptions{})
 	return err
 }
 
@@ -284,7 +284,7 @@ func (dc *controller) getNewMachineSet(ctx context.Context, d *v1alpha1.MachineD
 			}
 			dCopy := d.DeepCopy()
 			dCopy.Status = newStatus
-			if d, err = dc.controlMachineClient.MachineDeployments(dCopy.Namespace).UpdateStatus(ctx, dCopy, metav1.UpdateOptions{}); err != nil {
+			if _, err = dc.controlMachineClient.MachineDeployments(dCopy.Namespace).UpdateStatus(ctx, dCopy, metav1.UpdateOptions{}); err != nil {
 				return nil, err
 			}
 		}

pkg/controller/drain.go

@@ -486,7 +486,6 @@ func (o *DrainOptions) evictPodsWithoutPv(ctx context.Context, attemptEvict bool
 	for _, pod := range pods {
 		go o.evictPodWithoutPVInternal(ctx, attemptEvict, pod, policyGroupVersion, getPodFn, returnCh)
 	}
-	return
 }
 
 func sortPodsByPriority(pods []*corev1.Pod) {
@@ -550,7 +549,7 @@ func filterSharedPVs(pvMap map[string][]string) {
 	for pod, vols := range pvMap {
 		volList := []string{}
 		for _, vol := range vols {
-			if sharedVol[vol] == false {
+			if !sharedVol[vol] {
 				volList = append(volList, vol)
 			}
 		}
@@ -613,8 +612,6 @@ func (o *DrainOptions) evictPodsWithPv(ctx context.Context, attemptEvict bool, p
 			returnCh <- fmt.Errorf("Error deleting pod %s/%s from node %q", pod.Namespace, pod.Name, pod.Spec.NodeName)
 		}
 	}
-
-	return
 }
 
 func (o *DrainOptions) evictPodsWithPVInternal(ctx context.Context, attemptEvict bool, pods []*corev1.Pod, volMap map[string][]string,

pkg/controller/drain_test.go

@@ -257,7 +257,7 @@ var _ = Describe("drain", func() {
 					go func() {
 						defer wg.Done()
 						runPodDrainHandlers(pod)
-						fmt.Fprintf(GinkgoWriter, "Drained pod %s/%s in %s\n", pod.Namespace, pod.Name, time.Now().Sub(start).String())
+						fmt.Fprintf(GinkgoWriter, "Drained pod %s/%s in %s\n", pod.Namespace, pod.Name, time.Since(start).String())
 					}()
 
 					nEvictions++
@@ -289,7 +289,7 @@ var _ = Describe("drain", func() {
 					go func() {
 						defer wg.Done()
 						runPodDrainHandlers(pod)
-						fmt.Fprintf(GinkgoWriter, "Drained pod %s/%s in %s\n", pod.Namespace, pod.Name, time.Now().Sub(start).String())
+						fmt.Fprintf(GinkgoWriter, "Drained pod %s/%s in %s\n", pod.Namespace, pod.Name, time.Since(start).String())
 					}()
 				default:
 					err = fmt.Errorf("Expected type k8stesting.GetAction but got %T", action)

pkg/controller/machine.go

@@ -142,7 +142,7 @@ func (c *controller) reconcileClusterMachine(ctx context.Context, machine *v1alp
 	}()
 
 	if c.safetyOptions.MachineControllerFrozen && machine.DeletionTimestamp == nil {
-		message := "Machine controller has frozen. Retrying reconcile after 10 minutes"
+		message := "machine controller has frozen. Retrying reconcile after 10 minutes"
 		klog.V(3).Info(message)
 		return errors.New(message)
 	}
@@ -157,9 +157,9 @@ func (c *controller) reconcileClusterMachine(ctx context.Context, machine *v1alp
 	if err != nil {
 		return err
 	}
-	validationerr := validation.ValidateMachine(internalMachine)
-	if validationerr.ToAggregate() != nil && len(validationerr.ToAggregate().Errors()) > 0 {
-		klog.Errorf("Validation of Machine failed %s", validationerr.ToAggregate().Error())
+	validationErr := validation.ValidateMachine(internalMachine)
+	if validationErr.ToAggregate() != nil && len(validationErr.ToAggregate().Errors()) > 0 {
+		klog.Errorf("Validation of Machine failed %s", validationErr.ToAggregate().Error())
 		return nil
 	}
 
@@ -170,8 +170,8 @@ func (c *controller) reconcileClusterMachine(ctx context.Context, machine *v1alp
 		return nil
 	}
 
-	driver := driver.NewDriver(machine.Spec.ProviderID, secretData, machine.Spec.Class.Kind, MachineClass, machine.Name)
-	actualProviderID, err := driver.GetExisting()
+	d := driver.NewDriver(machine.Spec.ProviderID, secretData, machine.Spec.Class.Kind, MachineClass, machine.Name)
+	actualProviderID, err := d.GetExisting()
 	if err != nil {
 		return err
 	} else if actualProviderID == "fake" {
@@ -208,7 +208,7 @@ func (c *controller) reconcileClusterMachine(ctx context.Context, machine *v1alp
 
 	if machine.DeletionTimestamp != nil {
 		// Processing of delete event
-		if err := c.machineDelete(ctx, machine, driver); err != nil {
+		if err := c.machineDelete(ctx, machine, d); err != nil {
 			c.enqueueMachineAfter(machine, MachineEnqueueRetryPeriod)
 			return nil
 		}
@@ -222,7 +222,7 @@ func (c *controller) reconcileClusterMachine(ctx context.Context, machine *v1alp
 		if machine.Status.CurrentStatus.Phase == v1alpha1.MachineFailed {
 			return nil
 		} else if actualProviderID == "" {
-			if err := c.machineCreate(ctx, machine, driver); err != nil {
+			if err := c.machineCreate(ctx, machine, d); err != nil {
 				c.enqueueMachineAfter(machine, MachineEnqueueRetryPeriod)
 				return nil
 			}
@@ -305,7 +305,7 @@ func (c *controller) getMachineFromNode(nodeName string) (*v1alpha1.Machine, err
 	machines, _ := c.machineLister.List(selector)
 
 	if len(machines) > 1 {
-		return nil, errors.New("Multiple machines matching node")
+		return nil, errors.New("multiple machines matching node")
 	} else if len(machines) < 1 {
 		return nil, nil
 	}
@@ -334,18 +334,18 @@ func (c *controller) updateMachineState(ctx context.Context, machine *v1alpha1.M
 				nodeName = node.Name
 				clone := machine.DeepCopy()
 				clone.Status.Node = nodeName
-				clone, err = c.controlMachineClient.Machines(clone.Namespace).UpdateStatus(ctx, clone, metav1.UpdateOptions{})
+				_, err = c.controlMachineClient.Machines(clone.Namespace).UpdateStatus(ctx, clone, metav1.UpdateOptions{})
 				if err != nil {
 					klog.Errorf("Could not update status of the machine-object %s due to error %v", machine.Name, err)
 					return machine, err
 				}
 				break
 			}
 		}
-		// Couldnt adopt any node-object.
+		// Couldn't adopt any node-object.
 		if nodeName == "" {
 			// There are no objects mapped to this machine object
-			// Hence node status need not be propogated to machine object
+			// Hence node status need not be propagated to machine object
 			return machine, nil
 		}
 	}
@@ -519,7 +519,7 @@ func (c *controller) machineCreate(ctx context.Context, machine *v1alpha1.Machin
 				}
 
 				// Return with error
-				return fmt.Errorf("Couldn't find machine object, hence deleted orphan VM")
+				return fmt.Errorf("couldn't find machine object, hence deleted orphan VM")
 			}
 
 			klog.Warningf("Machine GET failed for %q. Retrying, error: %s", machineName, err)
@@ -660,7 +660,7 @@ func (c *controller) machineDelete(ctx context.Context, machine *v1alpha1.Machin
 		timeOutOccurred = utiltime.HasTimeOutOccurred(*machine.DeletionTimestamp, timeOutDuration)
 
 		if forceDeleteLabelPresent || timeOutOccurred {
-			// To perform forceful machine drain/delete either one of the below conditions must be satified
+			// To perform forceful machine drain/delete either one of the below conditions must be satisfied
 			// 1. force-deletion: "True" label must be present
 			// 2. Deletion operation is more than drain-timeout minutes old
 			forceDeleteMachine = true
@@ -699,7 +699,7 @@ func (c *controller) machineDelete(ctx context.Context, machine *v1alpha1.Machin
 		}
 
 		if machineID != "" && nodeName != "" {
-			// Begin drain logic only when the nodeName & providerID exist's for the machine
+			// Begin drain logic only when the nodeName & providerID exists for the machine
 			buf := bytes.NewBuffer([]byte{})
 			errBuf := bytes.NewBuffer([]byte{})
 
@@ -792,7 +792,7 @@ func (c *controller) machineDelete(ctx context.Context, machine *v1alpha1.Machin
 			// Delete node object
 			err = c.targetCoreClient.CoreV1().Nodes().Delete(ctx, nodeName, metav1.DeleteOptions{})
 			if err != nil && !apierrors.IsNotFound(err) {
-				// If its an error, and anyother error than object not found
+				// If it's an error, and another error than object not found
 				message := fmt.Sprintf("Deletion of Node Object %q failed due to error: %s", nodeName, err)
 				lastOperation := v1alpha1.LastOperation{
 					Description:    message,
@@ -812,7 +812,7 @@ func (c *controller) machineDelete(ctx context.Context, machine *v1alpha1.Machin
 		// Delete machine object
 		err = c.controlMachineClient.Machines(machine.Namespace).Delete(ctx, machine.Name, metav1.DeleteOptions{})
 		if err != nil && !apierrors.IsNotFound(err) {
-			// If its an error, and anyother error than object not found
+			// If it's an error, and another error than object not found
 			klog.Errorf("Deletion of Machine Object %q failed due to error: %s", machine.Name, err)
 			return err
 		}
@@ -919,7 +919,7 @@ func (c *controller) updateMachineConditions(ctx context.Context, machine *v1alp
 		objectRequiresUpdate = true
 
 	} else if c.isHealthy(clone) && clone.Status.CurrentStatus.Phase != v1alpha1.MachineRunning {
-		// If machine is healhy and current machinePhase is not running.
+		// If machine is healthy and current machinePhase is not running.
 		// indicates that the machine is not healthy and status needs to be updated.
 
 		if clone.Status.LastOperation.Type == v1alpha1.MachineOperationCreate &&
@@ -1084,7 +1084,7 @@ func (c *controller) checkMachineTimeout(ctx context.Context, machine *v1alpha1.
 					timeOutDuration,
 				)
 			} else {
-				// Timeour occurred due to machine being unhealthy for too long
+				// Timeout occurred due to machine being unhealthy for too long
 				description = fmt.Sprintf(
 					"Machine %s is not healthy since %s minutes. Changing status to failed. Node Conditions: %+v",
 					machine.Name,

pkg/controller/machine_safety.go

@@ -163,7 +163,7 @@ func (c *controller) reconcileClusterMachineSafetyAPIServer(key string) error {
 				// If timeout has not started
 				c.safetyOptions.APIserverInactiveStartTime = time.Now()
 			}
-			if time.Now().Sub(c.safetyOptions.APIserverInactiveStartTime) > statusCheckTimeout {
+			if time.Since(c.safetyOptions.APIserverInactiveStartTime) > statusCheckTimeout {
 				// If APIServer has been down for more than statusCheckTimeout
 				c.safetyOptions.MachineControllerFrozen = true
 				klog.V(2).Infof("SafetyController: Freezing Machine Controller")

pkg/controller/machine_safety_test.go

@@ -221,8 +221,6 @@ var _ = Describe("#machine_safety", func() {
 		machineDeploymentLabels      map[string]string
 		machineDeploymentConditions  []v1alpha1.MachineDeploymentCondition
 	}
-	type action struct {
-	}
 	type expect struct {
 		machineSetAnnotations        map[string]string
 		machineSetLabels             map[string]string
@@ -233,7 +231,6 @@ var _ = Describe("#machine_safety", func() {
 	}
 	type data struct {
 		setup  setup
-		action action
 		expect expect
 	}
 
@@ -807,11 +804,11 @@ var _ = Describe("#machine_safety", func() {
 			c.safetyOptions.APIserverInactiveStartTime = apiServerInactiveStartTime
 			c.safetyOptions.MachineControllerFrozen = preMachineControllerIsFrozen
 			if !controlAPIServerIsUp {
-				trackers.ControlMachine.SetError("APIServer is Not Reachable")
-				trackers.ControlCore.SetError("APIServer is Not Reachable")
+				Expect(trackers.ControlMachine.SetError("APIServer is Not Reachable")).NotTo(HaveOccurred())
+				Expect(trackers.ControlCore.SetError("APIServer is Not Reachable")).NotTo(HaveOccurred())
 			}
 			if !targetAPIServerIsUp {
-				trackers.TargetCore.SetError("APIServer is Not Reachable")
+				Expect(trackers.TargetCore.SetError("APIServer is Not Reachable")).NotTo(HaveOccurred())
 			}
 
 			c.reconcileClusterMachineSafetyAPIServer("")