Merge lp:~smoser/cloud-init/trunk.lp1602373 into lp:~cloud-init-dev/cloud-init/trunk

[Scott Moser]

ConfigDrive: fix writing of 'injected' files and legacy networking

Previous commit inadvertently disabled the consumption of 'injected' files
in configdrive (openstack server boot --file=/target/file=local-file)
unless the datasource was in 'pass' mode. The default mode is 'net' so
that would never happen.

Also here are:
a.) some comments to apply_network_config
b.) add backwards compatibility for distros that do not yet implement
    apply_network_config by converting the network config into ENI format
    and calling apply_network.

    This is required because prior to the previous commit, those distros
    would have had 'apply_network' called with the openstack provided
    ENI file. But after this change they will have apply_network_config
    called by cloudinit's main.

c.) add network_state_to_eni for converting net config to eni
    it supports the not-actually-correct 'hwaddress' field in ENI

[lp:~smoser/cloud-init/trunk.lp1602373]

1247. By Scott Moser on 2016-07-14

pass the return back up, shorten lines some.

1248. By Scott Moser on 2016-07-14

merge from trunk

1249. By Scott Moser on 2016-07-14

add test of apply_network fallback path.

we could do this more simply by mocking fbsd.apply_network
and checking it's inputs. but this pushes it through the whole
path that the other test does.

1250. By Scott Moser on 2016-07-14

flake8

=== modified file 'cloudinit/distros/__init__.py'
--- cloudinit/distros/__init__.py	2016-06-16 02:50:12 +0000
+++ cloudinit/distros/__init__.py	2016-07-14 18:21:43 +0000
@@ -32,6 +32,8 @@
 from cloudinit import importer
 from cloudinit import log as logging
 from cloudinit import net
+from cloudinit.net import eni
+from cloudinit.net import network_state
 from cloudinit import ssh_util
 from cloudinit import type_utils
 from cloudinit import util
@@ -138,9 +140,31 @@
             return self._bring_up_interfaces(dev_names)
         return False
 
+    def _apply_network_from_network_config(self, netconfig, bring_up=True):
+        distro = self.__class__
+        LOG.warn("apply_network_config is not currently implemented "
+                 "for distribution '%s'.  Attempting to use apply_network",
+                 distro)
+        header = '\n'.join([
+            "# Converted from network_config for distro %s" % distro,
+            "# Implmentation of _write_network_config is needed."
+        ])
+        ns = network_state.parse_net_config_data(netconfig)
+        contents = eni.network_state_to_eni(
+            ns, header=header, render_hwaddress=True)
+        return self.apply_network(contents, bring_up=bring_up)
+
     def apply_network_config(self, netconfig, bring_up=False):
-        # Write it out
-        dev_names = self._write_network_config(netconfig)
+        # apply network config netconfig
+        # This method is preferred to apply_network which only takes
+        # a much less complete network config format (interfaces(5)).
+        try:
+            dev_names = self._write_network_config(netconfig)
+        except NotImplementedError:
+            # backwards compat until all distros have apply_network_config
+            return self._apply_network_from_network_config(
+                netconfig, bring_up=bring_up)
+
         # Now try to bring them up
         if bring_up:
             return self._bring_up_interfaces(dev_names)

=== modified file 'cloudinit/net/eni.py'
--- cloudinit/net/eni.py	2016-07-13 22:10:54 +0000
+++ cloudinit/net/eni.py	2016-07-14 18:21:43 +0000
@@ -352,7 +352,7 @@
             content += down + route_line + eol
         return content
 
-    def _render_interfaces(self, network_state):
+    def _render_interfaces(self, network_state, render_hwaddress=False):
         '''Given state, emit etc/network/interfaces content.'''
 
         content = ""
@@ -397,6 +397,8 @@
                         iface['mode'] = 'dhcp'
 
                     content += _iface_start_entry(iface, index)
