Merge lp:~raharper/curtin/trunk.passthrough-netconfig into lp:~curtin-dev/curtin/trunk

[Ryan Harper]

Enable curtin to pass network configuration through to capable targets

Curtin can detect if the cloud-init package in a target is capable of
accepting network configuration, and if so, write out the network-config
into the target instead of rendering it during installation.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:466
https://jenkins.ubuntu.com/server/job/curtin-ci/526/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/526
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/526
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/526
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/526
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=vm-i386/526

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/526/rebuild

review: Approve (continuous-integration)

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:477
https://jenkins.ubuntu.com/server/job/curtin-ci/527/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/527
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/527
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/527
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/527
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=vm-i386/527

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/527/rebuild

review: Approve (continuous-integration)

[Scott Moser (smoser)]

well, first few comments
i didn't get very far. sorry.

[Ryan Harper (raharper)]

On Wed, May 31, 2017 at 4:11 PM, Scott Moser <email address hidden> wrote:

> well, first few comments
> i didn't get very far. sorry.
>
>
> Diff comments:
>
> > === modified file 'curtin/__init__.py'
> > --- curtin/__init__.py 2017-03-21 16:41:33 +0000
> > +++ curtin/__init__.py 2017-05-31 00:24:53 +0000
> > @@ -37,6 +37,8 @@
> > 'SUBCOMMAND_SYSTEM_UPGRADE',
> > # supports new format of apt configuration
> > 'APT_CONFIG_V1',
> > + # supports passing network config to target
> > + 'NETWORK_PASSTHROUGH',
>
> Why would someone need to know this ?
> i think it is related to our plan on implementing images that have builtin
> hooks but dont want to use them.
> that is worth an explanation in the commit message.
>

It's worth comment and *documentation*; will add. I think we might also
want to allow caller to
control whether we render eni or passthrough; ie, MaaS may want to *force*
an eni rendering
even if passthrough is possible.

>
> > # has version module
> > 'HAS_VERSION_MODULE',
> > ]
> >
> > === modified file 'curtin/block/__init__.py'
> > --- curtin/block/__init__.py 2017-05-19 18:52:21 +0000
> > +++ curtin/block/__init__.py 2017-05-31 00:24:53 +0000
> > @@ -978,4 +978,71 @@
> > else:
> > raise ValueError("wipe mode %s not supported" % mode)
> >
> > +
> > +def storage_config_required_packages(storage_config, mapping):
> > + """Read storage configuration dictionary and determine
> > + which packages are required for the supplied configuration
> > + to function. Return a list of packaged to install.
> > + """
> > +
> > + if not storage_config or not isinstance(storage_config, dict):
> > + raise ValueError('Invalid storage configuration. '
> > + 'Must be a dict:\n %s' % storage_config)
> > +
> > + if not mapping or not isinstance(mapping, dict):
> > + raise ValueError('Invalid storage mapping. Must be a dict')
> > +
> > + if 'storage' in storage_config:
> > + storage_config = storage_config.get('storage')
> > +
> > + needed_packages = []
> > +
> > + # get reqs by device operation type
> > + dev_configs = set(operation['type']
> > + for operation in storage_config['config'])
> > +
> > + for dev_type in dev_configs:
> > + if dev_type in mapping:
> > + needed_packages.extend(mapping[dev_type])
> > +
> > + # for any format operations, check the fstype and
> > + # determine if we need any mkfs tools as well.
> > + format_configs = set([operation['fstype']
> > + for operation in storage_config['config']
> > + if operation['type'] == 'format'])
> > + for format_type in format_configs:
> > + if format_type in mapping:
> > + needed_packages.extend(mapping[format_type])
> > +
> > + return needed_packages
> > +
> > +
> > +def detect_required_packages_mapping():
> > + """Return a dictionary providing a versioned configuration which
> maps
> > + storage configuration elements to the packages which are required
> > + for functionality.
> > +
> > + The mapping key is either...

[Scott Moser (smoser)]

Summary of review:
 - your commit message doesn't mention that you've modified the block/fs depends (i dont think)
 - more unit tests would be good.
 - I don't love the 'passthrough'. if we dont have an actual need for it i think i'd drop it for now.

but over all, seems sane.

[Ryan Harper (raharper)]

Thanks for the review, will fix ack'ed items.

[Scott Moser (smoser)]

Moved this to 'Work in progress'. Move it back to 'Needs Review' when you "fix ack'ed items.".

Thanks

[Ryan Harper (raharper)]

Updated branch with fixes for items marked ACK. Custom config deps discussion still needed.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:483
https://jenkins.ubuntu.com/server/job/curtin-ci/531/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/531
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/531
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/531
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/531
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=vm-i386/531

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/531/rebuild

review: Approve (continuous-integration)

[Scott Moser (smoser)]

inline comments, also:
 * Chad has started requesting Human readable """This unit tests does..."""
   in each unit test, and I generally agree.

[Scott Moser (smoser)]

shoot. hit submit before i was done... continuing.

[Scott Moser (smoser)]

a lot of prints in test cases.

[Ryan Harper (raharper)]

Thanks for the review, will update with ACK'ed fixes. I think the items that remain for discussion:

1) parsing shebang for which python interpreter to use for cloud-init in-target
2) side-effect = (Exception) which it appears there was some confusion with the assert_not_called (which I agree need to be replaced with call_count checks).

[Ryan Harper (raharper)]

Fixed all of the ACK's here, filed a bug in cloud-init for UpperCase MAC issue, and applied these fixes to centos-passthrough branch, mp here (https://code.launchpad.net/~raharper/curtin/centos-passthrough/+merge/327536)

[Ryan Harper (raharper)]

Thanks for the follow-up. I'll fix up (1) and (2) as discussed below.

[Scott Moser (smoser)]

marking this as rejected.
the changes here all went into
 https://code.launchpad.net/~raharper/curtin/centos-passthrough/+merge/327536
which got merged (see comments there).

Unmerged revisions

=== modified file 'curtin/block/__init__.py'
--- curtin/block/__init__.py	2017-05-19 18:52:21 +0000
+++ curtin/block/__init__.py	2017-06-19 16:35:41 +0000
@@ -978,4 +978,71 @@
     else:
         raise ValueError("wipe mode %s not supported" % mode)
 
+
+def storage_config_required_packages(storage_config, mapping):
+    """Read storage configuration dictionary and determine
+       which packages are required for the supplied configuration
+       to function.  Return a list of packaged to install.
+    """
+
+    if not storage_config or not isinstance(storage_config, dict):
+        raise ValueError('Invalid storage configuration.  '
+                         'Must be a dict:\n %s' % storage_config)
+
+    if not mapping or not isinstance(mapping, dict):
+        raise ValueError('Invalid storage mapping.  Must be a dict')
+
+    if 'storage' in storage_config:
+        storage_config = storage_config.get('storage')
+
+    needed_packages = []
+
+    # get reqs by device operation type
+    dev_configs = set(operation['type']
+                      for operation in storage_config['config'])
+
+    for dev_type in dev_configs:
+        if dev_type in mapping:
+            needed_packages.extend(mapping[dev_type])
+
+    # for any format operations, check the fstype and
+    # determine if we need any mkfs tools as well.
+    format_configs = set([operation['fstype']
+                         for operation in storage_config['config']
+                         if operation['type'] == 'format'])
+    for format_type in format_configs:
+        if format_type in mapping:
+            needed_packages.extend(mapping[format_type])
+
+    return needed_packages
+
+
+def detect_required_packages_mapping():
+    """Return a dictionary providing a versioned configuration which maps
+       storage configuration elements to the packages which are required
+       for functionality.
+
+       The mapping key is either a config type value, or an fstype value.
+
+    """
+    version = 1
+    mapping = {
+        version: {
+            'handler': storage_config_required_packages,
+            'mapping': {
+                'bcache': ['bcache-tools'],
+                'btrfs': ['btrfs-tools'],
+                'ext2': ['e2fsprogs'],
+                'ext3': ['e2fsprogs'],
+                'ext4': ['e2fsprogs'],
+                'lvm_partition': ['lvm2'],
+                'lvm_volgroup': ['lvm2'],
+                'raid': ['mdadm'],
+                'xfs': ['xfsprogs']
+            },
+        },
+    }
+    return mapping
+
+
 # vi: ts=4 expandtab syntax=python

=== modified file 'curtin/commands/apply_net.py'
--- curtin/commands/apply_net.py	2017-02-08 05:56:12 +0000
+++ curtin/commands/apply_net.py	2017-06-19 16:35:41 +0000
@@ -21,6 +21,7 @@
 from .. import log
 import curtin.net as net
 import curtin.util as util
+from curtin import config
 from . import populate_one_subcmd
 
 
@@ -89,15 +90,36 @@
         sys.stderr.write(msg + "\n")
         raise Exception(msg)
 
+    passthrough = False
     if network_state:
+        # NB: we cannot support passthrough until curtin can convert from
+        # network_state to network-config yaml
         ns = net.network_state.from_state_file(network_state)
+        raise ValueError('Not Supported; curtin lacks a network_state to '
+                         'network_config converter.')
     elif network_config:
-        ns = net.parse_net_config(network_config)
-
-    net.render_network_state(target=target, network_state=ns)
+        netcfg = config.load_config(network_config)
+
+        # curtin will pass-through the netconfig into the target
+        # for rendering at runtime, unless:
+        #   1) target OS does not support (cloud-init too old)
+        LOG.info('Checking cloud-init in target [%s] for network '
+                 'configuration passthrough support.', target)
+        passthrough = net.netconfig_passthrough_available(target)
+        LOG.debug('passthrough available via in-target: %s', passthrough)
+
+        if passthrough:
+            LOG.info('Passing network configuration through to target: %s',
+                     target)
+            net.render_netconfig_passthrough(target, netconfig=netcfg)
+        else:
+            ns = net.parse_net_config_data(netcfg.get('network', {}))
+
+    if not passthrough:
+        LOG.info('Rendering network configuration in target')
+        net.render_network_state(target=target, network_state=ns)
 
     _maybe_remove_legacy_eth0(target)
-    LOG.info('Attempting to remove ipv6 privacy extensions')
     _disable_ipv6_privacy_extensions(target)
     _patch_ifupdown_ipv6_mtu_hook(target)
 
@@ -130,6 +152,7 @@
        by default; this races with the cloud-image desire to disable them.
        Resolve this by allowing the cloud-image setting to win. """
 
