Merge lp:~raharper/curtin/fix-network-package-install into lp:~curtin-dev/curtin/trunk

[Ryan Harper]

- Fixes lp:1491994
- Enforce order on rendering of /etc/network/interfaces (physical ifaces before bonds)
- Don't emit hwaddress for anything except bonds or bridges
- Introduce a bonding vmtest
- lint and unit-test fixes.

[Scott Moser (smoser)]

llooks good.

review: Approve

=== modified file 'curtin/commands/curthooks.py'
--- curtin/commands/curthooks.py	2015-09-04 17:01:28 +0000
+++ curtin/commands/curthooks.py	2015-09-04 19:23:11 +0000
@@ -573,24 +573,39 @@
     update_initramfs(target, all_kernels=True)
 
 
-def install_missing_storage_packages(cfg, target):
-    # Do nothing if no custom storage.
-    if 'storage' not in cfg:
-        return
-
-    all_types = set(
-        operation['type']
-        for operation in cfg['storage']['config']
-        )
+def install_missing_packages(cfg, target):
+    ''' describe which operation types will require specific packages
+
+    'custom_config_key': {
+         'pkg1': ['op_name_1', 'op_name_2', ...]
+     }
+    '''
+    custom_configs = {
+        'storage': {
+            'lvm2': ['lvm_volgroup', 'lvm_partition'],
+            'mdadm': ['raid'],
+            'bcache-tools': ['bcache']},
+        'network': {
+            'vlan': ['vlan'],
+            'ifenslave': ['bond'],
+            'bridge-utils': ['bridge']},
+    }
+
     needed_packages = []
     installed_packages = get_installed_packages(target)
-    if ('lvm_volgroup' in all_types or 'lvm_partition' in all_types) and \
-            'lvm2' not in installed_packages:
-        needed_packages.append('lvm2')
-    if 'raid' in all_types and 'mdadm' not in installed_packages:
-        needed_packages.append('mdadm')
-    if 'bcache' in all_types and 'bcache-tools' not in installed_packages:
-        needed_packages.append('bcache-tools')
+    for cust_cfg, pkg_reqs in custom_configs.items():
+        if cust_cfg not in cfg:
+            continue
+
+        all_types = set(
+            operation['type']
+            for operation in cfg[cust_cfg]['config']
+            )
+        for pkg, types in pkg_reqs.items():
+            if set(types).intersection(all_types) and \
+               pkg not in installed_packages:
+                needed_packages.append(pkg)
+
     if needed_packages:
         util.install_packages(needed_packages, target=target)
 
@@ -630,7 +645,7 @@
 
     detect_and_handle_multipath(cfg, target)
 
-    install_missing_storage_packages(cfg, target)
+    install_missing_packages(cfg, target)
 
     # If a crypttab file was created by block_meta than it needs to be copied
     # onto the target system, and update_initramfs() needs to be run, so that
 
=== modified file 'curtin/net/__init__.py'
--- curtin/net/__init__.py	2015-09-02 13:55:39 +0000
+++ curtin/net/__init__.py	2015-09-04 19:23:11 +0000
@@ -265,7 +265,6 @@
         'metric',
         'gateway',
         'pointopoint',
-        'hwaddress',
         'mtu',
         'scope',
     ]
@@ -289,6 +288,9 @@
         'index',
         'subnets',
     ]
+    if iface['type'] not in ['bond', 'bridge']:
+        ignore_map.append('mac_address')
+
     for key, value in iface.items():
         if value and key not in ignore_map:
             if type(value) == list:
@@ -319,7 +321,18 @@
 
     content = ""
     interfaces = network_state.get('interfaces')
-    for iface in interfaces.values():
+    ''' Apply a sort order to ensure that we write out
+        the physical interfaces first; this is critical for
+        bonding
+    '''
+    order = {
+        'physical': 0,
+        'bond': 1,
+        'bridge': 2,
+        'vlan': 3,
+    }
+    for iface in sorted(interfaces.values(),
+                        key=lambda k: (order[k['type']], k['name'])):
         content += "auto {name}\n".format(**iface)
 
         subnets = iface.get('subnets', {})
 
=== modified file 'curtin/net/network_state.py'
--- curtin/net/network_state.py	2015-09-02 13:55:39 +0000
+++ curtin/net/network_state.py	2015-09-04 19:23:11 +0000
@@ -116,6 +116,8 @@
 
         interfaces = self.network_state.get('interfaces')
         iface = interfaces.get(command['name'], {})
