Merge ~raharper/cloud-init:bug-lp-1709180-v2-params into cloud-init:master

[Ryan Harper]

network: add v2 passthrough and fix parsing v2 config with parameters

If the network-config sent to cloud-init is in version: 2 format then
when rendering netplan, we can pass the content through and avoid
consuming network_state elements. This removes the need for trying to
map many v2 features onto network state where other renderers won't be
able to use anyhow (for example match parameters for multi-interface
configuration and wifi configuration support).

Additionally ensure we retain bond/bridge v2 configuration in network
state so when rendering to eni or sysconfig we don't lose the configuration

- Drop the NotImplemented wifi exception, log a warning that it works for
  netplan only
- Adjust unittests to new code path and output
- Fix issue with v2 macaddress values getting dropped
- Add unittests for consuming/validating v2 configurations

LP: #1709180

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

PASSED: Continuous integration, rev:09863394a2119a9e91d34abdfab7e2f14205d053
https://jenkins.ubuntu.com/server/job/cloud-init-ci/131/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: CentOS 6 & 7: Build & Test
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/131/rebuild

review: Approve (continuous-integration)

[Chad Smith (chad.smith)]

Approved with minor nits questions inline. +1!

Validated and inspected netplan output with the following:

PYTHONPATH=. ./tools/net-convert.py --network-data=net-config.yaml --kind=yaml --output-kind=netplan --directory=out.d --mac=eth0,52:54:00:12:34:00 --mac=ens4,52:54:00:12:34:02

Prior to this branch netplan output was empty/broken:
 cat out2.d/etc/netplan/50-cloud-init.yaml

network:
    version: 2
    ethernets:
        ens4: {}
        eth0: {}

[Chad Smith (chad.smith)]

review: Approve

[Ryan Harper (raharper)]

Thanks for the review, will fix the bikeshed name. Not sure how to do a py2/py3 iterator cleanly; list isn't huge though. I'm going to push another update to this branch to address more v2 -> v1 -> v2 issues found and instead allow the netplan renderer to write out the original v2 config.

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

PASSED: Continuous integration, rev:fb634903af397678a636f390e1bc4df45e9148be
https://jenkins.ubuntu.com/server/job/cloud-init-ci/135/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: CentOS 6 & 7: Build & Test
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/135/rebuild

review: Approve (continuous-integration)

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

PASSED: Continuous integration, rev:c3c19526beec952d865e483de0837fa0a27c1d74
https://jenkins.ubuntu.com/server/job/cloud-init-ci/136/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: CentOS 6 & 7: Build & Test
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/136/rebuild

review: Approve (continuous-integration)

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

PASSED: Continuous integration, rev:dc2bd79949492bccdc1d7df0132f98c354d51943
https://jenkins.ubuntu.com/server/job/cloud-init-ci/145/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: CentOS 6 & 7: Build & Test
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/145/rebuild

review: Approve (continuous-integration)

diff --git a/cloudinit/net/netplan.py b/cloudinit/net/netplan.py
index 9f35b72..3b06fbf 100644
--- a/cloudinit/net/netplan.py
+++ b/cloudinit/net/netplan.py
@@ -4,7 +4,7 @@ import copy
 import os
 
 from . import renderer
-from .network_state import subnet_is_ipv6
+from .network_state import subnet_is_ipv6, NET_CONFIG_TO_V2
 
 from cloudinit import log as logging
 from cloudinit import util
