Add portgroups to support LAG interfaces - net

Ironic should be able to provide the requisite connectivity
information to the Neutron ML2 plugin to allow drivers to
provision the top-of-rack switch for the bare metal server.
The addition of portgroups in Ironic allows the concept of
link aggregation to be handled in Ironic in order to provide
support for cases where multiple interfaces on the bare metal
server connect to switch ports of a single LAG.

This commit includes changes to:
- the port vif mapping to also use portgroups

Partial-bug: #1526403
DocImpact
Co-Authored-By: Jenny Moorehead (jenny.moorehead@sap.com)
Co-Authored-By: Will Stevenson (will.stevenson@sap.com)
Co-Authored-By: Vasyl Saienko (vsaienko@mirantis.com)

Change-Id: I7f1cba65275078da750aa49ae83ba7345d6fd5e0

ironic/common/dhcp_factory.py

@@ -89,8 +89,18 @@ class DHCPFactory(object):
                              'opt_value': '123.123.123.456'},
                             {'opt_name': 'tftp-server',
                              'opt_value': '123.123.123.123'}]
-        :param ports: a list of Neutron port dicts to update DHCP options on.
-            If None, will get the list of ports from the Ironic port objects.
+
+        :param ports: A dict with keys 'ports' and 'portgroups' and
+            dicts as values. Each dict has key/value pairs of the form
+            <ironic UUID>:<neutron port UUID>. e.g.
+
+                          ::
+
+                           {'ports': {'port.uuid': vif.id},
+                            'portgroups': {'portgroup.uuid': vif.id}}
+
+            If the value is None, will get the list of ports/portgroups
+            from the Ironic port/portgroup objects.
         """
         self.provider.update_dhcp_opts(task, dhcp_opts, ports)
 

ironic/common/network.py

@@ -19,12 +19,26 @@ def get_node_vif_ids(task):
     This function does not handle multi node operations.
 
     :param task: a TaskManager instance.
-    :returns: A dict of the Node's port UUIDs and their associated VIFs
+    :returns: A dict of Node's neutron ports where keys are
+        'ports' & 'portgroups' and the values are dict of UUIDs
+        and their associated VIFs, e.g.
 
+              ::
+
+               {'ports': {'port.uuid': vif.id},
+                'portgroups': {'portgroup.uuid': vif.id}}
     """
+    vifs = {}
+    portgroup_vifs = {}
     port_vifs = {}
+    for portgroup in task.portgroups:
+        vif = portgroup.extra.get('vif_port_id')
+        if vif:
+            portgroup_vifs[portgroup.uuid] = vif
+    vifs['portgroups'] = portgroup_vifs
     for port in task.ports:
         vif = port.extra.get('vif_port_id')
         if vif:
             port_vifs[port.uuid] = vif
-    return port_vifs
+    vifs['ports'] = port_vifs
+    return vifs

ironic/dhcp/base.py

@@ -74,21 +74,27 @@ class BaseDHCP(object):
                           {'opt_name': 'tftp-server',
                            'opt_value': '123.123.123.123'}]
 
-         :param vifs: a dict of Neutron port dicts to update DHCP options on.
-            The keys should be Ironic port UUIDs, and the values should be
-            Neutron port UUIDs
-            If the value is None, will get the list of ports from the Ironic
-            port objects.
+        :param vifs: A dict with keys 'ports' and 'portgroups' and
+            dicts as values. Each dict has key/value pairs of the form
+            <ironic UUID>:<neutron port UUID>. e.g.
 
+                          ::
+
+                           {'ports': {'port.uuid': vif.id},
+                            'portgroups': {'portgroup.uuid': vif.id}}
+
+            If the value is None, will get the list of ports/portgroups
+            from the Ironic port/portgroup objects.
         :raises: FailedToUpdateDHCPOptOnPort
         """
 
     @abc.abstractmethod
     def get_ip_addresses(self, task):
-        """Get IP addresses for all ports in `task`.
+        """Get IP addresses for all ports/portgroups in `task`.
 
-        :param task: a TaskManager instance.
-        :returns: List of IP addresses associated with task.ports
+        :param task: A TaskManager instance.
+        :returns: List of IP addresses associated with
+            task's ports and portgroups.
         """
 
     def clean_dhcp_opts(self, task):

