Revert "network: use nmstate for bridge options"

This reverts commit 35e919f.
The revert is temporary until a fix for [1] is merged.
[1] https://bugzilla.redhat.com/2108974

Change-Id: I45d2a7be386d8e192f12597403be870fa8a92ce3
Signed-off-by: Eitan Raviv <[email protected]>

lib/vdsm/network/link/setup.py

@@ -20,75 +20,38 @@
 from __future__ import absolute_import
 from __future__ import division
 
-
-class NmstateBridgeOpts:
-    def __init__(self):
-        self._types_dict = {
-            'mac-ageing-time': int,
-            'group-forward-mask': int,
-            'hash-max': int,
-            'multicast-snooping': self._str_to_bool,
-            'multicast-router': int,
-            'multicast-last-member-count': int,
-            'multicast-last-member-interval': int,
-            'multicast-membership-interval': int,
-            'multicast-querier': self._str_to_bool,
-            'multicast-querier-interval': int,
-            'multicast-query-use-ifaddr': self._str_to_bool,
-            'multicast-query-interval': int,
-            'multicast-query-response-interval': int,
-            'multicast-startup-query-count': int,
-            'multicast-startup-query-interval': int,
-        }
-
-    @property
-    def types_dict(self):
-        return self._types_dict
-
-    def convert_value(self, key, value):
-        conversion_func = self._types_dict[key]
-        return conversion_func(value)
-
-    @staticmethod
-    def _str_to_bool(str_value):
-        str_value = str_value.lower()
-        false_values = ['false', '0']
-        return str_value not in false_values
-
-
-class NmstateBridgeOptionNotSupported(Exception):
-    pass
+from vdsm.network.link.bridge import Bridge
 
 
 def parse_bond_options(options):
     """
     Parse bonding options into a dictionary.
     """
+
+    def _string_to_dict(str, div, eq):
+        if options == '':
+            return {}
+        return dict(
+            option.split(eq, 1) for option in str.strip(div).split(div)
+        )
+
     if options:
         d_options = _string_to_dict(options, ' ', '=')
         return d_options
     else:
         return {}
 
 
-def parse_nets_bridge_opts(opts):
-
-    if not opts:
-        return {}
-    opts_str = opts.replace('_', '-')
-    bridge_opts = _string_to_dict(opts_str, ' ', '=')
-    try:
-        nmstate_bridge_opts = NmstateBridgeOpts()
-        for key, value in bridge_opts.items():
-            bridge_opts[key] = nmstate_bridge_opts.convert_value(key, value)
-    except KeyError:
-        raise NmstateBridgeOptionNotSupported(
-            f'{key} is not a valid nmstate bridge option'
-        )
-    return bridge_opts
+def setup_custom_bridge_opts(nets):
+    for name, opts in parse_nets_bridge_opts(nets):
+        Bridge(name, opts)
 
 
-def _string_to_dict(str, div, eq):
-    if str == '':
-        return {}
-    return dict(option.split(eq, 1) for option in str.strip(div).split(div))
+def parse_nets_bridge_opts(nets):
+    for name, opts in nets.items():
+        opts_str = opts.get('custom', {}).get('bridge_opts')
+        if opts_str:
+            bridge_opts = dict(
+                opt.split('=', 1) for opt in opts_str.split(' ')
+            )
+            yield name, bridge_opts

lib/vdsm/network/netswitch/configurator.py

@@ -32,6 +32,7 @@
 from vdsm.network.configurators import qos
 from vdsm.network.dhcp_monitor import MonitoredItemPool
 from vdsm.network.link import nic
+from vdsm.network.link import setup as link_setup
 from vdsm.network.link.iface import iface as iface_obj
 from vdsm.network.netconfpersistence import RunningConfig, Transaction
 from vdsm.network.netlink import waitfor
@@ -106,6 +107,7 @@ def _setup_nmstate(networks, bondings, options, in_rollback):
             if not bond_attrs.get('remove'):
                 config.setBonding(bond_name, bond_attrs)
         config.save()
+        link_setup.setup_custom_bridge_opts(networks)
         connectivity.check(options)
 
 

lib/vdsm/network/nmstate/bridge_util.py

@@ -40,7 +40,6 @@ def __init__(self, name, attrs):
         self.bridged = attrs.get('bridged')
         self.stp = attrs.get('stp')
         self.mtu = attrs.get('mtu', DEFAULT_MTU)
