Generate necessary network metadata for ironic port groups

In ironic it is now possible to attach neutron ports to ironic port
groups. To configure them on the instance side, we need to pass the
information about them to the configdrive, which is then read by
cloud-init.

This patch amends the network metadata generated by nova because:

* neutron port's MAC can be changed by ironic during attach call,
  metadata needs to contain an up-to-date address;

* metadata needs to include additional information about the port
  group, such as it's mode and any properties set on ironic side;

* metadata needs to contain information about ports that belong to the
  selected port group.

Implements: blueprint ironic-portgroups-support
Co-Authored-By: Sam Betts <sambetts@cisco.com>
Depends-On: I0dca2c2d98184e370c08c3e05aa3edadead869af
Depends-On: Id8afa902026ce4466e96cc7bfb7fb97447d65809
Change-Id: Ic9bf6ca9e3de0068a289cbe6760cd8c4526ec7e0

4 files changed, 139 insertions, 12 deletions

diff --git a/nova/tests/unit/virt/ironic/test_driver.py b/nova/tests/unit/virt/ironic/test_driver.py
index 4712bc3537..71039f1e49 100644
--- a/nova/tests/unit/virt/ironic/test_driver.py
+++ b/nova/tests/unit/virt/ironic/test_driver.py
@@ -1707,13 +1707,75 @@ class IronicDriverGenerateConfigDriveTestCase(test.NoDBTestCase):
             request_context=None)
 
     @mock.patch.object(FAKE_CLIENT.node, 'list_ports')
-    def test_generate_network_metadata_ports_only(
-        self, mock_ports, mock_cd_builder, mock_instance_meta):
+    @mock.patch.object(FAKE_CLIENT.portgroup, 'list')
+    def _test_generate_network_metadata(self, mock_portgroups, mock_ports,
+                                        address=None, vif_internal_info=True):
+        internal_info = ({'tenant_vif_port_id': utils.FAKE_VIF_UUID}
+                         if vif_internal_info else {})
+        extra = ({'vif_port_id': utils.FAKE_VIF_UUID}
+                 if not vif_internal_info else {})
+        portgroup = ironic_utils.get_test_portgroup(
+            node_uuid=self.node.uuid, address=address,
+            extra=extra, internal_info=internal_info,
+            properties={'bond_miimon': 100, 'xmit_hash_policy': 'layer3+4'}
+        )
+        port1 = ironic_utils.get_test_port(uuid=uuidutils.generate_uuid(),
+                                           node_uuid=self.node.uuid,
+                                           address='00:00:00:00:00:01',
+                                           portgroup_uuid=portgroup.uuid)
+        port2 = ironic_utils.get_test_port(uuid=uuidutils.generate_uuid(),
+                                           node_uuid=self.node.uuid,
+                                           address='00:00:00:00:00:02',
+                                           portgroup_uuid=portgroup.uuid)
+        mock_ports.return_value = [port1, port2]
+        mock_portgroups.return_value = [portgroup]
+
+        metadata = self.driver._get_network_metadata(self.node,
+                                                     self.network_info)
+
+        pg_vif = metadata['links'][0]
+        self.assertEqual('bond', pg_vif['type'])
+        self.assertEqual('active-backup', pg_vif['bond_mode'])
+        self.assertEqual(address if address else utils.FAKE_VIF_MAC,
+                         pg_vif['ethernet_mac_address'])
+        self.assertEqual('layer3+4',
+                         pg_vif['bond_xmit_hash_policy'])
+        self.assertEqual(100, pg_vif['bond_miimon'])
+        self.assertEqual([port1.uuid, port2.uuid],
+                         pg_vif['bond_links'])
+        self.assertEqual([{'id': port1.uuid, 'type': 'phy',
+                           'ethernet_mac_address': port1.address},
+                          {'id': port2.uuid, 'type': 'phy',
+                           'ethernet_mac_address': port2.address}],
+                         metadata['links'][1:])
+        # assert there are no duplicate links
+        link_ids = [link['id'] for link in metadata['links']]
+        self.assertEqual(len(set(link_ids)), len(link_ids),
+                         'There are duplicate link IDs: %s' % link_ids)
+
+    def test_generate_network_metadata_with_pg_address(self, mock_cd_builder,
+                                       mock_instance_meta):
+        self._test_generate_network_metadata(address='00:00:00:00:00:00')
+
+    def test_generate_network_metadata_no_pg_address(self, mock_cd_builder,
+                                                     mock_instance_meta):
+        self._test_generate_network_metadata()
+
+    def test_generate_network_metadata_vif_in_extra(self, mock_cd_builder,
+                                                    mock_instance_meta):
+        self._test_generate_network_metadata(vif_internal_info=False)
+
+    @mock.patch.object(FAKE_CLIENT.node, 'list_ports')
+    @mock.patch.object(FAKE_CLIENT.portgroup, 'list')
+    def test_generate_network_metadata_ports_only(self, mock_portgroups,
+                                                  mock_ports, mock_cd_builder,
+                                                  mock_instance_meta):
         address = self.network_info[0]['address']
         port = ironic_utils.get_test_port(
             node_uuid=self.node.uuid, address=address,
             internal_info={'tenant_vif_port_id': utils.FAKE_VIF_UUID})
         mock_ports.return_value = [port]
+        mock_portgroups.return_value = []
 
         metadata = self.driver._get_network_metadata(self.node,
                                                      self.network_info)
diff --git a/nova/tests/unit/virt/ironic/utils.py b/nova/tests/unit/virt/ironic/utils.py
index 86f1150e38..8320600554 100644
--- a/nova/tests/unit/virt/ironic/utils.py
+++ b/nova/tests/unit/virt/ironic/utils.py
@@ -59,6 +59,23 @@ def get_test_port(**kw):
                 'address': kw.get('address', 'FF:FF:FF:FF:FF:FF'),
                 'extra': kw.get('extra', {}),
                 'internal_info': kw.get('internal_info', {}),