+                    if render_hwaddress and iface.get('mac_address'):
+                        content += "    hwaddress %s" % iface['mac_address']
                     content += _iface_add_subnet(iface, subnet)
                     content += _iface_add_attrs(iface)
                     for route in subnet.get('routes', []):
@@ -411,8 +413,6 @@
         for route in network_state.iter_routes():
             content += self._render_route(route)
 
-        # global replacements until v2 format
-        content = content.replace('mac_address', 'hwaddress')
         return content
 
     def render_network_state(self, target, network_state):
@@ -448,3 +448,21 @@
                     ""
                 ])
                 util.write_file(fname, content)
+
+
+def network_state_to_eni(network_state, header=None, render_hwaddress=False):
+    # render the provided network state, return a string of equivalent eni
+    eni_path = 'etc/network/interfaces'
+    renderer = Renderer({
+        'eni_path': eni_path,
+        'eni_header': header,
+        'links_path_prefix': None,
+        'netrules_path': None,
+    })
+    if not header:
+        header = ""
+    if not header.endswith("\n"):
+        header += "\n"
+    contents = renderer._render_interfaces(
+        network_state, render_hwaddress=render_hwaddress)
+    return header + contents

=== modified file 'cloudinit/sources/DataSourceConfigDrive.py'
--- cloudinit/sources/DataSourceConfigDrive.py	2016-06-10 21:16:51 +0000
+++ cloudinit/sources/DataSourceConfigDrive.py	2016-07-14 18:21:43 +0000
@@ -107,12 +107,19 @@
         if self.dsmode == sources.DSMODE_DISABLED:
             return False
 
-        # This is legacy and sneaky.  If dsmode is 'pass' then write
-        # 'injected files' and apply legacy ENI network format.
         prev_iid = get_previous_iid(self.paths)
         cur_iid = md['instance-id']
-        if prev_iid != cur_iid and self.dsmode == sources.DSMODE_PASS:
-            on_first_boot(results, distro=self.distro)
+        if prev_iid != cur_iid:
+            # better would be to handle this centrally, allowing
+            # the datasource to do something on new instance id
+            # note, networking is only rendered here if dsmode is DSMODE_PASS
+            # which means "DISABLED, but render files and networking"
+            on_first_boot(results, distro=self.distro,
+                          network=self.dsmode == sources.DSMODE_PASS)
+
+        # This is legacy and sneaky.  If dsmode is 'pass' then do not claim
+        # the datasource was used, even though we did run on_first_boot above.
+        if self.dsmode == sources.DSMODE_PASS:
             LOG.debug("%s: not claiming datasource, dsmode=%s", self,
                       self.dsmode)
             return False
@@ -184,15 +191,16 @@
         return None
 
 
-def on_first_boot(data, distro=None):
+def on_first_boot(data, distro=None, network=True):
     """Performs any first-boot actions using data read from a config-drive."""
     if not isinstance(data, dict):
         raise TypeError("Config-drive data expected to be a dict; not %s"
                         % (type(data)))
-    net_conf = data.get("network_config", '')
-    if net_conf and distro:
-        LOG.warn("Updating network interfaces from config drive")
-        distro.apply_network(net_conf)
+    if network:
+        net_conf = data.get("network_config", '')
+        if net_conf and distro:
+            LOG.warn("Updating network interfaces from config drive")
+            distro.apply_network(net_conf)
     write_injected_files(data.get('files'))
 
 

