Merge ~smoser/cloud-init:bond_name into cloud-init:master

[Scott Moser]

[Scott Moser (smoser)]

this was originally submitted at https://code.launchpad.net/~mgagne/cloud-init/+git/cloud-init/+merge/301953 and then working with mgagne, we have some additional things here.

[Scott Moser (smoser)]

this isnt completely ready yet, but i hope close.
it definitely does solve some issues.

[Matthew Thode (prometheanfire)]

I just tested this and it solved my issue of the bonding_masters file causing an error to be thrown.

review: Approve

[Mathieu Gagné (mgagne)]

Same on my side. All issues identified in bug report were fixed. +1

There is still an issue I can't reproduce or explain where default gateway is not properly configured. I rebooted 10+ times, rebuilt the baremetal nodes 10+ and still can't reproduce. But I know it exists because so far, it happened 2 times. But I think it can be addressed in an other merge request.

review: Approve

diff --git a/cloudinit/net/eni.py b/cloudinit/net/eni.py
index eff5b92..cd533dd 100644
--- a/cloudinit/net/eni.py
+++ b/cloudinit/net/eni.py
@@ -399,7 +399,7 @@ class Renderer(renderer.Renderer):
         else:
             # ifenslave docs say to auto the slave devices
             lines = []
-            if 'bond-master' in iface:
+            if 'bond-master' in iface or 'bond-slaves' in iface:
                 lines.append("auto {name}".format(**iface))
             lines.append("iface {name} {inet} {mode}".format(**iface))
             lines.extend(_iface_add_attrs(iface, index=0))
diff --git a/cloudinit/sources/helpers/openstack.py b/cloudinit/sources/helpers/openstack.py
index 84322e0..a5a2a1d 100644
--- a/cloudinit/sources/helpers/openstack.py
+++ b/cloudinit/sources/helpers/openstack.py
@@ -539,6 +539,10 @@ def convert_net_json(network_json=None, known_macs=None):
     networks = network_json.get('networks', [])
     services = network_json.get('services', [])
 
+    link_updates = []
+    link_id_info = {}
+    bond_name_fmt = "bond%d"
+    bond_number = 0
     config = []
     for link in links:
         subnets = []
@@ -551,6 +555,13 @@ def convert_net_json(network_json=None, known_macs=None):
         if 'name' in link:
             cfg['name'] = link['name']
 