+    LOG.debug('Attempting to remove ipv6 privacy extensions')
     cfg = util.target_path(target, path=path)
     if not os.path.exists(cfg):
         LOG.warn('Failed to find ipv6 privacy conf file %s', cfg)
@@ -143,7 +166,7 @@
         lines = [f.strip() for f in contents.splitlines()
                  if not f.startswith("#")]
         if lines == known_contents:
-            LOG.info('deleting file: %s', cfg)
+            LOG.info('Found expected contents, deleting file: %s', cfg)
             util.del_file(cfg)
             msg = "removed %s with known contents" % cfg
             curtin_contents = '\n'.join(
@@ -153,9 +176,10 @@
                  "# net.ipv6.conf.default.use_tempaddr = 2"])
             util.write_file(cfg, curtin_contents)
         else:
-            LOG.info('skipping, content didnt match')
-            LOG.debug("found content:\n%s", lines)
-            LOG.debug("expected contents:\n%s", known_contents)
+            LOG.debug('skipping removal of %s, expected content not found',
+                      cfg)
+            LOG.debug("Found content in file %s:\n%s", cfg, lines)
+            LOG.debug("Expected contents in file %s:\n%s", cfg, known_contents)
             msg = (bmsg + " '%s' exists with user configured content." % cfg)
     except Exception as e:
         msg = bmsg + " %s exists, but could not be read. %s" % (cfg, e)

=== modified file 'curtin/commands/curthooks.py'
--- curtin/commands/curthooks.py	2017-05-12 00:49:43 +0000
+++ curtin/commands/curthooks.py	2017-06-19 16:35:41 +0000
@@ -25,6 +25,7 @@
 
 from curtin import config
 from curtin import block
+from curtin import net
 from curtin import futil
 from curtin.log import LOG
 from curtin import swap
@@ -648,6 +649,38 @@
     update_initramfs(target, all_kernels=True)
 
 
+def detect_required_packages(cfg):
+    """
+    detect packages that will be required in-target by custom config items
+    """
+
+    mapping = {
+        'storage': block.detect_required_packages_mapping(),
+        'network': net.detect_required_packages_mapping(),
+    }
+
+    needed_packages = []
+    for cfg_type, cfg_map in mapping.items():
+
+        # skip missing or invalid config items, configs may
+        # only have network or storage, not always both
+        if not isinstance(cfg.get(cfg_type), dict):
+            continue
+
+        cfg_version = cfg[cfg_type].get('version')
+        if not isinstance(cfg_version, int) or cfg_version not in cfg_map:
+            msg = ('Supplied configuration version "%s", for config type'
+                   '"%s" is not present in the known mapping.' % (cfg_version,
+                                                                  cfg_type))
+            raise ValueError(msg)
+
+        mapped_config = cfg_map[cfg_version]
+        found_reqs = mapped_config['handler'](cfg, mapped_config['mapping'])
+        needed_packages.extend(found_reqs)
+
+    return needed_packages
+
+
 def install_missing_packages(cfg, target):
     ''' describe which operation types will require specific packages
 
@@ -655,46 +688,10 @@
          'pkg1': ['op_name_1', 'op_name_2', ...]
      }
     '''
-    custom_configs = {
-        'storage': {
-            'lvm2': ['lvm_volgroup', 'lvm_partition'],
-            'mdadm': ['raid'],
-            'bcache-tools': ['bcache']},
-        'network': {
-            'vlan': ['vlan'],
-            'ifenslave': ['bond'],
-            'bridge-utils': ['bridge']},
-    }
-
-    format_configs = {
-        'xfsprogs': ['xfs'],
-        'e2fsprogs': ['ext2', 'ext3', 'ext4'],
-        'btrfs-tools': ['btrfs'],
-    }
-
-    needed_packages = []
+
     installed_packages = util.get_installed_packages(target)
-    for cust_cfg, pkg_reqs in custom_configs.items():
-        if cust_cfg not in cfg:
-            continue
-
-        all_types = set(
-            operation['type']
-            for operation in cfg[cust_cfg]['config']
-            )
-        for pkg, types in pkg_reqs.items():
-            if set(types).intersection(all_types) and \
-               pkg not in installed_packages:
-                needed_packages.append(pkg)
-
-        format_types = set(
-            [operation['fstype']
-             for operation in cfg[cust_cfg]['config']
-             if operation['type'] == 'format'])
-        for pkg, fstypes in format_configs.items():
-            if set(fstypes).intersection(format_types) and \
-               pkg not in installed_packages:
-                needed_packages.append(pkg)
+    needed_packages = [pkg for pkg in detect_required_packages(cfg)
+                       if pkg not in installed_packages]
 
     arch_packages = {
         's390x': [('s390-tools', 'zipl')],
@@ -852,7 +849,11 @@
         disable_overlayroot(cfg, target)
 
     # packages may be needed prior to installing kernel
-    install_missing_packages(cfg, target)
+    with events.ReportEventStack(
+            name=stack_prefix + '/installing-missing-packages',
+            reporting_enabled=True, level="INFO",
+            description="installing missing packages"):
+        install_missing_packages(cfg, target)
 
     # If a /etc/iscsi/nodes/... file was created by block_meta then it
     # needs to be copied onto the target system
@@ -880,10 +881,15 @@
         setup_zipl(cfg, target)
         install_kernel(cfg, target)
         run_zipl(cfg, target)
-
         restore_dist_interfaces(cfg, target)
 
     with events.ReportEventStack(
+            name=stack_prefix + '/system-upgrade',
+            reporting_enabled=True, level="INFO",
+            description="updating packages on target system"):
+        system_upgrade(cfg, target)
+
+    with events.ReportEventStack(
             name=stack_prefix + '/setting-up-swap',
             reporting_enabled=True, level="INFO",
             description="setting up swap"):
@@ -907,18 +913,6 @@
             description="configuring multipath"):
         detect_and_handle_multipath(cfg, target)
 
-    with events.ReportEventStack(
-            name=stack_prefix + '/installing-missing-packages',
-            reporting_enabled=True, level="INFO",
-            description="installing missing packages"):
-        install_missing_packages(cfg, target)
-
-    with events.ReportEventStack(
-            name=stack_prefix + '/system-upgrade',
-            reporting_enabled=True, level="INFO",
-            description="updating packages on target system"):
-        system_upgrade(cfg, target)
-
     # If a crypttab file was created by block_meta than it needs to be copied
     # onto the target system, and update_initramfs() needs to be run, so that
     # the cryptsetup hooks are properly configured on the installed system and

