Merge pull request #3546 from hashicorp/f-heuristic

Better interface selection heuristic

CHANGELOG.md

@@ -17,6 +17,8 @@ IMPROVEMENTS:
  * cli: Allocation create and modify times are displayed in a human readable
    relative format like `6 h ago` [GH-3449]
  * client: Added metrics to track state transitions of allocations [GH-3061]
+ * client: When `network_interface` is unspecified use interface attached to
+   default route [GH-3546]
  * driver/docker: Detect OOM kill event [GH-3459]
  * driver/docker: Adds support for adding host device to container via `--device` [GH-2938]
  * driver/qemu: Support graceful shutdowns on unix platforms [GH-3411]

client/fingerprint/network.go

@@ -5,6 +5,7 @@ import (
 	"log"
 	"net"
 
+	sockaddr "github.com/hashicorp/go-sockaddr"
 	"github.com/hashicorp/nomad/client/config"
 	"github.com/hashicorp/nomad/nomad/structs"
 )
@@ -167,47 +168,25 @@ func (f *NetworkFingerprint) createNetworkResources(throughput int, intf *net.In
 	return nwResources, nil
 }
 
-// Checks if the device is marked UP by the operator
-func (f *NetworkFingerprint) isDeviceEnabled(intf *net.Interface) bool {
-	return intf.Flags&net.FlagUp != 0
-}
-
-// Checks if the device has any IP address configured
-func (f *NetworkFingerprint) deviceHasIpAddress(intf *net.Interface) bool {
-	addrs, err := f.interfaceDetector.Addrs(intf)
-	return err == nil && len(addrs) != 0
-}
-
-func (n *NetworkFingerprint) isDeviceLoopBackOrPointToPoint(intf *net.Interface) bool {
-	return intf.Flags&(net.FlagLoopback|net.FlagPointToPoint) != 0
-}
-
-// Returns the interface with the name passed by user
-// If the name is blank then it iterates through all the devices
-// and finds one which is routable and marked as UP
-// It excludes PPP and lo devices unless they are specifically asked
+// Returns the interface with the name passed by user. If the name is blank, we
+// use the interface attached to the default route.
 func (f *NetworkFingerprint) findInterface(deviceName string) (*net.Interface, error) {
-	var interfaces []net.Interface
-	var err error
-
-	if deviceName != "" {
-		return f.interfaceDetector.InterfaceByName(deviceName)
-	}
-
-	var intfs []net.Interface
-
-	if intfs, err = f.interfaceDetector.Interfaces(); err != nil {
-		return nil, err
-	}
+	// If we aren't given a device, look it up by using the interface with the default route
+	if deviceName == "" {
+		ri, err := sockaddr.NewRouteInfo()
+		if err != nil {
+			return nil, err
+		}
 
-	for _, intf := range intfs {
-		if f.isDeviceEnabled(&intf) && !f.isDeviceLoopBackOrPointToPoint(&intf) && f.deviceHasIpAddress(&intf) {
-			interfaces = append(interfaces, intf)
+		defaultIfName, err := ri.GetDefaultInterfaceName()
+		if err != nil {
+			return nil, err
 		}
+		if defaultIfName == "" {
+			return nil, fmt.Errorf("no network_interface given and failed to determine interface attached to default route")
+		}
+		deviceName = defaultIfName
 	}
 
-	if len(interfaces) == 0 {
-		return nil, nil
-	}
-	return &interfaces[0], nil
+	return f.interfaceDetector.InterfaceByName(deviceName)
 }

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{lo, eth0, eth1, eth2, eth3, eth4}, nil
 }
 
 func (n *NetworkInterfaceDetectorMultipleInterfaces) InterfaceByName(name string) (*net.Interface, error) {
@@ -224,23 +225,6 @@ func TestNetworkFingerprint_basic(t *testing.T) {
 	}
 }
 
-func TestNetworkFingerprint_no_devices(t *testing.T) {
-	f := &NetworkFingerprint{logger: testLogger(), interfaceDetector: &NetworkIntefaceDetectorNoDevices{}}
-	node := &structs.Node{
-		Attributes: make(map[string]string),
-	}
-	cfg := &config.Config{NetworkSpeed: 100}
-
-	ok, err := f.Fingerprint(cfg, node)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-
-	if ok {
-		t.Fatalf("ok: %v", ok)
-	}
-}
-
 func TestNetworkFingerprint_default_device_absent(t *testing.T) {
 	f := &NetworkFingerprint{logger: testLogger(), interfaceDetector: &NetworkInterfaceDetectorOnlyLo{}}
 	node := &structs.Node{
@@ -301,61 +285,6 @@ func TestNetworkFingerPrint_default_device(t *testing.T) {
 	}
 }
 
-func TestNetworkFingerPrint_excludelo_down_interfaces(t *testing.T) {
-	f := &NetworkFingerprint{logger: testLogger(), interfaceDetector: &NetworkInterfaceDetectorMultipleInterfaces{}}
-	node := &structs.Node{
-		Attributes: make(map[string]string),
-	}
-	cfg := &config.Config{NetworkSpeed: 100}
-
-	ok, err := f.Fingerprint(cfg, node)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	if !ok {
-		t.Fatalf("should apply")
-	}
-
-	assertNodeAttributeContains(t, node, "unique.network.ip-address")
-
-	ip := node.Attributes["unique.network.ip-address"]
-	match := net.ParseIP(ip)
-	if match == nil {
-		t.Fatalf("Bad IP match: %s", ip)
-	}
-
-	if node.Resources == nil || len(node.Resources.Networks) == 0 {
-		t.Fatal("Expected to find Network Resources")
-	}
-
-	// Test at least the first Network Resource
-	net := node.Resources.Networks[0]
-	if net.IP == "" {
-		t.Fatal("Expected Network Resource to have an IP")
-	}
-	if net.CIDR == "" {
-		t.Fatal("Expected Network Resource to have a CIDR")
-	}
-	if net.Device != "eth0" {
-		t.Fatal("Expected Network Resource to be eth0. Actual: ", net.Device)
-	}
-	if net.MBits == 0 {
-		t.Fatal("Expected Network Resource to have a non-zero bandwidth")
-	}
-
-	// Test the CIDR of the IPs
-	if node.Resources.Networks[0].CIDR != "100.64.0.0/32" {
-		t.Fatalf("bad CIDR: %v", node.Resources.Networks[0].CIDR)
-	}
-	if node.Resources.Networks[1].CIDR != "2001:db8:85a3::/128" {
-		t.Fatalf("bad CIDR: %v", node.Resources.Networks[1].CIDR)
-	}
-	// Ensure that the link local address isn't fingerprinted
-	if len(node.Resources.Networks) != 2 {
-		t.Fatalf("bad number of IPs %v", len(node.Resources.Networks))
-	}
-}
-
 func TestNetworkFingerPrint_LinkLocal_Allowed(t *testing.T) {
 	f := &NetworkFingerprint{logger: testLogger(), interfaceDetector: &NetworkInterfaceDetectorMultipleInterfaces{}}
 	node := &structs.Node{

website/source/docs/agent/configuration/client.html.md

@@ -49,11 +49,12 @@ client {
 - `meta` `(map[string]string: nil)` - Specifies a key-value map that annotates
   with user-defined metadata.
 
-- `network_interface` `(string: "lo | lo0")` - Specifies the name of the
-  interface to force network fingerprinting on. This defaults to the loopback
-  interface. All addresses on the interface are fingerprinted except the ones
-  which are scoped local for IPv6. When allocating ports for tasks, the
-  scheduler will choose from the IPs of the fingerprinted interface.
+- `network_interface` `(string: varied)` - Specifies the name of the interface
+  to force network fingerprinting on. When run in dev mode, this defaults to the
+  loopback interface. When not in dev mode, the interface attached to the
+  default route is used. All IP addresses except those scoped local for IPV6 on
+  the chosen interface are fingerprinted. The scheduler chooses from those IP
+  addresses when allocating ports for tasks.
 
 - `network_speed` `(int: 0)` - Specifies an override for the network link speed.
   This value, if set, overrides any detected or defaulted link speed. Most