Support 802.3ad port groups on Cumulus devices

Since Ic3e10d19315b776662188f41c552fe0676a12782, multiple links in a
port group are configured. This typically works for bond modes that do
not require switch-side configuration, such as active/passive, TLB and
ALB.

In some cases this may also work for 802.3ad link aggregates, if
local_link_connection.port_id in the ports is set to the name of the
port group interface. However some switches require different commands
to be used when configuring port groups vs switch port interfaces. For
example, NVIDIA Cumulus switches require to use 'net add bond...'
instead of 'net add interface ...'.

This change adds support for devices that require different commands to
configure port groups, and provides an implementation for NVIDIA Cumulus
switches.

Closes-Bug: #1976382
Related-Bug: #1759000

Change-Id: I0693c495170aa821a2f571038f387c50a2f6c599
(cherry picked from commit 0de7a26)

Author: markgoddard

networking_generic_switch/devices/__init__.py

@@ -151,3 +151,11 @@ def plug_port_to_network(self, port_id, segmentation_id):
     @abc.abstractmethod
     def delete_port(self, port_id, segmentation_id):
         pass
+
+    def plug_bond_to_network(self, bond_id, segmentation_id):
+        # Fall back to interface method.
+        return self.plug_port_to_network(bond_id, segmentation_id)
+
+    def unplug_bond_from_network(self, bond_id, segmentation_id):
+        # Fall back to interface method.
+        return self.delete_port(bond_id, segmentation_id)

networking_generic_switch/devices/netmiko_devices/__init__.py

@@ -71,6 +71,10 @@ class NetmikoSwitch(devices.GenericSwitchDevice):
 
     DELETE_PORT = None
 
+    PLUG_BOND_TO_NETWORK = None
+
+    UNPLUG_BOND_FROM_NETWORK = None
+
     ADD_NETWORK_TO_TRUNK = None
 
     REMOVE_NETWORK_FROM_TRUNK = None
@@ -79,6 +83,10 @@ class NetmikoSwitch(devices.GenericSwitchDevice):
 
     DISABLE_PORT = None
 
+    ENABLE_BOND = None
+
+    DISABLE_BOND = None
+
     SAVE_CONFIGURATION = None
 
     ERROR_MSG_PATTERNS = ()
@@ -284,6 +292,47 @@ def delete_port(self, port, segmentation_id):
             cmds += self._format_commands(self.DISABLE_PORT, port=port)
         return self.send_commands_to_device(cmds)
 