=== modified file 'curtin/net/__init__.py'
--- curtin/net/__init__.py	2017-02-06 20:58:37 +0000
+++ curtin/net/__init__.py	2017-06-19 16:35:41 +0000
@@ -520,7 +520,63 @@
     return content
 
 
+def netconfig_passthrough_available(target, feature='NETWORK_CONFIG_V2'):
+    """
+    Determine if curtin can pass v2 network config to in target cloud-init
+    """
+    LOG.debug('Checking in-target cloud-init features')
+    cmd = ("from cloudinit import version;"
+           "print('{}' in getattr(version, 'FEATURES', []))"
+           .format(feature))
+    with util.ChrootableTarget(target) as in_chroot:
+
+        def run_cmd(cmd):
+            (out, _) = in_chroot.subp(cmd, capture=True)
+            return out.strip()
+
+        cloudinit = util.which('cloud-init', target=target)
+        if not cloudinit:
+            LOG.warning('Target does not have cloud-init installed')
+            return False
+
+        # here we read shebang from cloud-init and extract python path  as we
+        # cannot use the presence of the python package to determine  which
+        # python cloud-init will use. E.g trusty has python3 support but
+        # cloud-init uses python2.7
+        python = util.load_file(util.target_path(target, path=cloudinit))
+        python = python.splitlines()[0].split("#!")[-1].strip()
+        try:
+            feature_available = run_cmd([python, '-c', cmd])
+        except util.ProcessExecutionError:
+            LOG.exception("An error occurred while probing cloudinit features")
+            return False
+
+        available = config.value_as_boolean(feature_available)
+        LOG.debug('%s available? %s', feature, available)
+        return available
+
+
+def render_netconfig_passthrough(target, netconfig=None):
+    """
+    Extract original network config and pass it
+    through to cloud-init in target
+    """
+    LOG.debug("rendering passthrough netconfig")
+    cc = 'etc/cloud/cloud.cfg.d/curtin-networking.cfg'
+    if not isinstance(netconfig, dict):
+        raise ValueError('Network config must be a dictionary')
+
+    if 'network' not in netconfig:
+        raise ValueError("Network config must contain the key 'network'")
+
+    content = config.dump_config(netconfig)
+    cc_passthrough = os.path.sep.join((target, cc,))
+    LOG.info('Writing ' + cc_passthrough)
+    util.write_file(cc_passthrough, content=content)
+
+
 def render_network_state(target, network_state):
+    LOG.debug("rendering eni from netconfig")
     eni = 'etc/network/interfaces'
     netrules = 'etc/udev/rules.d/70-persistent-net.rules'
     cc = 'etc/cloud/cloud.cfg.d/curtin-disable-cloudinit-networking.cfg'
@@ -542,4 +598,59 @@
     """Returns the string value of an interface's MAC Address"""
     return read_sys_net(ifname, "address", enoent=False)
 
+
+def network_config_required_packages(network_config, mapping=None):
+
+    if not network_config or not isinstance(network_config, dict):
+        raise ValueError('Invalid network configuration.  Must be a dict')
+
+    if not mapping or not isinstance(mapping, dict):
+        raise ValueError('Invalid network mapping.  Must be a dict')
+
+    # allow top-level 'network' key
+    if 'network' in network_config:
+        network_config = network_config.get('network')
+
+    # v1 has 'config' key and uses type: devtype elements
+    if 'config' in network_config:
+        dev_configs = set(device['type']
+                          for device in network_config['config'])
+    else:
+        # v2 has no config key
+        dev_configs = set(cfgtype for (cfgtype, cfg) in
+                          network_config.items() if cfgtype not in ['version'])
+
+    needed_packages = []
+    for dev_type in dev_configs:
+        if dev_type in mapping:
+            needed_packages.extend(mapping[dev_type])
+
+    return needed_packages
+
+
+def detect_required_packages_mapping():
+    """Return a dictionary providing a versioned configuration which maps
+       network configuration elements to the packages which are required
+       for functionality.
+    """
+    mapping = {
+        1: {
+            'handler': network_config_required_packages,
+            'mapping': {
+                'bond': ['ifenslave'],
+                'bridge': ['bridge-utils'],
+                'vlan': ['vlan']},
+        },
+        2: {
+            'handler': network_config_required_packages,
+            'mapping': {
+                'bonds': ['ifenslave'],
+                'bridges': ['bridge-utils'],
+                'vlans': ['vlan']}
+        },
+    }
+
+    return mapping
+
+
 # vi: ts=4 expandtab syntax=python

=== modified file 'curtin/util.py'
--- curtin/util.py	2017-06-05 02:55:31 +0000
+++ curtin/util.py	2017-06-19 16:35:41 +0000
@@ -1347,6 +1347,9 @@
     if not path:
         return target
 
+    if not isinstance(path, string_types):
+        raise ValueError("Unexpected input for path: %s" % path)
+
     # os.path.join("/etc", "/foo") returns "/foo". Chomp all leading /.
     while len(path) and path[0] == "/":
         path = path[1:]

=== modified file 'examples/network-ipv6-bond-vlan.yaml'
--- examples/network-ipv6-bond-vlan.yaml	2016-08-12 17:24:13 +0000
+++ examples/network-ipv6-bond-vlan.yaml	2017-06-19 16:35:41 +0000
@@ -3,10 +3,10 @@
   config:
     - name: interface0
       type: physical
-      mac_address: BC:76:4E:06:96:B3
+      mac_address: bc:76:4e:06:96:b3
     - name: interface1
       type: physical
-      mac_address: BC:76:4E:04:88:41
+      mac_address: bc:76:4e:04:88:41
     - type: bond
       bond_interfaces:
         - interface0

=== added file 'examples/tests/network_v2_passthrough.yaml'
--- examples/tests/network_v2_passthrough.yaml	1970-01-01 00:00:00 +0000
+++ examples/tests/network_v2_passthrough.yaml	2017-06-19 16:35:41 +0000
@@ -0,0 +1,8 @@
+showtrace: true
+network:
+    version: 2
+    ethernets:
+        interface0:
+            match:
+                mac_address: "52:54:00:12:34:00"
+            dhcp4: true