+                'portgroup_uuid': kw.get('portgroup_uuid'),
+                'created_at': kw.get('created_at'),
+                'updated_at': kw.get('updated_at')})()
+
+
+def get_test_portgroup(**kw):
+    return type('portgroup', (object,),
+               {'uuid': kw.get('uuid', 'deaffeed-1234-5678-9012-fedcbafedcba'),
+                'node_uuid': kw.get('node_uuid', get_test_node().uuid),
+                'address': kw.get('address', 'EE:EE:EE:EE:EE:EE'),
+                'extra': kw.get('extra', {}),
+                'internal_info': kw.get('internal_info', {}),
+                'properties': kw.get('properties', {}),
+                'mode': kw.get('mode', 'active-backup'),
+                'name': kw.get('name'),
+                'standalone_ports_supported': kw.get(
+                    'standalone_ports_supported', True),
                 'created_at': kw.get('created_at'),
                 'updated_at': kw.get('updated_at')})()
 
@@ -110,6 +127,12 @@ class FakePortClient(object):
         pass
 
 
+class FakePortgroupClient(object):
+
+    def list(self, node=None, detail=False):
+        pass
+
+
 class FakeNodeClient(object):
 
     def list(self, detail=False):
@@ -147,3 +170,4 @@ class FakeClient(object):
 
     node = FakeNodeClient()
     port = FakePortClient()
+    portgroup = FakePortgroupClient()
diff --git a/nova/virt/ironic/driver.py b/nova/virt/ironic/driver.py
index c89173cfdf..c3261bb0a1 100644
--- a/nova/virt/ironic/driver.py
+++ b/nova/virt/ironic/driver.py
@@ -694,25 +694,59 @@ class IronicDriver(virt_driver.ComputeDriver):
         """
         base_metadata = netutils.get_network_metadata(network_info)
 
+        # TODO(vdrok): change to doing a single "detailed vif list" call,
+        # when added to ironic API, response to that will contain all
+        # necessary information. Then we will be able to avoid looking at
+        # internal_info/extra fields.
         ports = self.ironicclient.call("node.list_ports",
                                        node.uuid, detail=True)
-
-        # TODO(vsaienko) add support of portgroups
-        vif_id_to_objects = {'ports': {}}
-        for p in ports:
-            vif_id = (p.internal_info.get('tenant_vif_port_id') or
-                      p.extra.get('vif_port_id'))
-            if vif_id:
-                vif_id_to_objects['ports'][vif_id] = p
-
+        portgroups = self.ironicclient.call("portgroup.list", node=node.uuid,
+                                            detail=True)
+        vif_id_to_objects = {'ports': {}, 'portgroups': {}}
+        for collection, name in ((ports, 'ports'), (portgroups, 'portgroups')):
+            for p in collection:
+                vif_id = (p.internal_info.get('tenant_vif_port_id') or
+                          p.extra.get('vif_port_id'))
+                if vif_id:
+                    vif_id_to_objects[name][vif_id] = p
+
+        additional_links = []
         for link in base_metadata['links']:
             vif_id = link['vif_id']
-            if vif_id in vif_id_to_objects['ports']:
+            if vif_id in vif_id_to_objects['portgroups']:
+                pg = vif_id_to_objects['portgroups'][vif_id]
+                pg_ports = [p for p in ports if p.portgroup_uuid == pg.uuid]
+                link.update({'type': 'bond', 'bond_mode': pg.mode,
+                             'bond_links': []})
+                # If address is set on the portgroup, an (ironic) vif-attach
+                # call has already updated neutron with the port address;
+                # reflect it here. Otherwise, an address generated by neutron
+                # will be used instead (code is elsewhere to handle this case).
+                if pg.address:
+                    link.update({'ethernet_mac_address': pg.address})
+                for prop in pg.properties:
+                    # These properties are the bonding driver options described
+                    # at https://www.kernel.org/doc/Documentation/networking/bonding.txt  # noqa
+                    # cloud-init checks the same way, parameter name has to
+                    # start with bond
+                    key = prop if prop.startswith('bond') else 'bond_%s' % prop
+                    link[key] = pg.properties[prop]
+                for port in pg_ports:
+                    # This won't cause any duplicates to be added. A port
+                    # cannot be in more than one port group for the same
+                    # node.
+                    additional_links.append({
+                        'id': port.uuid,
+                        'type': 'phy', 'ethernet_mac_address': port.address,
+                    })
+                    link['bond_links'].append(port.uuid)
+            elif vif_id in vif_id_to_objects['ports']:
                 p = vif_id_to_objects['ports'][vif_id]
                 # Ironic updates neutron port's address during attachment
                 link.update({'ethernet_mac_address': p.address,
                              'type': 'phy'})
 
+        base_metadata['links'].extend(additional_links)
         return base_metadata
 
     def _generate_configdrive(self, context, instance, node, network_info,
diff --git a/releasenotes/notes/add-ironic-configdrive-network-metadata-4e8f06dfd6d6d6d4.yaml b/releasenotes/notes/add-ironic-configdrive-network-metadata-4e8f06dfd6d6d6d4.yaml
new file mode 100644
index 0000000000..837f7d5dd1
--- /dev/null
+++ b/releasenotes/notes/add-ironic-configdrive-network-metadata-4e8f06dfd6d6d6d4.yaml
@@ -0,0 +1,7 @@
+---
+features:
+  - |
+    Updates the network metadata that is passed to configdrive by the Ironic
+    virt driver. The metadata now includes network information about port
+    groups and their associated ports. It will be used to configure port
+    groups on the baremetal instance side.