cloud-init-dev team mailing list archive

[Ben Howard <bh@xxxxx>]

Ben Howard has proposed merging ~utlemming/cloud-init:lp-1676908 into cloud-init:master.

Requested reviews:
  cloud init development team (cloud-init-dev)
Related bugs:
  Bug #1676908 in cloud-init: "DigitalOcean network improvements"
  https://bugs.launchpad.net/cloud-init/+bug/1676908

For more details, see:
https://code.launchpad.net/~utlemming/cloud-init/+git/cloud-init-1/+merge/321183

This is a request to merge the improvements to the linked PR that improves the DigitalOcean datasource.

The changes:
- No longer bind the nameservers to a specific interface to bring it inline with the other DataSources like OpenStack and SmartOS.
- Fix mis-binding the IPV4all address to a secondary interface by considering 'eth0' or 'ens3' first
- Consider all network definitions, not just 'public' or 'private'.
-- 
Your team cloud init development team is requested to review the proposed merge of ~utlemming/cloud-init:lp-1676908 into cloud-init:master.

diff --git a/cloudinit/sources/helpers/digitalocean.py b/cloudinit/sources/helpers/digitalocean.py
index 72f7bde..a9f1816 100644
--- a/cloudinit/sources/helpers/digitalocean.py
+++ b/cloudinit/sources/helpers/digitalocean.py
@@ -10,6 +10,7 @@ from cloudinit import net as cloudnet
 from cloudinit import url_helper
 from cloudinit import util
 
+FIRST_NICS = ['eth0', 'ens3']
 NIC_MAP = {'public': 'eth0', 'private': 'eth1'}
 
 LOG = logging.getLogger(__name__)
@@ -22,8 +23,13 @@ def assign_ipv4_link_local(nic=None):
        address is random.
     """
 
+    # if there is an eth0, then it has already been mapped from ens3 to eth0
+    c_devs = FIRST_NICS
+    c_devs.extend([x for x in sorted(cloudnet.get_devicelist())
+                   if x not in FIRST_NICS])
+
     if not nic:
-        for cdev in sorted(cloudnet.get_devicelist()):
+        for cdev in c_devs:
             if cloudnet.is_physical(cdev):
                 nic = cdev
                 LOG.debug("assigned nic '%s' for link-local discovery", nic)
@@ -107,15 +113,12 @@ def convert_network_configuration(config, dns_servers):
         }
     """
 
-    def _get_subnet_part(pcfg, nameservers=None):
+    def _get_subnet_part(pcfg):
         subpart = {'type': 'static',
                    'control': 'auto',
                    'address': pcfg.get('ip_address'),
                    'gateway': pcfg.get('gateway')}
 
-        if nameservers:
-            subpart['dns_nameservers'] = nameservers
-
         if ":" in pcfg.get('ip_address'):
             subpart['address'] = "{0}/{1}".format(pcfg.get('ip_address'),
                                                   pcfg.get('cidr'))
@@ -124,15 +127,13 @@ def convert_network_configuration(config, dns_servers):
 
         return subpart
 
-    all_nics = []
-    for k in ('public', 'private'):
-        if k in config:
-            all_nics.extend(config[k])
-
-    macs_to_nics = cloudnet.get_interfaces_by_mac()
     nic_configs = []
+    macs_to_nics = cloudnet.get_interfaces_by_mac()
+    LOG.debug("nic mapping: %s", macs_to_nics)
 
-    for nic in all_nics:
+    for n in config:
+        nic = config[n][0]
+        LOG.debug("considering %s", nic)
 
         mac_address = nic.get('mac')
         sysfs_name = macs_to_nics.get(mac_address)
@@ -157,13 +158,8 @@ def convert_network_configuration(config, dns_servers):
                 continue
 
             sub_part = _get_subnet_part(raw_subnet)
