cloud-init-dev team mailing list archive

[Scott Moser <smoser@xxxxxxxxxx>]

Scott Moser has proposed merging ~smoser/cloud-init:bug/1663045-archlinux-empty-dns into cloud-init:master.

Requested reviews:
  cloud-init commiters (cloud-init-dev)
Related bugs:
  Bug #1663045 in cloud-init: "Arch distro fails to write network config with empty dns-nameservers"
  https://bugs.launchpad.net/cloud-init/+bug/1663045

For more details, see:
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/328114
-- 
Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:bug/1663045-archlinux-empty-dns into cloud-init:master.

diff --git a/cloudinit/distros/arch.py b/cloudinit/distros/arch.py
index b4c0ba7..123b4b7 100644
--- a/cloudinit/distros/arch.py
+++ b/cloudinit/distros/arch.py
@@ -14,6 +14,8 @@ from cloudinit.distros.parsers.hostname import HostnameConf
 
 from cloudinit.settings import PER_INSTANCE
 
+import os
+
 LOG = logging.getLogger(__name__)
 
 
@@ -52,31 +54,10 @@ class Distro(distros.Distro):
         entries = net_util.translate_network(settings)
         LOG.debug("Translated ubuntu style network settings %s into %s",
                   settings, entries)
-        dev_names = entries.keys()
-        # Format for netctl
-        for (dev, info) in entries.items():
-            nameservers = []
-            net_fn = self.network_conf_dir + dev
-            net_cfg = {
-                'Connection': 'ethernet',
-                'Interface': dev,
-                'IP': info.get('bootproto'),
-                'Address': "('%s/%s')" % (info.get('address'),
-                                          info.get('netmask')),
-                'Gateway': info.get('gateway'),
-                'DNS': str(tuple(info.get('dns-nameservers'))).replace(',', '')
-            }
-            util.write_file(net_fn, convert_netctl(net_cfg))
-            if info.get('auto'):
-                self._enable_interface(dev)
-            if 'dns-nameservers' in info:
-                nameservers.extend(info['dns-nameservers'])
-
-        if nameservers:
-            util.write_file(self.resolve_conf_fn,
-                            convert_resolv_conf(nameservers))
-
-        return dev_names
+        return _render_network(
+            entries, resolv_conf=self.resolv_conf_fn,
+            conf_dir=self.network_conf_dir,
+            enable_func=self._enable_interface)
 
     def _enable_interface(self, device_name):
         cmd = ['netctl', 'reenable', device_name]
@@ -173,13 +154,57 @@ class Distro(distros.Distro):
                          ["-y"], freq=PER_INSTANCE)
 
 
+def _render_network(entries, target="/", conf_dir="etc/netctl",
+                    resolv_conf="etc/resolv.conf", enable_func=None):
+    """Render the translate_network format into netctl files in target.
+    Paths will be rendered under target.
+    """
+
+    devs = []
+    nameservers = []
+    resolv_conf = util.target_path(target, resolv_conf)
+    conf_dir = util.target_path(target, conf_dir)
+
+    for (dev, info) in entries.items():
+        devs.append(dev)
+        net_fn = os.path.join(conf_dir, dev)
+        net_cfg = {
+            'Connection': 'ethernet',
+            'Interface': dev,
+            'IP': info.get('bootproto'),
+            'Address': "%s/%s" % (info.get('address'),
+                                  info.get('netmask')),
+            'Gateway': info.get('gateway'),
+            'DNS': info.get('dns-nameservers', []),
+        }
+        util.write_file(net_fn, convert_netctl(net_cfg))
+        if enable_func and info.get('auto'):
+            enable_func(dev)
+        if 'dns-nameservers' in info:
+            nameservers.extend(info['dns-nameservers'])
+
+    if nameservers:
+        util.write_file(resolv_conf,
+                        convert_resolv_conf(nameservers))
+    return devs
+
+
 def convert_netctl(settings):
-    """Returns a settings string formatted for netctl."""
-    result = ''
-    if isinstance(settings, dict):
-        for k, v in settings.items():
-            result = result + '%s=%s\n' % (k, v)
-        return result
+    """Given a dictionary, returns a string in netctl profile format.
+
+    netctl profile is described at:
+    https://git.archlinux.org/netctl.git/tree/docs/netctl.profile.5.txt
+
+    Note that the 'Special Quoting Rules' are not handled here."""
+    result = []
+    for key in sorted(settings):
+        val = settings[key]
+        if val is None:
+            val = ""
+        elif isinstance(val, (tuple, list)):
+            val = "(" + ' '.join("'%s'" % v for v in val) + ")"
+        result.append("%s=%s\n" % (key, val))
+    return ''.join(result)
 
 
 def convert_resolv_conf(settings):
diff --git a/tests/unittests/test_distros/__init__.py b/tests/unittests/test_distros/__init__.py
index e69de29..f8d8414 100644
--- a/tests/unittests/test_distros/__init__.py
+++ b/tests/unittests/test_distros/__init__.py
@@ -0,0 +1,20 @@
+# This file is part of cloud-init. See LICENSE file for license information.
+import copy
+
+from cloudinit import distros
+from cloudinit import helpers
+from cloudinit import settings
+
+def _get_distro(dtype, system_info=None):
+    """Return a Distro class of distro 'dtype'.
+
+    cfg is format of CFG_BUILTIN['system_info'].
+
+    example: _get_distro("debian")
+    """
+    if system_info is None:
+        system_info = copy.deepcopy(settings.CFG_BUILTIN['system_info'])
+    system_info['distro'] = dtype
+    paths = helpers.Paths(system_info['paths'])
+    distro_cls = distros.fetch(dtype)
+    return distro_cls(dtype, system_info, paths)
diff --git a/tests/unittests/test_distros/test_arch.py b/tests/unittests/test_distros/test_arch.py
new file mode 100644
index 0000000..2301ee1
--- /dev/null
+++ b/tests/unittests/test_distros/test_arch.py
@@ -0,0 +1,42 @@
+# This file is part of cloud-init. See LICENSE file for license information.
+
+from cloudinit.distros.arch import _render_network
+from cloudinit import util
+
+from ..helpers import (CiTestCase, dir2dict, mock)
+
+from . import _get_distro
+
+
+class TestArch(CiTestCase):
+
+    def test_get_distro(self):
+        distro = _get_distro("arch")
+        hostname = "myhostname"
+        hostfile = self.tmp_path("hostfile")
+        distro._write_hostname(hostname, hostfile)
+        self.assertEqual(hostname + "\n", util.load_file(hostfile))
+
+
+class TestRenderNetwork(CiTestCase):
+    def test_basic_static(self):
+        """Just the most basic static config."""
+        entries = {'eth0': {'auto': True,
+                            'dns-nameservers': ['8.8.8.8'],
+                            'bootproto': 'static',
+                            'address': '10.0.0.2',
+                            'gateway': '10.0.0.1',
+                            'netmask': '255.255.255.0'}}
+        target = self.tmp_dir()
+        devs = _render_network(entries, target=target)
+        files = dir2dict(target, prefix=target)
+        self.assertEqual(['eth0'], devs)
+        self.assertEqual(
+            {'/etc/netctl/eth0': '\n'.join([
+                "Address=10.0.0.2/255.255.255.0",
+                "Connection=ethernet",
+                "DNS=('8.8.8.8')",
+                "Gateway=10.0.0.1",
+                "IP=static",
+                "Interface=eth0", ""]),
+             '/etc/resolv.conf': 'nameserver 8.8.8.8\n'}, files)