diff --git a/networking_generic_switch/devices/__init__.py b/networking_generic_switch/devices/__init__.py index 1feb79cb..7ad5b61e 100644 --- a/networking_generic_switch/devices/__init__.py +++ b/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) diff --git a/networking_generic_switch/devices/netmiko_devices/__init__.py b/networking_generic_switch/devices/netmiko_devices/__init__.py index 00843730..6620e973 100644 --- a/networking_generic_switch/devices/netmiko_devices/__init__.py +++ b/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. diff --git a/networking_generic_switch/devices/netmiko_devices/cumulus.py b/networking_generic_switch/devices/netmiko_devices/cumulus.py index f6a5b40f..9095348e 100644 --- a/networking_generic_switch/devices/netmiko_devices/cumulus.py +++ b/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', ] diff --git a/networking_generic_switch/generic_switch_mech.py b/networking_generic_switch/generic_switch_mech.py index a5e153d9..43365aa0 100644 --- a/networking_generic_switch/generic_switch_mech.py +++ b/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 " diff --git a/networking_generic_switch/tests/unit/netmiko/test_cumulus.py b/networking_generic_switch/tests/unit/netmiko/test_cumulus.py index e783e83f..2ea84e0c 100644 --- a/networking_generic_switch/tests/unit/netmiko/test_cumulus.py +++ b/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 diff --git a/networking_generic_switch/tests/unit/test_devices.py b/networking_generic_switch/tests/unit/test_devices.py index 57a22b72..1eb8d8d4 100644 --- a/networking_generic_switch/tests/unit/test_devices.py +++ b/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): diff --git a/networking_generic_switch/tests/unit/test_generic_switch_mech.py b/networking_generic_switch/tests/unit/test_generic_switch_mech.py index 549b09f0..6c3356c9 100644 --- a/networking_generic_switch/tests/unit/test_generic_switch_mech.py +++ b/networking_generic_switch/tests/unit/test_generic_switch_mech.py @@ -280,6 +280,40 @@ def test_delete_portgroup_postcommit(self, m_list): [mock.call(2222, 123), mock.call(3333, 123)]) + def test_delete_portgroup_postcommit_802_3ad(self, m_list): + driver = gsm.GenericSwitchDriver() + driver.initialize() + mock_context = mock.create_autospec(driver_context.PortContext) + mock_context.current = {'binding:profile': + {'local_link_information': + [ + { + 'switch_info': 'foo', + 'port_id': 2222 + }, + { + 'switch_info': 'foo', + 'port_id': 3333 + }, + ], + 'local_group_information': + { + 'bond_mode': '4' + } + }, + 'binding:vnic_type': 'baremetal', + 'binding:vif_type': 'other', + 'id': 'aaaa-bbbb-cccc'} + mock_context.network = mock.Mock() + mock_context.network.current = {'provider:segmentation_id': 123, + 'id': 'aaaa-bbbb-cccc'} + mock_context.segments_to_bind = [mock_context.network.current] + + driver.delete_port_postcommit(mock_context) + self.switch_mock.unplug_bond_from_network.assert_has_calls( + [mock.call(2222, 123), + mock.call(3333, 123)]) + def test_delete_port_postcommit_failure(self, m_list): driver = gsm.GenericSwitchDriver() driver.initialize() @@ -513,6 +547,45 @@ def test_update_portgroup_postcommit_complete_provisioning(self, resources.PORT, 'GENERICSWITCH')]) + @mock.patch.object(provisioning_blocks, 'provisioning_complete') + def test_update_portgroup_postcommit_complete_provisioning_802_3ad(self, + m_pc, + m_list): + driver = gsm.GenericSwitchDriver() + driver.initialize() + mock_context = mock.create_autospec(driver_context.PortContext) + mock_context._plugin_context = mock.MagicMock() + mock_context.current = {'binding:profile': + {'local_link_information': + [ + { + 'switch_info': 'foo', + 'port_id': 2222 + }, + { + 'switch_info': 'foo', + 'port_id': 3333 + }, + ], + 'local_group_information': + { + 'bond_mode': '802.3ad' + } + }, + 'binding:vnic_type': 'baremetal', + 'id': '123', + 'binding:vif_type': 'other'} + mock_context.original = {'binding:profile': {}, + 'binding:vnic_type': 'baremetal', + 'id': '123', + 'binding:vif_type': 'unbound'} + driver.update_port_postcommit(mock_context) + self.switch_mock.plug_bond_to_network.assert_not_called() + m_pc.assert_has_calls([mock.call(mock_context._plugin_context, + mock_context.current['id'], + resources.PORT, + 'GENERICSWITCH')]) + @mock.patch.object(provisioning_blocks, 'provisioning_complete') def test_update_port_postcommit_unbind_not_bound(self, m_pc, m_list): driver = gsm.GenericSwitchDriver() @@ -754,6 +827,54 @@ def test_bind_portgroup(self, m_apc, m_list): resources.PORT, 'GENERICSWITCH')]) + @mock.patch.object(provisioning_blocks, 'add_provisioning_component') + def test_bind_portgroup_802_3ad(self, m_apc, m_list): + driver = gsm.GenericSwitchDriver() + driver.initialize() + mock_context = mock.create_autospec(driver_context.PortContext) + mock_context._plugin_context = mock.MagicMock() + mock_context.current = {'binding:profile': + {'local_link_information': + [ + { + 'switch_info': 'foo', + 'port_id': 2222 + }, + { + 'switch_info': 'foo', + 'port_id': 3333 + }, + ], + 'local_group_information': + { + 'bond_mode': '802.3ad' + } + }, + 'binding:vnic_type': 'baremetal', + 'id': '123'} + mock_context.network.current = { + 'provider:physical_network': 'physnet1' + } + mock_context.segments_to_bind = [ + { + 'segmentation_id': None, + 'id': 123 + } + ] + + driver.bind_port(mock_context) + self.switch_mock.plug_bond_to_network.assert_has_calls( + [mock.call(2222, 1), + mock.call(3333, 1)] + ) + mock_context.set_binding.assert_has_calls( + [mock.call(123, 'other', {})] + ) + m_apc.assert_has_calls([mock.call(mock_context._plugin_context, + mock_context.current['id'], + resources.PORT, + 'GENERICSWITCH')]) + @mock.patch.object(provisioning_blocks, 'add_provisioning_component') def test_bind_port_with_physnet(self, m_apc, m_list): driver = gsm.GenericSwitchDriver() @@ -878,3 +999,34 @@ def test_empty_methods(self, m_list): driver.create_port_postcommit(mock_context) driver.update_port_precommit(mock_context) driver.delete_port_precommit(mock_context) + + +class TestGenericSwitchDriverStaticMethods(unittest.TestCase): + + def test__is_802_3ad_no_lgi(self): + driver = gsm.GenericSwitchDriver() + port = {'binding:profile': {}} + self.assertFalse(driver._is_802_3ad(port)) + + def test__is_802_3ad_no_bond_mode(self): + driver = gsm.GenericSwitchDriver() + port = {'binding:profile': {'local_group_information': {}}} + self.assertFalse(driver._is_802_3ad(port)) + + def test__is_802_3ad_wrong_bond_mode(self): + driver = gsm.GenericSwitchDriver() + port = {'binding:profile': {'local_group_information': + {'bond_mode': 42}}} + self.assertFalse(driver._is_802_3ad(port)) + + def test__is_802_3ad_numeric(self): + driver = gsm.GenericSwitchDriver() + port = {'binding:profile': {'local_group_information': + {'bond_mode': '4'}}} + self.assertTrue(driver._is_802_3ad(port)) + + def test__is_802_3ad_string(self): + driver = gsm.GenericSwitchDriver() + port = {'binding:profile': {'local_group_information': + {'bond_mode': '802.3ad'}}} + self.assertTrue(driver._is_802_3ad(port)) diff --git a/releasenotes/notes/cumulus-802.3ad-da9bffe131995f98.yaml b/releasenotes/notes/cumulus-802.3ad-da9bffe131995f98.yaml new file mode 100644 index 00000000..facb726d --- /dev/null +++ b/releasenotes/notes/cumulus-802.3ad-da9bffe131995f98.yaml @@ -0,0 +1,4 @@ +--- +features: + - | + Adds support for 802.3ad port groups on NVIDIA Cumulus devices.