+        for param, val in command.get('params', {}).items():
+            iface.update({param: val})
         iface.update({
             'name': command.get('name'),
             'type': command.get('type'),
@@ -160,15 +162,20 @@
     #/etc/network/interfaces
     auto eth0
     iface eth0 inet manual
+        bond-master bond0
+        bond-mode 802.3ad
 
     auto eth1
     iface eth1 inet manual
+        bond-master bond0
+        bond-mode 802.3ad
 
     auto bond0
     iface bond0 inet static
          address 192.168.0.10
          gateway 192.168.0.1
          netmask 255.255.255.0
+         bond-slaves none
          bond-mode 802.3ad
          bond-miimon 100
          bond-downdelay 200
@@ -190,6 +197,7 @@
         iface = interfaces.get(command.get('name'), {})
         for param, val in command.get('params').items():
             iface.update({param: val})
+        iface.update({'bond-slaves': 'none'})
         self.network_state['interfaces'].update({iface['name']: iface})
 
         # handle bond slaves
@@ -205,6 +213,9 @@
             interfaces = self.network_state.get('interfaces')
             bond_if = interfaces.get(ifname)
             bond_if['bond-master'] = command.get('name')
+            # copy in bond config into slave
+            for param, val in command.get('params').items():
+                bond_if.update({param: val})
             self.network_state['interfaces'].update({ifname: bond_if})
 
     def handle_bridge(self, command):
@@ -354,7 +365,24 @@
         print("NS1 == NS2 ?=> {}".format(
             ns1.network_state == ns2.network_state))
 
+    def test_output(network_config):
+        (version, config) = load_config(network_config)
+        ns1 = NetworkState(version=version, config=config)
+        ns1.parse_config()
+        random.shuffle(config)
+        ns2 = NetworkState(version=version, config=config)
+        ns2.parse_config()
+        print("NS1 == NS2 ?=> {}".format(
+            ns1.network_state == ns2.network_state))
+        eni_1 = net.render_interfaces(ns1.network_state)
+        eni_2 = net.render_interfaces(ns2.network_state)
+        print(eni_1)
+        print(eni_2)
+        print("eni_1 == eni_2 ?=> {}".format(
+            eni_1 == eni_2))
+
     y = curtin_config.load_config(sys.argv[1])
     network_config = y.get('network')
     test_parse(network_config)
     test_dump_and_load(network_config)
+    test_output(network_config)
 
=== added file 'examples/tests/bonding_network.yaml'
--- examples/tests/bonding_network.yaml	1970-01-01 00:00:00 +0000
+++ examples/tests/bonding_network.yaml	2015-09-04 19:23:11 +0000
@@ -0,0 +1,27 @@
+network:
+    version: 1
+    config:
+        # Physical interfaces.
+        - type: physical
+          name: eth0
+          mac_address: "52:54:00:12:34:00"
+          subnets:
+              - type: dhcp4
+        - type: physical
+          name: eth1
+          mac_address: "52:54:00:12:34:02"
+        - type: physical
+          name: eth2
+          mac_address: "52:54:00:12:34:04"
+        # Bond.
+        - type: bond
+          name: bond0
+          mac_address: "52:54:00:12:34:06"
+          bond_interfaces:
+            - eth1
+            - eth2
+          params:
+            bond-mode: active-backup
+          subnets:
+              - type: static
+                address: 10.23.23.2/24
 
=== modified file 'tests/unittests/test_net.py'
--- tests/unittests/test_net.py	2015-09-02 13:55:39 +0000
+++ tests/unittests/test_net.py	2015-09-04 19:23:11 +0000
@@ -307,10 +307,8 @@
 
     def test_render_interfaces(self):
         ns = self.get_net_state()
-        ifaces = ('auto eth1\n' + 'iface eth1 inet manual\n' +
-                  '    hwaddress cf:d6:af:48:e8:80\n\n' +
-                  'auto eth0\n' + 'iface eth0 inet dhcp\n' +
-                  '    hwaddress c0:d6:9f:2c:e8:80\n\n')
+        ifaces = ('auto eth0\n' + 'iface eth0 inet dhcp\n\n' +
+                  'auto eth1\n' + 'iface eth1 inet manual\n\n')
         net_ifaces = net.render_interfaces(ns.network_state)
         self.assertEqual(sorted(ifaces.split('\n')),
                          sorted(net_ifaces.split('\n')))
 
=== added file 'tests/vmtests/test_bonding.py'
--- tests/vmtests/test_bonding.py	1970-01-01 00:00:00 +0000
+++ tests/vmtests/test_bonding.py	2015-09-04 19:23:11 +0000
@@ -0,0 +1,235 @@
+from . import VMBaseClass
+from unittest import TestCase
+
+import ipaddress
+import os
+import re
+import textwrap
+import yaml
+
+
+def iface_extract(input):
+    mo = re.search(r'^(?P<interface>\w+|\w+:\d+)\s+' +
+                   r'Link encap:(?P<link_encap>\S+)\s+' +
+                   r'(HWaddr\s+(?P<mac_address>\S+))?' +
+                   r'(\s+inet addr:(?P<address>\S+))?' +
+                   r'(\s+Bcast:(?P<broadcast>\S+)\s+)?' +
+                   r'(Mask:(?P<netmask>\S+)\s+)?',
+                   input, re.MULTILINE)
+
+    mtu = re.search(r'(\s+MTU:(?P<mtu>\d+)\s+)\s+', input, re.MULTILINE)
+    mtu_info = mtu.groupdict('')
+    mtu_info['mtu'] = int(mtu_info['mtu'])
+
+    if mo:
+        info = mo.groupdict('')
+        info['running'] = False
+        info['up'] = False
+        info['multicast'] = False
+        if 'RUNNING' in input:
+            info['running'] = True
+        if 'UP' in input:
+            info['up'] = True
+        if 'MULTICAST' in input:
+            info['multicast'] = True
+        info.update(mtu_info)
+        return info
+    return {}
+
+
+def ifconfig_to_dict(ifconfig):
+    interfaces = {}
+    for iface in [iface_extract(iface) for iface in ifconfig.split('\n\n')
+                  if iface.strip()]:
+        interfaces[iface['interface']] = iface
+
+    return interfaces
+
+
+class TestNetworkAbs(VMBaseClass):
+    __test__ = False
+    interactive = False
+    conf_file = "examples/tests/bonding_network.yaml"
+    install_timeout = 600
+    boot_timeout = 600
+    extra_disks = []
+    extra_nics = []
+    user_data = textwrap.dedent("""\
+        #cloud-config
+        password: passw0rd
+        chpasswd: { expire: False }
+        bootcmd:
+          - mkdir -p /media/output
+          - mount /dev/vdb /media/output
+        runcmd:
+          - ifconfig -a > /media/output/ifconfig_a
+          - cp -av /etc/network/interfaces /media/output
+          - cp -av /etc/udev/rules.d/70-persistent-net.rules /media/output
+          - ip -o route show > /media/output/ip_route_show
+          - route -n > /media/output/route_n
+          - dpkg-query -W -f '${Status}' ifenslave > \
+                /media/output/ifenslave_installed
+        power_state:
+          mode: poweroff
+        """)
+
+    def test_output_files_exist(self):
+        self.output_files_exist(["ifconfig_a",
+                                 "interfaces",
+                                 "70-persistent-net.rules",
+                                 "ip_route_show",
+                                 "ifenslave_installed",
+                                 "route_n"])
+
+    def test_ifenslave_installed(self):
+        with open(os.path.join(self.td.mnt, "ifenslave_installed")) as fp:
+            status = fp.read().strip()
+            print('ifenslave installed: {}'.format(status))
+            self.assertEqual('install ok installed', status)
+
+    def test_etc_network_interfaces(self):
+        with open(os.path.join(self.td.mnt, "interfaces")) as fp:
+            eni = fp.read()
+            print('etc/network/interfaces:\n{}'.format(eni))
+
+        expected_eni = self.get_expected_etc_network_interfaces()
+        eni_lines = eni.split('\n')
+        for line in expected_eni.split('\n'):
+            self.assertTrue(line in eni_lines)
+
+    def test_ifconfig_output(self):
+        '''check ifconfig output with test input'''
+        network_state = self.get_network_state()
+        print('expected_network_state:\n{}'.format(
+            yaml.dump(network_state, default_flow_style=False, indent=4)))
+
+        with open(os.path.join(self.td.mnt, "ifconfig_a")) as fp:
+            ifconfig_a = fp.read()
+            print('ifconfig -a:\n{}'.format(ifconfig_a))
+
+        ifconfig_dict = ifconfig_to_dict(ifconfig_a)
+        print('parsed ifcfg dict:\n{}'.format(
+            yaml.dump(ifconfig_dict, default_flow_style=False, indent=4)))
+
+        with open(os.path.join(self.td.mnt, "ip_route_show")) as fp:
+            ip_route_show = fp.read()
+            print("ip route show:\n{}".format(ip_route_show))
+            for line in [line for line in ip_route_show.split('\n')
+                         if 'src' in line]:
+                m = re.search(r'^(?P<network>\S+)\sdev\s' +
+                              r'(?P<devname>\S+)\s+' +
+                              r'proto kernel\s+scope link' +
+                              r'\s+src\s(?P<src_ip>\S+)',
+                              line)
+                route_info = m.groupdict('')
+                print(route_info)
+
+        with open(os.path.join(self.td.mnt, "route_n")) as fp:
+            route_n = fp.read()
+            print("route -n:\n{}".format(route_n))
+
+        interfaces = network_state.get('interfaces')
+        for iface in interfaces.values():
+            subnets = iface.get('subnets', {})
+            if subnets:
+                for index, subnet in zip(range(0, len(subnets)), subnets):
+                    iface['index'] = index
+                    if index == 0:
+                        ifname = "{name}".format(**iface)
+                    else:
+                        ifname = "{name}:{index}".format(**iface)
+
+                    self.check_interface(iface,
+                                         ifconfig_dict.get(ifname),
+                                         route_n)
+            else:
+                iface['index'] = 0
+                self.check_interface(iface,
+                                     ifconfig_dict.get(iface['name']),
+                                     route_n)
+
+    def check_interface(self, iface, ifconfig, route_n):
+        print('testing iface:\n{}\n\nifconfig:\n{}'.format(
+              iface, ifconfig))
+        subnets = iface.get('subnets', {})
+        if subnets and iface['index'] != 0:
+            ifname = "{name}:{index}".format(**iface)
+        else:
+            ifname = "{name}".format(**iface)
+
+        # initial check, do we have the correct iface ?
+        print('ifname={}'.format(ifname))
+        print("ifconfig['interface']={}".format(ifconfig['interface']))
+        self.assertEqual(ifname, ifconfig['interface'])
+
+        # check physical interface attributes
+        # FIXME: can't check mac_addr under bonding since
+        # the bond might change slave mac addrs
+        for key in ['mtu']:
+            if key in iface and iface[key]:
+                self.assertEqual(iface[key],
+                                 ifconfig[key])
+
+        def __get_subnet(subnets, subidx):
+            for index, subnet in zip(range(0, len(subnets)), subnets):
+                if index == subidx:
+                    break
+            return subnet
+
+        # check subnet related attributes, and specifically only
+        # the subnet specified by iface['index']
+        subnets = iface.get('subnets', {})
+        if subnets:
+            subnet = __get_subnet(subnets, iface['index'])
+            if 'address' in subnet and subnet['address']:
+                if ':' in subnet['address']:
+                    inet_iface = ipaddress.IPv6Interface(
+                        subnet['address'])
+                else:
+                    inet_iface = ipaddress.IPv4Interface(
+                        subnet['address'])
+
+                # check ip addr
+                self.assertEqual(str(inet_iface.ip),
+                                 ifconfig['address'])
+
+                self.assertEqual(str(inet_iface.netmask),
+                                 ifconfig['netmask'])
+
+                self.assertEqual(
+                    str(inet_iface.network.broadcast_address),
+                    ifconfig['broadcast'])
+
+            # handle gateway by looking at routing table
+            if 'gateway' in subnet and subnet['gateway']:
+                gw_ip = subnet['gateway']
+                gateways = [line for line in route_n.split('\n')
+                            if 'UG' in line and gw_ip in line]
+                print('matching gateways:\n{}'.format(gateways))
+                self.assertEqual(len(gateways), 1)
+                [gateways] = gateways
+                (dest, gw, genmask, flags, metric, ref, use, iface) = \
+                    gateways.split()
+                print('expected gw:{} found gw:{}'.format(gw_ip, gw))
+                self.assertEqual(gw_ip, gw)
+
+
+class TrustyTestBasic(TestNetworkAbs, TestCase):
+    __test__ = False
+    repo = "maas-daily"
+    release = "trusty"
+    arch = "amd64"
+
+
+class WilyTestBasic(TestNetworkAbs, TestCase):
+    __test__ = True
+    repo = "maas-daily"
+    release = "wily"
+    arch = "amd64"
+
+
+class VividTestBasic(TestNetworkAbs, TestCase):
+    __test__ = True
+    repo = "maas-daily"
+    release = "vivid"
+    arch = "amd64"