← Back to team overview

cloud-init-dev team mailing list archive

[Merge] ~smoser/cloud-init:feature/net-render-priority into cloud-init:master

 

Scott Moser has proposed merging ~smoser/cloud-init:feature/net-render-priority into cloud-init:master.

Commit message:
net: add renderers for automatically selecting the renderer.

previously, the distro had hard coded which network renderer it would
use. This adds support for just picking the right renderer based
on what is available.

Now, that can be set via a 'priority' in system_info, but should
generally work. That config looks like:
 system_info:
   network:
     renderers: ["eni", "sysconfig"]

Requested reviews:
  cloud init development team (cloud-init-dev)

For more details, see:
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/320233
-- 
Your team cloud init development team is requested to review the proposed merge of ~smoser/cloud-init:feature/net-render-priority into cloud-init:master.
diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py
index f3d395b..77956a6 100755
--- a/cloudinit/distros/__init__.py
+++ b/cloudinit/distros/__init__.py
@@ -22,6 +22,7 @@ from cloudinit import log as logging
 from cloudinit import net
 from cloudinit.net import eni
 from cloudinit.net import network_state
+from cloudinit.net import renderers
 from cloudinit import ssh_util
 from cloudinit import type_utils
 from cloudinit import util
@@ -50,6 +51,7 @@ class Distro(object):
     hostname_conf_fn = "/etc/hostname"
     tz_zone_dir = "/usr/share/zoneinfo"
     init_cmd = ['service']  # systemctl, service etc
+    renderer_configs = {}
 
     def __init__(self, name, cfg, paths):
         self._paths = paths
@@ -69,6 +71,15 @@ class Distro(object):
     def _write_network_config(self, settings):
         raise NotImplementedError()
 
+    def _supported_write_network_config(self, network_config):
+        priority = util.get_cfg_by_path(
+            self._cfg, ('network', 'renderers'), None)
+
+        name, render_cls = renderers.select(priority=priority)
+        renderer = render_cls(config=self.renderer_configs.get(name))
+        renderer.render_network_config(network_config=network_config)
+        return []
+
     def _find_tz_file(self, tz):
         tz_file = os.path.join(self.tz_zone_dir, str(tz))
         if not os.path.isfile(tz_file):
diff --git a/cloudinit/distros/debian.py b/cloudinit/distros/debian.py
index 48ccec8..1101f02 100644
--- a/cloudinit/distros/debian.py
+++ b/cloudinit/distros/debian.py
@@ -13,8 +13,6 @@ import os
 from cloudinit import distros
 from cloudinit import helpers
 from cloudinit import log as logging
-from cloudinit.net import eni
-from cloudinit.net.network_state import parse_net_config_data
 from cloudinit import util
 
 from cloudinit.distros.parsers.hostname import HostnameConf
@@ -38,11 +36,18 @@ ENI_HEADER = """# This file is generated from information provided by
 # network: {config: disabled}
 """
 
+NETWORK_CONF_FN = "/etc/network/interfaces.d/50-cloud-init.cfg"
+
 
 class Distro(distros.Distro):
     hostname_conf_fn = "/etc/hostname"
     locale_conf_fn = "/etc/default/locale"
-    network_conf_fn = "/etc/network/interfaces.d/50-cloud-init.cfg"
+    renderer_configs = {
+        'eni': {
+            'eni_path': NETWORK_CONF_FN,
+            'eni_header': ENI_HEADER,
+        }
+    }
 
     def __init__(self, name, cfg, paths):
         distros.Distro.__init__(self, name, cfg, paths)
@@ -51,12 +56,6 @@ class Distro(distros.Distro):
         # should only happen say once per instance...)
         self._runner = helpers.Runners(paths)
         self.osfamily = 'debian'
-        self._net_renderer = eni.Renderer({
-            'eni_path': self.network_conf_fn,
-            'eni_header': ENI_HEADER,
-            'links_path_prefix': None,
-            'netrules_path': None,
-        })
 
     def apply_locale(self, locale, out_fn=None):
         if not out_fn:
@@ -76,14 +75,12 @@ class Distro(distros.Distro):
         self.package_command('install', pkgs=pkglist)
 
     def _write_network(self, settings):
-        util.write_file(self.network_conf_fn, settings)
+        util.write_file(NETWORK_CONF_FN, settings)
         return ['all']
 
     def _write_network_config(self, netconfig):
-        ns = parse_net_config_data(netconfig)
-        self._net_renderer.render_network_state("/", ns)
         _maybe_remove_legacy_eth0()