ironic/dhcp/neutron.py

@@ -30,6 +30,7 @@ from ironic.common import keystone
 from ironic.common import network
 from ironic.dhcp import base
 from ironic.drivers.modules import ssh
+from ironic.objects.port import Port
 
 
 neutron_opts = [
@@ -152,37 +153,44 @@ class NeutronDHCPApi(base.BaseDHCP):
                              'opt_value': '123.123.123.456'},
                             {'opt_name': 'tftp-server',
                              'opt_value': '123.123.123.123'}]
-        :param vifs: a dict of Neutron port dicts to update DHCP options on.
-            The keys should be Ironic port UUIDs, and the values should be
-            Neutron port UUIDs
-            If the value is None, will get the list of ports from the Ironic
-            port objects.
+        :param vifs: a dict of Neutron port/portgroup dicts
+            to update DHCP options on. The port/portgroup dict key
+            should be Ironic port UUIDs, and the values should be
+            Neutron port UUIDs, e.g.
+
+            ::
+
+            {'ports': {'port.uuid': vif.id},
+             'portgroups': {'portgroup.uuid': vif.id}}
+            If the value is None, will get the list of ports/portgroups
+            from the Ironic port/portgroup objects.
         """
         if vifs is None:
             vifs = network.get_node_vif_ids(task)
-        if not vifs:
+        if not (vifs['ports'] or vifs['portgroups']):
             raise exception.FailedToUpdateDHCPOptOnPort(
                 _("No VIFs found for node %(node)s when attempting "
                   "to update DHCP BOOT options.") %
                 {'node': task.node.uuid})
 
         failures = []
-        for port_id, port_vif in vifs.items():
+        vif_list = [vif for pdict in vifs.values() for vif in pdict.values()]
+        for vif in vif_list:
             try:
-                self.update_port_dhcp_opts(port_vif, options,
+                self.update_port_dhcp_opts(vif, options,
                                            token=task.context.auth_token)
             except exception.FailedToUpdateDHCPOptOnPort:
-                failures.append(port_id)
+                failures.append(vif)
 
         if failures:
-            if len(failures) == len(vifs):
+            if len(failures) == len(vif_list):
                 raise exception.FailedToUpdateDHCPOptOnPort(_(
                     "Failed to set DHCP BOOT options for any port on node %s.")
                     % task.node.uuid)
             else:
                 LOG.warning(_LW("Some errors were encountered when updating "
                                 "the DHCP BOOT options for node %(node)s on "
-                                "the following ports: %(ports)s."),
+                                "the following Neutron ports: %(ports)s."),
                             {'node': task.node.uuid, 'ports': failures})
 
         # TODO(adam_g): Hack to workaround bug 1334447 until we have a
@@ -195,7 +203,7 @@ class NeutronDHCPApi(base.BaseDHCP):
             time.sleep(15)
 
     def _get_fixed_ip_address(self, port_uuid, client):
-        """Get a port's fixed ip address.
+        """Get a Neutron port's fixed ip address.
 
         :param port_uuid: Neutron port id.
         :param client: Neutron client instance.
@@ -231,55 +239,81 @@ class NeutronDHCPApi(base.BaseDHCP):
                       port_uuid)
             raise exception.FailedToGetIPAddressOnPort(port_id=port_uuid)
 
-    def _get_port_ip_address(self, task, port_uuid, client):
-        """Get ip address of ironic port assigned by neutron.
+    def _get_port_ip_address(self, task, p_obj, client):
+        """Get ip address of ironic port/portgroup assigned by Neutron.
 
         :param task: a TaskManager instance.
-        :param port_uuid: ironic Node's port UUID.
+        :param p_obj: Ironic port or portgroup object.
         :param client: Neutron client instance.
-        :returns:  Neutron port ip address associated with Node's port.
+        :returns: List of Neutron vif ip address associated with
+            Node's port/portgroup.
         :raises: FailedToGetIPAddressOnPort
         :raises: InvalidIPv4Address
         """
 
