fix(node.go): improve logic for GetNodeObject

Before the logic ran like the following in terms of preference:

1. Prefer environment var NODE_NAME
2. `Use os.Hostname()`
3. Fallback to `--hostname-override` passed by user

This didn't make a whole lot of sense, as `--hostname-override` is
directly, and supposedly intentionally set by the user, therefore it
should be the MOST preferred, not the least preferred. Additionally,
none of the errors encountered were passed back to the user so that
future conditions could be considered, so if there was an error at the
API level, that error was swallowed. Now the logic looks like:

1. Prefer `--hostname-override` if it is set. If it is set and we
   weren't able to resolve to a node object, return the error
2. Use environment var NODE_NAME if it is set. If it is set and we
   weren't able to resolve to a node object, return the error
3. Fallback to `os.Hostname()`. If we weren't able to resolve to a node
   object then return the error and give the user options

Author: aauren

pkg/controllers/netpol/network_policy_controller_test.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"fmt"
 	"net"
+	"os"
 	"strings"
 	"sync"
 	"testing"
@@ -754,6 +755,7 @@ func (ips *fakeIPSet) Name(name string) string {
 }
 
 func TestNetworkPolicyController(t *testing.T) {
+	curHostname, _ := os.Hostname()
 	testCases := []tNetPolConfigTestCase{
 		{
 			"Default options are successful",
@@ -765,7 +767,8 @@ func TestNetworkPolicyController(t *testing.T) {
 			"Missing nodename fails appropriately",
 			newMinimalKubeRouterConfig([]string{""}, "", "", nil, nil, false),
 			true,
-			"failed to identify the node by NODE_NAME, hostname or --hostname-override",
+			fmt.Sprintf("failed to identify the node by NODE_NAME, %s or --hostname-override: nodes \"%s\""+
+				" not found", curHostname, curHostname),
 		},
 		{
 			"Test bad cluster CIDR (not properly formatting ip address)",

pkg/utils/node.go

@@ -17,31 +17,33 @@ import (
 
 // GetNodeObject returns the node API object for the node
 func GetNodeObject(clientset kubernetes.Interface, hostnameOverride string) (*apiv1.Node, error) {
+	// if env NODE_NAME is not set and node is not registered with hostname, then use host name override
+	if hostnameOverride != "" {
+		node, err := clientset.CoreV1().Nodes().Get(context.Background(), hostnameOverride, metav1.GetOptions{})
+		if err != nil {
+			return nil, fmt.Errorf("unable to get node %s, due to: %v", hostnameOverride, err)
+		}
+		return node, nil
+	}
+
 	// assuming kube-router is running as pod, first check env NODE_NAME
 	nodeName := os.Getenv("NODE_NAME")
 	if nodeName != "" {
 		node, err := clientset.CoreV1().Nodes().Get(context.Background(), nodeName, metav1.GetOptions{})
-		if err == nil {
-			return node, nil
+		if err != nil {
+			return nil, fmt.Errorf("unable to get node %s, due to: %v", nodeName, err)
 		}
+		return node, nil
 	}
 
 	// if env NODE_NAME is not set then check if node is register with hostname
 	hostName, _ := os.Hostname()
 	node, err := clientset.CoreV1().Nodes().Get(context.Background(), hostName, metav1.GetOptions{})
-	if err == nil {
-		return node, nil
-	}
-
-	// if env NODE_NAME is not set and node is not registered with hostname, then use host name override
-	if hostnameOverride != "" {
-		node, err = clientset.CoreV1().Nodes().Get(context.Background(), hostnameOverride, metav1.GetOptions{})
-		if err == nil {
-			return node, nil
-		}
+	if err != nil {
+		return nil, fmt.Errorf("failed to identify the node by NODE_NAME, %s or --hostname-override: %v", hostName, err)
 	}
 
-	return nil, fmt.Errorf("failed to identify the node by NODE_NAME, hostname or --hostname-override")
+	return node, nil
 }
 
 // GetPrimaryNodeIP returns the most valid external facing IP address for a node.

pkg/utils/node_test.go

@@ -38,7 +38,7 @@ func Test_GetNodeObject(t *testing.T) {
 			nil,
 		},
 		{
-			"node with hostname overrie exists",
+			"node with hostname override exists",
 			"something-else",
 			"test-node",
 			&apiv1.Node{
@@ -50,8 +50,8 @@ func Test_GetNodeObject(t *testing.T) {
 		},
 		{
 			"node with current hostname exists",
-			"something-else",
-			"something-else",
+			"",
+			"",
 			&apiv1.Node{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: curHostname,
@@ -68,7 +68,7 @@ func Test_GetNodeObject(t *testing.T) {
 					Name: "another-node",
 				},
 			},
-			errors.New("failed to identify the node by NODE_NAME, hostname or --hostname-override"),
+			errors.New("unable to get node test-node, due to: nodes \"test-node\" not found"),
 		},
 	}