Merge pull request openshift#751 from kikisdeliveryservice/refactor-r…

…eadychecks

mcc: refactor node controller to use error check helper function

pkg/controller/node/node_controller.go

@@ -205,17 +205,10 @@ func (ctrl *Controller) updateNode(old, cur interface{}) {
 	var changed bool
 	oldReadyErr := checkNodeReady(oldNode)
 	newReadyErr := checkNodeReady(curNode)
-	var oldReady, newReady string
-	if oldReadyErr != nil {
-		oldReady = oldReadyErr.Error()
-	} else {
-		oldReady = ""
-	}
-	if newReadyErr != nil {
-		newReady = newReadyErr.Error()
-	} else {
-		newReady = ""
-	}
+
+	oldReady := getErrorString(oldReadyErr)
+	newReady := getErrorString(newReadyErr)
+
 	if oldReady != newReady {
 		changed = true
 		if newReadyErr != nil {
@@ -471,7 +464,7 @@ func (ctrl *Controller) syncMachineConfigPool(key string) error {
 	maxunavail, err := maxUnavailable(pool, nodes)
 	if err != nil {
 		return err
-	}	
+	}
 
 	candidates := getCandidateMachines(pool, nodes, maxunavail)
 	for _, node := range candidates {
@@ -573,3 +566,11 @@ func maxUnavailable(pool *mcfgv1.MachineConfigPool, nodes []*corev1.Node) (int,
 	}
 	return maxunavail, nil
 }
+
+// getErrorString returns error string if not nil and empty string if error is nil
+func getErrorString(err error) string {
+	if err != nil {
+		return err.Error()
+	}
+	return ""
+}

pkg/controller/node/status.go

@@ -165,7 +165,7 @@ func isNodeMCDState(node *corev1.Node, state string) bool {
 func isNodeMCDFailing(node *corev1.Node) bool {
 	if node.Annotations[daemonconsts.CurrentMachineConfigAnnotationKey] == node.Annotations[daemonconsts.DesiredMachineConfigAnnotationKey] {
 		return false
-	}	
+	}
 	return isNodeMCDState(node, daemonconsts.MachineConfigDaemonStateDegraded) ||
 		isNodeMCDState(node, daemonconsts.MachineConfigDaemonStateUnreconcilable)
 }
@@ -204,18 +204,18 @@ func checkNodeReady(node *corev1.Node) error {
 		// - NodeOutOfDisk condition status is ConditionFalse,
 		// - NodeNetworkUnavailable condition status is ConditionFalse.
 		if cond.Type == corev1.NodeReady && cond.Status != corev1.ConditionTrue {
-			return fmt.Errorf("Node %s is reporting NotReady", node.Name)
+			return fmt.Errorf("node %s is reporting NotReady", node.Name)
 		}
 		if cond.Type == corev1.NodeOutOfDisk && cond.Status != corev1.ConditionFalse {
-			return fmt.Errorf("Node %s is reporting OutOfDisk", node.Name)
+			return fmt.Errorf("node %s is reporting OutOfDisk", node.Name)
 		}
 		if cond.Type == corev1.NodeNetworkUnavailable && cond.Status != corev1.ConditionFalse {
-			return fmt.Errorf("Node %s is reporting NetworkUnavailable", node.Name)
+			return fmt.Errorf("node %s is reporting NetworkUnavailable", node.Name)
 		}
 	}
 	// Ignore nodes that are marked unschedulable
 	if node.Spec.Unschedulable {
-		return fmt.Errorf("Node %s is reporting Unschedulable", node.Name)
+		return fmt.Errorf("node %s is reporting Unschedulable", node.Name)
 	}
 	return nil
 }