-        vifs = network.get_node_vif_ids(task)
-        if not vifs:
+        vif = p_obj.extra.get('vif_port_id')
+        if not vif:
+            obj_name = 'portgroup'
+            if isinstance(p_obj, Port):
+                obj_name = 'port'
             LOG.warning(_LW("No VIFs found for node %(node)s when attempting "
-                            " to get port IP address."),
-                        {'node': task.node.uuid})
-            raise exception.FailedToGetIPAddressOnPort(port_id=port_uuid)
-
-        port_vif = vifs[port_uuid]
+                            "to get IP address for %(obj_name)s: %(obj_id)."),
+                        {'node': task.node.uuid, 'obj_name': obj_name,
+                        'obj_id': p_obj.uuid})
+            raise exception.FailedToGetIPAddressOnPort(port_id=p_obj.uuid)
 
-        port_ip_address = self._get_fixed_ip_address(port_vif, client)
-        return port_ip_address
+        vif_ip_address = self._get_fixed_ip_address(vif, client)
+        return vif_ip_address
 
-    def get_ip_addresses(self, task):
-        """Get IP addresses for all ports in `task`.
+    def _get_ip_addresses(self, task, pobj_list, client):
+        """Get IP addresses for all ports/portgroups.
 
         :param task: a TaskManager instance.
-        :returns: List of IP addresses associated with task.ports.
+        :param pobj_list: List of port or portgroup objects.
+        :param client: Neutron client instance.
+        :returns: List of IP addresses associated with
+                  task's ports/portgroups.
         """
-        client = _build_client(task.context.auth_token)
         failures = []
         ip_addresses = []
-        for port in task.ports:
+        for obj in pobj_list:
             try:
-                port_ip_address = self._get_port_ip_address(task, port.uuid,
-                                                            client)
-                ip_addresses.append(port_ip_address)
+                vif_ip_address = self._get_port_ip_address(task, obj,
+                                                           client)
+                ip_addresses.append(vif_ip_address)
             except (exception.FailedToGetIPAddressOnPort,
                     exception.InvalidIPv4Address):
-                failures.append(port.uuid)
+                    failures.append(obj.uuid)
 
         if failures:
-            LOG.warning(_LW("Some errors were encountered on node %(node)s"
-                            " while retrieving IP address on the following"
-                            " ports: %(ports)s."),
-                        {'node': task.node.uuid, 'ports': failures})
+            obj_name = 'portgroups'
+            if isinstance(pobj_list[0], Port):
+                obj_name = 'ports'
+
+            LOG.warning(_LW(
+                "Some errors were encountered on node %(node)s "
+                "while retrieving IP addresses on the following "
+                "%(obj_name)s: %(failures)s."),
+                {'node': task.node.uuid, 'obj_name': obj_name,
+                 'failures': failures})
 
         return ip_addresses
 
+    def get_ip_addresses(self, task):
+        """Get IP addresses for all ports/portgroups in `task`.
+
+        :param task: a TaskManager instance.
+        :returns: List of IP addresses associated with
+                  task's ports/portgroups.
+        """
+        client = _build_client(task.context.auth_token)
+
+        port_ip_addresses = self._get_ip_addresses(task, task.ports, client)
+        portgroup_ip_addresses = self._get_ip_addresses(
+            task, task.portgroups, client)
+
+        return port_ip_addresses + portgroup_ip_addresses
+
     def create_cleaning_ports(self, task):
         """Create neutron ports for each port on task.node to boot the ramdisk.
 

ironic/tests/unit/common/test_network.py

@@ -30,8 +30,9 @@ class TestNetwork(db_base.DbTestCase):
         mgr_utils.mock_the_extension_manager(driver='fake')
         self.node = object_utils.create_test_node(self.context)
 
-    def test_get_node_vif_ids_no_ports(self):
-        expected = {}
+    def test_get_node_vif_ids_no_ports_no_portgroups(self):
+        expected = {'portgroups': {},
+                    'ports': {}}
         with task_manager.acquire(self.context, self.node.uuid) as task:
             result = network.get_node_vif_ids(task)
         self.assertEqual(expected, result)