+        if link.get('ethernet_mac_address'):
+            link_id_info[link['id']] = link.get('ethernet_mac_address')
+
+        curinfo = {'name': cfg.get('name'),
+                   'mac': link.get('ethernet_mac_address'),
+                   'id': link['id'], 'type': link['type']}
+
         for network in [n for n in networks
                         if n['link'] == link['id']]:
             subnet = dict((k, v) for k, v in network.items()
@@ -582,31 +593,56 @@ def convert_net_json(network_json=None, known_macs=None):
                     continue
                 elif k.startswith('bond'):
                     params.update({k: v})
-            cfg.update({
-                'bond_interfaces': copy.deepcopy(link['bond_links']),
-                'params': params,
-            })
+
+            # openstack does not provide a name for the bond.
+            # they do provide an 'id', but that is possibly non-sensical.
+            # so we just create our own name.
+            link_name = bond_name_fmt % bond_number
+            bond_number += 1
+
+            # bond_links reference links by their id, but we need to add
+            # to the network config by their nic name.
+            # store that in bond_links_needed, and update these later.
+            link_updates.append(
+                (cfg, 'bond_interfaces', '%s',
+                 copy.deepcopy(link['bond_links']))
+            )
+            cfg.update({'params': params, 'name': link_name})
+
+            curinfo['name'] = link_name
         elif link['type'] in ['vlan']:
+            name = "%s.%s" % (link['vlan_link'], link['vlan_id'])
             cfg.update({
-                'name': "%s.%s" % (link['vlan_link'],
-                                   link['vlan_id']),
-                'vlan_link': link['vlan_link'],
+                'name': name,
                 'vlan_id': link['vlan_id'],
                 'mac_address': link['vlan_mac_address'],
             })
+            link_updates.append((cfg, 'vlan_link', '%s', link['vlan_link']))
+            link_updates.append((cfg, 'name', "%%s.%s" % link['vlan_id'],
+                                 link['vlan_link']))
+            curinfo.update({'mac': link['vlan_mac_address'],
+                            'name': name})
         else:
             raise ValueError(
                 'Unknown network_data link type: %s' % link['type'])
 
         config.append(cfg)
+        link_id_info[curinfo['id']] = curinfo
 
     need_names = [d for d in config
                   if d.get('type') == 'physical' and 'name' not in d]
 
-    if need_names:
+    if need_names or link_updates:
         if known_macs is None:
             known_macs = net.get_interfaces_by_mac()
 
+        # go through and fill out the link_id_info with names
+        for link_id, info in link_id_info.items():
+            if info.get('name'):
+                continue
+            if info.get('mac') in known_macs:
+                info['name'] = known_macs[info['mac']]
+
         for d in need_names:
             mac = d.get('mac_address')
             if not mac:
@@ -615,6 +651,12 @@ def convert_net_json(network_json=None, known_macs=None):
                 raise ValueError("Unable to find a system nic for %s" % d)
             d['name'] = known_macs[mac]
 
+        for cfg, key, fmt, target in link_updates:
+            if isinstance(target, (list, tuple)):
+                cfg[key] = [fmt % link_id_info[l]['name'] for l in target]
+            else:
+                cfg[key] = fmt % link_id_info[target]['name']
+
     for service in services:
         cfg = service
         cfg.update({'type': 'nameserver'})
diff --git a/tests/unittests/test_datasource/test_configdrive.py b/tests/unittests/test_datasource/test_configdrive.py
index d026994..98ff97a 100644
--- a/tests/unittests/test_datasource/test_configdrive.py
+++ b/tests/unittests/test_datasource/test_configdrive.py
@@ -137,12 +137,71 @@ NETWORK_DATA_3 = {
     ]
 }
 
