From 61ded1b1d57409d57465820e705491b41847aa2b Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Tue, 27 Oct 2015 14:14:25 -0700 Subject: [PATCH 01/17] Using the Go stdlib APIs to detect IP Address of a device --- client/fingerprint/network_unix.go | 118 ++++++----------------------- 1 file changed, 25 insertions(+), 93 deletions(-) diff --git a/client/fingerprint/network_unix.go b/client/fingerprint/network_unix.go index 020c99516..de06191e3 100644 --- a/client/fingerprint/network_unix.go +++ b/client/fingerprint/network_unix.go @@ -10,7 +10,6 @@ import ( "net" "os/exec" "regexp" - "runtime" "strconv" "strings" @@ -46,10 +45,10 @@ func (f *NetworkFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) return false, err } - for _, i := range intfs { - if (i.Flags&net.FlagUp != 0) && (i.Flags&(net.FlagLoopback|net.FlagPointToPoint) == 0) { - if ip := f.ipAddress(i.Name); ip != "" { - defaultDevice = i.Name + for _, intf := range intfs { + if f.isDeviceEnabled(&intf) && f.isDeviceLoopBackOrPointToPoint(&intf) && f.deviceHasIpAddress(&intf) { + if ip, err := f.ipAddress(&intf); err == nil { + defaultDevice = intf.Name node.Attributes["network.ip-address"] = ip newNetwork.IP = ip newNetwork.CIDR = newNetwork.IP + "/32" @@ -150,99 +149,32 @@ func (f *NetworkFingerprint) linkSpeedEthtool(path, device string) int { return mbs } -// ipAddress returns the first IPv4 address on the configured default interface -// Tries Golang native functions and falls back onto ifconfig -func (f *NetworkFingerprint) ipAddress(device string) string { - if ip, err := f.nativeIpAddress(device); err == nil { - return ip +func (f *NetworkFingerprint) ipAddress(intf *net.Interface) (string, error) { + var ( + addrs []net.Addr + err error + ) + if addrs, err = intf.Addrs(); err != nil { + return "", err } - return f.ifConfig(device) + if len(addrs) == 0 { + return "", errors.New(fmt.Sprintf("Interface %s has no IP address", intf.Name)) + } + return addrs[0].String(), nil } -func (f *NetworkFingerprint) nativeIpAddress(device string) (string, error) { - // Find IP address on configured interface - var ip string - ifaces, err := net.Interfaces() - if err != nil { - return "", errors.New("could not retrieve interface list") - } - - // TODO: should we handle IPv6 here? How do we determine precedence? - for _, i := range ifaces { - if i.Name != device { - continue - } - - addrs, err := i.Addrs() - if err != nil { - return "", errors.New("could not retrieve interface IP addresses") - } - - for _, a := range addrs { - switch v := a.(type) { - case *net.IPNet: - if v.IP.To4() != nil { - ip = v.IP.String() - } - case *net.IPAddr: - if v.IP.To4() != nil { - ip = v.IP.String() - } - } - } - } - - if net.ParseIP(ip) == nil { - return "", errors.New(fmt.Sprintf("could not parse IP address `%s`", ip)) - } - - return ip, nil +func (f *NetworkFingerprint) isDeviceEnabled(intf *net.Interface) bool { + return intf.Flags&net.FlagUp != 0 } -// ifConfig returns the IP Address for this node according to ifConfig, for the -// specified device. -func (f *NetworkFingerprint) ifConfig(device string) string { - ifConfigPath, _ := exec.LookPath("ifconfig") - if ifConfigPath == "" { - f.logger.Println("[WARN] fingerprint.network: ifconfig not found") - return "" +func (f *NetworkFingerprint) deviceHasIpAddress(intf *net.Interface) bool { + if addrs, err := intf.Addrs(); err == nil { + return len(addrs) > 0 } - - outBytes, err := exec.Command(ifConfigPath, device).Output() - if err != nil { - f.logger.Printf("[WARN] fingerprint.network: Error calling ifconfig (%s %s): %v", ifConfigPath, device, err) - return "" - } - - // Parse out the IP address returned from ifconfig for this device - // Tested on Ubuntu, the matching part of ifconfig output for eth0 is like - // so: - // inet addr:10.0.2.15 Bcast:10.0.2.255 Mask:255.255.255.0 - // For OS X and en0, we have: - // inet 192.168.0.7 netmask 0xffffff00 broadcast 192.168.0.255 - output := strings.TrimSpace(string(outBytes)) - - // re is a regular expression, which can vary based on the OS - var re *regexp.Regexp - - if "darwin" == runtime.GOOS { - re = regexp.MustCompile("inet [0-9].+") - } else { - re = regexp.MustCompile("inet addr:[0-9].+") - } - args := strings.Split(re.FindString(output), " ") - - var ip string - if len(args) > 1 { - ip = strings.TrimPrefix(args[1], "addr:") - } - - // validate what we've sliced out is a valid IP - if net.ParseIP(ip) == nil { - f.logger.Printf("[WARN] fingerprint.network: Unable to parse IP in output of '%s %s'", ifConfigPath, device) - return "" - } - - return ip + return false +} + +func (n *NetworkFingerprint) isDeviceLoopBackOrPointToPoint(intf *net.Interface) bool { + return intf.Flags&(net.FlagLoopback|net.FlagPointToPoint) == 0 } From 48d71cc20ab686b972224268f52769ee5ee97f83 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 28 Oct 2015 11:11:13 -0700 Subject: [PATCH 02/17] Exctracted a method to detect network interface --- client/fingerprint/network_unix.go | 84 ++++++++++++++++++------------ 1 file changed, 50 insertions(+), 34 deletions(-) diff --git a/client/fingerprint/network_unix.go b/client/fingerprint/network_unix.go index 52a2b3426..4ca815184 100644 --- a/client/fingerprint/network_unix.go +++ b/client/fingerprint/network_unix.go @@ -32,45 +32,34 @@ func NewNetworkFingerprinter(logger *log.Logger) Fingerprint { func (f *NetworkFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) { // newNetwork is populated and addded to the Nodes resources newNetwork := &structs.NetworkResource{} - defaultDevice := "" - ip := "" + var ip string - // 1. Use user-defined network device - // 2. Use first interface found in the system for non-dev mode. (dev mode uses lo by default.) - if cfg.NetworkInterface != "" { - defaultDevice = cfg.NetworkInterface - if intf, err := net.InterfaceByName(defaultDevice); err == nil { - ip, _ = f.ipAddress(intf) - } - - } else { - - intfs, err := net.Interfaces() - if err != nil { - return false, err - } - - for _, intf := range intfs { - if f.isDeviceEnabled(&intf) && f.isDeviceLoopBackOrPointToPoint(&intf) && f.deviceHasIpAddress(&intf) { - var err error - if ip, err = f.ipAddress(&intf); err == nil { - defaultDevice = intf.Name - break - } - } - } + interfaces, err := f.findInterfaces(cfg.NetworkInterface) + if err != nil { + f.logger.Println(fmt.Sprintf("[DEBUG] Error while detecting network interface during fingerprinting: %s", err.Error())) + return false, err } - if (defaultDevice != "") && (ip != "") { - newNetwork.Device = defaultDevice - node.Attributes["network.ip-address"] = ip - newNetwork.IP = ip - newNetwork.CIDR = newNetwork.IP + "/32" - } else { - return false, fmt.Errorf("Unable to find any network interface which has IP address") + if len(interfaces) == 0 { + f.logger.Println("[DEBUG] No network interfaces were detected") + return false, errors.New("Unable to find any interface") } - if throughput := f.linkSpeed(defaultDevice); throughput > 0 { + // Use the first interface that we have detected. + intf := interfaces[0] + if ip, err = f.ipAddress(intf); err != nil { + f.logger.Println("[DEBUG] Unable to find IP address of interface ", intf.Name) + return false, err + } + + newNetwork.Device = intf.Name + node.Attributes["network.ip-address"] = ip + newNetwork.IP = ip + newNetwork.CIDR = newNetwork.IP + "/32" + + f.logger.Println("[DEBUG] Detected interface ", intf.Name, " with IP ", ip, " during fingerprinting") + + if throughput := f.linkSpeed(intf.Name); throughput > 0 { newNetwork.MBits = throughput } else { f.logger.Printf("[DEBUG] fingerprint.network: Unable to read link speed; setting to default %v", cfg.NetworkSpeed) @@ -187,3 +176,30 @@ func (f *NetworkFingerprint) deviceHasIpAddress(intf *net.Interface) bool { func (n *NetworkFingerprint) isDeviceLoopBackOrPointToPoint(intf *net.Interface) bool { return intf.Flags&(net.FlagLoopback|net.FlagPointToPoint) == 0 } + +func (f *NetworkFingerprint) findInterfaces(deviceName string) ([]*net.Interface, error) { + var ( + interfaces []*net.Interface + err error + ) + if deviceName != "" { + if intf, err := net.InterfaceByName(deviceName); err == nil { + interfaces = append(interfaces, intf) + } + return interfaces, err + } + + var intfs []net.Interface + + if intfs, err = net.Interfaces(); err != nil { + return nil, err + } + + for _, intf := range intfs { + if f.isDeviceEnabled(&intf) && f.isDeviceLoopBackOrPointToPoint(&intf) && f.deviceHasIpAddress(&intf) { + interfaces = append(interfaces, &intf) + } + } + + return interfaces, nil +} From 61a56c1ab2cf0272c0d0163910729af895e0ad00 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 28 Oct 2015 13:38:28 -0700 Subject: [PATCH 03/17] Selecting the ipv4 address if there are ipv4 and ipv6 addresses configured for an interface --- client/fingerprint/network_unix.go | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/client/fingerprint/network_unix.go b/client/fingerprint/network_unix.go index 4ca815184..45933eda2 100644 --- a/client/fingerprint/network_unix.go +++ b/client/fingerprint/network_unix.go @@ -156,10 +156,26 @@ func (f *NetworkFingerprint) ipAddress(intf *net.Interface) (string, error) { if len(addrs) == 0 { return "", errors.New(fmt.Sprintf("Interface %s has no IP address", intf.Name)) } - if ip, _, err := net.ParseCIDR(addrs[0].String()); err == nil { - return ip.String(), nil + var ipV4 net.IP + for _, addr := range addrs { + var ip net.IP + switch v := (addr).(type) { + case *net.IPNet: + ip = v.IP + case *net.IPAddr: + ip = v.IP + } + if ip.To4() != nil { + ipV4 = ip + break + } } - return "", errors.New(fmt.Sprintf("Couldn't parse IP address for interface %s with addr %s", intf.Name, addrs[0].String())) + + if ipV4 == nil { + return "", errors.New(fmt.Sprintf("Couldn't parse IP address for interface %s with addr %s", intf.Name)) + } + return ipV4.String(), nil + } func (f *NetworkFingerprint) isDeviceEnabled(intf *net.Interface) bool { From 2c3def89b3d1babf73bd2c2a7968a7dfba92c6d2 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 28 Oct 2015 14:03:33 -0700 Subject: [PATCH 04/17] Added more comments --- client/fingerprint/network_unix.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/client/fingerprint/network_unix.go b/client/fingerprint/network_unix.go index 45933eda2..3a8a0a93a 100644 --- a/client/fingerprint/network_unix.go +++ b/client/fingerprint/network_unix.go @@ -144,6 +144,7 @@ func (f *NetworkFingerprint) linkSpeedEthtool(path, device string) int { return mbs } +// Gets the ipv4 addr for a network interface func (f *NetworkFingerprint) ipAddress(intf *net.Interface) (string, error) { var ( addrs []net.Addr @@ -178,13 +179,15 @@ func (f *NetworkFingerprint) ipAddress(intf *net.Interface) (string, error) { } +// 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 { - if addrs, err := intf.Addrs(); err == nil { - return len(addrs) > 0 + if _, err := f.ipAddress(intf); err == nil { + return true } return false } @@ -193,6 +196,8 @@ func (n *NetworkFingerprint) isDeviceLoopBackOrPointToPoint(intf *net.Interface) return intf.Flags&(net.FlagLoopback|net.FlagPointToPoint) == 0 } +// Returns interfaces which are routable and marked as UP +// Tries to get the specific interface if the user has specified name func (f *NetworkFingerprint) findInterfaces(deviceName string) ([]*net.Interface, error) { var ( interfaces []*net.Interface @@ -216,6 +221,5 @@ func (f *NetworkFingerprint) findInterfaces(deviceName string) ([]*net.Interface interfaces = append(interfaces, &intf) } } - return interfaces, nil } From ae8b07e85a040cc73fc75f1f592878e2980b5dd9 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 28 Oct 2015 14:32:13 -0700 Subject: [PATCH 05/17] Adding more information to errors --- client/fingerprint/network_unix.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/client/fingerprint/network_unix.go b/client/fingerprint/network_unix.go index 3a8a0a93a..6fcea7436 100644 --- a/client/fingerprint/network_unix.go +++ b/client/fingerprint/network_unix.go @@ -36,20 +36,17 @@ func (f *NetworkFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) interfaces, err := f.findInterfaces(cfg.NetworkInterface) if err != nil { - f.logger.Println(fmt.Sprintf("[DEBUG] Error while detecting network interface during fingerprinting: %s", err.Error())) - return false, err + return false, fmt.Errorf("Error while detecting network interface during fingerprinting: %s", err.Error()) } if len(interfaces) == 0 { - f.logger.Println("[DEBUG] No network interfaces were detected") - return false, errors.New("Unable to find any interface") + return false, errors.New("No network interfaces were detected") } // Use the first interface that we have detected. intf := interfaces[0] if ip, err = f.ipAddress(intf); err != nil { - f.logger.Println("[DEBUG] Unable to find IP address of interface ", intf.Name) - return false, err + return false, fmt.Errorf("Unable to find IP address of interface: %s, err: %s", intf.Name, err.Error()) } newNetwork.Device = intf.Name From 345ccb112a653cc9f9dcaba9cc82c1c392028e6d Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 28 Oct 2015 14:41:13 -0700 Subject: [PATCH 06/17] Added more information to log --- client/fingerprint/network_unix.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/fingerprint/network_unix.go b/client/fingerprint/network_unix.go index 6fcea7436..5aec3154b 100644 --- a/client/fingerprint/network_unix.go +++ b/client/fingerprint/network_unix.go @@ -54,7 +54,7 @@ func (f *NetworkFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) newNetwork.IP = ip newNetwork.CIDR = newNetwork.IP + "/32" - f.logger.Println("[DEBUG] Detected interface ", intf.Name, " with IP ", ip, " during fingerprinting") + f.logger.Println("[DEBUG] fingerprint.network Detected interface ", intf.Name, " with IP ", ip, " during fingerprinting") if throughput := f.linkSpeed(intf.Name); throughput > 0 { newNetwork.MBits = throughput From 65b47b824a348c3e8cbcd2bda0c1bf46051261fb Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 28 Oct 2015 14:44:46 -0700 Subject: [PATCH 07/17] We don't want lo and PPP in production --- client/fingerprint/network_unix.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/fingerprint/network_unix.go b/client/fingerprint/network_unix.go index 5aec3154b..a8c492fc1 100644 --- a/client/fingerprint/network_unix.go +++ b/client/fingerprint/network_unix.go @@ -214,7 +214,7 @@ func (f *NetworkFingerprint) findInterfaces(deviceName string) ([]*net.Interface } for _, intf := range intfs { - if f.isDeviceEnabled(&intf) && f.isDeviceLoopBackOrPointToPoint(&intf) && f.deviceHasIpAddress(&intf) { + if f.isDeviceEnabled(&intf) && !f.isDeviceLoopBackOrPointToPoint(&intf) && f.deviceHasIpAddress(&intf) { interfaces = append(interfaces, &intf) } } From e470eb230dc9d912cdf89773e4cc8236370979a5 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 28 Oct 2015 15:03:11 -0700 Subject: [PATCH 08/17] Some coding style changes --- client/fingerprint/network_unix.go | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/client/fingerprint/network_unix.go b/client/fingerprint/network_unix.go index a8c492fc1..04d539757 100644 --- a/client/fingerprint/network_unix.go +++ b/client/fingerprint/network_unix.go @@ -36,7 +36,7 @@ func (f *NetworkFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) interfaces, err := f.findInterfaces(cfg.NetworkInterface) if err != nil { - return false, fmt.Errorf("Error while detecting network interface during fingerprinting: %s", err.Error()) + return false, fmt.Errorf("Error while detecting network interface during fingerprinting: %v", err) } if len(interfaces) == 0 { @@ -46,7 +46,7 @@ func (f *NetworkFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) // Use the first interface that we have detected. intf := interfaces[0] if ip, err = f.ipAddress(intf); err != nil { - return false, fmt.Errorf("Unable to find IP address of interface: %s, err: %s", intf.Name, err.Error()) + return false, fmt.Errorf("Unable to find IP address of interface: %s, err: %v", intf.Name, err) } newNetwork.Device = intf.Name @@ -143,10 +143,9 @@ func (f *NetworkFingerprint) linkSpeedEthtool(path, device string) int { // Gets the ipv4 addr for a network interface func (f *NetworkFingerprint) ipAddress(intf *net.Interface) (string, error) { - var ( - addrs []net.Addr - err error - ) + var addrs []net.Addr + var err error + if addrs, err = intf.Addrs(); err != nil { return "", err } @@ -170,7 +169,7 @@ func (f *NetworkFingerprint) ipAddress(intf *net.Interface) (string, error) { } if ipV4 == nil { - return "", errors.New(fmt.Sprintf("Couldn't parse IP address for interface %s with addr %s", intf.Name)) + return "", fmt.Errorf("Couldn't parse IP address for interface %s with addr %s", intf.Name) } return ipV4.String(), nil @@ -196,10 +195,9 @@ func (n *NetworkFingerprint) isDeviceLoopBackOrPointToPoint(intf *net.Interface) // Returns interfaces which are routable and marked as UP // Tries to get the specific interface if the user has specified name func (f *NetworkFingerprint) findInterfaces(deviceName string) ([]*net.Interface, error) { - var ( - interfaces []*net.Interface - err error - ) + var interfaces []*net.Interface + var err error + if deviceName != "" { if intf, err := net.InterfaceByName(deviceName); err == nil { interfaces = append(interfaces, intf) From e61c4ad1302571dbf24f6f8b0129a12d9ad0e36f Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 28 Oct 2015 15:48:08 -0700 Subject: [PATCH 09/17] Refactored the findInterfaces method to make it more clear --- client/fingerprint/network_unix.go | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/client/fingerprint/network_unix.go b/client/fingerprint/network_unix.go index 04d539757..04f54285f 100644 --- a/client/fingerprint/network_unix.go +++ b/client/fingerprint/network_unix.go @@ -34,17 +34,11 @@ func (f *NetworkFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) newNetwork := &structs.NetworkResource{} var ip string - interfaces, err := f.findInterfaces(cfg.NetworkInterface) + intf, err := f.findInterface(cfg.NetworkInterface) if err != nil { return false, fmt.Errorf("Error while detecting network interface during fingerprinting: %v", err) } - if len(interfaces) == 0 { - return false, errors.New("No network interfaces were detected") - } - - // Use the first interface that we have detected. - intf := interfaces[0] if ip, err = f.ipAddress(intf); err != nil { return false, fmt.Errorf("Unable to find IP address of interface: %s, err: %v", intf.Name, err) } @@ -192,17 +186,16 @@ func (n *NetworkFingerprint) isDeviceLoopBackOrPointToPoint(intf *net.Interface) return intf.Flags&(net.FlagLoopback|net.FlagPointToPoint) == 0 } -// Returns interfaces which are routable and marked as UP -// Tries to get the specific interface if the user has specified name -func (f *NetworkFingerprint) findInterfaces(deviceName string) ([]*net.Interface, error) { +// 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 +func (f *NetworkFingerprint) findInterface(deviceName string) (*net.Interface, error) { var interfaces []*net.Interface var err error if deviceName != "" { - if intf, err := net.InterfaceByName(deviceName); err == nil { - interfaces = append(interfaces, intf) - } - return interfaces, err + return net.InterfaceByName(deviceName) } var intfs []net.Interface @@ -216,5 +209,9 @@ func (f *NetworkFingerprint) findInterfaces(deviceName string) ([]*net.Interface interfaces = append(interfaces, &intf) } } - return interfaces, nil + + if len(interfaces) == 0 { + return nil, errors.New("No network interfaces were detected") + } + return interfaces[0], nil } From 764a6bdf1dfdeec10b9e7a561894f0c76bf95e47 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 28 Oct 2015 15:58:40 -0700 Subject: [PATCH 10/17] Introduced an interface to detect network devices so that we can mock it for tests --- client/fingerprint/network_unix.go | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/client/fingerprint/network_unix.go b/client/fingerprint/network_unix.go index 04f54285f..053743e63 100644 --- a/client/fingerprint/network_unix.go +++ b/client/fingerprint/network_unix.go @@ -19,13 +19,30 @@ import ( // NetworkFingerprint is used to fingerprint the Network capabilities of a node type NetworkFingerprint struct { - logger *log.Logger + logger *log.Logger + interfaceDetector NetworkInterfaceDetector +} + +type NetworkInterfaceDetector interface { + Interfaces() ([]net.Interface, error) + InterfaceByName(name string) (*net.Interface, error) +} + +type BasicNetworkInterfaceDetector struct { +} + +func (b *BasicNetworkInterfaceDetector) Interfaces() ([]net.Interface, error) { + return net.Interfaces() +} + +func (b *BasicNetworkInterfaceDetector) InterfaceByName(name string) (*net.Interface, error) { + return net.InterfaceByName(name) } // NewNetworkFingerprinter returns a new NetworkFingerprinter with the given // logger func NewNetworkFingerprinter(logger *log.Logger) Fingerprint { - f := &NetworkFingerprint{logger: logger} + f := &NetworkFingerprint{logger: logger, interfaceDetector: &BasicNetworkInterfaceDetector{}} return f } @@ -195,12 +212,12 @@ func (f *NetworkFingerprint) findInterface(deviceName string) (*net.Interface, e var err error if deviceName != "" { - return net.InterfaceByName(deviceName) + return f.interfaceDetector.InterfaceByName(deviceName) } var intfs []net.Interface - if intfs, err = net.Interfaces(); err != nil { + if intfs, err = f.interfaceDetector.Interfaces(); err != nil { return nil, err } From 617edcdddd0a9df0e37cfcf5579f5055438c750d Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Thu, 29 Oct 2015 11:01:15 -0700 Subject: [PATCH 11/17] Added tests for testing detecting default device --- client/fingerprint/network_test.go | 153 ++++++++++++++++++++++++++++- client/fingerprint/network_unix.go | 7 +- 2 files changed, 158 insertions(+), 2 deletions(-) diff --git a/client/fingerprint/network_test.go b/client/fingerprint/network_test.go index 077d5a573..f99c91830 100644 --- a/client/fingerprint/network_test.go +++ b/client/fingerprint/network_test.go @@ -1,6 +1,7 @@ package fingerprint import ( + "fmt" "net" "testing" @@ -8,8 +9,81 @@ import ( "github.com/hashicorp/nomad/nomad/structs" ) +var ( + lo = net.Interface{ + Index: 2, + MTU: 65536, + Name: "lo", + HardwareAddr: []byte{23, 43, 54, 54}, + Flags: net.FlagUp | net.FlagLoopback, + } +) + +type LoAddrV4 struct { +} + +func (l LoAddrV4) Network() string { + return "ip+net" +} + +func (l LoAddrV4) String() string { + return "127.0.0.1/8" +} + +type LoAddrV6 struct { +} + +func (l LoAddrV6) Network() string { + return "ip+net" +} + +func (l LoAddrV6) String() string { + return "::1/128" +} + +// A fake network detector which returns no devices +type NetworkIntefaceDetectorNoDevices struct { +} + +func (f *NetworkIntefaceDetectorNoDevices) Interfaces() ([]net.Interface, error) { + return make([]net.Interface, 0), nil +} + +func (f *NetworkIntefaceDetectorNoDevices) InterfaceByName(name string) (*net.Interface, error) { + return nil, fmt.Errorf("Device with name %s doesn't exist", name) +} + +func (f *NetworkIntefaceDetectorNoDevices) Addrs(intf *net.Interface) ([]net.Addr, error) { + return nil, fmt.Errorf("No interfaces found for device %v", intf.Name) +} + +// A fake network detector which returns only loopback +type NetworkInterfaceDetectorOnlyLo struct { +} + +func (n *NetworkInterfaceDetectorOnlyLo) Interfaces() ([]net.Interface, error) { + return []net.Interface{lo}, nil +} + +func (n *NetworkInterfaceDetectorOnlyLo) InterfaceByName(name string) (*net.Interface, error) { + if name == "lo" { + return &lo, nil + } + + return nil, fmt.Errorf("No device with name %v found", name) +} + +func (n *NetworkInterfaceDetectorOnlyLo) Addrs(intf *net.Interface) ([]net.Addr, error) { + if intf.Name == "lo" { + _, ipnet1, _ := net.ParseCIDR("127.0.0.1/8") + _, ipnet2, _ := net.ParseCIDR("2001:DB8::/48") + return []net.Addr{ipnet1, ipnet2}, nil + } + return nil, fmt.Errorf("Can't find addresses for device: %v", intf.Name) +} + func TestNetworkFingerprint_basic(t *testing.T) { - f := NewNetworkFingerprinter(testLogger()) + f := &NetworkFingerprint{logger: testLogger(), interfaceDetector: &BasicNetworkInterfaceDetector{}} node := &structs.Node{ Attributes: make(map[string]string), } @@ -50,3 +124,80 @@ func TestNetworkFingerprint_basic(t *testing.T) { t.Fatal("Expected Network Resource to have a non-zero bandwith") } } + +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{ + Attributes: make(map[string]string), + } + cfg := &config.Config{NetworkSpeed: 100, NetworkInterface: "eth0"} + + 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(t *testing.T) { + f := &NetworkFingerprint{logger: testLogger(), interfaceDetector: &NetworkInterfaceDetectorOnlyLo{}} + node := &structs.Node{ + Attributes: make(map[string]string), + } + cfg := &config.Config{NetworkSpeed: 100, NetworkInterface: "lo"} + + ok, err := f.Fingerprint(cfg, node) + if err != nil { + t.Fatalf("err: %v", err) + } + if !ok { + t.Fatalf("should apply") + } + + assertNodeAttributeContains(t, node, "network.ip-address") + + ip := node.Attributes["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 not be empty") + } + if net.CIDR == "" { + t.Fatal("Expected Network Resource to have a CIDR") + } + if net.Device == "" { + t.Fatal("Expected Network Resource to have a Device Name") + } + if net.MBits == 0 { + t.Fatal("Expected Network Resource to have a non-zero bandwith") + } +} diff --git a/client/fingerprint/network_unix.go b/client/fingerprint/network_unix.go index 053743e63..fea9076e7 100644 --- a/client/fingerprint/network_unix.go +++ b/client/fingerprint/network_unix.go @@ -26,6 +26,7 @@ type NetworkFingerprint struct { type NetworkInterfaceDetector interface { Interfaces() ([]net.Interface, error) InterfaceByName(name string) (*net.Interface, error) + Addrs(intf *net.Interface) ([]net.Addr, error) } type BasicNetworkInterfaceDetector struct { @@ -39,6 +40,10 @@ func (b *BasicNetworkInterfaceDetector) InterfaceByName(name string) (*net.Inter return net.InterfaceByName(name) } +func (b *BasicNetworkInterfaceDetector) Addrs(intf *net.Interface) ([]net.Addr, error) { + return intf.Addrs() +} + // NewNetworkFingerprinter returns a new NetworkFingerprinter with the given // logger func NewNetworkFingerprinter(logger *log.Logger) Fingerprint { @@ -157,7 +162,7 @@ func (f *NetworkFingerprint) ipAddress(intf *net.Interface) (string, error) { var addrs []net.Addr var err error - if addrs, err = intf.Addrs(); err != nil { + if addrs, err = f.interfaceDetector.Addrs(intf); err != nil { return "", err } From 24b1a8eb3e033b5ebefba7f39970b0e5645b2392 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Thu, 29 Oct 2015 11:05:58 -0700 Subject: [PATCH 12/17] Added some coments --- client/fingerprint/network_unix.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/client/fingerprint/network_unix.go b/client/fingerprint/network_unix.go index fea9076e7..1f82d52fd 100644 --- a/client/fingerprint/network_unix.go +++ b/client/fingerprint/network_unix.go @@ -23,12 +23,16 @@ type NetworkFingerprint struct { interfaceDetector NetworkInterfaceDetector } +// An interface to isolate calls to various api in net package +// This facilitates testing where we can implement +// fake interfaces and addresses to test varios code paths type NetworkInterfaceDetector interface { Interfaces() ([]net.Interface, error) InterfaceByName(name string) (*net.Interface, error) Addrs(intf *net.Interface) ([]net.Addr, error) } +// Implements the interface detector which calls net directly type BasicNetworkInterfaceDetector struct { } From e85ec5c2e2fe130500a6fee64db8a49a3ba2f8f8 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Thu, 29 Oct 2015 15:14:13 -0700 Subject: [PATCH 13/17] Added a test to ensure we are not selecting devices which are not marked as UP or loopback and have no IP addresses --- client/fingerprint/network_test.go | 142 ++++++++++++++++++++++++----- client/fingerprint/network_unix.go | 8 +- 2 files changed, 124 insertions(+), 26 deletions(-) diff --git a/client/fingerprint/network_test.go b/client/fingerprint/network_test.go index f99c91830..650488bb1 100644 --- a/client/fingerprint/network_test.go +++ b/client/fingerprint/network_test.go @@ -17,30 +17,32 @@ var ( HardwareAddr: []byte{23, 43, 54, 54}, Flags: net.FlagUp | net.FlagLoopback, } + + eth0 = net.Interface{ + Index: 3, + MTU: 1500, + Name: "eth0", + HardwareAddr: []byte{23, 44, 54, 67}, + Flags: net.FlagUp | net.FlagMulticast | net.FlagBroadcast, + } + + eth1 = net.Interface{ + Index: 4, + MTU: 1500, + Name: "eth1", + HardwareAddr: []byte{23, 44, 54, 69}, + Flags: net.FlagMulticast | net.FlagBroadcast, + } + + eth2 = net.Interface{ + Index: 4, + MTU: 1500, + Name: "eth2", + HardwareAddr: []byte{23, 44, 54, 70}, + Flags: net.FlagUp | net.FlagBroadcast | net.FlagMulticast, + } ) -type LoAddrV4 struct { -} - -func (l LoAddrV4) Network() string { - return "ip+net" -} - -func (l LoAddrV4) String() string { - return "127.0.0.1/8" -} - -type LoAddrV6 struct { -} - -func (l LoAddrV6) Network() string { - return "ip+net" -} - -func (l LoAddrV6) String() string { - return "::1/128" -} - // A fake network detector which returns no devices type NetworkIntefaceDetectorNoDevices struct { } @@ -79,6 +81,59 @@ func (n *NetworkInterfaceDetectorOnlyLo) Addrs(intf *net.Interface) ([]net.Addr, _, ipnet2, _ := net.ParseCIDR("2001:DB8::/48") return []net.Addr{ipnet1, ipnet2}, nil } + + return nil, fmt.Errorf("Can't find addresses for device: %v", intf.Name) +} + +// A fake network detector which simulates the presence of multiple interfaces +type NetworkInterfaceDetectorMultipleInterfaces struct { +} + +func (n *NetworkInterfaceDetectorMultipleInterfaces) Interfaces() ([]net.Interface, error) { + return []net.Interface{lo, eth0, eth1, eth2}, nil +} + +func (n *NetworkInterfaceDetectorMultipleInterfaces) InterfaceByName(name string) (*net.Interface, error) { + var intf *net.Interface + switch name { + case "lo": + intf = &lo + case "eth0": + intf = ð0 + case "eth1": + intf = ð1 + case "eth2": + intf = ð2 + } + if intf != nil { + return intf, nil + } + + return nil, fmt.Errorf("No device with name %v found", name) +} + +func (n *NetworkInterfaceDetectorMultipleInterfaces) Addrs(intf *net.Interface) ([]net.Addr, error) { + if intf.Name == "lo" { + _, ipnet1, _ := net.ParseCIDR("127.0.0.1/8") + _, ipnet2, _ := net.ParseCIDR("2001:DB8::/48") + return []net.Addr{ipnet1, ipnet2}, nil + } + + if intf.Name == "eth0" { + _, ipnet1, _ := net.ParseCIDR("100.64.0.11/10") + _, ipnet2, _ := net.ParseCIDR("2005:DB6::/48") + return []net.Addr{ipnet1, ipnet2}, nil + } + + if intf.Name == "eth1" { + _, ipnet1, _ := net.ParseCIDR("100.64.0.10/10") + _, ipnet2, _ := net.ParseCIDR("2003:DB8::/48") + return []net.Addr{ipnet1, ipnet2}, nil + } + + if intf.Name == "eth2" { + return []net.Addr{}, nil + } return nil, fmt.Errorf("Can't find addresses for device: %v", intf.Name) } @@ -201,3 +256,46 @@ func TestNetworkFingerPrint_default_device(t *testing.T) { t.Fatal("Expected Network Resource to have a non-zero bandwith") } } + +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, "network.ip-address") + + ip := node.Attributes["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 bandwith") + } +} diff --git a/client/fingerprint/network_unix.go b/client/fingerprint/network_unix.go index 1f82d52fd..0e47787ca 100644 --- a/client/fingerprint/network_unix.go +++ b/client/fingerprint/network_unix.go @@ -209,7 +209,7 @@ func (f *NetworkFingerprint) deviceHasIpAddress(intf *net.Interface) bool { } func (n *NetworkFingerprint) isDeviceLoopBackOrPointToPoint(intf *net.Interface) bool { - return intf.Flags&(net.FlagLoopback|net.FlagPointToPoint) == 0 + return intf.Flags&(net.FlagLoopback|net.FlagPointToPoint) != 0 } // Returns the interface with the name passed by user @@ -217,7 +217,7 @@ 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 interfaces []net.Interface var err error if deviceName != "" { @@ -232,12 +232,12 @@ func (f *NetworkFingerprint) findInterface(deviceName string) (*net.Interface, e for _, intf := range intfs { if f.isDeviceEnabled(&intf) && !f.isDeviceLoopBackOrPointToPoint(&intf) && f.deviceHasIpAddress(&intf) { - interfaces = append(interfaces, &intf) + interfaces = append(interfaces, intf) } } if len(interfaces) == 0 { return nil, errors.New("No network interfaces were detected") } - return interfaces[0], nil + return &interfaces[0], nil } From d684b8e0e79612e17edbcb3736a1a4a3e6ec78a0 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Thu, 29 Oct 2015 15:15:44 -0700 Subject: [PATCH 14/17] Fixed style of a debug log --- client/fingerprint/network_unix.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/fingerprint/network_unix.go b/client/fingerprint/network_unix.go index 0e47787ca..c41deca1d 100644 --- a/client/fingerprint/network_unix.go +++ b/client/fingerprint/network_unix.go @@ -74,7 +74,7 @@ func (f *NetworkFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) newNetwork.IP = ip newNetwork.CIDR = newNetwork.IP + "/32" - f.logger.Println("[DEBUG] fingerprint.network Detected interface ", intf.Name, " with IP ", ip, " during fingerprinting") + f.logger.Println("[DEBUG] fingerprint.network: Detected interface ", intf.Name, " with IP ", ip, " during fingerprinting") if throughput := f.linkSpeed(intf.Name); throughput > 0 { newNetwork.MBits = throughput From 06ac210eec4ec9a608076bb59d050eef69ddbb1f Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Thu, 29 Oct 2015 15:17:40 -0700 Subject: [PATCH 15/17] Fixed a debug message --- client/fingerprint/network_unix.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/fingerprint/network_unix.go b/client/fingerprint/network_unix.go index c41deca1d..0ae89dbaa 100644 --- a/client/fingerprint/network_unix.go +++ b/client/fingerprint/network_unix.go @@ -189,7 +189,7 @@ func (f *NetworkFingerprint) ipAddress(intf *net.Interface) (string, error) { } if ipV4 == nil { - return "", fmt.Errorf("Couldn't parse IP address for interface %s with addr %s", intf.Name) + return "", fmt.Errorf("Couldn't parse IP address for interface %s", intf.Name) } return ipV4.String(), nil From 804d88d7f0195cdb50d9d04a037b02ec1f7767d6 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Thu, 29 Oct 2015 15:55:49 -0700 Subject: [PATCH 16/17] Renamed BasicNetworkInterfaceDetector to DefaultNetworkInterfaceDetector --- client/fingerprint/network_unix.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/client/fingerprint/network_unix.go b/client/fingerprint/network_unix.go index 0ae89dbaa..bf4b7b4ee 100644 --- a/client/fingerprint/network_unix.go +++ b/client/fingerprint/network_unix.go @@ -33,25 +33,25 @@ type NetworkInterfaceDetector interface { } // Implements the interface detector which calls net directly -type BasicNetworkInterfaceDetector struct { +type DefaultNetworkInterfaceDetector struct { } -func (b *BasicNetworkInterfaceDetector) Interfaces() ([]net.Interface, error) { +func (b *DefaultNetworkInterfaceDetector) Interfaces() ([]net.Interface, error) { return net.Interfaces() } -func (b *BasicNetworkInterfaceDetector) InterfaceByName(name string) (*net.Interface, error) { +func (b *DefaultNetworkInterfaceDetector) InterfaceByName(name string) (*net.Interface, error) { return net.InterfaceByName(name) } -func (b *BasicNetworkInterfaceDetector) Addrs(intf *net.Interface) ([]net.Addr, error) { +func (b *DefaultNetworkInterfaceDetector) Addrs(intf *net.Interface) ([]net.Addr, error) { return intf.Addrs() } // NewNetworkFingerprinter returns a new NetworkFingerprinter with the given // logger func NewNetworkFingerprinter(logger *log.Logger) Fingerprint { - f := &NetworkFingerprint{logger: logger, interfaceDetector: &BasicNetworkInterfaceDetector{}} + f := &NetworkFingerprint{logger: logger, interfaceDetector: &DefaultNetworkInterfaceDetector{}} return f } From a2bdd2ae0c33594396207e3a9f21d280f45f8e88 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Thu, 29 Oct 2015 16:16:10 -0700 Subject: [PATCH 17/17] Refactored code --- client/fingerprint/network_test.go | 2 +- client/fingerprint/network_unix.go | 17 +++++------------ 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/client/fingerprint/network_test.go b/client/fingerprint/network_test.go index 650488bb1..a42b1d7b3 100644 --- a/client/fingerprint/network_test.go +++ b/client/fingerprint/network_test.go @@ -138,7 +138,7 @@ func (n *NetworkInterfaceDetectorMultipleInterfaces) Addrs(intf *net.Interface) } func TestNetworkFingerprint_basic(t *testing.T) { - f := &NetworkFingerprint{logger: testLogger(), interfaceDetector: &BasicNetworkInterfaceDetector{}} + f := &NetworkFingerprint{logger: testLogger(), interfaceDetector: &DefaultNetworkInterfaceDetector{}} node := &structs.Node{ Attributes: make(map[string]string), } diff --git a/client/fingerprint/network_unix.go b/client/fingerprint/network_unix.go index bf4b7b4ee..4278384e9 100644 --- a/client/fingerprint/network_unix.go +++ b/client/fingerprint/network_unix.go @@ -74,7 +74,7 @@ func (f *NetworkFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) newNetwork.IP = ip newNetwork.CIDR = newNetwork.IP + "/32" - f.logger.Println("[DEBUG] fingerprint.network: Detected interface ", intf.Name, " with IP ", ip, " during fingerprinting") + f.logger.Printf("[DEBUG] fingerprint.network: Detected interface %v with IP %v during fingerprinting", intf.Name, ip) if throughput := f.linkSpeed(intf.Name); throughput > 0 { newNetwork.MBits = throughput @@ -173,7 +173,6 @@ func (f *NetworkFingerprint) ipAddress(intf *net.Interface) (string, error) { if len(addrs) == 0 { return "", errors.New(fmt.Sprintf("Interface %s has no IP address", intf.Name)) } - var ipV4 net.IP for _, addr := range addrs { var ip net.IP switch v := (addr).(type) { @@ -183,15 +182,11 @@ func (f *NetworkFingerprint) ipAddress(intf *net.Interface) (string, error) { ip = v.IP } if ip.To4() != nil { - ipV4 = ip - break + return ip.String(), nil } } - if ipV4 == nil { - return "", fmt.Errorf("Couldn't parse IP address for interface %s", intf.Name) - } - return ipV4.String(), nil + return "", fmt.Errorf("Couldn't parse IP address for interface %s", intf.Name) } @@ -202,10 +197,8 @@ func (f *NetworkFingerprint) isDeviceEnabled(intf *net.Interface) bool { // Checks if the device has any IP address configured func (f *NetworkFingerprint) deviceHasIpAddress(intf *net.Interface) bool { - if _, err := f.ipAddress(intf); err == nil { - return true - } - return false + _, err := f.ipAddress(intf) + return err == nil } func (n *NetworkFingerprint) isDeviceLoopBackOrPointToPoint(intf *net.Interface) bool {