@@ -27,31 +27,6 @@ network:
 """
 
 LOG = logging.getLogger(__name__)
-NET_CONFIG_TO_V2 = {
-    'bond': {'bond-ad-select': 'ad-select',
-             'bond-arp-interval': 'arp-interval',
-             'bond-arp-ip-target': 'arp-ip-target',
-             'bond-arp-validate': 'arp-validate',
-             'bond-downdelay': 'down-delay',
-             'bond-fail-over-mac': 'fail-over-mac-policy',
-             'bond-lacp-rate': 'lacp-rate',
-             'bond-miimon': 'mii-monitor-interval',
-             'bond-min-links': 'min-links',
-             'bond-mode': 'mode',
-             'bond-num-grat-arp': 'gratuitious-arp',
-             'bond-primary-reselect': 'primary-reselect-policy',
-             'bond-updelay': 'up-delay',
-             'bond-xmit-hash-policy': 'transmit-hash-policy'},
-    'bridge': {'bridge_ageing': 'ageing-time',
-               'bridge_bridgeprio': 'priority',
-               'bridge_fd': 'forward-delay',
-               'bridge_gcint': None,
-               'bridge_hello': 'hello-time',
-               'bridge_maxage': 'max-age',
-               'bridge_maxwait': None,
-               'bridge_pathcost': 'path-cost',
-               'bridge_portprio': None,
-               'bridge_waitport': None}}
 
 
 def _get_params_dict_by_match(config, match):
@@ -247,6 +222,14 @@ class Renderer(renderer.Renderer):
             util.subp(cmd, capture=True)
 
     def _render_content(self, network_state):
+
+        # if content already in netplan format, pass it back
+        if network_state.version == 2:
+            LOG.debug('V2 to V2 passthrough')
+            return util.yaml_dumps({'network': network_state.config},
+                                   explicit_start=False,
+                                   explicit_end=False)
+
         ethernets = {}
         wifis = {}
         bridges = {}
diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py
index 87a7222..6faf01b 100644
--- a/cloudinit/net/network_state.py
+++ b/cloudinit/net/network_state.py
@@ -23,6 +23,33 @@ NETWORK_V2_KEY_FILTER = [
     'match', 'mtu', 'nameservers', 'renderer', 'set-name', 'wakeonlan'
 ]
 
+NET_CONFIG_TO_V2 = {
+    'bond': {'bond-ad-select': 'ad-select',
+             'bond-arp-interval': 'arp-interval',
+             'bond-arp-ip-target': 'arp-ip-target',
+             'bond-arp-validate': 'arp-validate',
+             'bond-downdelay': 'down-delay',
+             'bond-fail-over-mac': 'fail-over-mac-policy',
+             'bond-lacp-rate': 'lacp-rate',
+             'bond-miimon': 'mii-monitor-interval',
+             'bond-min-links': 'min-links',
+             'bond-mode': 'mode',
+             'bond-num-grat-arp': 'gratuitious-arp',
+             'bond-primary': 'primary',
+             'bond-primary-reselect': 'primary-reselect-policy',
+             'bond-updelay': 'up-delay',
+             'bond-xmit-hash-policy': 'transmit-hash-policy'},
+    'bridge': {'bridge_ageing': 'ageing-time',
+               'bridge_bridgeprio': 'priority',
+               'bridge_fd': 'forward-delay',
+               'bridge_gcint': None,
+               'bridge_hello': 'hello-time',
+               'bridge_maxage': 'max-age',
+               'bridge_maxwait': None,
+               'bridge_pathcost': 'path-cost',
+               'bridge_portprio': None,
+               'bridge_waitport': None}}
+
 
 def parse_net_config_data(net_config, skip_broken=True):
     """Parses the config, returns NetworkState object
@@ -120,6 +147,10 @@ class NetworkState(object):
         self.use_ipv6 = network_state.get('use_ipv6', False)
 
     @property
+    def config(self):
+        return self._network_state['config']
+
+    @property
     def version(self):
         return self._version
 
@@ -166,12 +197,14 @@ class NetworkStateInterpreter(object):
             'search': [],
         },
         'use_ipv6': False,
+        'config': None,
     }
 
     def __init__(self, version=NETWORK_STATE_VERSION, config=None):
         self._version = version
         self._config = config
         self._network_state = copy.deepcopy(self.initial_network_state)
+        self._network_state['config'] = config
         self._parsed = False
 
     @property
@@ -460,12 +493,15 @@ class NetworkStateInterpreter(object):
         v2_command = {
           bond0: {
             'interfaces': ['interface0', 'interface1'],
-            'miimon': 100,
-            'mode': '802.3ad',
-            'xmit_hash_policy': 'layer3+4'},
+            'parameters': {
+               'mii-monitor-interval': 100,
+               'mode': '802.3ad',
+               'xmit_hash_policy': 'layer3+4'}},
           bond1: {
             'bond-slaves': ['interface2', 'interface7'],
-            'mode': 1
+            'parameters': {
+                'mode': 1,
+            }
           }
         }
 
@@ -554,6 +590,7 @@ class NetworkStateInterpreter(object):
             if not mac_address:
                 LOG.debug('NetworkState Version2: missing "macaddress" info '
                           'in config entry: %s: %s', eth, str(cfg))
+            phy_cmd.update({'mac_address': mac_address})
 
             for key in ['mtu', 'match', 'wakeonlan']:
                 if key in cfg:
@@ -598,8 +635,8 @@ class NetworkStateInterpreter(object):
             self.handle_vlan(vlan_cmd)
 
     def handle_wifis(self, command):