+    @check_output('plug bond')
+    def plug_bond_to_network(self, bond, segmentation_id):
+        cmds = []
+        if self._disable_inactive_ports() and self.ENABLE_BOND:
+            cmds += self._format_commands(self.ENABLE_BOND, bond=bond)
+        ngs_port_default_vlan = self._get_port_default_vlan()
+        if ngs_port_default_vlan:
+            cmds += self._format_commands(
+                self.UNPLUG_BOND_FROM_NETWORK,
+                bond=bond,
+                segmentation_id=ngs_port_default_vlan)
+        cmds += self._format_commands(
+            self.PLUG_BOND_TO_NETWORK,
+            bond=bond,
+            segmentation_id=segmentation_id)
+        return self.send_commands_to_device(cmds)
+
+    @check_output('unplug bond')
+    def unplug_bond_from_network(self, bond, segmentation_id):
+        cmds = self._format_commands(self.UNPLUG_BOND_FROM_NETWORK,
+                                     bond=bond,
+                                     segmentation_id=segmentation_id)
+        ngs_port_default_vlan = self._get_port_default_vlan()
+        if ngs_port_default_vlan:
+            # NOTE(mgoddard): Pass network_id and segmentation_id for drivers
+            # not yet using network_name.
+            network_name = self._get_network_name(ngs_port_default_vlan,
+                                                  ngs_port_default_vlan)
+            cmds += self._format_commands(
+                self.ADD_NETWORK,
+                segmentation_id=ngs_port_default_vlan,
+                network_id=ngs_port_default_vlan,
+                network_name=network_name)
+            cmds += self._format_commands(
+                self.PLUG_BOND_TO_NETWORK,
+                bond=bond,
+                segmentation_id=ngs_port_default_vlan)
+        if self._disable_inactive_ports() and self.DISABLE_BOND:
+            cmds += self._format_commands(self.DISABLE_BOND, bond=bond)
+        return self.send_commands_to_device(cmds)
+
     def send_config_set(self, net_connect, cmd_set):
         """Send a set of configuration lines to the device.
 

networking_generic_switch/devices/netmiko_devices/cumulus.py

@@ -51,6 +51,14 @@ class Cumulus(netmiko_devices.NetmikoSwitch):
         'net del interface {port} bridge access {segmentation_id}',
     ]
 
+    PLUG_BOND_TO_NETWORK = [
+        'net add bond {bond} bridge access {segmentation_id}',
+    ]
+
+    UNPLUG_BOND_FROM_NETWORK = [
+        'net del bond {bond} bridge access {segmentation_id}',
+    ]
+
     ENABLE_PORT = [
         'net del interface {port} link down',
     ]
@@ -59,6 +67,14 @@ class Cumulus(netmiko_devices.NetmikoSwitch):
         'net add interface {port} link down',
     ]
 
+    ENABLE_BOND = [
+        'net del bond {bond} link down',
+    ]
+
+    DISABLE_BOND = [
+        'net add bond {bond} link down',
+    ]
+
     SAVE_CONFIGURATION = [
         'net commit',
     ]

networking_generic_switch/generic_switch_mech.py

@@ -442,28 +442,32 @@ def bind_port(self, context):
             # of the links should be processed.
             if not self._is_link_valid(port, network):
                 return
-            else:
-                for link in local_link_information:
-                    port_id = link.get('port_id')
-                    switch_info = link.get('switch_info')
-                    switch_id = link.get('switch_id')
-                    switch = device_utils.get_switch_device(
-                        self.switches, switch_info=switch_info,
-                        ngs_mac_address=switch_id)
-
-                    segments = context.segments_to_bind
-                    # If segmentation ID is None, set vlan 1
-                    segmentation_id = segments[0].get('segmentation_id') or 1
-                    LOG.debug("Putting port %(port_id)s on %(switch_info)s "
-                              "to vlan: %(segmentation_id)s",
-                              {'port_id': port_id, 'switch_info': switch_info,
-                               'segmentation_id': segmentation_id})
-                    # Move port to network
+
+            is_802_3ad = self._is_802_3ad(port)
+            for link in local_link_information:
+                port_id = link.get('port_id')
+                switch_info = link.get('switch_info')
+                switch_id = link.get('switch_id')
+                switch = device_utils.get_switch_device(
+                    self.switches, switch_info=switch_info,
+                    ngs_mac_address=switch_id)
+
+                segments = context.segments_to_bind
+                # If segmentation ID is None, set vlan 1
+                segmentation_id = segments[0].get('segmentation_id') or 1
+                LOG.debug("Putting port %(port_id)s on %(switch_info)s "
+                          "to vlan: %(segmentation_id)s",
+                          {'port_id': port_id, 'switch_info': switch_info,
+                           'segmentation_id': segmentation_id})
+                # Move port to network
+                if is_802_3ad and hasattr(switch, 'plug_bond_to_network'):
+                    switch.plug_bond_to_network(port_id, segmentation_id)
+                else:
                     switch.plug_port_to_network(port_id, segmentation_id)
-                    LOG.info("Successfully bound port %(port_id)s in segment "
-                             "%(segment_id)s on device %(device)s",
-                             {'port_id': port['id'], 'device': switch_info,
-                              'segment_id': segmentation_id})
+                LOG.info("Successfully bound port %(port_id)s in segment "
+                         "%(segment_id)s on device %(device)s",
+                         {'port_id': port['id'], 'device': switch_info,
+                          'segment_id': segmentation_id})
 
             context.set_binding(segments[0][api.ID],
                                 portbindings.VIF_TYPE_OTHER, {})
@@ -541,6 +545,21 @@ def _is_port_bound(port):
         vif_type = port[portbindings.VIF_TYPE]
         return vif_type == portbindings.VIF_TYPE_OTHER
 
+    @staticmethod
+    def _is_802_3ad(port):
+        """Return whether a port is using 802.3ad link aggregation.
+
+        :param port: The port to check
+        :returns: Whether the port is a port group using 802.3ad link
+                  aggregation.
+        """
+        binding_profile = port['binding:profile']
+        local_group_information = binding_profile.get(
+            'local_group_information')
+        if not local_group_information:
+            return False
+        return local_group_information.get('bond_mode') in ['4', '802.3ad']
+
     def _unplug_port_from_network(self, port, network):
         """Unplug a port from a network.
 