-        self.bridge_opts = attrs.get('custom', {}).get('bridge_opts')
 
         self.ipv4addr = attrs.get('ipaddr')
         self.ipv4netmask = attrs.get('netmask')

lib/vdsm/network/nmstate/linux_bridge.py

@@ -19,8 +19,6 @@
 
 from collections import defaultdict
 
-from vdsm.network.link.setup import parse_nets_bridge_opts
-
 from .bridge_util import DEFAULT_MTU
 from .bridge_util import is_iface_up
 from .bridge_util import is_default_mtu
@@ -213,14 +211,11 @@ def _create_bridge_iface(self, port, options=None):
         return bridge_state
 
     def _create_bridge_options(self):
-        opts = parse_nets_bridge_opts(self._netconf.bridge_opts)
-        birdge_opts_dict = {
+        return {
             LinuxBridge.STP_SUBTREE: {
                 LinuxBridge.STP.ENABLED: self._netconf.stp
             }
         }
-        birdge_opts_dict.update(opts)
-        return birdge_opts_dict
 
     def _add_ip(self, sb_iface, vlan_iface, bridge_iface):
         ip_addr = IpAddress(self._netconf, self._auto_dns)

tests/network/functional/bridge_test.py

@@ -70,7 +70,9 @@ def test_add_bridge_with_custom_opts(self, adapter, switch, nic0):
         NET_ATTRS = {
             'nic': nic0,
             'switch': switch,
-            'custom': {'bridge_opts': 'multicast_snooping=0'},
+            'custom': {
+                'bridge_opts': 'multicast_snooping=0 multicast_router=0'
+            },
         }
         NETCREATE = {NETWORK_NAME: NET_ATTRS}
         with adapter.setupNetworks(NETCREATE, {}, nftestlib.NOCHK):

tests/network/functional/netrestore_test.py

@@ -45,8 +45,12 @@ def nic1():
 @pytest.mark.legacy_switch
 class TestRestoreLegacyBridge(object):
     def test_restore_bridge_with_custom_opts(self, adapter, nic0):
-        CUSTOM_OPTS1 = {'bridge_opts': 'multicast_snooping=0'}
-        CUSTOM_OPTS2 = {'bridge_opts': 'multicast_snooping=1'}
+        CUSTOM_OPTS1 = {
+            'bridge_opts': 'multicast_router=0 multicast_snooping=0'
+        }
+        CUSTOM_OPTS2 = {
+            'bridge_opts': 'multicast_router=0 multicast_snooping=1'
+        }
         NETCREATE = {
             NETWORK_NAME: {
                 'nic': nic0,

tests/network/functional/rollback_test.py

@@ -21,7 +21,6 @@
 import pytest
 
 from vdsm.network import errors as ne
-from vdsm.network.link.setup import NmstateBridgeOptionNotSupported
 
 from . import netfunctestlib as nftestlib
 from .netfunctestlib import SetupNetworksError
@@ -239,7 +238,7 @@ def test_setup_invalid_bridge_opts_fails(adapter, nic0):
         'custom': {'bridge_opts': 'foo=0'},
     }
 
-    with pytest.raises(NmstateBridgeOptionNotSupported):
+    with pytest.raises(SetupNetworksError):
         adapter.setupNetworks({NETWORK_NAME: net_attrs}, {}, NOCHK)
 
     adapter.update_netinfo()

tests/network/unit/link_setup_test.py

@@ -25,14 +25,21 @@
 
 def test_parse_nets_bridge_opts():
     nets = {
-        'br1': 'multicast_router=0 multicast_snooping=0',
-        'br2': 'multicast_router=1 multicast_snooping=1',
+        'br1': {
+            'custom': {
+                'bridge_opts': 'multicast_router=0 multicast_snooping=0'
+            }
+        },
+        'br2': {
+            'custom': {
+                'bridge_opts': 'multicast_router=1 multicast_snooping=1'
+            }
+        },
     }
     expected = {
-        'br1': {'multicast-router': 0, 'multicast-snooping': False},
-        'br2': {'multicast-router': 1, 'multicast-snooping': True},
+        'br1': {'multicast_router': '0', 'multicast_snooping': '0'},
+        'br2': {'multicast_router': '1', 'multicast_snooping': '1'},
     }
 
-    for name, opts in nets.items():
-        parsed_opts = linksetup.parse_nets_bridge_opts(opts)
-        assert expected[name] == parsed_opts
+    for name, opts in linksetup.parse_nets_bridge_opts(nets):
+        assert expected[name] == opts