From f1fd127c34f07624b472e07c3e589080499b29dd Mon Sep 17 00:00:00 2001 From: Bartosz Bezak Date: Thu, 27 Jul 2023 13:11:12 +0000 Subject: [PATCH] Add STP option for bridge interfaces For Rocky Linux 9, Kayobe will now disable STP on a bridge by default, to preserve compatibility with network scripts, as Network Manager enables STP on all bridges by default. Enabling STP can lead to port down event if BPDU guard is enabled on the switch. Closes-Bug: #2028775 Change-Id: I35eaa92f4243af00697306aa801e5a733885ce4f --- .../configuration/reference/network.rst | 9 +++++++ kayobe/plugins/filter/networkd.py | 3 +++ kayobe/plugins/filter/networks.py | 27 +++++++++++++++++++ .../unit/plugins/filter/test_networkd.py | 6 ++++- .../overrides.yml.j2 | 2 ++ .../tests/test_overcloud_host_configure.py | 4 +++ ...dd-bridge-stp-option-9be6bf35f6425548.yaml | 12 +++++++++ 7 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/add-bridge-stp-option-9be6bf35f6425548.yaml diff --git a/doc/source/configuration/reference/network.rst b/doc/source/configuration/reference/network.rst index 7a73deafb..f3d37da74 100644 --- a/doc/source/configuration/reference/network.rst +++ b/doc/source/configuration/reference/network.rst @@ -359,6 +359,15 @@ The following attributes are supported: ``bridge_ports`` For bridge interfaces, a list of names of network interfaces to add to the bridge. +``bridge_stp`` + .. note:: + + For Rocky Linux 9, the ``bridge_stp`` attribute is set to false to preserve + backwards compatibility with network scripts. This is because the Network + Manager sets STP to true by default on bridges. + + Enable or disable the Spanning Tree Protocol (STP) on this bridge. Should be + set to a boolean value. The default is not set on Ubuntu systems. ``bond_mode`` For bond interfaces, the bond's mode, e.g. 802.3ad. ``bond_ad_select`` diff --git a/kayobe/plugins/filter/networkd.py b/kayobe/plugins/filter/networkd.py index 221272239..191c5ecc3 100644 --- a/kayobe/plugins/filter/networkd.py +++ b/kayobe/plugins/filter/networkd.py @@ -117,6 +117,7 @@ def _bridge_netdev(context, name, inventory_hostname): """ device = networks.net_interface(context, name, inventory_hostname) mtu = networks.net_mtu(context, name, inventory_hostname) + stp = networks.net_bridge_stp(context, name, inventory_hostname) config = [ { 'NetDev': [ @@ -126,6 +127,8 @@ def _bridge_netdev(context, name, inventory_hostname): ] } ] + if stp is not None: + config[0]['Bridge'] = [{'STP': stp}] return _filter_options(config) diff --git a/kayobe/plugins/filter/networks.py b/kayobe/plugins/filter/networks.py index 1d54a8c7b..a89462eba 100644 --- a/kayobe/plugins/filter/networks.py +++ b/kayobe/plugins/filter/networks.py @@ -274,6 +274,30 @@ def net_mtu(context, name, inventory_hostname=None): return mtu +@jinja2.pass_context +def net_bridge_stp(context, name, inventory_hostname=None): + """Return the Spanning Tree Protocol (STP) state for a bridge. + + On RL9 if STP is not defined, default it to 'false' to preserve + compatibility with network scripts. STP is 'true' in NetworkManager + by default, so we set it to 'false' here. + + :param context: Jinja2 Context object. + :param name: The name of the network. + :param inventory_hostname: Ansible inventory hostname. + :returns: A string "true" or "false" representing the STP state. + """ + bridge_stp = net_attr(context, name, 'bridge_stp', inventory_hostname) + os_family = context['ansible_facts']['os_family'] + if bridge_stp is None: + if os_family == 'RedHat': + return 'false' + else: + return None + bridge_stp = str(utils.call_bool_filter(context, bridge_stp)).lower() + return bridge_stp + + net_routes = _make_attr_filter('routes') net_rules = _make_attr_filter('rules') net_physical_network = _make_attr_filter('physical_network') @@ -431,6 +455,7 @@ def net_bridge_obj(context, name, inventory_hostname=None): defroute = net_defroute(context, name, inventory_hostname) ethtool_opts = net_ethtool_opts(context, name, inventory_hostname) zone = net_zone(context, name, inventory_hostname) + stp = net_bridge_stp(context, name, inventory_hostname) vip_address = net_vip_address(context, name, inventory_hostname) allowed_addresses = [vip_address] if vip_address else None _validate_rules(rules) @@ -450,6 +475,7 @@ def net_bridge_obj(context, name, inventory_hostname=None): 'zone': zone, 'allowed_addresses': allowed_addresses, 'onboot': 'yes', + 'stp': stp, } interface = {k: v for k, v in interface.items() if v is not None} return interface @@ -734,6 +760,7 @@ def get_filters(): 'net_defroute': net_defroute, 'net_ethtool_opts': net_ethtool_opts, 'net_zone': net_zone, + 'net_bridge_stp': net_bridge_stp, 'net_interface_obj': net_interface_obj, 'net_bridge_obj': net_bridge_obj, 'net_bond_obj': net_bond_obj, diff --git a/kayobe/tests/unit/plugins/filter/test_networkd.py b/kayobe/tests/unit/plugins/filter/test_networkd.py index f2253877c..37fb272c4 100644 --- a/kayobe/tests/unit/plugins/filter/test_networkd.py +++ b/kayobe/tests/unit/plugins/filter/test_networkd.py @@ -65,6 +65,7 @@ class BaseNetworkdTest(unittest.TestCase): # Bandit complains about Jinja2 autoescaping without nosec. self.env = jinja2.Environment() # nosec self.env.filters['bool'] = to_bool + self.variables.update({'ansible_facts': {'os_family': 'Debian'}}) self.context = self._make_context(self.variables) def _make_context(self, parent): @@ -202,7 +203,7 @@ class TestNetworkdNetDevs(BaseNetworkdTest): self.assertEqual(expected, devs) def test_bridge_all_options(self): - self._update_context({"net3_mtu": 1400}) + self._update_context({"net3_mtu": 1400, "net3_bridge_stp": True}) devs = networkd.networkd_netdevs(self.context, ["net3"]) expected = { "50-kayobe-br0": [ @@ -211,6 +212,9 @@ class TestNetworkdNetDevs(BaseNetworkdTest): {"Name": "br0"}, {"Kind": "bridge"}, {"MTUBytes": 1400}, + ], + "Bridge": [ + {"STP": "true"}, ] }, ] diff --git a/playbooks/kayobe-overcloud-host-configure-base/overrides.yml.j2 b/playbooks/kayobe-overcloud-host-configure-base/overrides.yml.j2 index a88e0ef6e..da0ea7765 100644 --- a/playbooks/kayobe-overcloud-host-configure-base/overrides.yml.j2 +++ b/playbooks/kayobe-overcloud-host-configure-base/overrides.yml.j2 @@ -56,6 +56,7 @@ test_net_eth_vlan_zone: test-zone1 test_net_bridge_cidr: 192.168.36.0/24 test_net_bridge_interface: br0 test_net_bridge_bridge_ports: [dummy3, dummy4] +test_net_bridge_bridge_stp: false test_net_bridge_zone: test-zone2 # br0.43: VLAN subinterface of br0. @@ -80,6 +81,7 @@ test_net_bond_vlan_zone: public test_net_bridge_noip_cidr: 192.168.40.0/24 test_net_bridge_noip_interface: br1 test_net_bridge_noip_bridge_ports: [dummy7] +test_net_bridge_noip_bridge_stp: true test_net_bridge_noip_no_ip: true {% if ansible_os_family == "Debian" %} diff --git a/playbooks/kayobe-overcloud-host-configure-base/tests/test_overcloud_host_configure.py b/playbooks/kayobe-overcloud-host-configure-base/tests/test_overcloud_host_configure.py index 896adae14..70c33ca7a 100644 --- a/playbooks/kayobe-overcloud-host-configure-base/tests/test_overcloud_host_configure.py +++ b/playbooks/kayobe-overcloud-host-configure-base/tests/test_overcloud_host_configure.py @@ -56,6 +56,8 @@ def test_network_bridge(host): interface = host.interface('br0') assert interface.exists assert '192.168.36.1' in interface.addresses + stp_status = host.file('/sys/class/net/br0/bridge/stp_state').content_string.strip() + assert '0' == stp_status ports = ['dummy3', 'dummy4'] sys_ports = host.check_output('ls -1 /sys/class/net/br0/brif') assert sys_ports == "\n".join(ports) @@ -100,6 +102,8 @@ def test_network_bridge_no_ip(host): interface = host.interface('br1') assert interface.exists assert not '192.168.40.1' in interface.addresses + stp_status = host.file('/sys/class/net/br1/bridge/stp_state').content_string.strip() + assert '1' == stp_status @pytest.mark.skipif(not _is_apt(), diff --git a/releasenotes/notes/add-bridge-stp-option-9be6bf35f6425548.yaml b/releasenotes/notes/add-bridge-stp-option-9be6bf35f6425548.yaml new file mode 100644 index 000000000..067d32acb --- /dev/null +++ b/releasenotes/notes/add-bridge-stp-option-9be6bf35f6425548.yaml @@ -0,0 +1,12 @@ +features: + - | + The Spanning Tree Protocol (STP) can now be configured on bridge interfaces. + Enable or disable STP by setting the ``bridge_stp`` attribute for a network. + Note that STP is not set by default on Ubuntu, but it is disabled on Rocky + Linux 9 for compatibility with network scripts, as NetworkManager enables + STP on all bridges by default. +upgrade: + - | + For Rocky Linux 9, Kayobe now disables STP on a bridge by default. This + action will cause the bridge interface to restart during the host + configuration process.