-        return []
+        return self._supported_write_network_config(netconfig)
 
     def _bring_up_interfaces(self, device_names):
         use_all = False
diff --git a/cloudinit/distros/rhel.py b/cloudinit/distros/rhel.py
index 7498c63..372c7d0 100644
--- a/cloudinit/distros/rhel.py
+++ b/cloudinit/distros/rhel.py
@@ -11,8 +11,6 @@
 from cloudinit import distros
 from cloudinit import helpers
 from cloudinit import log as logging
-from cloudinit.net.network_state import parse_net_config_data
-from cloudinit.net import sysconfig
 from cloudinit import util
 
 from cloudinit.distros import net_util
@@ -49,16 +47,13 @@ class Distro(distros.Distro):
         # should only happen say once per instance...)
         self._runner = helpers.Runners(paths)
         self.osfamily = 'redhat'
-        self._net_renderer = sysconfig.Renderer()
         cfg['ssh_svcname'] = 'sshd'
 
     def install_packages(self, pkglist):
         self.package_command('install', pkgs=pkglist)
 
     def _write_network_config(self, netconfig):
-        ns = parse_net_config_data(netconfig)
-        self._net_renderer.render_network_state("/", ns)
-        return []
+        return self._supported_write_network_config(netconfig)
 
     def _write_network(self, settings):
         # TODO(harlowja) fix this... since this is the ubuntu format
diff --git a/cloudinit/net/eni.py b/cloudinit/net/eni.py
index 5b249f1..f8c15ab 100644
--- a/cloudinit/net/eni.py
+++ b/cloudinit/net/eni.py
@@ -496,4 +496,14 @@ def network_state_to_eni(network_state, header=None, render_hwaddress=False):
         network_state, render_hwaddress=render_hwaddress)
     return header + contents
 
+
+def available(target=None):
+    expected = ['ifquery', 'ifup', 'ifdown']
+    search = ['/sbin', '/usr/sbin']
+    for p in expected:
+        if not util.which(p, search=search, target=target):
+            return False
+    return True
+
+
 # vi: ts=4 expandtab
diff --git a/cloudinit/net/renderer.py b/cloudinit/net/renderer.py
index 3a19243..a5b2b57 100644
--- a/cloudinit/net/renderer.py
+++ b/cloudinit/net/renderer.py
@@ -7,6 +7,7 @@
 
 import six
 
+from .network_state import parse_net_config_data
 from .udev import generate_udev_rule
 
 
@@ -36,4 +37,8 @@ class Renderer(object):
                                                  iface['mac_address']))
         return content.getvalue()
 
+    def render_network_config(self, network_config, target=None):
+        return self.render_network_state(
+            network_state=parse_net_config_data(network_config), target=target)
+
 # vi: ts=4 expandtab
diff --git a/cloudinit/net/renderers.py b/cloudinit/net/renderers.py
new file mode 100644
index 0000000..bd7d3f7
--- /dev/null
+++ b/cloudinit/net/renderers.py
@@ -0,0 +1,40 @@
+# This file is part of cloud-init. See LICENSE file for license information.
+
+from . import eni
+from . import sysconfig
+
+NAME_TO_RENDERER = {
+    "eni": eni,
+    "sysconfig": sysconfig,
+}
+
+DEFAULT_PRIORITY = ["eni", "sysconfig"]
+
+
+def search(priority=None, target=None, first=False):
+    if priority is None:
+        priority = DEFAULT_PRIORITY
+
+    available = NAME_TO_RENDERER
+
+    unknown = [i for i in priority if i not in available]
+    if unknown:
+        raise ValueError(
+            "Unknown renderers provided in priority list: %s" % unknown)
+
+    found = []
+    for name in priority:
+        render_mod = available[name]
+        if render_mod.available(target):
+            cur = (name, render_mod.Renderer)
+            if first:
+                return cur
+            found.append(cur)
+
+    return found
+
+
+def select(priority=None, target=None):
+    return search(priority, target=target, first=True)
+
+# vi: ts=4 expandtab
diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py
index 06de660..861c5cd 100644
--- a/cloudinit/net/sysconfig.py
+++ b/cloudinit/net/sysconfig.py
@@ -402,4 +402,21 @@ class Renderer(renderer.Renderer):
             netrules_path = os.path.join(target, self.netrules_path)
             util.write_file(netrules_path, netrules_content)
 
+
+def available(target=None):
+    expected = ['ifup', 'ifdown']
+    search = ['/sbin', '/usr/sbin']
+    for p in expected:
+        if not util.which(p, search=search, target=target):
+            return False
+
+    expected_paths = [
+        'etc/sysconfig/network-scripts/network-functions',
+        'etc/sysconfig/network-scripts/ifdown-eth']
+    for p in expected_paths:
+        if not os.path.isfile(util.target_path(target, p)):
+            return False
+    return True
+
+
 # vi: ts=4 expandtab