-        raise NotImplementedError("NetworkState V2: "
-                                  "Skipping wifi configuration")
+        LOG.warning('Wifi configuration is only available to distros with'
+                    'netplan rendering support.')
 
     def _v2_common(self, cfg):
         LOG.debug('v2_common: handling config:\n%s', cfg)
@@ -616,6 +653,11 @@ class NetworkStateInterpreter(object):
 
     def _handle_bond_bridge(self, command, cmd_type=None):
         """Common handler for bond and bridge types"""
+
+        # inverse mapping for v2 keynames to v1 keynames
+        v2key_to_v1 = dict((v, k) for k, v in
+                           NET_CONFIG_TO_V2.get(cmd_type).items())
+
         for item_name, item_cfg in command.items():
             item_params = dict((key, value) for (key, value) in
                                item_cfg.items() if key not in
@@ -624,14 +666,20 @@ class NetworkStateInterpreter(object):
                 'type': cmd_type,
                 'name': item_name,
                 cmd_type + '_interfaces': item_cfg.get('interfaces'),
-                'params': item_params,
+                'params': dict((v2key_to_v1[k], v) for k, v in
+                               item_params.get('parameters', {}).items())
             }
             subnets = self._v2_to_v1_ipcfg(item_cfg)
             if len(subnets) > 0:
                 v1_cmd.update({'subnets': subnets})
 
-            LOG.debug('v2(%ss) -> v1(%s):\n%s', cmd_type, cmd_type, v1_cmd)
-            self.handle_bridge(v1_cmd)
+            LOG.debug('v2(%s) -> v1(%s):\n%s', cmd_type, cmd_type, v1_cmd)
+            if cmd_type == "bridge":
+                self.handle_bridge(v1_cmd)
+            elif cmd_type == "bond":
+                self.handle_bond(v1_cmd)
+            else:
+                raise ValueError('Unknown command type: %s', cmd_type)
 
     def _v2_to_v1_ipcfg(self, cfg):
         """Common ipconfig extraction from v2 to v1 subnets array."""
@@ -651,12 +699,6 @@ class NetworkStateInterpreter(object):
                 'address': address,
             }
 
-            routes = []
-            for route in cfg.get('routes', []):
-                routes.append(_normalize_route(
-                    {'address': route.get('to'), 'gateway': route.get('via')}))
-            subnet['routes'] = routes
-
             if ":" in address:
                 if 'gateway6' in cfg and gateway6 is None:
                     gateway6 = cfg.get('gateway6')
@@ -667,6 +709,17 @@ class NetworkStateInterpreter(object):
                     subnet.update({'gateway': gateway4})
 
             subnets.append(subnet)
+
+        routes = []
+        for route in cfg.get('routes', []):
+            routes.append(_normalize_route(
+                {'destination': route.get('to'), 'gateway': route.get('via')}))
+
+        # v2 routes are bound to the interface, in v1 we add them under
+        # the first subnet since there isn't an equivalent interface level.
+        if len(subnets) and len(routes):
+            subnets[0]['routes'] = routes
+
         return subnets
 
 
@@ -721,7 +774,7 @@ def _normalize_net_keys(network, address_keys=()):
     elif netmask:
         prefix = mask_to_net_prefix(netmask)
     elif 'prefix' in net:
-        prefix = int(prefix)
+        prefix = int(net['prefix'])
     else:
         prefix = 64 if ipv6 else 24
 