=== added file 'tests/unittests/test_commands_apply_net.py'
--- tests/unittests/test_commands_apply_net.py	1970-01-01 00:00:00 +0000
+++ tests/unittests/test_commands_apply_net.py	2017-06-19 16:35:41 +0000
@@ -0,0 +1,351 @@
+from unittest import TestCase
+from mock import patch, call
+import copy
+import os
+
+from curtin.commands import apply_net
+from curtin import util
+
+
+class ApplyNetTestBase(TestCase):
+    def setUp(self):
+        super(ApplyNetTestBase, self).setUp()
+
+    def add_patch(self, target, attr):
+        """Patches specified target object and sets it as attr on test
+        instance also schedules cleanup"""
+        m = patch(target, autospec=True)
+        p = m.start()
+        self.addCleanup(m.stop)
+        setattr(self, attr, p)
+
+
+class TestApplyNet(ApplyNetTestBase):
+    def setUp(self):
+        super(TestApplyNet, self).setUp()
+
+        basepath = 'curtin.commands.apply_net.'
+        self.add_patch(basepath + '_maybe_remove_legacy_eth0', 'mock_legacy')
+        self.add_patch(basepath + '_disable_ipv6_privacy_extensions',
+                       'mock_ipv6_priv')
+        self.add_patch(basepath + '_patch_ifupdown_ipv6_mtu_hook',
+                       'mock_ipv6_mtu')
+        self.add_patch('curtin.net.netconfig_passthrough_available',
+                       'mock_netpass_avail')
+        self.add_patch('curtin.net.render_netconfig_passthrough',
+                       'mock_netpass_render')
+        self.add_patch('curtin.net.parse_net_config_data',
+                       'mock_net_parsedata')
+        self.add_patch('curtin.net.render_network_state',
+                       'mock_net_renderstate')
+        self.add_patch('curtin.net.network_state.from_state_file',
+                       'mock_ns_from_file')
+        self.add_patch('curtin.config.load_config', 'mock_load_config')
+
+        self.target = "my_target"
+        self.network_config = {
+            'network': {
+                'version': 1,
+                'config': {},
+            }
+        }
+        self.ns = {
+            'interfaces': {},
+            'routes': [],
+            'dns': {
+                'nameservers': [],
+                'search': [],
+            }
+        }
+
+    def test_apply_net_notarget(self):
+        self.assertRaises(Exception,
+                          apply_net.apply_net, None, "", "")
+
+    def test_apply_net_nostate_or_config(self):
+        self.assertRaises(Exception,
+                          apply_net.apply_net, "")
+
+    def test_apply_net_target_and_state(self):
+        self.mock_ns_from_file.return_value = self.ns
+
+        self.assertRaises(ValueError,
+                          apply_net.apply_net, self.target,
+                          network_state=self.ns, network_config=None)
+
+    def test_apply_net_target_and_config(self):
+        self.mock_load_config.return_value = self.network_config
+        self.mock_netpass_avail.return_value = False
+        self.mock_net_parsedata.return_value = self.ns
+
+        apply_net.apply_net(self.target, network_state=None,
+                            network_config=self.network_config)
+
+        self.mock_netpass_avail.assert_called_with(self.target)
+
+        self.mock_net_renderstate.assert_called_with(target=self.target,
+                                                     network_state=self.ns)
+        self.mock_legacy.assert_called_with(self.target)
+        self.mock_ipv6_priv.assert_called_with(self.target)
+        self.mock_ipv6_mtu.assert_called_with(self.target)
+
+    def test_apply_net_target_and_config_passthrough(self):
+        self.mock_load_config.return_value = self.network_config
+        self.mock_netpass_avail.return_value = True
+
+        netcfg = "network_config.yaml"
+        apply_net.apply_net(self.target, network_state=None,
+                            network_config=netcfg)
+
+        self.assertFalse(self.mock_ns_from_file.called)
+        self.mock_load_config.assert_called_with(netcfg)
+        self.mock_netpass_avail.assert_called_with(self.target)
+        nc = self.network_config
+        self.mock_netpass_render.assert_called_with(self.target, netconfig=nc)
+
+        self.assertFalse(self.mock_net_renderstate.called)
+        self.mock_legacy.assert_called_with(self.target)
+        self.mock_ipv6_priv.assert_called_with(self.target)
+        self.mock_ipv6_mtu.assert_called_with(self.target)
+
+    def test_apply_net_target_and_config_passthrough_nonet(self):
+        nc = {'storage': {}}
+        self.mock_load_config.return_value = nc
+        self.mock_netpass_avail.return_value = True
+
+        netcfg = "network_config.yaml"
+
+        apply_net.apply_net(self.target, network_state=None,
+                            network_config=netcfg)
+
+        self.assertFalse(self.mock_ns_from_file.called)
+        self.mock_load_config.assert_called_with(netcfg)
+        self.mock_netpass_avail.assert_called_with(self.target)
+        self.mock_netpass_render.assert_called_with(self.target, netconfig=nc)
+
+        self.assertFalse(self.mock_net_renderstate.called)
+        self.mock_legacy.assert_called_with(self.target)
+        self.mock_ipv6_priv.assert_called_with(self.target)
+        self.mock_ipv6_mtu.assert_called_with(self.target)
+
+    def test_apply_net_target_and_config_passthrough_v2_not_available(self):
+        nc = copy.deepcopy(self.network_config)
+        nc['network']['version'] = 2
+        self.mock_load_config.return_value = nc
+        self.mock_netpass_avail.return_value = False
+        self.mock_net_parsedata.return_value = self.ns
+
+        netcfg = "network_config.yaml"
+
+        apply_net.apply_net(self.target, network_state=None,
+                            network_config=netcfg)
+
+        self.assertFalse(self.mock_ns_from_file.called)
+        self.mock_load_config.assert_called_with(netcfg)
+        self.mock_netpass_avail.assert_called_with(self.target)
+        self.assertFalse(self.mock_netpass_render.called)
+        self.mock_net_parsedata.assert_called_with(nc['network'])
+
+        self.mock_net_renderstate.assert_called_with(
+            target=self.target, network_state=self.ns)
+        self.mock_legacy.assert_called_with(self.target)
+        self.mock_ipv6_priv.assert_called_with(self.target)
+        self.mock_ipv6_mtu.assert_called_with(self.target)
+
+
+class TestApplyNetPatchIfupdown(ApplyNetTestBase):
+
+    @patch('curtin.util.write_file')
+    def test_apply_ipv6_mtu_hook(self, mock_write):
+        target = 'mytarget'
+        prehookfn = 'if-pre-up.d/mtuipv6'
+        posthookfn = 'if-up.d/mtuipv6'
+        mode = 0o755
+
+        apply_net._patch_ifupdown_ipv6_mtu_hook(target,
+                                                prehookfn=prehookfn,
+                                                posthookfn=posthookfn)
+
+        precfg = util.target_path(target, path=prehookfn)
+        postcfg = util.target_path(target, path=posthookfn)
+        precontents = apply_net.IFUPDOWN_IPV6_MTU_PRE_HOOK
+        postcontents = apply_net.IFUPDOWN_IPV6_MTU_POST_HOOK
+
+        hook_calls = [
+            call(precfg, precontents, mode=mode),
+            call(postcfg, postcontents, mode=mode),
+        ]
+        mock_write.assert_has_calls(hook_calls)
+
+    @patch('curtin.util.write_file')
+    def test_apply_ipv6_mtu_hook_write_fail(self, mock_write):
+        target = 'mytarget'
+        prehookfn = 'if-pre-up.d/mtuipv6'
+        posthookfn = 'if-up.d/mtuipv6'
+        mock_write.side_effect = (Exception)
+
+        self.assertRaises(Exception,
+                          apply_net._patch_ifupdown_ipv6_mtu_hook,
+                          target,
+                          prehookfn=prehookfn,
+                          posthookfn=posthookfn)
+
+    @patch('curtin.util.write_file')
+    def test_apply_ipv6_mtu_hook_invalid_target(self, mock_write):
+        """ Test that an invalid target will fail to build a
+            proper path for util.write_file
+        """
+        target = {}
+        prehookfn = 'if-pre-up.d/mtuipv6'
+        posthookfn = 'if-up.d/mtuipv6'
+        mock_write.side_effect = (Exception)
+
+        self.assertRaises(ValueError,
+                          apply_net._patch_ifupdown_ipv6_mtu_hook,
+                          target,
+                          prehookfn=prehookfn,
+                          posthookfn=posthookfn)
+
+    @patch('curtin.util.write_file')
+    def test_apply_ipv6_mtu_hook_invalid_prepost_fn(self, mock_write):
+        """ Test that invalid prepost filenames will fail to build a
+            proper path for util.write_file
+        """
+        target = "mytarget"
+        prehookfn = {'a': 1}
+        posthookfn = {'b': 2}
+        mock_write.side_effect = (Exception)
+
+        self.assertRaises(ValueError,
+                          apply_net._patch_ifupdown_ipv6_mtu_hook,
+                          target,
+                          prehookfn=prehookfn,
+                          posthookfn=posthookfn)
+
+
+class TestApplyNetPatchIpv6Priv(ApplyNetTestBase):
+
+    @patch('curtin.util.del_file')
+    @patch('curtin.util.load_file')
+    @patch('os.path')
+    @patch('curtin.util.write_file')
+    def test_disable_ipv6_priv_extentions(self, mock_write, mock_ospath,
+                                          mock_load, mock_del):
+        target = 'mytarget'
+        path = 'etc/sysctl.d/10-ipv6-privacy.conf'
+        ipv6_priv_contents = (
+            'net.ipv6.conf.all.use_tempaddr = 2\n'
+            'net.ipv6.conf.default.use_tempaddr = 2')
+        expected_ipv6_priv_contents = '\n'.join(
+            ["# IPv6 Privacy Extensions (RFC 4941)",
+             "# Disabled by curtin",
+             "# net.ipv6.conf.all.use_tempaddr = 2",
+             "# net.ipv6.conf.default.use_tempaddr = 2"])
+        mock_ospath.exists.return_value = True
+        mock_load.side_effect = [ipv6_priv_contents]
+
+        apply_net._disable_ipv6_privacy_extensions(target)
+
+        cfg = util.target_path(target, path=path)
+        mock_write.assert_called_with(cfg, expected_ipv6_priv_contents)
+
+    @patch('curtin.util.load_file')
+    @patch('os.path')
+    def test_disable_ipv6_priv_extentions_decoderror(self, mock_ospath,
+                                                     mock_load):
+        target = 'mytarget'
+        mock_ospath.exists.return_value = True
+
+        # simulate loading of binary data
+        mock_load.side_effect = (Exception)
+
+        self.assertRaises(Exception,
+                          apply_net._disable_ipv6_privacy_extensions,
+                          target)
+
+    @patch('curtin.util.load_file')
+    @patch('os.path')
+    def test_disable_ipv6_priv_extentions_notfound(self, mock_ospath,
+                                                   mock_load):
+        target = 'mytarget'
+        path = 'foo.conf'
+        mock_ospath.exists.return_value = False
+
+        apply_net._disable_ipv6_privacy_extensions(target, path=path)
+
+        # source file not found
+        cfg = util.target_path(target, path)
+        mock_ospath.exists.assert_called_with(cfg)
+        mock_load.assert_not_called()
+
+
+class TestApplyNetRemoveLegacyEth0(ApplyNetTestBase):
+
+    @patch('curtin.util.del_file')
+    @patch('curtin.util.load_file')
+    @patch('os.path')
+    def test_remove_legacy_eth0(self, mock_ospath, mock_load, mock_del):
+        target = 'mytarget'
+        path = 'eth0.cfg'
+        cfg = util.target_path(target, path)
+        legacy_eth0_contents = (
+            'auto eth0\n'
+            'iface eth0 inet dhcp')
+
+        mock_ospath.exists.return_value = True
+        mock_load.side_effect = [legacy_eth0_contents]
+
+        apply_net._maybe_remove_legacy_eth0(target, path)
+
+        mock_del.assert_called_with(cfg)
+
+    @patch('curtin.util.del_file')
+    @patch('curtin.util.load_file')
+    @patch('os.path')
+    def test_remove_legacy_eth0_nomatch(self, mock_ospath, mock_load,
+                                        mock_del):
+        target = 'mytarget'
+        path = 'eth0.cfg'
+        legacy_eth0_contents = "nomatch"
+        mock_ospath.join.side_effect = os.path.join
+        mock_ospath.exists.return_value = True
+        mock_load.side_effect = [legacy_eth0_contents]
+
+        self.assertRaises(Exception,
+                          apply_net._maybe_remove_legacy_eth0,
+                          target, path)
+
+        mock_del.assert_not_called()
+
+    @patch('curtin.util.del_file')
+    @patch('curtin.util.load_file')
+    @patch('os.path')
+    def test_remove_legacy_eth0_badload(self, mock_ospath, mock_load,
+                                        mock_del):
+        target = 'mytarget'
+        path = 'eth0.cfg'
+        mock_ospath.exists.return_value = True
+        mock_load.side_effect = (Exception)
+
+        self.assertRaises(Exception,
+                          apply_net._maybe_remove_legacy_eth0,
+                          target, path)
+
+        mock_del.assert_not_called()
+
+    @patch('curtin.util.del_file')
+    @patch('curtin.util.load_file')
+    @patch('os.path')
+    def test_remove_legacy_eth0_notfound(self, mock_ospath, mock_load,
+                                         mock_del):
+        target = 'mytarget'
+        path = 'eth0.conf'
+        mock_ospath.exists.return_value = False
+
+        apply_net._maybe_remove_legacy_eth0(target, path)
+
+        # source file not found
+        cfg = util.target_path(target, path)
+        mock_ospath.exists.assert_called_with(cfg)
+        mock_load.assert_not_called()
+        mock_del.assert_not_called()

