Merge lp:~ltrager/maas/fix_xenial_net_detect into lp:~maas-committers/maas/trunk

[Lee Trager]

Fix cluster controller networking auto detection

* Don't require that a valid NIC has a kernel module loaded
* When falling back to scanning ip addr for devices look for network devices which follow the preedictable network interface names standard.

Auto detection of the cluster controllers network devices was failing for two reasons

1. MAAS's primary method of discovering NICs is to iterate through /sys/class/net. A device was only identified as a physical device that MAAS can use if its /sys entry contained device/driver/module. In Xenial the virtio network driver is built into the kernel thus the module subdirectory doesn't exist. Now we just check that it contains the device in its /sys entry.
2. As a fall back MAAS iterates over the output of ip addr. Xenial is following the preedictable network interface names standard[1] which creates ethernet devices starting with 'en' instead of 'eth'. MAAS was checking for devices that start with 'em' while devices only come up with 'en.' Many articulates on the internet came out describing that 'em' would be used and many still report that today[2]. However the freedesktop.org spec[1] links to a comment in the udev source code[3] which explains how devices will be named starting with 'en'

1. http://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/
2. https://en.wikipedia.org/wiki/Consistent_Network_Device_Naming
3. https://github.com/systemd/systemd/blob/master/src/udev/udev-builtin-net_id.c#L20

[Mike Pontillo (mpontillo)]

Looks good, but please keep the startswith('em') in there; that's still a valid for embedded NICs.[1]

[1]:
https://en.wikipedia.org/wiki/Consistent_Network_Device_Naming

review: Approve

[MAAS Lander (maas-lander)]

The attempt to merge lp:~ltrager/maas/fix_xenial_net_detect into lp:maas failed. Below is the output from the failed tests.

Get:1 http://prodstack-zone-1.clouds.archive.ubuntu.com xenial InRelease [218 kB]
Hit http://security.ubuntu.com xenial-security InRelease
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com xenial-updates InRelease
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com xenial-backports InRelease
Hit http://security.ubuntu.com xenial-security/main Sources
Hit http://security.ubuntu.com xenial-security/restricted Sources
Hit http://security.ubuntu.com xenial-security/universe Sources
Hit http://security.ubuntu.com xenial-security/multiverse Sources
Hit http://security.ubuntu.com xenial-security/main amd64 Packages
Hit http://security.ubuntu.com xenial-security/restricted amd64 Packages
Hit http://security.ubuntu.com xenial-security/universe amd64 Packages
Hit http://security.ubuntu.com xenial-security/multiverse amd64 Packages
Hit http://security.ubuntu.com xenial-security/main Translation-en
Hit http://security.ubuntu.com xenial-security/multiverse Translation-en
Hit http://security.ubuntu.com xenial-security/restricted Translation-en
Hit http://security.ubuntu.com xenial-security/universe Translation-en
Get:2 http://prodstack-zone-1.clouds.archive.ubuntu.com xenial/main Sources [1,119 kB]
Get:3 http://prodstack-zone-1.clouds.archive.ubuntu.com xenial/restricted Sources [7,228 B]
Get:4 http://prodstack-zone-1.clouds.archive.ubuntu.com xenial/universe Sources [7,487 kB]
Get:5 http://prodstack-zone-1.clouds.archive.ubuntu.com xenial/multiverse Sources [178 kB]
Get:6 http://prodstack-zone-1.clouds.archive.ubuntu.com xenial/main amd64 Packages [1,446 kB]
Get:7 http://prodstack-zone-1.clouds.archive.ubuntu.com xenial/restricted amd64 Packages [15.8 kB]
Get:8 http://prodstack-zone-1.clouds.archive.ubuntu.com xenial/universe amd64 Packages [6,983 kB]
Get:9 http://prodstack-zone-1.clouds.archive.ubuntu.com xenial/multiverse amd64 Packages [138 kB]
Get:10 http://prodstack-zone-1.clouds.archive.ubuntu.com xenial/main Translation-en [844 kB]
Get:11 http://prodstack-zone-1.clouds.archive.ubuntu.com xenial/multiverse Translation-en [108 kB]
Get:12 http://prodstack-zone-1.clouds.archive.ubuntu.com xenial/restricted Translation-en [4,302 B]
Get:13 http://prodstack-zone-1.clouds.archive.ubuntu.com xenial/universe Translation-en [4,716 kB]
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com xenial-updates/main Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com xenial-updates/restricted Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com xenial-updates/universe Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com xenial-updates/multiverse Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com xenial-updates/main amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com xenial-updates/restricted amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com xenial-updates/universe amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com xenial-updates/multiverse amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com xenial-updates/main Translation-en
Hit ht...

=== modified file 'src/provisioningserver/network.py'
--- src/provisioningserver/network.py	2015-10-23 21:26:12 +0000
+++ src/provisioningserver/network.py	2015-11-25 20:22:18 +0000
@@ -163,6 +163,7 @@
         for network in networks
         if 'interface' in network and
            (network['interface'].startswith('eth') or
+            network['interface'].startswith('en') or
             network['interface'].startswith('em') or
             network['interface'].startswith('vlan') or
             network['interface'].startswith('bond'))