diff --git a/tests/unittests/test_distros/test_netconfig.py b/tests/unittests/test_distros/test_netconfig.py
index 2f505d9..6d89dba 100644
--- a/tests/unittests/test_distros/test_netconfig.py
+++ b/tests/unittests/test_distros/test_netconfig.py
@@ -135,7 +135,7 @@ network:
 V2_NET_CFG = {
     'ethernets': {
         'eth7': {
-            'addresses': ['192.168.1.5/255.255.255.0'],
+            'addresses': ['192.168.1.5/24'],
             'gateway4': '192.168.1.254'},
         'eth9': {
             'dhcp4': True}
@@ -151,7 +151,6 @@ V2_TO_V2_NET_CFG_OUTPUT = """
 # /etc/cloud/cloud.cfg.d/99-disable-network-config.cfg with the following:
 # network: {config: disabled}
 network:
-    version: 2
     ethernets:
         eth7:
             addresses:
@@ -159,6 +158,7 @@ network:
             gateway4: 192.168.1.254
         eth9:
             dhcp4: true
+    version: 2
 """
 
 
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index 4653be1..f251024 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -1059,6 +1059,100 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true
                   - type: static
                     address: 2001:1::1/92
             """),
+        'expected_netplan': textwrap.dedent("""
+         network:
+             version: 2
+             ethernets:
+                 bond0s0:
+                     match:
+                         macaddress: aa:bb:cc:dd:e8:00
+                     set-name: bond0s0
+                 bond0s1:
+                     match:
+                         macaddress: aa:bb:cc:dd:e8:01
+                     set-name: bond0s1
+             bonds:
+                 bond0:
+                     addresses:
+                     - 192.168.0.2/24
+                     - 192.168.1.2/24
+                     - 2001:1::1/92
+                     gateway4: 192.168.0.1
+                     interfaces:
+                     - bond0s0
+                     - bond0s1
+                     parameters:
+                         mii-monitor-interval: 100
+                         mode: active-backup
+                         transmit-hash-policy: layer3+4
+                     routes:
+                     -   to: 10.1.3.0/24
+                         via: 192.168.0.3
+        """),
+        'yaml-v2': textwrap.dedent("""
+            version: 2
+            ethernets:
+              eth0:
+                match:
+                    driver: "virtio_net"
+                    macaddress: "aa:bb:cc:dd:e8:00"
+              vf0:
+                set-name: vf0
+                match:
+                    driver: "e1000"
+                    macaddress: "aa:bb:cc:dd:e8:01"
+            bonds:
+              bond0:
+                addresses:
+                - 192.168.0.2/24
+                - 192.168.1.2/24
+                - 2001:1::1/92
+                gateway4: 192.168.0.1
+                interfaces:
+                - eth0
+                - vf0
+                parameters:
+                    mii-monitor-interval: 100
+                    mode: active-backup
+                    primary: vf0
+                    transmit-hash-policy: "layer3+4"
+                routes:
+                -   to: 10.1.3.0/24
+                    via: 192.168.0.3
+            """),
+        'expected_netplan-v2': textwrap.dedent("""
+         network:
+             bonds:
+                 bond0:
+                     addresses:
+                     - 192.168.0.2/24
+                     - 192.168.1.2/24
+                     - 2001:1::1/92
+                     gateway4: 192.168.0.1
+                     interfaces:
+                     - eth0
+                     - vf0
+                     parameters:
+                         mii-monitor-interval: 100
+                         mode: active-backup
+                         primary: vf0
+                         transmit-hash-policy: layer3+4
+                     routes:
+                     -   to: 10.1.3.0/24
+                         via: 192.168.0.3
+             ethernets:
+                 eth0:
+                     match:
+                         driver: virtio_net
+                         macaddress: aa:bb:cc:dd:e8:00
+                 vf0:
+                     match:
+                         driver: e1000
+                         macaddress: aa:bb:cc:dd:e8:01
+                     set-name: vf0
+             version: 2
+        """),
+
         'expected_sysconfig': {
             'ifcfg-bond0': textwrap.dedent("""\
         BONDING_MASTER=yes
@@ -2159,6 +2253,27 @@ class TestNetplanRoundTrip(CiTestCase):
         renderer.render_network_state(ns, target)
         return dir2dict(target)
 
+    def testsimple_render_bond_netplan(self):
+        entry = NETWORK_CONFIGS['bond']
+        files = self._render_and_read(network_config=yaml.load(entry['yaml']))
+        print(entry['expected_netplan'])
+        print('-- expected ^ | v rendered --')
+        print(files['/etc/netplan/50-cloud-init.yaml'])
+        self.assertEqual(
+            entry['expected_netplan'].splitlines(),
+            files['/etc/netplan/50-cloud-init.yaml'].splitlines())
+
+    def testsimple_render_bond_v2_input_netplan(self):
+        entry = NETWORK_CONFIGS['bond']
+        files = self._render_and_read(
+            network_config=yaml.load(entry['yaml-v2']))
+        print(entry['expected_netplan-v2'])
+        print('-- expected ^ | v rendered --')
+        print(files['/etc/netplan/50-cloud-init.yaml'])
+        self.assertEqual(
+            entry['expected_netplan-v2'].splitlines(),
+            files['/etc/netplan/50-cloud-init.yaml'].splitlines())
+
     def testsimple_render_small_netplan(self):
         entry = NETWORK_CONFIGS['small']
         files = self._render_and_read(network_config=yaml.load(entry['yaml']))