=== modified file 'tests/unittests/test_distros/test_netconfig.py'
--- tests/unittests/test_distros/test_netconfig.py	2016-05-12 20:43:11 +0000
+++ tests/unittests/test_distros/test_netconfig.py	2016-07-14 18:21:43 +0000
@@ -319,3 +319,63 @@
 '''
             self.assertCfgEquals(expected_buf, str(write_buf))
             self.assertEqual(write_buf.mode, 0o644)
+
+    def test_apply_network_config_fallback(self):
+        fbsd_distro = self._get_distro('freebsd')
+
+        # a weak attempt to verify that we don't have an implementation
+        # of _write_network_config or apply_network_config in fbsd now,
+        # which would make this test not actually test the fallback.
+        self.assertRaises(
+            NotImplementedError, fbsd_distro._write_network_config,
+            BASE_NET_CFG)
+
+        # now run
+        mynetcfg = {
+            'config': [{"type": "physical", "name": "eth0",
+                        "mac_address": "c0:d6:9f:2c:e8:80",
+                        "subnets": [{"type": "dhcp"}]}],
+            'version': 1}
+
+        write_bufs = {}
+        read_bufs = {
+            '/etc/rc.conf': '',
+            '/etc/resolv.conf': '',
+        }
+
+        def replace_write(filename, content, mode=0o644, omode="wb"):
+            buf = WriteBuffer()
+            buf.mode = mode
+            buf.omode = omode
+            buf.write(content)
+            write_bufs[filename] = buf
+
+        def replace_read(fname, read_cb=None, quiet=False):
+            if fname not in read_bufs:
+                if fname in write_bufs:
+                    return str(write_bufs[fname])
+                raise IOError("%s not found" % fname)
+            else:
+                if fname in write_bufs:
+                    return str(write_bufs[fname])
+                return read_bufs[fname]
+
+        with ExitStack() as mocks:
+            mocks.enter_context(
+                mock.patch.object(util, 'subp', return_value=('vtnet0', '')))
+            mocks.enter_context(
+                mock.patch.object(os.path, 'exists', return_value=False))
+            mocks.enter_context(
+                mock.patch.object(util, 'write_file', replace_write))
+            mocks.enter_context(
+                mock.patch.object(util, 'load_file', replace_read))
+
+            fbsd_distro.apply_network_config(mynetcfg, bring_up=False)
+
+            self.assertIn('/etc/rc.conf', write_bufs)
+            write_buf = write_bufs['/etc/rc.conf']
+            expected_buf = '''
+ifconfig_vtnet0="DHCP"
+'''
+            self.assertCfgEquals(expected_buf, str(write_buf))
+            self.assertEqual(write_buf.mode, 0o644)

=== modified file 'tests/unittests/test_net.py'
--- tests/unittests/test_net.py	2016-06-15 23:11:24 +0000
+++ tests/unittests/test_net.py	2016-07-14 18:21:43 +0000
@@ -269,6 +269,37 @@
         self.assertEqual(expected.lstrip(), contents.lstrip())
 
 
+class TestEniNetworkStateToEni(TestCase):
+    mycfg = {
+        'config': [{"type": "physical", "name": "eth0",
+                    "mac_address": "c0:d6:9f:2c:e8:80",
+                    "subnets": [{"type": "dhcp"}]}],
+        'version': 1}
+    my_mac = 'c0:d6:9f:2c:e8:80'
+
+    def test_no_header(self):
+        rendered = eni.network_state_to_eni(
+            network_state=network_state.parse_net_config_data(self.mycfg),
+            render_hwaddress=True)
+        self.assertIn(self.my_mac, rendered)
+        self.assertIn("hwaddress", rendered)
+
+    def test_with_header(self):
+        header = "# hello world\n"
+        rendered = eni.network_state_to_eni(
+            network_state=network_state.parse_net_config_data(self.mycfg),
+            header=header, render_hwaddress=True)
+        self.assertIn(header, rendered)
+        self.assertIn(self.my_mac, rendered)
+
+    def test_no_hwaddress(self):
+        rendered = eni.network_state_to_eni(
+            network_state=network_state.parse_net_config_data(self.mycfg),
+            render_hwaddress=False)
+        self.assertNotIn(self.my_mac, rendered)
+        self.assertNotIn("hwaddress", rendered)
+
+
 class TestCmdlineConfigParsing(TestCase):
     simple_cfg = {
         'config': [{"type": "physical", "name": "eth0",