=== modified file 'src/provisioningserver/utils/ipaddr.py'
--- src/provisioningserver/utils/ipaddr.py	2015-10-23 10:28:17 +0000
+++ src/provisioningserver/utils/ipaddr.py	2015-11-25 20:22:18 +0000
@@ -281,11 +281,8 @@
             return 'ethernet.bond'
         if os.path.isfile('%s/%s' % (proc_net_vlan, ifname)):
             return 'ethernet.vlan'
-        module_name_link = os.path.join(sys_path, 'device', 'driver', 'module')
-        if not os.path.islink(module_name_link):
-            return 'ethernet'
-        module_name = os.readlink(module_name_link).split('/')[-1]
-        if module_name:
+        device_path = os.path.join(sys_path, 'device')
+        if os.path.islink(device_path):
             device_80211 = os.path.join(sys_path, 'device', 'ieee80211')
             if os.path.isdir(device_80211):
                 return 'ethernet.wireless'

=== modified file 'src/provisioningserver/utils/tests/test_ipaddr.py'
--- src/provisioningserver/utils/tests/test_ipaddr.py	2015-10-23 10:28:17 +0000
+++ src/provisioningserver/utils/tests/test_ipaddr.py	2015-11-25 20:22:18 +0000
@@ -349,13 +349,9 @@
                 f.write(b"%s\n" % ' '.join(bonded_interfaces).encode('utf-8'))
                 f.close()
         if is_physical or is_wireless:
-            os.mkdir(os.path.join(ifdir, 'device'))
-            os.mkdir(os.path.join(ifdir, 'device', 'driver'))
-            # Note: the symlink here doesn't actually need to resolve
-            # to a real file.
-            os.symlink(
-                os.path.join('..', 'driver_module_name'),
-                os.path.join(ifdir, 'device', 'driver', 'module'))
+            device_real = os.path.join(ifdir, 'device.real')
+            os.mkdir(device_real)
+            os.symlink(device_real, os.path.join(ifdir, 'device'))
         if is_wireless:
             os.mkdir(os.path.join(ifdir, 'device', 'ieee80211'))
 
@@ -480,6 +476,10 @@
 
     def test__filters_based_on_name_by_default(self):
         input_networks = [
+            {"interface": "eno1"},
+            {"interface": "ens3"},
+            {"interface": "enp0s25"},
+            {"interface": "enx78e7d1ea46da"},
             {"interface": "em0"},
             {"interface": "eth0"},
             {"interface": "vlan0"},
@@ -490,6 +490,10 @@
         ]
         actual_networks = filter_and_annotate_networks(input_networks)
         expected_networks = [
+            {"interface": "eno1"},
+            {"interface": "ens3"},
+            {"interface": "enp0s25"},
+            {"interface": "enx78e7d1ea46da"},
             {"interface": "em0"},
             {"interface": "eth0"},
             {"interface": "vlan0"},
@@ -499,6 +503,10 @@
 
     def test__filters_and_annotates_based_on_json_data_if_available(self):
         input_networks = [
+            {"interface": "eno1"},
+            {"interface": "ens3"},
+            {"interface": "enp0s25"},
+            {"interface": "enx78e7d1ea46da"},
             {"interface": "em0"},
             {"interface": "eth0"},
             {"interface": "vlan0"},
@@ -521,7 +529,7 @@
                     "bridged_interfaces": ["avian0"]},
             "br3": {"type": "ethernet.bridge"},
             "wlan0": {"type": "ethernet.bond",
-                      "bonded_interfaces": ["em0", "eth0"]}
+                      "bonded_interfaces": ["eno1", "eth0"]}
         }
         actual_networks = filter_and_annotate_networks(
             input_networks, json.dumps(input_json))
@@ -533,12 +541,16 @@
              "bridged_interfaces": "avian0"},
             {"interface": "br3", 'type': "ethernet.bridge"},
             {"interface": "wlan0", 'type': "ethernet.bond",
-             "bonded_interfaces": "em0 eth0"},
+             "bonded_interfaces": "eno1 eth0"},
         ]
         self.assertThat(actual_networks, Equals(expected_networks))
 
     def test__falls_back_to_names_if_no_interfaces_found(self):
         input_networks = [
+            {"interface": "eno1"},
+            {"interface": "ens3"},
+            {"interface": "enp0s25"},
+            {"interface": "enx78e7d1ea46da"},
             {"interface": "em0"},
             {"interface": "eth0"},
             {"interface": "vlan0"},
@@ -552,6 +564,10 @@
         actual_networks = filter_and_annotate_networks(
             input_networks, json.dumps(input_json))
         expected_networks = [
+            {"interface": "eno1"},
+            {"interface": "ens3"},
+            {"interface": "enp0s25"},
+            {"interface": "enx78e7d1ea46da"},
             {"interface": "em0"},
             {"interface": "eth0"},
             {"interface": "vlan0"},