Merge pull request #27 from stackhpc/backport/wallaby-port-groups

Backport port group support (wallaby)

Author: markgoddard

networking_generic_switch/generic_switch_mech.py

@@ -343,13 +343,14 @@ def update_port_postcommit(self, context):
                 'local_link_information')
             if not local_link_information:
                 return
-            switch_info = local_link_information[0].get('switch_info')
-            switch_id = local_link_information[0].get('switch_id')
-            switch = device_utils.get_switch_device(
-                self.switches, switch_info=switch_info,
-                ngs_mac_address=switch_id)
-            if not switch:
-                return
+            for link in local_link_information:
+                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)
+                if not switch:
+                    return
             provisioning_blocks.provisioning_complete(
                 context._plugin_context, port['id'], resources.PORT,
                 GENERIC_SWITCH_ENTITY)
@@ -432,45 +433,86 @@ def bind_port(self, context):
         """
 
         port = context.current
+        network = context.network.current
         binding_profile = port['binding:profile']
         local_link_information = binding_profile.get('local_link_information')
+
         if self._is_port_supported(port) and local_link_information:
-            switch_info = local_link_information[0].get('switch_info')
-            switch_id = local_link_information[0].get('switch_id')
+            # NOTE(jamesdenton): If any link of the port is invalid, none
+            # 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
+                    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})
+
+            context.set_binding(segments[0][api.ID],
+                                portbindings.VIF_TYPE_OTHER, {})
+            provisioning_blocks.add_provisioning_component(
+                context._plugin_context, port['id'], resources.PORT,
+                GENERIC_SWITCH_ENTITY)
+
+    def _is_link_valid(self, port, network):
+        """Return whether a link references valid switch and physnet.
+
+        If the local link information refers to a switch that is not
+        known to NGS or the switch is not associated with the respective
+        physnet, the port will not be processed and no exception will
+        be raised.
+
+        :param port: The port to check
+        :param network: the network mapped to physnet
+        :returns: Whether the link refers to a configured switch and/or switch
+                  is associated with physnet
+        """
+
+        binding_profile = port['binding:profile']
+        local_link_information = binding_profile.get('local_link_information')
+
+        for link in local_link_information:
+            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)
             if not switch:
-                return
-            network = context.network.current
+                LOG.error("Cannot bind port %(port)s as device %(device)s "
+                          "is not configured. Check baremetal port link "
+                          "configuration.",
+                          {'port': port['id'],
+                           'device': switch_info})
+                return False
+
             physnet = network['provider:physical_network']
             switch_physnets = switch._get_physical_networks()
+
             if switch_physnets and physnet not in switch_physnets:
-                LOG.error("Cannot bind port %(port)s as device %(device)s is "
-                          "not on physical network %(physnet)",
-                          {'port_id': port['id'], 'device': switch_info,
+                LOG.error("Cannot bind port %(port)s as device %(device)s "
+                          "is not on physical network %(physnet)s. Check "
+                          "baremetal port link configuration.",
+                          {'port': port['id'], 'device': switch_info,
                            'physnet': physnet})
-                return
-            port_id = local_link_information[0].get('port_id')
-            segments = context.segments_to_bind
-            # If segmentation ID is None, set vlan 1
-            segmentation_id = segments[0].get('segmentation_id') or 1
-            provisioning_blocks.add_provisioning_component(
-                context._plugin_context, port['id'], resources.PORT,
-                GENERIC_SWITCH_ENTITY)
-            LOG.debug("Putting port {port} on {switch_info} to vlan: "
-                      "{segmentation_id}".format(
-                          port=port_id,
-                          switch_info=switch_info,
-                          segmentation_id=segmentation_id))
-            # Move port to network
-            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})
-            context.set_binding(segments[0][api.ID],
-                                portbindings.VIF_TYPE_OTHER, {})
+                return False
+        return True
 
     @staticmethod
     def _is_port_supported(port):
@@ -513,34 +555,34 @@ def _unplug_port_from_network(self, port, network):
         local_link_information = binding_profile.get('local_link_information')
         if not local_link_information:
             return
-        switch_info = local_link_information[0].get('switch_info')
-        switch_id = local_link_information[0].get('switch_id')
-        switch = device_utils.get_switch_device(
-            self.switches, switch_info=switch_info,
-            ngs_mac_address=switch_id)
-        if not switch:
-            return
-        port_id = local_link_information[0].get('port_id')
-        # If segmentation ID is None, set vlan 1
-        segmentation_id = network.get('provider:segmentation_id') or 1
-        LOG.debug("Unplugging port {port} on {switch_info} from vlan: "
-                  "{segmentation_id}".format(
-                      port=port_id,
-                      switch_info=switch_info,
-                      segmentation_id=segmentation_id))
-        try:
-            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 "
-                      "reason: %(exc)s",
-                      {'port_id': port['id'], 'net_id': network['id'],
-                       'switch': switch_info, 'exc': e})
-            raise e
-        LOG.info('Port %(port_id)s has been unplugged from network '
-                 '%(net_id)s on device %(device)s',
-                 {'port_id': port['id'], 'net_id': network['id'],
-                  'device': switch_info})
+        for link in local_link_information:
+            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)
+            if not switch:
+                continue
+            port_id = link.get('port_id')
+            # If segmentation ID is None, set vlan 1
+            segmentation_id = network.get('provider:segmentation_id') or 1
+            LOG.debug("Unplugging port %(port)s on %(switch_info)s from vlan: "
+                      "%(segmentation_id)s",
+                      {'port': port_id, 'switch_info': switch_info,
+                       'segmentation_id': segmentation_id})
+            try:
+                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 "
+                          "reason: %(exc)s",
+                          {'port_id': port['id'], 'net_id': network['id'],
+                           'switch': switch_info, 'exc': e})
+                raise e
+            LOG.info('Port %(port_id)s has been unplugged from network '
+                     '%(net_id)s on device %(device)s',
+                     {'port_id': port['id'], 'net_id': network['id'],
+                      'device': switch_info})
 
     def _get_devices_by_physnet(self, physnet):
         """Generator yielding switches on a particular physical network.