-            if nic_type == 'public' and 'anchor' not in netdef:
-                # add DNS resolvers to the public interfaces only
-                sub_part = _get_subnet_part(raw_subnet, dns_servers)
-            else:
-                # remove the gateway any non-public interfaces
-                if 'gateway' in sub_part:
-                    del sub_part['gateway']
+            if netdef in ('private', 'anchor_ipv4', 'anchor_ipv6'):
+                del sub_part['gateway']
 
             subnets.append(sub_part)
 
@@ -171,6 +167,10 @@ def convert_network_configuration(config, dns_servers):
         nic_configs.append(ncfg)
         LOG.debug("nic '%s' configuration: %s", if_name, ncfg)
 
+    if dns_servers:
+        LOG.debug("added dns servers")
+        nic_configs.append({'type': 'nameserver', 'address': dns_servers})
+
     return {'version': 1, 'config': nic_configs}
 
 
diff --git a/tests/unittests/test_datasource/test_digitalocean.py b/tests/unittests/test_datasource/test_digitalocean.py
index 9be6bc1..c022e05 100644
--- a/tests/unittests/test_datasource/test_digitalocean.py
+++ b/tests/unittests/test_datasource/test_digitalocean.py
@@ -204,17 +204,24 @@ class TestNetworkConvert(TestCase):
         netcfg = self._get_networking()
         self.assertIsNotNone(netcfg)
 
-        for nic_def in netcfg.get('config'):
-            print(json.dumps(nic_def, indent=3))
-            n_type = nic_def.get('type')
-            n_subnets = nic_def.get('type')
-            n_name = nic_def.get('name')
-            n_mac = nic_def.get('mac_address')
+        for part in netcfg.get('config'):
+            print(json.dumps(part, indent=3))
+            n_type = part.get('type')
+
+            if n_type == 'nameserver':
+                n_address = part.get('address')
+                self.assertIsNotNone(n_address)
+                self.assertEqual(len(n_address), 3)
 
-            self.assertIsNotNone(n_type)
-            self.assertIsNotNone(n_subnets)
-            self.assertIsNotNone(n_name)
-            self.assertIsNotNone(n_mac)
+            else:
+                n_subnets = part.get('type')
+                n_name = part.get('name')
+                n_mac = part.get('mac_address')
+
+                self.assertIsNotNone(n_type)
+                self.assertIsNotNone(n_subnets)
+                self.assertIsNotNone(n_name)
+                self.assertIsNotNone(n_mac)
 
     def _get_nic_definition(self, int_type, expected_name):
         """helper function to return if_type (i.e. public) and the expected
@@ -255,12 +262,6 @@ class TestNetworkConvert(TestCase):
         self.assertEqual(meta_def.get('mac'), nic_def.get('mac_address'))
         self.assertEqual('physical', nic_def.get('type'))
 
-    def _check_dns_nameservers(self, subn_def):
-        self.assertIn('dns_nameservers', subn_def)
-        expected_nameservers = DO_META['dns']['nameservers']
-        nic_nameservers = subn_def.get('dns_nameservers')
-        self.assertEqual(expected_nameservers, nic_nameservers)
-
     def test_public_interface_ipv6(self):
         """test public ipv6 addressing"""
         (nic_def, meta_def) = self._get_nic_definition('public', 'eth0')
@@ -275,7 +276,6 @@ class TestNetworkConvert(TestCase):
 
         self.assertEqual(cidr_notated_address, subn_def.get('address'))
         self.assertEqual(ipv6_def.get('gateway'), subn_def.get('gateway'))
-        self._check_dns_nameservers(subn_def)
 
     def test_public_interface_ipv4(self):
         """test public ipv4 addressing"""
@@ -288,7 +288,6 @@ class TestNetworkConvert(TestCase):
 
         self.assertEqual(ipv4_def.get('netmask'), subn_def.get('netmask'))
         self.assertEqual(ipv4_def.get('gateway'), subn_def.get('gateway'))
-        self._check_dns_nameservers(subn_def)
 
     def test_public_interface_anchor_ipv4(self):
         """test public ipv4 addressing"""