OpenNebula: Improve network configuration support.

Network configuration in OpenNebula would only work if the host correctly
guessed the names of the devices in the guest. OpenNebula provided data
in its context.sh like 'ETH0_NETWORK', but if the guest named devices
differently then results were not predictable. This would occur with
Predictable Network Interface Names. To address this,
newer versions (of OpenNebula provide the mac address ETH0_MAC.
This function is present in 4.14 and documented officially in 5.0 docs.

This provides support for reading the mac addresses from the context.sh.
It also fixes cases where context.sh provided a field (ETH0_NETWORK
or ETH0_MASK) with a empty string. Previously the empty string would
be used rather than falling back to the default.

LP: #1719157, #1716397, #1736750

4 files changed, 241 insertions, 104 deletions

diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py
index a1b0db10..c015e793 100644
--- a/cloudinit/net/__init__.py
+++ b/cloudinit/net/__init__.py
@@ -18,7 +18,7 @@ SYS_CLASS_NET = "/sys/class/net/"
 DEFAULT_PRIMARY_INTERFACE = 'eth0'
 
 
-def _natural_sort_key(s, _nsre=re.compile('([0-9]+)')):
+def natural_sort_key(s, _nsre=re.compile('([0-9]+)')):
     """Sorting for Humans: natural sort order. Can be use as the key to sort
     functions.
     This will sort ['eth0', 'ens3', 'ens10', 'ens12', 'ens8', 'ens0'] as
@@ -224,7 +224,7 @@ def find_fallback_nic(blacklist_drivers=None):
 
     # if eth0 exists use it above anything else, otherwise get the interface
     # that we can read 'first' (using the sorted defintion of first).
-    names = list(sorted(potential_interfaces, key=_natural_sort_key))
+    names = list(sorted(potential_interfaces, key=natural_sort_key))
     if DEFAULT_PRIMARY_INTERFACE in names:
         names.remove(DEFAULT_PRIMARY_INTERFACE)
         names.insert(0, DEFAULT_PRIMARY_INTERFACE)
diff --git a/cloudinit/sources/DataSourceOpenNebula.py b/cloudinit/sources/DataSourceOpenNebula.py
index f66c95d7..ce47b6bd 100644
--- a/cloudinit/sources/DataSourceOpenNebula.py
+++ b/cloudinit/sources/DataSourceOpenNebula.py
@@ -12,6 +12,7 @@
 #
 # This file is part of cloud-init. See LICENSE file for license information.
 
+import collections
 import os
 import pwd
 import re
@@ -19,6 +20,7 @@ import string
 
 from cloudinit import log as logging
 from cloudinit import net
+from cloudinit.net import eni
 from cloudinit import sources
 from cloudinit import util
 
@@ -89,11 +91,18 @@ class DataSourceOpenNebula(sources.DataSource):
             return False
 
         self.seed = seed
-        self.network_eni = results.get("network_config")
+        self.network_eni = results.get('network-interfaces')
         self.metadata = md
         self.userdata_raw = results.get('userdata')
         return True
 
+    @property
+    def network_config(self):
+        if self.network_eni is not None:
+            return eni.convert_eni_data(self.network_eni)
+        else:
+            return None
+
     def get_hostname(self, fqdn=False, resolve_ip=None):
         if resolve_ip is None:
             if self.dsmode == sources.DSMODE_NETWORK:
@@ -116,58 +125,53 @@ class OpenNebulaNetwork(object):
         self.context = context
         if system_nics_by_mac is None:
             system_nics_by_mac = get_physical_nics_by_mac()
-        self.ifaces = system_nics_by_mac
+        self.ifaces = collections.OrderedDict(
+            [k for k in sorted(system_nics_by_mac.items(),
+                               key=lambda k: net.natural_sort_key(k[1]))])
+
+        # OpenNebula 4.14+ provide macaddr for ETHX in variable ETH_MAC.
+        # context_devname provides {mac.lower():ETHX, mac2.lower():ETHX}
+        self.context_devname = {}
+        for k, v in context.items():
+            m = re.match(r'^(.+)_MAC$', k)
+            if m:
+                self.context_devname[v.lower()] = m.group(1)
 
     def mac2ip(self, mac):
-        components = mac.split(':')[2:]
-        return [str(int(c, 16)) for c in components]
+        return '.'.join([str(int(c, 16)) for c in mac.split(':')[2:]])
 
-    def get_ip(self, dev, components):
-        var_name = dev.upper() + '_IP'
-        if var_name in self.context:
-            return self.context[var_name]
-        else:
-            return '.'.join(components)
+    def mac2network(self, mac):
+        return self.mac2ip(mac).rpartition(".")[0] + ".0"
 
-    def get_mask(self, dev):
-        var_name = dev.upper() + '_MASK'
-        if var_name in self.context:
-            return self.context[var_name]
-        else:
-            return '255.255.255.0'
+    def get_dns(self, dev):
+        return self.get_field(dev, "dns", "").split()
 
-    def get_network(self, dev, components):
-        var_name = dev.upper() + '_NETWORK'
-        if var_name in self.context:
-            return self.context[var_name]
-        else:
-            return '.'.join(components[:-1]) + '.0'
+    def get_domain(self, dev):
+        return self.get_field(dev, "domain")
+
+    def get_ip(self, dev, mac):
+        return self.get_field(dev, "ip", self.mac2ip(mac))
 
     def get_gateway(self, dev):
-        var_name = dev.upper() + '_GATEWAY'
-        if var_name in self.context:
-            return self.context[var_name]
-        else:
-            return None
+        return self.get_field(dev, "gateway")
 
-    def get_dns(self, dev):
-        var_name = dev.upper() + '_DNS'
-        if var_name in self.context:
-            return self.context[var_name]
-        else:
-            return None
+    def get_mask(self, dev):
+        return self.get_field(dev, "mask", "255.255.255.0")
 
-    def get_domain(self, dev):
-        var_name = dev.upper() + '_DOMAIN'
-        if var_name in self.context:
-            return self.context[var_name]
-        else:
-            return None
+    def get_network(self, dev, mac):
+        return self.get_field(dev, "network", self.mac2network(mac))
+
+    def get_field(self, dev, name, default=None):
+        """return the field name in context for device dev.
+
+        context stores <dev>_<NAME> (example: eth0_DOMAIN).
+        an empty string for value will return default."""
+        val = self.context.get('_'.join((dev, name,)).upper())
+        # allow empty string to return the default.
+        return default if val in (None, "") else val
 
     def gen_conf(self):
-        global_dns = []
-        if 'DNS' in self.context:
-            global_dns.append(self.context['DNS'])
+        global_dns = self.context.get('DNS', "").split()
 
         conf = []
         conf.append('auto lo')
@@ -175,29 +179,31 @@ class OpenNebulaNetwork(object):
         conf.append('')
 
         for mac, dev in self.ifaces.items():
-            ip_components = self.mac2ip(mac)
+            mac = mac.lower()
+
+            # c_dev stores name in context 'ETHX' for this device.
+            # dev stores the current system name.
+            c_dev = self.context_devname.get(mac, dev)
 
             conf.append('auto ' + dev)
             conf.append('iface ' + dev + ' inet static')
-            conf.append('  address ' + self.get_ip(dev, ip_components))
-            conf.append('  network ' + self.get_network(dev, ip_components))
-            conf.append('  netmask ' + self.get_mask(dev))
+            conf.append('  #hwaddress %s' % mac)
+            conf.append('  address ' + self.get_ip(c_dev, mac))
+            conf.append('  network ' + self.get_network(c_dev, mac))
+            conf.append('  netmask ' + self.get_mask(c_dev))
 
-            gateway = self.get_gateway(dev)
+            gateway = self.get_gateway(c_dev)
             if gateway:
                 conf.append('  gateway ' + gateway)
 
-            domain = self.get_domain(dev)
+            domain = self.get_domain(c_dev)
             if domain:
                 conf.append('  dns-search ' + domain)
 
             # add global DNS servers to all interfaces
-            dns = self.get_dns(dev)
+            dns = self.get_dns(c_dev)
             if global_dns or dns:
-                all_dns = global_dns
-                if dns:
-                    all_dns.append(dns)
-                conf.append('  dns-nameservers ' + ' '.join(all_dns))
+                conf.append('  dns-nameservers ' + ' '.join(global_dns + dns))
 
             conf.append('')
 
diff --git a/tests/unittests/test_datasource/test_opennebula.py b/tests/unittests/test_datasource/test_opennebula.py
index 2326dd58..5c3ba012 100644
--- a/tests/unittests/test_datasource/test_opennebula.py
+++ b/tests/unittests/test_datasource/test_opennebula.py
@@ -4,6 +4,7 @@ from cloudinit import helpers
 from cloudinit.sources import DataSourceOpenNebula as ds
 from cloudinit import util
 from cloudinit.tests.helpers import mock, populate_dir, CiTestCase
+from textwrap import dedent
 
 import os
 import pwd
@@ -30,6 +31,8 @@ USER_DATA = '#cloud-config\napt_upgrade: true'
 SSH_KEY = 'ssh-rsa AAAAB3NzaC1....sIkJhq8wdX+4I3A4cYbYP ubuntu@server-460-%i'
 HOSTNAME = 'foo.example.com'
 PUBLIC_IP = '10.0.0.3'
+MACADDR = '02:00:0a:12:01:01'
+IP_BY_MACADDR = '10.18.1.1'
 
 DS_PATH = "cloudinit.sources.DataSourceOpenNebula"
 
@@ -195,24 +198,96 @@ class TestOpenNebulaDataSource(CiTestCase):
 
     @mock.patch(DS_PATH + ".get_physical_nics_by_mac")
     def test_hostname(self, m_get_phys_by_mac):
-        m_get_phys_by_mac.return_value = {'02:00:0a:12:01:01': 'eth0'}
-        for k in ('HOSTNAME', 'PUBLIC_IP', 'IP_PUBLIC', 'ETH0_IP'):
-            my_d = os.path.join(self.tmp, k)
-            populate_context_dir(my_d, {k: PUBLIC_IP})
-            results = ds.read_context_disk_dir(my_d)
+        for dev in ('eth0', 'ens3'):
+            m_get_phys_by_mac.return_value = {MACADDR: dev}
+            for k in ('HOSTNAME', 'PUBLIC_IP', 'IP_PUBLIC', 'ETH0_IP'):
+                my_d = os.path.join(self.tmp, k)
+                populate_context_dir(my_d, {k: PUBLIC_IP})
+                results = ds.read_context_disk_dir(my_d)
 
-            self.assertTrue('metadata' in results)
-            self.assertTrue('local-hostname' in results['metadata'])
-            self.assertEqual(PUBLIC_IP, results['metadata']['local-hostname'])
+                self.assertTrue('metadata' in results)
+                self.assertTrue('local-hostname' in results['metadata'])
+                self.assertEqual(
+                    PUBLIC_IP, results['metadata']['local-hostname'])
 
     @mock.patch(DS_PATH + ".get_physical_nics_by_mac")
     def test_network_interfaces(self, m_get_phys_by_mac):
-        m_get_phys_by_mac.return_value = {'02:00:0a:12:01:01': 'eth0'}
-        populate_context_dir(self.seed_dir, {'ETH0_IP': '1.2.3.4'})
-        results = ds.read_context_disk_dir(self.seed_dir)
-
-        self.assertTrue('network-interfaces' in results)
-        self.assertTrue('1.2.3.4' in results['network-interfaces'])
+        for dev in ('eth0', 'ens3'):
+            m_get_phys_by_mac.return_value = {MACADDR: dev}
+
+            # without ETH0_MAC
+            # for Older OpenNebula?
+            populate_context_dir(self.seed_dir, {'ETH0_IP': IP_BY_MACADDR})
+            results = ds.read_context_disk_dir(self.seed_dir)
+
+            self.assertTrue('network-interfaces' in results)
+            self.assertTrue(IP_BY_MACADDR in results['network-interfaces'])
+
+            # ETH0_IP and ETH0_MAC
+            populate_context_dir(
+                self.seed_dir, {'ETH0_IP': IP_BY_MACADDR, 'ETH0_MAC': MACADDR})
+            results = ds.read_context_disk_dir(self.seed_dir)
+
+            self.assertTrue('network-interfaces' in results)
+            self.assertTrue(IP_BY_MACADDR in results['network-interfaces'])
+
+            # ETH0_IP with empty string and ETH0_MAC
+            # in the case of using Virtual Network contains
+            # "AR = [ TYPE = ETHER ]"
+            populate_context_dir(
+                self.seed_dir, {'ETH0_IP': '', 'ETH0_MAC': MACADDR})
+            results = ds.read_context_disk_dir(self.seed_dir)
+
+            self.assertTrue('network-interfaces' in results)
+            self.assertTrue(IP_BY_MACADDR in results['network-interfaces'])
+
+            # ETH0_NETWORK
+            populate_context_dir(
+                self.seed_dir, {
+                    'ETH0_IP': IP_BY_MACADDR,
+                    'ETH0_MAC': MACADDR,
+                    'ETH0_NETWORK': '10.18.0.0'
+                })
+            results = ds.read_context_disk_dir(self.seed_dir)
+
+            self.assertTrue('network-interfaces' in results)
+            self.assertTrue('10.18.0.0' in results['network-interfaces'])
+
+            # ETH0_NETWORK with empty string
+            populate_context_dir(
+                self.seed_dir, {
+                    'ETH0_IP': IP_BY_MACADDR,
+                    'ETH0_MAC': MACADDR,
+                    'ETH0_NETWORK': ''
+                })
+            results = ds.read_context_disk_dir(self.seed_dir)
+
+            self.assertTrue('network-interfaces' in results)
+            self.assertTrue('10.18.1.0' in results['network-interfaces'])
+
+            # ETH0_MASK
+            populate_context_dir(
+                self.seed_dir, {
+                    'ETH0_IP': IP_BY_MACADDR,
+                    'ETH0_MAC': MACADDR,
+                    'ETH0_MASK': '255.255.0.0'
+                })
+            results = ds.read_context_disk_dir(self.seed_dir)
+
+            self.assertTrue('network-interfaces' in results)
+            self.assertTrue('255.255.0.0' in results['network-interfaces'])
+
+            # ETH0_MASK with empty string
+            populate_context_dir(
+                self.seed_dir, {
+                    'ETH0_IP': IP_BY_MACADDR,
+                    'ETH0_MAC': MACADDR,
+                    'ETH0_MASK': ''
+                })
+            results = ds.read_context_disk_dir(self.seed_dir)
+
+            self.assertTrue('network-interfaces' in results)
+            self.assertTrue('255.255.255.0' in results['network-interfaces'])
 
     def test_find_candidates(self):
         def my_devs_with(criteria):
@@ -233,7 +308,7 @@ class TestOpenNebulaDataSource(CiTestCase):
 
 class TestOpenNebulaNetwork(unittest.TestCase):
 
-    system_nics = {'02:00:0a:12:01:01': 'eth0'}
+    system_nics = ('eth0', 'ens3')
 
     def test_lo(self):
         net = ds.OpenNebulaNetwork(context={}, system_nics_by_mac={})
@@ -244,45 +319,101 @@ iface lo inet loopback
 
     @mock.patch(DS_PATH + ".get_physical_nics_by_mac")
     def test_eth0(self, m_get_phys_by_mac):
-        m_get_phys_by_mac.return_value = self.system_nics
-        net = ds.OpenNebulaNetwork({})
-        self.assertEqual(net.gen_conf(), u'''\
-auto lo
-iface lo inet loopback
-
-auto eth0
-iface eth0 inet static
-  address 10.18.1.1
-  network 10.18.1.0
-  netmask 255.255.255.0
-''')
+        for nic in self.system_nics:
+            m_get_phys_by_mac.return_value = {MACADDR: nic}
+            net = ds.OpenNebulaNetwork({})
+            self.assertEqual(net.gen_conf(), dedent("""\
+                auto lo
+                iface lo inet loopback
+
+                auto {dev}
+                iface {dev} inet static
+                  #hwaddress {macaddr}
+                  address 10.18.1.1
+                  network 10.18.1.0
+                  netmask 255.255.255.0
+                """.format(dev=nic, macaddr=MACADDR)))
 
     def test_eth0_override(self):
         context = {
             'DNS': '1.2.3.8',
-            'ETH0_IP': '1.2.3.4',
-            'ETH0_NETWORK': '1.2.3.0',
+            'ETH0_IP': '10.18.1.1',
+            'ETH0_NETWORK': '10.18.0.0',
             'ETH0_MASK': '255.255.0.0',
             'ETH0_GATEWAY': '1.2.3.5',
             'ETH0_DOMAIN': 'example.com',
-            'ETH0_DNS': '1.2.3.6 1.2.3.7'
+            'ETH0_DNS': '1.2.3.6 1.2.3.7',
+            'ETH0_MAC': '02:00:0a:12:01:01'
         }
-
-        net = ds.OpenNebulaNetwork(context,
-                                   system_nics_by_mac=self.system_nics)
-        self.assertEqual(net.gen_conf(), u'''\
-auto lo
-iface lo inet loopback
-
-auto eth0
-iface eth0 inet static
-  address 1.2.3.4
-  network 1.2.3.0
-  netmask 255.255.0.0
-  gateway 1.2.3.5
-  dns-search example.com
-  dns-nameservers 1.2.3.8 1.2.3.6 1.2.3.7
-''')
+        for nic in self.system_nics:
+            expected = dedent("""\
+                auto lo
+                iface lo inet loopback
+
+                auto {dev}
+                iface {dev} inet static
+                  #hwaddress {macaddr}
+                  address 10.18.1.1
+                  network 10.18.0.0
+                  netmask 255.255.0.0
+                  gateway 1.2.3.5
+                  dns-search example.com
+                  dns-nameservers 1.2.3.8 1.2.3.6 1.2.3.7
+                  """).format(dev=nic, macaddr=MACADDR)
+            net = ds.OpenNebulaNetwork(context,
+                                       system_nics_by_mac={MACADDR: nic})
+            self.assertEqual(expected, net.gen_conf())
+
+    def test_multiple_nics(self):
+        """Test rendering multiple nics with names that differ from context."""
+        MAC_1 = "02:00:0a:12:01:01"
+        MAC_2 = "02:00:0a:12:01:02"
+        context = {
+            'DNS': '1.2.3.8',
+            'ETH0_IP': '10.18.1.1',
+            'ETH0_NETWORK': '10.18.0.0',
+            'ETH0_MASK': '255.255.0.0',
+            'ETH0_GATEWAY': '1.2.3.5',
+            'ETH0_DOMAIN': 'example.com',
+            'ETH0_DNS': '1.2.3.6 1.2.3.7',
+            'ETH0_MAC': MAC_2,
+            'ETH3_IP': '10.3.1.3',
+            'ETH3_NETWORK': '10.3.0.0',
+            'ETH3_MASK': '255.255.0.0',
+            'ETH3_GATEWAY': '10.3.0.1',
+            'ETH3_DOMAIN': 'third.example.com',
+            'ETH3_DNS': '10.3.1.2',
+            'ETH3_MAC': MAC_1,
+        }
+        net = ds.OpenNebulaNetwork(
+            context, system_nics_by_mac={MAC_1: 'enp0s25', MAC_2: 'enp1s2'})
+
+        expected = dedent("""\
+            auto lo
+            iface lo inet loopback
+
+            auto enp0s25
+            iface enp0s25 inet static
+              #hwaddress 02:00:0a:12:01:01
+              address 10.3.1.3
+              network 10.3.0.0
+              netmask 255.255.0.0
+              gateway 10.3.0.1
+              dns-search third.example.com
+              dns-nameservers 1.2.3.8 10.3.1.2
+
+            auto enp1s2
+            iface enp1s2 inet static
+              #hwaddress 02:00:0a:12:01:02
+              address 10.18.1.1
+              network 10.18.0.0
+              netmask 255.255.0.0
+              gateway 1.2.3.5
+              dns-search example.com
+              dns-nameservers 1.2.3.8 1.2.3.6 1.2.3.7
+            """)
+
+        self.assertEqual(expected, net.gen_conf())
 
 
 class TestParseShellConfig(unittest.TestCase):
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index f3fa2a30..ddea13d7 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -1,9 +1,9 @@
 # This file is part of cloud-init. See LICENSE file for license information.
 
 from cloudinit import net
-from cloudinit.net import _natural_sort_key
 from cloudinit.net import cmdline
 from cloudinit.net import eni
+from cloudinit.net import natural_sort_key
 from cloudinit.net import netplan
 from cloudinit.net import network_state
 from cloudinit.net import renderers
@@ -2708,11 +2708,11 @@ class TestInterfacesSorting(CiTestCase):
     def test_natural_order(self):
         data = ['ens5', 'ens6', 'ens3', 'ens20', 'ens13', 'ens2']
         self.assertEqual(
-            sorted(data, key=_natural_sort_key),
+            sorted(data, key=natural_sort_key),
             ['ens2', 'ens3', 'ens5', 'ens6', 'ens13', 'ens20'])
         data2 = ['enp2s0', 'enp2s3', 'enp0s3', 'enp0s13', 'enp0s8', 'enp1s2']
         self.assertEqual(
-            sorted(data2, key=_natural_sort_key),
+            sorted(data2, key=natural_sort_key),
             ['enp0s3', 'enp0s8', 'enp0s13', 'enp1s2', 'enp2s0', 'enp2s3'])