diff --git a/cloudinit/settings.py b/cloudinit/settings.py
index 692ff5e..dbafead 100644
--- a/cloudinit/settings.py
+++ b/cloudinit/settings.py
@@ -46,6 +46,7 @@ CFG_BUILTIN = {
             'templates_dir': '/etc/cloud/templates/',
         },
         'distro': 'ubuntu',
+        'network': {'renderers': None},
     },
     'vendor_data': {'enabled': True, 'prefix': []},
 }
diff --git a/cloudinit/util.py b/cloudinit/util.py
index 7196a7c..82f2f76 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -2099,21 +2099,36 @@ def get_mount_info(path, log=LOG):
         return parse_mount(path)
 
 
-def which(program):
-    # Return path of program for execution if found in path
-    def is_exe(fpath):
-        return os.path.isfile(fpath) and os.access(fpath, os.X_OK)
-
-    _fpath, _ = os.path.split(program)
-    if _fpath:
-        if is_exe(program):
+def is_exe(fpath):
+    # return boolean indicating if fpath exists and is executable.
+    return os.path.isfile(fpath) and os.access(fpath, os.X_OK)
+
+
+def which(program, search=None, target=None):
+    target = target_path(target)
+
+    if os.path.sep in program:
+        # if program had a '/' in it, then do not search PATH
+        # 'which' does consider cwd here. (cd / && which bin/ls) = bin/ls
+        # so effectively we set cwd to / (or target)
+        if is_exe(target_path(target, program)):
             return program
-    else:
-        for path in os.environ.get("PATH", "").split(os.pathsep):
-            path = path.strip('"')
-            exe_file = os.path.join(path, program)
-            if is_exe(exe_file):
-                return exe_file
+
+    if search is None:
+        paths = [p.strip('"') for p in
+                 os.environ.get("PATH", "").split(os.pathsep)]
+        if target == "/":
+            search = paths
+        else:
+            search = [p for p in paths if p.startswith("/")]
+
+    # normalize path input
+    search = [os.path.abspath(p) for p in search]
+
+    for path in search:
+        ppath = os.path.sep.join((path, program))
+        if is_exe(target_path(target, ppath)):
+            return ppath
 
     return None
 
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index 8d25310..af7905a 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -4,6 +4,7 @@ from cloudinit import net
 from cloudinit.net import cmdline
 from cloudinit.net import eni
 from cloudinit.net import network_state
+from cloudinit.net import renderers
 from cloudinit.net import sysconfig
 from cloudinit.sources.helpers import openstack
 from cloudinit import util
@@ -1006,6 +1007,34 @@ class TestEniRoundTrip(CiTestCase):
             expected, [line for line in found if line])
 
 
+class TestNetRenderers(CiTestCase):
+    @mock.patch("cloudinit.net.renderers.sysconfig.available")
+    @mock.patch("cloudinit.net.renderers.eni.available")
+    def test_eni_and_sysconfig_available(self, m_eni_avail, m_sysc_avail):
+        m_eni_avail.return_value = True
+        m_sysc_avail.return_value = True
+        found = renderers.search(priority=['sysconfig', 'eni'], first=False)
+        names = [f[0] for f in found]
+        self.assertEqual(['sysconfig', 'eni'], names)
+
+    @mock.patch("cloudinit.net.renderers.sysconfig.available")
+    @mock.patch("cloudinit.net.renderers.eni.available")
+    def test_first_in_priority(self, m_eni_avail, m_sysc_avail):
+        # available should only be called until one is found.
+        m_eni_avail.return_value = True
+        m_sysc_avail.side_effect = Exception("Should not call me")
+        found = renderers.search(priority=['eni', 'sysconfig'], first=True)
+        self.assertEqual(['eni'], [found[0]])
+
+    @mock.patch("cloudinit.net.renderers.sysconfig.available")
+    @mock.patch("cloudinit.net.renderers.eni.available")
+    def test_select(self, m_eni_avail, m_sysc_avail):
+        m_eni_avail.return_value = True
+        m_sysc_avail.return_value = False
+        found = renderers.select(priority=['sysconfig', 'eni'])
+        self.assertEqual('eni', found[0])
+
+
 def _gzip_data(data):
     with io.BytesIO() as iobuf:
         gzfp = gzip.GzipFile(mode="wb", fileobj=iobuf)

References