@@ -42,7 +43,19 @@ class TestNetwork(db_base.DbTestCase):
                                           uuid=uuidutils.generate_uuid(),
                                           extra={'vif_port_id': 'test-vif-A'},
                                           driver='fake')
-        expected = {port1.uuid: 'test-vif-A'}
+        expected = {'portgroups': {},
+                    'ports': {port1.uuid: 'test-vif-A'}}
+        with task_manager.acquire(self.context, self.node.uuid) as task:
+            result = network.get_node_vif_ids(task)
+        self.assertEqual(expected, result)
+
+    def test_get_node_vif_ids_one_portgroup(self):
+        pg1 = db_utils.create_test_portgroup(
+            node_id=self.node.id,
+            extra={'vif_port_id': 'test-vif-A'})
+
+        expected = {'portgroups': {pg1.uuid: 'test-vif-A'},
+                    'ports': {}}
         with task_manager.acquire(self.context, self.node.uuid) as task:
             result = network.get_node_vif_ids(task)
         self.assertEqual(expected, result)
@@ -58,7 +71,26 @@ class TestNetwork(db_base.DbTestCase):
                                           uuid=uuidutils.generate_uuid(),
                                           extra={'vif_port_id': 'test-vif-B'},
                                           driver='fake')
-        expected = {port1.uuid: 'test-vif-A', port2.uuid: 'test-vif-B'}
+        expected = {'portgroups': {},
+                    'ports': {port1.uuid: 'test-vif-A',
+                              port2.uuid: 'test-vif-B'}}
+        with task_manager.acquire(self.context, self.node.uuid) as task:
+            result = network.get_node_vif_ids(task)
+        self.assertEqual(expected, result)
+
+    def test_get_node_vif_ids_two_portgroups(self):
+        pg1 = db_utils.create_test_portgroup(
+            node_id=self.node.id,
+            extra={'vif_port_id': 'test-vif-A'})
+        pg2 = db_utils.create_test_portgroup(
+            uuid=uuidutils.generate_uuid(),
+            address='dd:ee:ff:aa:bb:cc',
+            node_id=self.node.id,
+            name='barname',
+            extra={'vif_port_id': 'test-vif-B'})
+        expected = {'portgroups': {pg1.uuid: 'test-vif-A',
+                                   pg2.uuid: 'test-vif-B'},
+                    'ports': {}}
         with task_manager.acquire(self.context, self.node.uuid) as task:
             result = network.get_node_vif_ids(task)
         self.assertEqual(expected, result)

ironic/tests/unit/dhcp/test_neutron.py

@@ -197,7 +197,8 @@ class TestNeutron(db_base.DbTestCase):
     @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts')
     @mock.patch('ironic.common.network.get_node_vif_ids')
     def test_update_dhcp(self, mock_gnvi, mock_updo):
-        mock_gnvi.return_value = {'port-uuid': 'vif-uuid'}
+        mock_gnvi.return_value = {'ports': {'port-uuid': 'vif-uuid'},
+                                  'portgroups': {}}
         with task_manager.acquire(self.context,
                                   self.node.uuid) as task:
             opts = pxe_utils.dhcp_options_for_instance(task)
@@ -209,7 +210,7 @@ class TestNeutron(db_base.DbTestCase):
     @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts')
     @mock.patch('ironic.common.network.get_node_vif_ids')
     def test_update_dhcp_no_vif_data(self, mock_gnvi, mock_updo):
-        mock_gnvi.return_value = {}
+        mock_gnvi.return_value = {'portgroups': {}, 'ports': {}}
         with task_manager.acquire(self.context,
                                   self.node.uuid) as task:
             api = dhcp_factory.DHCPFactory()
@@ -221,7 +222,8 @@ class TestNeutron(db_base.DbTestCase):
     @mock.patch('ironic.common.network.get_node_vif_ids')
     def test_update_dhcp_some_failures(self, mock_gnvi, mock_updo):
         # confirm update is called twice, one fails, but no exception raised
