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(-) 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