=== modified file 'tests/unittests/test_curthooks.py'
--- tests/unittests/test_curthooks.py	2017-05-12 14:35:53 +0000
+++ tests/unittests/test_curthooks.py	2017-06-19 16:35:41 +0000
@@ -577,4 +577,188 @@
         with self.assertRaises(ValueError):
             curthooks.handle_cloudconfig([], target="foobar")
 
+
+class TestDetectRequiredPackages(TestCase):
+    test_config = {
+        'storage': {
+            1: {
+                'bcache': {
+                    'type': 'bcache', 'name': 'bcache0', 'id': 'cache0',
+                    'backing_device': 'sda3', 'cache_device': 'sdb'},
+                'lvm_partition': {
+                    'id': 'lvol1', 'name': 'lv1', 'volgroup': 'vg1',
+                    'type': 'lvm_partition'},
+                'lvm_volgroup': {
+                    'id': 'vol1', 'name': 'vg1', 'devices': ['sda', 'sdb'],
+                    'type': 'lvm_volgroup'},
+                'raid': {
+                    'id': 'mddevice', 'name': 'md0', 'type': 'raid',
+                    'raidlevel': 5, 'devices': ['sda1', 'sdb1', 'sdc1']},
+                'ext2': {
+                    'id': 'format0', 'fstype': 'ext2', 'type': 'format'},
+                'ext3': {
+                    'id': 'format1', 'fstype': 'ext3', 'type': 'format'},
+                'ext4': {
+                    'id': 'format2', 'fstype': 'ext4', 'type': 'format'},
+                'btrfs': {
+                    'id': 'format3', 'fstype': 'btrfs', 'type': 'format'},
+                'xfs': {
+                    'id': 'format4', 'fstype': 'xfs', 'type': 'format'}}
+        },
+        'network': {
+            1: {
+                'bond': {
+                    'name': 'bond0', 'type': 'bond',
+                    'bond_interfaces': ['interface0', 'interface1'],
+                    'params': {'bond-mode': 'active-backup'},
+                    'subnets': [
+                        {'type': 'static', 'address': '10.23.23.2/24'},
+                        {'type': 'static', 'address': '10.23.24.2/24'}]},
+                'vlan': {
+                    'id': 'interface1.2667', 'mtu': 1500, 'name':
+                    'interface1.2667', 'type': 'vlan', 'vlan_id': 2667,
+                    'vlan_link': 'interface1',
+                    'subnets': [{'address': '10.245.184.2/24',
+                                 'dns_nameservers': [], 'type': 'static'}]},
+                'bridge': {
+                    'name': 'br0', 'bridge_interfaces': ['eth0', 'eth1'],
+                    'type': 'bridge', 'params': {
+                        'bridge_stp': 'off', 'bridge_fd': 0,
+                        'bridge_maxwait': 0},
+                    'subnets': [
+                        {'type': 'static', 'address': '192.168.14.2/24'},
+                        {'type': 'static', 'address': '2001:1::1/64'}]}},
+            2: {
+                'vlan': {
+                    'vlans': {
+                        'en-intra': {'id': 1, 'link': 'eno1', 'dhcp4': 'yes'},
+                        'en-vpn': {'id': 2, 'link': 'eno1'}}},
+                'bridge': {
+                    'bridges': {
+                        'br0': {
+                            'interfaces': ['wlp1s0', 'switchports'],
+                            'dhcp4': True}}}}
+        },
+    }
+
+    def _fmt_config(self, config_items):
+        res = {}
+        for item, item_confs in config_items.items():
+            version = item_confs['version']
+            res[item] = {'version': version}
+            if version == 1:
+                res[item]['config'] = [self.test_config[item][version][i]
+                                       for i in item_confs['items']]
+            elif version == 2 and item == 'network':
+                for cfg_item in item_confs['items']:
+                    res[item].update(self.test_config[item][version][cfg_item])
+            else:
+                raise NotImplementedError
+        return res
+
+    def _test_req_mappings(self, req_mappings):
+        for (config_items, expected_reqs) in req_mappings:
+            cfg = self._fmt_config(config_items)
+            actual_reqs = curthooks.detect_required_packages(cfg)
+            self.assertEqual(set(actual_reqs), set(expected_reqs),
+                             'failed for config: {}'.format(config_items))
+
+    def test_storage_v1_detect(self):
+        self._test_req_mappings((
+            ({'storage': {
+                'version': 1,
+                'items': ('lvm_partition', 'lvm_volgroup', 'btrfs', 'xfs')}},
+             ('lvm2', 'xfsprogs', 'btrfs-tools')),
+            ({'storage': {
+                'version': 1,
+                'items': ('raid', 'bcache', 'ext3', 'xfs')}},
+             ('mdadm', 'bcache-tools', 'e2fsprogs', 'xfsprogs')),
+            ({'storage': {
+                'version': 1,
+                'items': ('raid', 'lvm_volgroup', 'lvm_partition', 'ext3',
+                          'ext4', 'btrfs')}},
+             ('lvm2', 'mdadm', 'e2fsprogs', 'btrfs-tools')),
+            ({'storage': {
+                'version': 1,
+                'items': ('bcache', 'lvm_volgroup', 'lvm_partition', 'ext2')}},
+             ('bcache-tools', 'lvm2', 'e2fsprogs')),
+        ))
+
+    def test_network_v1_detect(self):
+        self._test_req_mappings((
+            ({'network': {
+                'version': 1,
+                'items': ('bridge',)}},
+             ('bridge-utils',)),
+            ({'network': {
+                'version': 1,
+                'items': ('vlan', 'bond')}},
+             ('vlan', 'ifenslave')),
+            ({'network': {
+                'version': 1,
+                'items': ('bond', 'bridge')}},
+             ('ifenslave', 'bridge-utils')),
+            ({'network': {
+                'version': 1,
+                'items': ('vlan', 'bridge', 'bond')}},
+             ('ifenslave', 'bridge-utils', 'vlan')),
+        ))
+
+    def test_mixed_v1_detect(self):
+        self._test_req_mappings((
+            ({'storage': {
+                'version': 1,
+                'items': ('raid', 'bcache', 'ext4')},
+              'network': {
+                  'version': 1,
+                  'items': ('vlan',)}},
+             ('mdadm', 'bcache-tools', 'e2fsprogs', 'vlan')),
+            ({'storage': {
+                'version': 1,
+                'items': ('lvm_partition', 'lvm_volgroup', 'xfs')},
+              'network': {
+                  'version': 1,
+                  'items': ('bridge', 'bond')}},
+             ('lvm2', 'xfsprogs', 'bridge-utils', 'ifenslave')),
+            ({'storage': {
+                'version': 1,
+                'items': ('ext3', 'ext4', 'btrfs')},
+              'network': {
+                  'version': 1,
+                  'items': ('bond', 'vlan')}},
+             ('e2fsprogs', 'btrfs-tools', 'vlan', 'ifenslave')),
+        ))
+
+    def test_network_v2_detect(self):
+        self._test_req_mappings((
+            ({'network': {
+                'version': 2,
+                'items': ('bridge',)}},
+             ('bridge-utils',)),
+            ({'network': {
+                'version': 2,
+                'items': ('vlan',)}},
+             ('vlan',)),
+            ({'network': {
+                'version': 2,
+                'items': ('vlan', 'bridge')}},
+             ('vlan', 'bridge-utils')),
+        ))
+
+    def test_mixed_storage_v1_network_v2_detect(self):
+        self._test_req_mappings((
+            ({'network': {
+                'version': 2,
+                'items': ('bridge', 'vlan')},
+             'storage': {
+                 'version': 1,
+                 'items': ('raid', 'bcache', 'ext4')}},
+             ('vlan', 'bridge-utils', 'mdadm', 'bcache-tools', 'e2fsprogs')),
+        ))
+
+    def test_invalid_version_in_config(self):
+        with self.assertRaises(ValueError):
+            curthooks.detect_required_packages({'network': {'version': 3}})
+
+
 # vi: ts=4 expandtab syntax=python

