From 5beecdf88b630a397b3722ddb299e9a37ff02737 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Wed, 15 Mar 2017 14:33:45 -0400 Subject: 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"] When no renderers are found, a specific RendererNotFoundError is raised. stages.py is modified to catch that and log it at error level. This path should not really be exercised, but could occur if for example an Ubuntu system did not have ifupdown, or a rhel system did not have sysconfig. In such a system previously we would have quietly rendered ENI configuration but that would have been ignored. This is one step better in that we at least log the error. --- cloudinit/distros/__init__.py | 13 +++++++++++ cloudinit/distros/debian.py | 23 +++++++++---------- cloudinit/distros/rhel.py | 7 +----- cloudinit/net/__init__.py | 5 +++++ cloudinit/net/eni.py | 14 ++++++++++++ cloudinit/net/renderer.py | 5 +++++ cloudinit/net/renderers.py | 51 +++++++++++++++++++++++++++++++++++++++++++ cloudinit/net/sysconfig.py | 17 +++++++++++++++ cloudinit/settings.py | 1 + cloudinit/stages.py | 6 ++++- cloudinit/util.py | 43 ++++++++++++++++++++++++------------ tests/unittests/test_net.py | 45 ++++++++++++++++++++++++++++++++++++++ 12 files changed, 196 insertions(+), 34 deletions(-) create mode 100644 cloudinit/net/renderers.py diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index f3d395b9..803ac74e 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,17 @@ 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) + LOG.debug("Selected renderer '%s' from priority list: %s", + name, 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 48ccec8c..1101f02d 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 7498c63a..372c7d0f 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/__init__.py b/cloudinit/net/__init__.py index ea649cc2..1cf98ef5 100755 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -364,4 +364,9 @@ def get_interfaces_by_mac(devs=None): ret[mac] = name return ret + +class RendererNotFoundError(RuntimeError): + pass + + # vi: ts=4 expandtab diff --git a/cloudinit/net/eni.py b/cloudinit/net/eni.py index efa034bf..9d39a2b0 100644 --- a/cloudinit/net/eni.py +++ b/cloudinit/net/eni.py @@ -500,4 +500,18 @@ 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 + eni = util.target_path(target, 'etc/network/interfaces') + if not os.path.is_file(eni): + return False + + return True + + # vi: ts=4 expandtab diff --git a/cloudinit/net/renderer.py b/cloudinit/net/renderer.py index 3a192436..a5b2b573 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 00000000..5ad84553 --- /dev/null +++ b/cloudinit/net/renderers.py @@ -0,0 +1,51 @@ +# This file is part of cloud-init. See LICENSE file for license information. + +from . import eni +from . import RendererNotFoundError +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): + found = search(priority, target=target, first=True) + if not found: + if priority is None: + priority = DEFAULT_PRIORITY + tmsg = "" + if target and target != "/": + tmsg = " in target=%s" % target + raise RendererNotFoundError( + "No available network renderers found%s. Searched " + "through list: %s" % (tmsg, priority)) + return found + +# vi: ts=4 expandtab diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py index 4eeaaa8a..117b515c 100644 --- a/cloudinit/net/sysconfig.py +++ b/cloudinit/net/sysconfig.py @@ -404,4 +404,21 @@ class Renderer(renderer.Renderer): netrules_path = util.target_path(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 692ff5e5..dbafead5 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/stages.py b/cloudinit/stages.py index 5bed9032..12165433 100644 --- a/cloudinit/stages.py +++ b/cloudinit/stages.py @@ -646,9 +646,13 @@ class Init(object): src, bring_up, netcfg) try: return self.distro.apply_network_config(netcfg, bring_up=bring_up) + except net.RendererNotFoundError as e: + LOG.error("Unable to render networking. Network config is " + "likely broken: %s", e) + return except NotImplementedError: LOG.warn("distro '%s' does not implement apply_network_config. " - "networking may not be configured properly." % + "networking may not be configured properly.", self.distro) return diff --git a/cloudinit/util.py b/cloudinit/util.py index 7196a7ca..82f2f76b 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 dca44b37..902204a0 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 @@ -1050,6 +1051,50 @@ 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.eni.available") + def test_search_returns_empty_on_none(self, m_eni_avail): + m_eni_avail.return_value = False + found = renderers.search(priority=['eni'], first=False) + self.assertEqual([], found) + + @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_positive(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]) + + @mock.patch("cloudinit.net.renderers.sysconfig.available") + @mock.patch("cloudinit.net.renderers.eni.available") + def test_select_none_found_raises(self, m_eni_avail, m_sysc_avail): + # if select finds nothing, should raise exception. + m_eni_avail.return_value = False + m_sysc_avail.return_value = False + + self.assertRaises(net.RendererNotFoundError, renderers.select, + priority=['sysconfig', 'eni']) + + def _gzip_data(data): with io.BytesIO() as iobuf: gzfp = gzip.GzipFile(mode="wb", fileobj=iobuf) -- cgit v1.2.3