From 0523d9375cc9b7de423e35da81f65e75612a879f Mon Sep 17 00:00:00 2001
From: Alex Dadgar <alex.dadgar@gmail.com>
Date: Mon, 13 Nov 2017 15:10:02 -0800
Subject: [PATCH] Better interface selection heuristic

This PR introduces a better interface selection heuristic such that we
select interfaces with globally routable unicast addresses over link
local addresses.

Fixes https://github.com/hashicorp/nomad/issues/3487
---
 client/fingerprint/network.go      | 55 ++++++++++++++++++++++++++----
 client/fingerprint/network_test.go |  7 ++--
 2 files changed, 53 insertions(+), 9 deletions(-)

diff --git a/client/fingerprint/network.go b/client/fingerprint/network.go
index beeca77b9f64..25effe93c13d 100644
--- a/client/fingerprint/network.go
+++ b/client/fingerprint/network.go
@@ -187,7 +187,6 @@ func (n *NetworkFingerprint) isDeviceLoopBackOrPointToPoint(intf *net.Interface)
 // and finds one which is routable and marked as UP
 // It excludes PPP and lo devices unless they are specifically asked
 func (f *NetworkFingerprint) findInterface(deviceName string) (*net.Interface, error) {
-	var interfaces []net.Interface
 	var err error
 
 	if deviceName != "" {
@@ -200,14 +199,56 @@ func (f *NetworkFingerprint) findInterface(deviceName string) (*net.Interface, e
 		return nil, err
 	}
 
+	// Since we aren't given an interface to use we have to pick using a
+	// heuristic. To choose between interfaces we avoid down interfaces, those
+	// that are loopback, point to point, or don't have an address. Of the
+	// remaining interfaces we rank them positively for each global unicast
+	// address and slightly negatively for a link local address. This makes
+	// us select interfaces that have more globally routable address over those
+	// that do not.
+	var bestChoice *net.Interface
+	bestScore := -1000
 	for _, intf := range intfs {
-		if f.isDeviceEnabled(&intf) && !f.isDeviceLoopBackOrPointToPoint(&intf) && f.deviceHasIpAddress(&intf) {
-			interfaces = append(interfaces, intf)
+		// We require a non-loopback interface that is enabled with a valid
+		// address.
+		if !f.isDeviceEnabled(&intf) || f.isDeviceLoopBackOrPointToPoint(&intf) || !f.deviceHasIpAddress(&intf) {
+			continue
+		}
+
+		addrs, err := f.interfaceDetector.Addrs(&intf)
+		if err != nil {
+			return nil, err
 		}
-	}
 
-	if len(interfaces) == 0 {
-		return nil, nil
+		seenLinkLocal := false
+		intfScore := 0
+		for _, addr := range addrs {
+			// Find the IP Addr and the CIDR from the Address
+			var ip net.IP
+			switch v := (addr).(type) {
+			case *net.IPNet:
+				ip = v.IP
+			case *net.IPAddr:
+				ip = v.IP
+			}
+
+			if !seenLinkLocal && (ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast()) {
+				intfScore -= 1
+			}
+
+			if ip.IsGlobalUnicast() {
+				intfScore += 10
+			}
+		}
+
+		if intfScore > bestScore {
+			// Make a copy on the stack so we grab a pointer to the correct
+			// interface
+			local := intf
+			bestChoice = &local
+			bestScore = intfScore
+		}
 	}
-	return &interfaces[0], nil
+
+	return bestChoice, nil
 }
diff --git a/client/fingerprint/network_test.go b/client/fingerprint/network_test.go
index cb17d9e7eb6c..29586e5d74b1 100644
--- a/client/fingerprint/network_test.go
+++ b/client/fingerprint/network_test.go
@@ -113,7 +113,8 @@ type NetworkInterfaceDetectorMultipleInterfaces struct {
 }
 
 func (n *NetworkInterfaceDetectorMultipleInterfaces) Interfaces() ([]net.Interface, error) {
-	return []net.Interface{lo, eth0, eth1, eth2}, nil
+	// Return link local first to test we don't prefer it
+	return []net.Interface{eth3, lo, eth0, eth1, eth2, eth4}, nil
 }
 
 func (n *NetworkInterfaceDetectorMultipleInterfaces) InterfaceByName(name string) (*net.Interface, error) {
@@ -301,7 +302,9 @@ func TestNetworkFingerPrint_default_device(t *testing.T) {
 	}
 }
 
-func TestNetworkFingerPrint_excludelo_down_interfaces(t *testing.T) {
+// This tests that we prefer skipping link local and down interfaces as well as
+// those without global unicast addresses
+func TestNetworkFingerPrint_preferential_interfaces(t *testing.T) {
 	f := &NetworkFingerprint{logger: testLogger(), interfaceDetector: &NetworkInterfaceDetectorMultipleInterfaces{}}
 	node := &structs.Node{
 		Attributes: make(map[string]string),