+NETWORK_DATA_BOND = {
+    "services": [
+        {"type": "dns", "address": "1.1.1.191"},
+        {"type": "dns", "address": "1.1.1.4"},
+    ],
+    "networks": [
+        {"id": "network2-ipv4", "ip_address": "2.2.2.13",
+         "link": "vlan2", "netmask": "255.255.255.248",
+         "network_id": "4daf5ce8-38cf-4240-9f1a-04e86d7c6117",
+         "type": "ipv4",
+         "routes": [{"netmask": "0.0.0.0", "network": "0.0.0.0",
+                    "gateway": "2.2.2.9"}]},
+        {"id": "network3-ipv4", "ip_address": "10.0.1.5",
+         "link": "vlan3", "netmask": "255.255.255.248",
+         "network_id": "a9e2f47c-3c43-4782-94d0-e1eeef1c8c9d",
+         "type": "ipv4",
+         "routes": [{"netmask": "255.255.255.255",
+                    "network": "192.168.1.0", "gateway": "10.0.1.1"}]}
+    ],
+    "links": [
+        {"ethernet_mac_address": "0c:c4:7a:34:6e:3c",
+         "id": "eth0", "mtu": 1500, "type": "phy"},
+        {"ethernet_mac_address": "0c:c4:7a:34:6e:3d",
+         "id": "eth1", "mtu": 1500, "type": "phy"},
+        {"bond_links": ["eth0", "eth1"],
+         "bond_miimon": 100, "bond_mode": "4",
+         "bond_xmit_hash_policy": "layer3+4",
+         "ethernet_mac_address": "0c:c4:7a:34:6e:3c",
+         "id": "bond0", "type": "bond"},
+        {"ethernet_mac_address": "fa:16:3e:b3:72:30",
+         "id": "vlan2", "type": "vlan", "vlan_id": 602,
+         "vlan_link": "bond0", "vlan_mac_address": "fa:16:3e:b3:72:30"},
+        {"ethernet_mac_address": "fa:16:3e:66:ab:a6",
+         "id": "vlan3", "type": "vlan", "vlan_id": 612, "vlan_link": "bond0",
+         "vlan_mac_address": "fa:16:3e:66:ab:a6"}
+    ]
+}
+
+NETWORK_DATA_VLAN = {
+    "services": [{"type": "dns", "address": "1.1.1.191"}],
+    "networks": [
+        {"id": "network1-ipv4", "ip_address": "10.0.1.5",
+         "link": "vlan1", "netmask": "255.255.255.248",
+         "network_id": "a9e2f47c-3c43-4782-94d0-e1eeef1c8c9d",
+         "type": "ipv4",
+         "routes": [{"netmask": "255.255.255.255",
+                    "network": "192.168.1.0", "gateway": "10.0.1.1"}]}
+    ],
+    "links": [
+        {"ethernet_mac_address": "fa:16:3e:69:b0:58",
+         "id": "eth0", "mtu": 1500, "type": "phy"},
+        {"ethernet_mac_address": "fa:16:3e:b3:72:30",
+         "id": "vlan1", "type": "vlan", "vlan_id": 602,
+         "vlan_link": "eth0", "vlan_mac_address": "fa:16:3e:b3:72:30"},
+    ]
+}
+
 KNOWN_MACS = {
     'fa:16:3e:69:b0:58': 'enp0s1',
     'fa:16:3e:d4:57:ad': 'enp0s2',
     'fa:16:3e:dd:50:9a': 'foo1',
     'fa:16:3e:a8:14:69': 'foo2',
     'fa:16:3e:ed:9a:59': 'foo3',
+    '0c:c4:7a:34:6e:3d': 'oeth1',
+    '0c:c4:7a:34:6e:3c': 'oeth0',
 }
 
 CFG_DRIVE_FILES_V2 = {
@@ -599,6 +658,52 @@ class TestConvertNetworkData(TestCase):
                 physicals.add(i['name'])
         self.assertEqual(physicals, set(('foo1', 'foo2')))
 
+    def test_bond_conversion(self):
+        # light testing of bond conversion and eni rendering of bond
+        ncfg = openstack.convert_net_json(NETWORK_DATA_BOND,
+                                          known_macs=KNOWN_MACS)
+        eni_renderer = eni.Renderer()
+        eni_renderer.render_network_state(
+            self.tmp, network_state.parse_net_config_data(ncfg))
+        with open(os.path.join(self.tmp, "etc",
+                               "network", "interfaces"), 'r') as f:
+            eni_rendering = f.read()
+
+        # Verify there are expected interfaces in the net config.
+        interfaces = sorted(
+            [i['name'] for i in ncfg['config']
+             if i['type'] in ('vlan', 'bond', 'physical')])
+        self.assertEqual(
+            sorted(["oeth0", "oeth1", "bond0", "bond0.602", "bond0.612"]),
+            interfaces)
+
+        words = eni_rendering.split()
+        # 'eth0' and 'eth1' are the ids. because their mac adresses
+        # map to other names, we should not see them in the ENI
+        self.assertNotIn('eth0', words)
+        self.assertNotIn('eth1', words)
+
+        # oeth0 and oeth1 are the interface names for eni.
+        # bond0 will be generated for the bond. Each should be auto.
+        self.assertIn("auto oeth0", eni_rendering)
+        self.assertIn("auto oeth1", eni_rendering)
+        self.assertIn("auto bond0", eni_rendering)
+
+    def test_vlan(self):
+        # light testing of vlan config conversion and eni rendering
+        ncfg = openstack.convert_net_json(NETWORK_DATA_VLAN,
+                                          known_macs=KNOWN_MACS)
+        eni_renderer = eni.Renderer()
+        eni_renderer.render_network_state(
+            self.tmp, network_state.parse_net_config_data(ncfg))
+        with open(os.path.join(self.tmp, "etc",
+                               "network", "interfaces"), 'r') as f:
+            eni_rendering = f.read()
+
+        self.assertIn("iface enp0s1", eni_rendering)
+        self.assertIn("address 10.0.1.5", eni_rendering)
+        self.assertIn("auto enp0s1.602", eni_rendering)
+
 
 def cfg_ds_from_dir(seed_d):
     cfg_ds = ds.DataSourceConfigDrive(settings.CFG_BUILTIN, None,