Merge "Add portgroups to support LAG interfaces - net"

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.
+        :param vifs: A dict with keys 'ports' and 'portgroups' and
-            The keys should be Ironic port UUIDs, and the values should be
+            dicts as values. Each dict has key/value pairs of the form
-            Neutron port UUIDs
+            <ironic UUID>:<neutron port UUID>. e.g.
-            If the value is None, will get the list of ports from the Ironic
-            port objects.
 
+                          ::
+
+                           {'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.
+        :param task: A TaskManager instance.
-        :returns: List of IP addresses associated with task.ports
+        :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.
+        :param vifs: a dict of Neutron port/portgroup dicts
-            The keys should be Ironic port UUIDs, and the values should be
+            to update DHCP options on. The port/portgroup dict key
-            Neutron port UUIDs
+            should be Ironic port UUIDs, and the values should be
-            If the value is None, will get the list of ports from the Ironic
+            Neutron port UUIDs, e.g.
-            port objects.
+
+            ::
+
+            {'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):
+    def _get_port_ip_address(self, task, p_obj, client):
-        """Get ip address of ironic port assigned by neutron.
+        """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)
+        vif = p_obj.extra.get('vif_port_id')
-        if not vifs:
+        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."),
+                            "to get IP address for %(obj_name)s: %(obj_id)."),
-                        {'node': task.node.uuid})
+                        {'node': task.node.uuid, 'obj_name': obj_name,
-            raise exception.FailedToGetIPAddressOnPort(port_id=port_uuid)
+                        'obj_id': p_obj.uuid})
+            raise exception.FailedToGetIPAddressOnPort(port_id=p_obj.uuid)
 
-        port_vif = vifs[port_uuid]
+        vif_ip_address = self._get_fixed_ip_address(vif, client)
+        return vif_ip_address
 
-        port_ip_address = self._get_fixed_ip_address(port_vif, client)
+    def _get_ip_addresses(self, task, pobj_list, client):
-        return port_ip_address
+        """Get IP addresses for all ports/portgroups.
-
-    def get_ip_addresses(self, task):
-        """Get IP addresses for all ports in `task`.
 
         :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,
+                vif_ip_address = self._get_port_ip_address(task, obj,
                                                            client)
-                ip_addresses.append(port_ip_address)
+                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"
+            obj_name = 'portgroups'
-                            " while retrieving IP address on the following"
+            if isinstance(pobj_list[0], Port):
-                            " ports: %(ports)s."),
+                obj_name = 'ports'
-                        {'node': task.node.uuid, 'ports': failures})
+
+            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):
+    def test_get_node_vif_ids_no_ports_no_portgroups(self):
-        expected = {}
+        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_gfia):
-    def test__get_port_ip_address(self, mock_gnvi, 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_for_portgroup(self, mock_gfia):
-    def test__get_port_ip_address_with_exception(self, mock_gnvi, mock_gfia):
         expected = "192.168.1.3"
-        port = object_utils.create_test_port(self.context,
+        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_gnvi.return_value = None
+        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(),
+                                             driver='fake')
         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