networking_generic_switch/tests/unit/netmiko/test_dell.py

@@ -256,8 +256,8 @@ def test_check_output(self):
     def _test_check_output_error(self, output):
         msg = ("Found invalid configuration in device response. Operation: "
                "fake op. Output: %s" % output)
-        self.assertRaisesRegexp(exc.GenericSwitchNetmikoConfigError, msg,
-                                self.switch.check_output, output, 'fake op')
+        self.assertRaisesRegex(exc.GenericSwitchNetmikoConfigError, msg,
+                               self.switch.check_output, output, 'fake op')
 
     def test_check_output_incomplete_command(self):
         output = """

networking_generic_switch/tests/unit/netmiko/test_juniper.py

@@ -166,10 +166,10 @@ def test_save_configuration_db_locked_timeout(self, m_stop, m_wait):
         mock_connection.commit.side_effect = ValueError(
             "Commit failed with the following errors:\n\n{0}".format(output))
 
-        self.assertRaisesRegexp(exc.GenericSwitchNetmikoConfigError,
-                                "Reached timeout waiting for",
-                                self.switch.save_configuration,
-                                mock_connection)
+        self.assertRaisesRegex(exc.GenericSwitchNetmikoConfigError,
+                               "Reached timeout waiting for",
+                               self.switch.save_configuration,
+                               mock_connection)
         self.assertGreater(mock_connection.commit.call_count, 1)
         m_stop.assert_called_once_with(60)
         m_wait.assert_called_once_with(5)
@@ -214,10 +214,10 @@ def test_save_configuration_warn_already_exists_timeout(
         mock_connection.commit.side_effect = ValueError(
             "Commit failed with the following errors:\n\n{0}".format(output))
 
-        self.assertRaisesRegexp(exc.GenericSwitchNetmikoConfigError,
-                                "Reached timeout while attempting",
-                                self.switch.save_configuration,
-                                mock_connection)
+        self.assertRaisesRegex(exc.GenericSwitchNetmikoConfigError,
+                               "Reached timeout while attempting",
+                               self.switch.save_configuration,
+                               mock_connection)
         self.assertGreater(mock_connection.commit.call_count, 1)
         m_stop.assert_called_once_with(60)
         m_wait.assert_called_once_with(5)
@@ -262,10 +262,10 @@ def test_save_configuration_warn_does_not_exist_timeout(
         mock_connection.commit.side_effect = ValueError(
             "Commit failed with the following errors:\n\n{0}".format(output))
 
-        self.assertRaisesRegexp(exc.GenericSwitchNetmikoConfigError,
-                                "Reached timeout while attempting",
-                                self.switch.save_configuration,
-                                mock_connection)
+        self.assertRaisesRegex(exc.GenericSwitchNetmikoConfigError,
+                               "Reached timeout while attempting",
+                               self.switch.save_configuration,
+                               mock_connection)
         self.assertGreater(mock_connection.commit.call_count, 1)
         m_stop.assert_called_once_with(60)
         m_wait.assert_called_once_with(5)
@@ -284,10 +284,10 @@ def test_save_configuration_error(self):
         mock_connection.commit.side_effect = ValueError(
             "Commit failed with the following errors:\n\n{0}".format(output))
 
-        self.assertRaisesRegexp(exc.GenericSwitchNetmikoConfigError,
-                                "Failed to commit configuration",
-                                self.switch.save_configuration,
-                                mock_connection)
+        self.assertRaisesRegex(exc.GenericSwitchNetmikoConfigError,
+                               "Failed to commit configuration",
+                               self.switch.save_configuration,
+                               mock_connection)
         mock_connection.commit.assert_called_once_with()
 
     @mock.patch.object(netmiko_devices.tenacity, 'wait_fixed',

networking_generic_switch/tests/unit/netmiko/test_netmiko_base.py

@@ -382,5 +382,5 @@ def test_check_output_error(self):
 """
         msg = ("Found invalid configuration in device response. Operation: "
                "fake op. Output: %s" % output)
-        self.assertRaisesRegexp(exc.GenericSwitchNetmikoConfigError, msg,
-                                self.switch.check_output, output, 'fake op')
+        self.assertRaisesRegex(exc.GenericSwitchNetmikoConfigError, msg,
+                               self.switch.check_output, output, 'fake op')

networking_generic_switch/tests/unit/test_devices.py

@@ -156,7 +156,7 @@ def test__validate_network_name_format(self):
 
     def test__validate_network_name_format_failure(self):
         device_cfg = {'ngs_network_name_format': '{invalid}'}
-        self.assertRaisesRegexp(
+        self.assertRaisesRegex(
             exc.GenericSwitchNetworkNameFormatInvalid,
             r"Invalid value for 'ngs_network_name_format'",
             FakeDevice, device_cfg)