From dc2bd79949492bccdc1d7df0132f98c354d51943 Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Wed, 9 Aug 2017 14:44:20 -0500 Subject: network: add v2 passthrough and fix parsing v2 config with bonds/bridge params 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 --- cloudinit/net/network_state.py | 85 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 69 insertions(+), 16 deletions(-) (limited to 'cloudinit/net/network_state.py') diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py index 87a7222d..6faf01b7 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 @@ -119,6 +146,10 @@ class NetworkState(object): self._version = version 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 -- cgit v1.2.3 From 57e2e01c703cdd1818d4f4ab8a67f37037d78582 Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Tue, 3 Oct 2017 18:56:52 -0500 Subject: network: bridge_stp value not always correct Update network_state to store the bridge_stp value as a boolean. The various renderers then can map the boolean value to the correct output as needed; eni uses 'on/off', sysconfig uses 'yes/no' and netplan will use the boolean directly. Update unittest values for sysconfig and netplan. Both contained the network_state string value which resulted in not correctly enable/disable STP in the target system. Update network_state comment (fd -> forward-delay, add stp as boolean) on bridge commands to match the expected format of a netplan bridge command. LP: #1721157 --- cloudinit/net/eni.py | 3 +++ cloudinit/net/netplan.py | 5 +++-- cloudinit/net/network_state.py | 17 +++++++++++++++-- tests/unittests/test_net.py | 5 +++-- 4 files changed, 24 insertions(+), 6 deletions(-) (limited to 'cloudinit/net/network_state.py') diff --git a/cloudinit/net/eni.py b/cloudinit/net/eni.py index bb80ec02..c6a71d16 100644 --- a/cloudinit/net/eni.py +++ b/cloudinit/net/eni.py @@ -95,6 +95,9 @@ def _iface_add_attrs(iface, index): ignore_map.append('mac_address') for key, value in iface.items(): + # convert bool to string for eni + if type(value) == bool: + value = 'on' if iface[key] else 'off' if not value or key in ignore_map: continue if key in multiline_keys: diff --git a/cloudinit/net/netplan.py b/cloudinit/net/netplan.py index 3b06fbf0..d3788af8 100644 --- a/cloudinit/net/netplan.py +++ b/cloudinit/net/netplan.py @@ -244,9 +244,9 @@ class Renderer(renderer.Renderer): for config in network_state.iter_interfaces(): ifname = config.get('name') - # filter None entries up front so we can do simple if key in dict + # filter None (but not False) entries up front ifcfg = dict((key, value) for (key, value) in config.items() - if value) + if value is not None) if_type = ifcfg.get('type') if if_type == 'physical': @@ -318,6 +318,7 @@ class Renderer(renderer.Renderer): (port, cost) = costval.split() newvalue[port] = int(cost) br_config.update({newname: newvalue}) + if len(br_config) > 0: bridge.update({'parameters': br_config}) _extract_addresses(ifcfg, bridge) diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py index 6faf01b7..890dbf8d 100644 --- a/cloudinit/net/network_state.py +++ b/cloudinit/net/network_state.py @@ -48,6 +48,7 @@ NET_CONFIG_TO_V2 = { 'bridge_maxwait': None, 'bridge_pathcost': 'path-cost', 'bridge_portprio': None, + 'bridge_stp': 'stp', 'bridge_waitport': None}} @@ -465,6 +466,18 @@ class NetworkStateInterpreter(object): for param, val in command.get('params', {}).items(): iface.update({param: val}) + # convert value to boolean + bridge_stp = iface.get('bridge_stp') + if bridge_stp and type(bridge_stp) != bool: + if bridge_stp in ['on', '1', 1]: + bridge_stp = True + elif bridge_stp in ['off', '0', 0]: + bridge_stp = False + else: + raise ValueError("Cannot convert bridge_stp value" + "(%s) to boolean", bridge_stp) + iface.update({'bridge_stp': bridge_stp}) + interfaces.update({iface['name']: iface}) @ensure_command_keys(['address']) @@ -525,8 +538,8 @@ class NetworkStateInterpreter(object): v2_command = { br0: { 'interfaces': ['interface0', 'interface1'], - 'fd': 0, - 'stp': 'off', + 'forward-delay': 0, + 'stp': False, 'maxwait': 0, } } diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index f2496151..17c9342b 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -756,6 +756,7 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true eth3: 50 eth4: 75 priority: 22 + stp: false routes: - to: ::/0 via: 2001:4800:78ff:1b::1 @@ -820,7 +821,7 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true NM_CONTROLLED=no ONBOOT=yes PRIO=22 - STP=off + STP=no TYPE=Bridge USERCTL=no"""), 'ifcfg-eth0': textwrap.dedent("""\ @@ -1296,7 +1297,7 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true NM_CONTROLLED=no ONBOOT=yes PRIO=22 - STP=off + STP=no TYPE=Bridge USERCTL=no """), -- cgit v1.2.3 From 45d361cb0b7f5e4e7d79522bd285871898358623 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Thu, 5 Oct 2017 14:26:34 -0600 Subject: net: Handle bridge stp values of 0 and convert to boolean type Update unit tests to pass a 0 instead of 'off' to validate that network state is properly written. --- cloudinit/net/network_state.py | 2 +- tests/unittests/test_net.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'cloudinit/net/network_state.py') diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py index 890dbf8d..0e830ee8 100644 --- a/cloudinit/net/network_state.py +++ b/cloudinit/net/network_state.py @@ -468,7 +468,7 @@ class NetworkStateInterpreter(object): # convert value to boolean bridge_stp = iface.get('bridge_stp') - if bridge_stp and type(bridge_stp) != bool: + if bridge_stp is not None and type(bridge_stp) != bool: if bridge_stp in ['on', '1', 1]: bridge_stp = True elif bridge_stp in ['off', '0', 0]: diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index 17c9342b..bbb63cb3 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -1283,7 +1283,7 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true - eth0 - eth1 params: - bridge_stp: 'off' + bridge_stp: 0 bridge_bridgeprio: 22 subnets: - type: static -- cgit v1.2.3