Merge "Generate necessary network metadata for ironic port groups"

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.