-        mock_gnvi.return_value = {'p1': 'v1', 'p2': 'v2'}
+        mock_gnvi.return_value = {'ports': {'p1': 'v1', 'p2': 'v2'},
+                                  'portgroups': {}}
         exc = exception.FailedToUpdateDHCPOptOnPort('fake exception')
         mock_updo.side_effect = [None, exc]
         with task_manager.acquire(self.context,
@@ -235,7 +237,8 @@ class TestNeutron(db_base.DbTestCase):
     @mock.patch('ironic.common.network.get_node_vif_ids')
     def test_update_dhcp_fails(self, mock_gnvi, mock_updo):
         # confirm update is called twice, both fail, and exception is raised
-        mock_gnvi.return_value = {'p1': 'v1', 'p2': 'v2'}
+        mock_gnvi.return_value = {'ports': {'p1': 'v1', 'p2': 'v2'},
+                                  'portgroups': {}}
         exc = exception.FailedToUpdateDHCPOptOnPort('fake exception')
         mock_updo.side_effect = [exc, exc]
         with task_manager.acquire(self.context,
@@ -307,8 +310,7 @@ class TestNeutron(db_base.DbTestCase):
         fake_client.show_port.assert_called_once_with(port_id)
 
     @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi._get_fixed_ip_address')
-    @mock.patch('ironic.common.network.get_node_vif_ids')
-    def test__get_port_ip_address(self, mock_gnvi, mock_gfia):
+    def test__get_port_ip_address(self, mock_gfia):
         expected = "192.168.1.3"
         port = object_utils.create_test_port(self.context,
                                              node_id=self.node.id,
@@ -317,29 +319,42 @@ class TestNeutron(db_base.DbTestCase):
                                              extra={'vif_port_id':
                                                     'test-vif-A'},
                                              driver='fake')
-        mock_gnvi.return_value = {port.uuid: 'vif-uuid'}
         mock_gfia.return_value = expected
         with task_manager.acquire(self.context,
                                   self.node.uuid) as task:
             api = dhcp_factory.DHCPFactory().provider
-            result = api._get_port_ip_address(task, port.uuid,
+            result = api._get_port_ip_address(task, port,
                                               mock.sentinel.client)
-            mock_gnvi.assert_called_once_with(task)
         self.assertEqual(expected, result)
-        mock_gfia.assert_called_once_with('vif-uuid', mock.sentinel.client)
+        mock_gfia.assert_called_once_with('test-vif-A', mock.sentinel.client)
 
     @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi._get_fixed_ip_address')
-    @mock.patch('ironic.common.network.get_node_vif_ids')
-    def test__get_port_ip_address_with_exception(self, mock_gnvi, mock_gfia):
+    def test__get_port_ip_address_for_portgroup(self, mock_gfia):
+        expected = "192.168.1.3"
+        pg = object_utils.create_test_portgroup(self.context,
+                                                node_id=self.node.id,
+                                                address='aa:bb:cc:dd:ee:ff',
+                                                uuid=uuidutils.generate_uuid(),
+                                                extra={'vif_port_id':
+                                                       'test-vif-A'},
+                                                driver='fake')
+        mock_gfia.return_value = expected
+        with task_manager.acquire(self.context,
+                                  self.node.uuid) as task:
+            api = dhcp_factory.DHCPFactory().provider
+            result = api._get_port_ip_address(task, pg,
+                                              mock.sentinel.client)
+        self.assertEqual(expected, result)
+        mock_gfia.assert_called_once_with('test-vif-A', mock.sentinel.client)
+
+    @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi._get_fixed_ip_address')
+    def test__get_port_ip_address_with_exception(self, mock_gfia):
         expected = "192.168.1.3"
         port = object_utils.create_test_port(self.context,
                                              node_id=self.node.id,
                                              address='aa:bb:cc:dd:ee:ff',
                                              uuid=uuidutils.generate_uuid(),
-                                             extra={'vif_port_id':
-                                                    'test-vif-A'},
                                              driver='fake')
-        mock_gnvi.return_value = None
         mock_gfia.return_value = expected
         with task_manager.acquire(self.context,
                                   self.node.uuid) as task:
@@ -347,7 +362,58 @@ class TestNeutron(db_base.DbTestCase):
             self.assertRaises(exception.FailedToGetIPAddressOnPort,
                               api._get_port_ip_address, task, port,
                               mock.sentinel.client)
-            mock_gnvi.assert_called_once_with(task)
+
+    @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi._get_fixed_ip_address')
+    def test__get_port_ip_address_for_portgroup_with_exception(
+            self, mock_gfia):
+        expected = "192.168.1.3"
+        pg = object_utils.create_test_portgroup(self.context,
+                                                node_id=self.node.id,
+                                                address='aa:bb:cc:dd:ee:ff',
+                                                uuid=uuidutils.generate_uuid(),
+                                                driver='fake')
+        mock_gfia.return_value = expected
+        with task_manager.acquire(self.context,
+                                  self.node.uuid) as task:
+            api = dhcp_factory.DHCPFactory().provider
+            self.assertRaises(exception.FailedToGetIPAddressOnPort,
+                              api._get_port_ip_address, task, pg,
+                              mock.sentinel.client)
+
+    @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi._get_fixed_ip_address')
+    def test__get_ip_addresses_ports(self, mock_gfia):
+        ip_address = '10.10.0.1'
+        expected = [ip_address]
+        port = object_utils.create_test_port(self.context,
+                                             node_id=self.node.id,
+                                             address='aa:bb:cc:dd:ee:ff',
+                                             uuid=uuidutils.generate_uuid(),
+                                             extra={'vif_port_id':
+                                                    'test-vif-A'},
+                                             driver='fake')
+        mock_gfia.return_value = ip_address
+        with task_manager.acquire(self.context, self.node.uuid) as task:
+            api = dhcp_factory.DHCPFactory().provider
+            result = api._get_ip_addresses(task, [port],
+                                           mock.sentinel.client)
+        self.assertEqual(expected, result)
+
+    @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi._get_fixed_ip_address')
+    def test__get_ip_addresses_portgroup(self, mock_gfia):
+        ip_address = '10.10.0.1'
+        expected = [ip_address]
+        pg = object_utils.create_test_portgroup(self.context,
+                                                node_id=self.node.id,
+                                                address='aa:bb:cc:dd:ee:ff',
+                                                uuid=uuidutils.generate_uuid(),
+                                                extra={'vif_port_id':
+                                                       'test-vif-A'},
+                                                driver='fake')
+        mock_gfia.return_value = ip_address
+        with task_manager.acquire(self.context, self.node.uuid) as task:
+            api = dhcp_factory.DHCPFactory().provider
+            result = api._get_ip_addresses(task, [pg], mock.sentinel.client)
+        self.assertEqual(expected, result)
 
     @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi._get_port_ip_address')
     def test_get_ip_addresses(self, get_ip_mock):
@@ -359,10 +425,26 @@ class TestNeutron(db_base.DbTestCase):
         with task_manager.acquire(self.context, self.node.uuid) as task:
             api = dhcp_factory.DHCPFactory().provider
             result = api.get_ip_addresses(task)
-            get_ip_mock.assert_called_once_with(task, self.ports[0].uuid,
+            get_ip_mock.assert_called_once_with(task, task.ports[0],
                                                 mock.ANY)
         self.assertEqual(expected, result)
 
+    @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi._get_port_ip_address')
+    def test_get_ip_addresses_for_port_and_portgroup(self, get_ip_mock):
+        object_utils.create_test_portgroup(self.context,
+                                           node_id=self.node.id,
+                                           address='aa:bb:cc:dd:ee:ff',
+                                           uuid=uuidutils.generate_uuid(),
+                                           extra={'vif_port_id':
+                                                  'test-vif-A'},
+                                           driver='fake')
+        with task_manager.acquire(self.context, self.node.uuid) as task:
+            api = dhcp_factory.DHCPFactory().provider
+            api.get_ip_addresses(task)
+            get_ip_mock.assert_has_calls(
+                [mock.call(task, task.ports[0], mock.ANY),
+                 mock.call(task, task.portgroups[0], mock.ANY)])
+
     @mock.patch.object(client.Client, 'create_port')
     def test_create_cleaning_ports(self, create_mock):
         # Ensure we can create cleaning ports for in band cleaning