@@ -555,6 +574,8 @@ def _unplug_port_from_network(self, port, network):
         local_link_information = binding_profile.get('local_link_information')
         if not local_link_information:
             return
+
+        is_802_3ad = self._is_802_3ad(port)
         for link in local_link_information:
             switch_info = link.get('switch_info')
             switch_id = link.get('switch_id')
@@ -571,7 +592,10 @@ def _unplug_port_from_network(self, port, network):
                       {'port': port_id, 'switch_info': switch_info,
                        'segmentation_id': segmentation_id})
             try:
-                switch.delete_port(port_id, segmentation_id)
+                if is_802_3ad and hasattr(switch, 'unplug_bond_from_network'):
+                    switch.unplug_bond_from_network(port_id, segmentation_id)
+                else:
+                    switch.delete_port(port_id, segmentation_id)
             except Exception as e:
                 LOG.error("Failed to unplug port %(port_id)s "
                           "on device: %(switch)s from network %(net_id)s "

networking_generic_switch/tests/unit/netmiko/test_cumulus.py

@@ -110,6 +110,51 @@ def test_delete_port_simple(self, mock_exec):
         mock_exec.assert_called_with(
             ['net del interface 3333 bridge access 33'])
 
+    @mock.patch('networking_generic_switch.devices.netmiko_devices.'
+                'NetmikoSwitch.send_commands_to_device',
+                return_value="")
+    def test_plug_bond_to_network(self, mock_exec):
+        self.switch.plug_bond_to_network(3333, 33)
+        mock_exec.assert_called_with(
+            ['net del bond 3333 link down',
+             'net del bond 3333 bridge access 123',
+             'net add bond 3333 bridge access 33'])
+
+    @mock.patch('networking_generic_switch.devices.netmiko_devices.'
+                'NetmikoSwitch.send_commands_to_device',
+                return_value="")
+    def test_plug_bond_simple(self, mock_exec):
+        switch = self._make_switch_device({
+            'ngs_disable_inactive_ports': 'false',
+            'ngs_port_default_vlan': '',
+        })
+        switch.plug_bond_to_network(3333, 33)
+        mock_exec.assert_called_with(
+            ['net add bond 3333 bridge access 33'])
+
+    @mock.patch('networking_generic_switch.devices.netmiko_devices.'
+                'NetmikoSwitch.send_commands_to_device',
+                return_value="")
+    def test_unplug_bond_from_network(self, mock_exec):
+        self.switch.unplug_bond_from_network(3333, 33)
+        mock_exec.assert_called_with(
+            ['net del bond 3333 bridge access 33',
+             'net add vlan 123',
+             'net add bond 3333 bridge access 123',
+             'net add bond 3333 link down'])
+
+    @mock.patch('networking_generic_switch.devices.netmiko_devices.'
+                'NetmikoSwitch.send_commands_to_device',
+                return_value="")
+    def test_unplug_bond_from_network_simple(self, mock_exec):
+        switch = self._make_switch_device({
+            'ngs_disable_inactive_ports': 'false',
+            'ngs_port_default_vlan': '',
+        })
+        switch.unplug_bond_from_network(3333, 33)
+        mock_exec.assert_called_with(
+            ['net del bond 3333 bridge access 33'])
+
     def test_save(self):
         mock_connect = mock.MagicMock()
         mock_connect.save_config.side_effect = NotImplementedError

networking_generic_switch/tests/unit/test_devices.py

@@ -57,6 +57,18 @@ def test_fault_class(self):
             self.assertIn(m, ex.exception.args[0])
         self.assertIsNone(device)
 
+    @mock.patch.object(FakeDevice, 'plug_port_to_network')
+    def test_plug_bond_to_network_fallback(self, mock_plug):
+        device = FakeDevice({'spam': 'ham'})
+        device.plug_bond_to_network(22, 33)
+        mock_plug.assert_called_once_with(22, 33)
+
+    @mock.patch.object(FakeDevice, 'delete_port')
+    def test_unplug_bond_from_network_fallback(self, mock_delete):
+        device = FakeDevice({'spam': 'ham'})
+        device.unplug_bond_from_network(22, 33)
+        mock_delete.assert_called_once_with(22, 33)
+
 
 class TestDeviceManager(unittest.TestCase):