=== modified file 'tests/unittests/test_net.py'
--- tests/unittests/test_net.py	2017-02-09 16:58:23 +0000
+++ tests/unittests/test_net.py	2017-06-19 16:35:41 +0000
@@ -1,10 +1,11 @@
 from unittest import TestCase
+import mock
 import os
 import shutil
 import tempfile
 import yaml
 
-from curtin import net
+from curtin import config, net, util
 import curtin.net.network_state as network_state
 from textwrap import dedent
 
@@ -654,6 +655,141 @@
         self.assertEqual(sorted(ifaces.split('\n')),
                          sorted(net_ifaces.split('\n')))
 
+    @mock.patch('curtin.util.load_file')
+    @mock.patch('curtin.util.subp')
+    @mock.patch('curtin.util.which')
+    @mock.patch.object(util.ChrootableTarget, "__enter__", new=lambda a: a)
+    def test_netconfig_passthrough_available(self, mock_which, mock_subp,
+                                             mock_load_file):
+        cloud_init = '/usr/bin/cloud-init'
+        python = '/usr/bin/python3'
+        mock_which.return_value = cloud_init
+        mock_load_file.return_value = "#! %s" % python
+        mock_subp.return_value = ('True', '')
+        feature = 'NETWORK_CONFIG_V2'
+        expected_cmd = (
+            "from cloudinit import version;"
+            "print('%s' in getattr(version, 'FEATURES', []))" % feature)
+
+        available = net.netconfig_passthrough_available(self.target)
+
+        self.assertEqual(True, available,
+                         "netconfig passthrough was NOT available")
+        mock_which.assert_called_with('cloud-init', target=self.target)
+        mock_load_file.assert_called_with(self.target + cloud_init)
+        mock_subp.assert_called_with([python, '-c', expected_cmd],
+                                     capture=True, target=self.target)
+
+    @mock.patch('curtin.util.load_file')
+    @mock.patch('curtin.util.subp')
+    @mock.patch('curtin.util.which')
+    @mock.patch.object(util.ChrootableTarget, "__enter__", new=lambda a: a)
+    def test_netconfig_passthrough_available_no_py3(self, mock_which,
+                                                    mock_subp, mock_load_file):
+        cloud_init = '/usr/bin/cloud-init'
+        python = '/usr/bin/python'
+        mock_which.return_value = cloud_init
+        mock_load_file.return_value = "#! %s" % python
+        mock_subp.return_value = ('True', '')
+        feature = 'NETWORK_CONFIG_V2'
+        expected_cmd = (
+            "from cloudinit import version;"
+            "print('%s' in getattr(version, 'FEATURES', []))" % feature)
+
+        available = net.netconfig_passthrough_available(self.target)
+
+        self.assertEqual(True, available,
+                         "netconfig passthrough was available")
+        mock_which.assert_called_with('cloud-init', target=self.target)
+        mock_load_file.assert_called_with(self.target + cloud_init)
+        mock_subp.assert_called_with([python, '-c', expected_cmd],
+                                     capture=True, target=self.target)
+
+    @mock.patch('curtin.net.LOG')
+    @mock.patch('curtin.util.subp')
+    @mock.patch('curtin.util.which')
+    @mock.patch.object(util.ChrootableTarget, "__enter__", new=lambda a: a)
+    def test_netconfig_passthrough_available_no_cloudinit(self, mock_which,
+                                                          mock_subp, mock_log):
+        mock_which.return_value = None
+
+        available = net.netconfig_passthrough_available(self.target)
+
+        self.assertEqual(False, available,
+                         "netconfig passthrough was available")
+        self.assertTrue(mock_log.warning.called)
+        self.assertFalse(mock_subp.called)
+
+    @mock.patch('curtin.util.load_file')
+    @mock.patch('curtin.util.subp')
+    @mock.patch('curtin.util.which')
+    @mock.patch.object(util.ChrootableTarget, "__enter__", new=lambda a: a)
+    def test_netconfig_passthrough_available_feature_not_found(self,
+                                                               mock_which,
+                                                               mock_subp,
+                                                               mock_load_file):
+        cloud_init = '/usr/bin/cloud-init'
+        python = '/usr/bin/python3'
+        mock_which.return_value = cloud_init
+        mock_load_file.return_value = "#! %s" % python
+        mock_subp.return_value = ('False', '')
+        feature = 'NETWORK_CONFIG_V2'
+        expected_cmd = (
+            "from cloudinit import version;"
+            "print('%s' in getattr(version, 'FEATURES', []))" % feature)
+
+        available = net.netconfig_passthrough_available(self.target)
+
+        self.assertEqual(False, available,
+                         "netconfig passthrough was available")
+        mock_which.assert_called_with('cloud-init', target=self.target)
+        mock_load_file.assert_called_with(self.target + cloud_init)
+        mock_subp.assert_called_with([python, '-c', expected_cmd],
+                                     capture=True, target=self.target)
+
+    @mock.patch('curtin.util.load_file')
+    @mock.patch('curtin.net.LOG')
+    @mock.patch('curtin.util.subp')
+    @mock.patch('curtin.util.which')
+    @mock.patch.object(util.ChrootableTarget, "__enter__", new=lambda a: a)
+    def test_netconfig_passthrough_available_exc(self, mock_which, mock_subp,
+                                                 mock_log, mock_load_file):
+        cloud_init = '/usr/bin/cloud-init'
+        python = '/usr/bin/python3'
+        mock_which.return_value = cloud_init
+        mock_load_file.return_value = "#! %s" % python
+        mock_subp.side_effect = util.ProcessExecutionError
+        feature = 'NETWORK_CONFIG_V2'
+        expected_cmd = (
+            "from cloudinit import version;"
+            "print('%s' in getattr(version, 'FEATURES', []))" % feature)
+
+        available = net.netconfig_passthrough_available(self.target)
+
+        self.assertEqual(False, available,
+                         "netconfig passthrough was available")
+        mock_which.assert_called_with('cloud-init', target=self.target)
+        mock_subp.assert_called_with([python, '-c', expected_cmd],
+                                     capture=True, target=self.target)
+        self.assertTrue(mock_log.exception.called)
+
+    @mock.patch('curtin.util.write_file')
+    def test_render_netconfig_passthrough(self, mock_writefile):
+        netcfg = yaml.safe_load(self.config)
+        pt_config = 'etc/cloud/cloud.cfg.d/curtin-networking.cfg'
+        target_config = os.path.sep.join((self.target, pt_config),)
+
+        net.render_netconfig_passthrough(self.target, netconfig=netcfg)
+
+        content = config.dump_config(netcfg)
+        mock_writefile.assert_called_with(target_config, content=content)
+
+    def test_render_netconfig_passthrough_nonetcfg(self):
+        netcfg = None
+        self.assertRaises(ValueError,
+                          net.render_netconfig_passthrough,
+                          self.target, netconfig=netcfg)
+
     def test_routes_rendered(self):
         # as reported in bug 1649652
         conf = [

=== modified file 'tests/vmtests/test_network.py'
--- tests/vmtests/test_network.py	2017-05-19 21:40:48 +0000
+++ tests/vmtests/test_network.py	2017-06-19 16:35:41 +0000
@@ -1,7 +1,12 @@
 from . import VMBaseClass, logger, helpers
 from .releases import base_vm_classes as relbase
 
+from unittest import SkipTest
+from curtin import config
+
+import glob
 import ipaddress
+import os
 import re
 import textwrap
 import yaml
@@ -14,26 +19,41 @@
     collect_scripts = [textwrap.dedent("""
         cd OUTPUT_COLLECT_D
         echo "waiting for ipv6 to settle" && sleep 5
-        ifconfig -a > ifconfig_a
-        ip link show > ip_link_show
-        ip a > ip_a
+        route -n | tee first_route_n
+        ifconfig -a | tee ifconfig_a
+        ip link show | tee ip_link_show
+        ip a | tee  ip_a
         find /etc/network/interfaces.d > find_interfacesd
         cp -av /etc/network/interfaces .
         cp -av /etc/network/interfaces.d .
         cp /etc/resolv.conf .
-        cp -av /etc/udev/rules.d/70-persistent-net.rules .
-        ip -o route show > ip_route_show
-        ip -6 -o route show > ip_6_route_show
-        route -n > route_n
-        route -6 -n > route_6_n
+        cp -av /etc/udev/rules.d/70-persistent-net.rules . ||:
+        ip -o route show | tee ip_route_show
+        ip -6 -o route show | tee ip_6_route_show
+        route -n |tee route_n
+        route -6 -n |tee route_6_n
         cp -av /run/network ./run_network
         cp -av /var/log/upstart ./upstart ||:
-        sleep 10 && ip a > ip_a
+        cp -av /etc/cloud ./etc_cloud
+        cp -av /var/log/cloud*.log ./
+        dpkg-query -W -f '${Version}' cloud-init |tee dpkg_cloud-init_version
+        dpkg-query -W -f '${Version}' nplan |tee dpkg_nplan_version
+        dpkg-query -W -f '${Version}' systemd |tee dpkg_systemd_version
+        V=/usr/lib/python*/dist-packages/cloudinit/version.py;
+        grep -c NETWORK_CONFIG_V2 $V > cloudinit_passthrough_available
+        mkdir -p etc_netplan
+        cp -av /etc/netplan/* ./etc_netplan/ ||:
+        networkctl |tee networkctl
+        mkdir -p run_systemd_network
+        cp -a /run/systemd/network/* ./run_systemd_network/ ||:
+        cp -a /run/systemd/netif ./run_systemd_netif ||:
+        cp -a /run/systemd/resolve ./run_systemd_resolve ||:
+        cp -a /etc/systemd ./etc_systemd ||:
+        journalctl --no-pager -b -x > journalctl_out
         """)]
 
     def test_output_files_exist(self):
         self.output_files_exist([
-            "70-persistent-net.rules",
             "find_interfacesd",
             "ifconfig_a",
             "interfaces",
@@ -44,17 +64,109 @@
             "route_n",
         ])
 
-    def test_etc_network_interfaces(self):
+    def read_eni(self):
+        eni = ""
+        eni_cfg = ""
+
         eni = self.load_collect_file("interfaces")
         logger.debug('etc/network/interfaces:\n{}'.format(eni))
 
+        # we don't use collect_path as we're building a glob
+        eni_dir = os.path.join(self.td.collect, "interfaces.d", "*.cfg")
+        for cfg in glob.glob(eni_dir):
+            with open(cfg) as fp:
+                eni_cfg += fp.read()
+
+        return (eni, eni_cfg)
+
+    def _network_renderer(self):
+        """ Determine if target uses eni/ifupdown or netplan/networkd """
+
+        etc_netplan = self.collect_path('etc_netplan')
+        networkd = self.collect_path('run_systemd_network')
+
+        if len(os.listdir(etc_netplan)) > 0 and len(os.listdir(networkd)) > 0:
+            print('Network Renderer: systemd-networkd')
+            return 'systemd-networkd'
+
+        print('Network Renderer: ifupdown')
+        return 'ifupdown'
+
+    def test_etc_network_interfaces(self):
+        if self._network_renderer() != "ifupdown":
+            reason = ("{}: using net-passthrough; "
+                      "deferring to cloud-init".format(self.__class__))
+            raise SkipTest(reason)
+
+        eni, eni_cfg = self.read_eni()
         expected_eni = self.get_expected_etc_network_interfaces()
-        eni_lines = eni.split('\n')
-        for line in expected_eni.split('\n'):
-            self.assertTrue(line in eni_lines, msg="missing line: %s" % line)
+
+        eni_lines = eni.split('\n') + eni_cfg.split('\n')
+        print("\n".join(eni_lines))
+        for line in [l for l in expected_eni.split('\n') if len(l) > 0]:
+            if line.startswith("#"):
+                continue
+            if "hwaddress ether" in line:
+                continue
+            print('expected line:\n%s' % line)
+            self.assertTrue(line in eni_lines, "not in eni: %s" % line)
+
+    def test_cloudinit_network_passthrough(self):
+        cc_passthrough = "cloud.cfg.d/curtin-networking.cfg"
+
+        avail_str = self.load_collect_file('cloudinit_passthrough_available')
+        available = int(avail_str) == 1
+        print('avail_str=%s available=%s' % (avail_str, available))
+
+        if not available:
+            raise SkipTest('not available on %s' % self.__class__)
+
+        print('passthrough was available')
+        pt_file = os.path.join(self.td.collect, 'etc_cloud',
+                               cc_passthrough)
+        print('checking if passthrough file written: %s' % pt_file)
+        self.assertTrue(os.path.exists(pt_file))
+
+        # compare
+        original = {'network':
+                    config.load_config(self.conf_file).get('network')}
+        intarget = config.load_config(pt_file)
+        self.assertEqual(original, intarget)
+
+    def test_cloudinit_network_disabled(self):
+        cc_disabled = 'cloud.cfg.d/curtin-disable-cloudinit-networking.cfg'
+
+        avail_str = self.load_collect_file('cloudinit_passthrough_available')
+        available = int(avail_str) == 1
+        print('avail_str=%s available=%s' % (avail_str, available))
+
+        if available:
+            raise SkipTest('passthrough available on %s' % self.__class__)
+
+        print('passthrough not available')
+        cc_disable_file = os.path.join(self.td.collect, 'etc_cloud',
+                                       cc_disabled)
+        print('checking if network:disable file written: %s' %
+              cc_disable_file)
+        self.assertTrue(os.path.exists(cc_disable_file))
+
+        # compare
+        original = {'network': {'config': 'disabled'}}
+        intarget = config.load_config(cc_disable_file)
+
+        print('checking cloud-init network-cfg content')
+        self.assertEqual(original, intarget)
 
     def test_etc_resolvconf(self):
-        resolvconf = self.load_collect_file("resolv.conf")
+        render2resolvconf = {
+            'ifupdown': "resolv.conf",
+            'systemd-networkd': "run_systemd_resolve/resolv.conf"
+        }
+        resolvconfpath = render2resolvconf.get(self._network_renderer(), None)
+        self.assertTrue(resolvconfpath is not None)
+        logger.debug('Selected path to resolvconf: %s', resolvconfpath)
+
+        resolvconf = self.load_collect_file(resolvconfpath)
         logger.debug('etc/resolv.conf:\n{}'.format(resolvconf))
 
         resolv_lines = resolvconf.split('\n')
@@ -111,12 +223,21 @@
     def test_static_routes(self):
         '''check routing table'''
         network_state = self.get_network_state()
+
+        # if we're using passthrough then we can't load state
+        cc_passthrough = "cloud.cfg.d/curtin-networking.cfg"
+        pt_file = os.path.join(self.td.collect, 'etc_cloud', cc_passthrough)
+        print('checking if passthrough file written: %s' % pt_file)
+        if not network_state and os.path.exists(pt_file):
+            raise SkipTest('passthrough enabled, skipping %s' % self.__class__)
+
         ip_route_show = self.load_collect_file("ip_route_show")
         route_n = self.load_collect_file("route_n")
 
         print("ip route show:\n%s" % ip_route_show)
         print("route -n:\n%s" % route_n)
-        routes = network_state.get('routes')
+        routes = network_state.get('routes', [])
+        print("found routes: [%s]" % routes)
         for route in routes:
             print('Checking static route: %s' % route)
             destnet = (
@@ -144,6 +265,12 @@
         print('parsed ip_a dict:\n{}'.format(
             yaml.dump(ip_dict, default_flow_style=False, indent=4)))
 
+        route_n = self.load_collect_file("route_n")
+        logger.debug("route -n:\n{}".format(route_n))
+
+        route_6_n = self.load_collect_file("route_6_n")
+        logger.debug("route -6 -n:\n{}".format(route_6_n))
+
         ip_route_show = self.load_collect_file("ip_route_show")
         logger.debug("ip route show:\n{}".format(ip_route_show))
         for line in [line for line in ip_route_show.split('\n')
@@ -156,12 +283,6 @@
             route_info = m.groupdict('')
             logger.debug(route_info)
 
-        route_n = self.load_collect_file("route_n")
-        logger.debug("route -n:\n{}".format(route_n))
-
-        route_6_n = self.load_collect_file("route_6_n")
-        logger.debug("route -6 -n:\n{}".format(route_6_n))
-
         routes = {
             '4': route_n,
             '6': route_6_n,
@@ -170,9 +291,10 @@
         for iface in interfaces.values():
             print("\nnetwork_state iface: %s" % (
                 yaml.dump(iface, default_flow_style=False, indent=4)))
+            ipcfg = ip_dict.get(iface['name'], {})
             self.check_interface(iface['name'],
                                  iface,
-                                 ip_dict.get(iface['name']),
+                                 ipcfg,
                                  routes)
 
     def check_interface(self, ifname, iface, ipcfg, routes):
@@ -182,8 +304,9 @@
         # FIXME: remove check?
         # initial check, do we have the correct iface ?
         print('ifname={}'.format(ifname))
+        self.assertTrue(type(ipcfg) == dict, "%s is not dict" % (ipcfg))
+        print("ipcfg['interface']={}".format(ipcfg['interface']))
         self.assertEqual(ifname, ipcfg['interface'])
-        print("ipcfg['interface']={}".format(ipcfg['interface']))
 
         # check physical interface attributes (skip bond members, macs change)
         if iface['type'] in ['physical'] and 'bond-master' not in iface:

=== modified file 'tests/vmtests/test_network_alias.py'
--- tests/vmtests/test_network_alias.py	2017-04-26 16:14:04 +0000
+++ tests/vmtests/test_network_alias.py	2017-06-19 16:35:41 +0000
@@ -1,5 +1,6 @@
 from .releases import base_vm_classes as relbase
 from .test_network import TestNetworkBaseTestsAbs
+from unittest import SkipTest
 
 
 class TestNetworkAliasAbs(TestNetworkBaseTestsAbs):
@@ -7,6 +8,11 @@
     """
     conf_file = "examples/tests/network_alias.yaml"
 
+    def test_etc_network_interfaces(self):
+        reason = ("%s: cloud-init and curtin eni rendering"
+                  " differ" % (self.__class__))
+        raise SkipTest(reason)
+
 
 class PreciseHWETTestNetworkAlias(relbase.precise_hwe_t, TestNetworkAliasAbs):
     # FIXME: off due to hang at test: Starting execute cloud user/final scripts

=== modified file 'tests/vmtests/test_network_bridging.py'
--- tests/vmtests/test_network_bridging.py	2017-05-19 21:40:48 +0000
+++ tests/vmtests/test_network_bridging.py	2017-06-19 16:35:41 +0000
@@ -111,7 +111,6 @@
         self.assertEqual('install ok installed', status)
 
     def test_bridge_params(self):
-        """ Test if configure bridge params match values on the device """
 
         def _load_sysfs_bridge_data():
             sysfs_br0 = sysfs_to_dict(self.collect_path("sysfs_br0"))
@@ -138,6 +137,7 @@
                         p not in bridge_params_uncheckable)]
 
         def _check_bridge_param(sysfs_vals, p, br):
+            print('Checking bridge %s param %s' % (br, p))
             value = br.get(param)
             if param in ['bridge_stp']:
                 if value in ['off', '0']:
@@ -146,25 +146,35 @@
                     value = 1
                 else:
                     print('bridge_stp not in known val list')
+            elif param in ['bridge_portprio']:
+                if self._network_renderer() == "systemd-networkd":
+                    reason = ("%s: skip until lp#1668347"
+                              " is fixed" % self.__class__)
+                    logger.warn('Skipping: %s', reason)
+                    print(reason)
+                    return
 
             print('key=%s value=%s' % (param, value))
             if type(value) == list:
                 for subval in value:
                     (port, pval) = subval.split(" ")
-                    print('key=%s port=%s pval=%s' % (param, port, pval))
+                    print('param=%s port=%s pval=%s' % (param, port, pval))
                     sys_file_val = _get_sysfs_value(sysfs_vals, br0['name'],
                                                     param, port)
 
-                    self.assertEqual(int(pval), int(sys_file_val))
+                    msg = "Source cfg: %s=%s on port %s" % (param, value, port)
+                    self.assertEqual(int(pval), int(sys_file_val), msg)
             else:
                 sys_file_val = _get_sysfs_value(sysfs_vals, br0['name'],
                                                 param, port=None)
-                self.assertEqual(int(value), int(sys_file_val))
+                self.assertEqual(int(value), int(sys_file_val),
+                                 "Source cfg: %s=%s" % (param, value))
 
         sysfs_vals = _load_sysfs_bridge_data()
-        print(sysfs_vals)
+        # print(sysfs_vals)
         br0 = _get_bridge_config()
         for param in _get_bridge_params(br0):
+            print('Checking param %s' % param)
             _check_bridge_param(sysfs_vals, param, br0)
 
 

=== modified file 'tests/vmtests/test_network_ipv6_enisource.py'
--- tests/vmtests/test_network_ipv6_enisource.py	2017-04-26 16:14:04 +0000
+++ tests/vmtests/test_network_ipv6_enisource.py	2017-06-19 16:35:41 +0000
@@ -1,10 +1,16 @@
 from .releases import base_vm_classes as relbase
 from .test_network_enisource import TestNetworkENISource
 
+import unittest
+
 
 class TestNetworkIPV6ENISource(TestNetworkENISource):
     conf_file = "examples/tests/network_source_ipv6.yaml"
 
+    @unittest.skip("FIXME: cloud-init.net needs update")
+    def test_etc_network_interfaces(self):
+        pass
+
 
 class PreciseTestNetworkIPV6ENISource(relbase.precise,
                                       TestNetworkIPV6ENISource):