Allow per-port modification of vnic_type and profile

Today neutronv2's bind_ports_to_host() method is being used
by the conductor during live-migration to bind neutron ports
to the destination host when neutron port binding API extention
is supported[1].

Until now, modifying the vnic_type and profile was not required
as the only vnic_type which was officially supported for live-migration
is the 'normal' vnic_type. Port profile updates were not required.

Support for live-migration with SR-IOV ports requires updates to
the port's profile with host specific information e.g the
claimed PCI device address on the destination node.

- This change modifies bind_ports_to_host to accept a per port
  dictionary for both vnic_type and port profile.
  Allowing the user to override each attribute on a per-port
  basis.

- This change updates bind_ports_to_host to generate
  a separate payload per vif instead of assuming all
  vifs attached to an interface share the same
  vnic_type and profile data.

- The change aims to extend the existing logic of
  bind_ports_to_host to allow the flexibility for the
  user to augment port attributes on a per-port basis.
  The change does not protect the user from wrongfully
  calling this method.

[1] https://blueprints.launchpad.net/nova/+spec/neutron-new-port-binding-api

Change-Id: I958685ee20676d45e5fbdf020b82d5844dcc85fe
Partial-Implements: blueprint libvirt-neutron-sriov-livemigration
Co-Authored-By: Adrian Chiris <[email protected]>

Author: SeanMooney

nova/network/base_api.py

@@ -399,7 +399,7 @@ def supports_port_binding_extension(self, context):
         return False
 
     def bind_ports_to_host(self, context, instance, host,
-                           vnic_type=None, profile=None):
+                           vnic_types=None, port_profiles=None):
         """Attempts to bind the ports from the instance on the given host
 
         If the ports are already actively bound to another host, like the
@@ -420,17 +420,17 @@ def bind_ports_to_host(self, context, instance, host,
         :param host: the host on which to bind the ports which
                      are attached to the instance
         :type host: str
-        :param vnic_type: optional vnic type string for the host
-                          port binding
-        :type vnic_type: str
-        :param profile: optional vif profile dict for the host port
-                        binding; note that the port binding profile is mutable
+        :param vnic_types: optional dict for the host port binding
+        :type vnic_types: dict of <port_id> : <vnic_type>
+        :param port_profiles: optional dict per port ID for the host port
+                        binding profile.
+                        note that the port binding profile is mutable
                         via the networking "Port Binding" API so callers that
                         pass in a profile should ensure they have the latest
                         version from neutron with their changes merged,
                         which can be determined using the "revision_number"
                         attribute of the port.
-        :type profile: dict
+        :type port_profiles: dict of <port_id> : <port_profile>
         :raises: PortBindingFailed if any of the ports failed to be bound to
                  the destination host
         :returns: dict, keyed by port ID, of a new host port

nova/network/neutronv2/api.py

@@ -1299,7 +1299,7 @@ def supports_port_binding_extension(self, context):
         return constants.PORT_BINDING_EXTENDED in self.extensions
 
     def bind_ports_to_host(self, context, instance, host,
-                           vnic_type=None, profile=None):
+                           vnic_types=None, port_profiles=None):
         """Attempts to bind the ports from the instance on the given host
 
         If the ports are already actively bound to another host, like the
@@ -1320,17 +1320,17 @@ def bind_ports_to_host(self, context, instance, host,
         :param host: the host on which to bind the ports which
                      are attached to the instance
         :type host: str
-        :param vnic_type: optional vnic type string for the host
-                          port binding
-        :type vnic_type: str
-        :param profile: optional vif profile dict for the host port
-                        binding; note that the port binding profile is mutable
+        :param vnic_types: optional dict for the host port binding
+        :type vnic_types: dict of <port_id> : <vnic_type>
+        :param port_profiles: optional dict per port ID for the host port
+                        binding profile.
+                        note that the port binding profile is mutable
                         via the networking "Port Binding" API so callers that
                         pass in a profile should ensure they have the latest
                         version from neutron with their changes merged,
                         which can be determined using the "revision_number"
                         attribute of the port.
-        :type profile: dict
+        :type port_profiles: dict of <port_id> : <port_profile>
         :raises: PortBindingFailed if any of the ports failed to be bound to
                  the destination host
         :returns: dict, keyed by port ID, of a new host port
@@ -1339,31 +1339,36 @@ def bind_ports_to_host(self, context, instance, host,
         # Get the current ports off the instance. This assumes the cache is
         # current.
         network_info = instance.get_network_info()
-        port_ids = [vif['id'] for vif in network_info]
 
-        if not port_ids:
+        if not network_info:
             # The instance doesn't have any ports so there is nothing to do.
             LOG.debug('Instance does not have any ports.', instance=instance)
             return {}
 
         client = _get_ksa_client(context, admin=True)
 
-        # Now bind each port to the destination host and keep track of each
-        # port that is bound to the resulting binding so we can rollback in
-        # the event of a failure, or return the results if everything is OK.
-        binding = dict(host=host)
-        if vnic_type:
-            binding['vnic_type'] = vnic_type
-        if profile:
-            binding['profile'] = profile
-        data = dict(binding=binding)
-
         # TODO(gibi): To support ports with resource request during server
         # live migrate operation we need to take care of 'allocation' key in
         # the binding profile per binding.
 
         bindings_by_port_id = {}
-        for port_id in port_ids:
+        for vif in network_info:
+            # Now bind each port to the destination host and keep track of each
+            # port that is bound to the resulting binding so we can rollback in
+            # the event of a failure, or return the results if everything is OK
+            port_id = vif['id']
+            binding = dict(host=host)
+            if vnic_types is None or port_id not in vnic_types:
+                binding['vnic_type'] = vif['vnic_type']
+            else:
+                binding['vnic_type'] = vnic_types[port_id]
+
+            if port_profiles is None or port_id not in port_profiles:
+                binding['profile'] = vif['profile']
+            else:
+                binding['profile'] = port_profiles[port_id]
+
+            data = dict(binding=binding)
             resp = self._create_port_binding(client, port_id, data)
             if resp:
                 bindings_by_port_id[port_id] = resp.json()['binding']

nova/tests/unit/network/test_neutronv2.py

@@ -6352,20 +6352,71 @@ def test_bind_ports_to_host_no_ports(self, mock_client):
     @mock.patch('nova.network.neutronv2.api._get_ksa_client')
     def test_bind_ports_to_host(self, mock_client):
         """Tests a single port happy path where everything is successful."""
+        def post_side_effect(*args, **kwargs):
+            self.assertDictEqual(binding, kwargs['json'])
+            return mock.DEFAULT
+
         nwinfo = model.NetworkInfo([model.VIF(uuids.port)])
         inst = objects.Instance(
             info_cache=objects.InstanceInfoCache(network_info=nwinfo))
         ctxt = context.get_context()
         binding = {'binding': {'host': 'fake-host',
                                'vnic_type': 'normal',
                                'profile': {'foo': 'bar'}}}
+
         resp = fake_req.FakeResponse(200, content=jsonutils.dumps(binding))
         mock_client.return_value.post.return_value = resp
+        mock_client.return_value.post.side_effect = post_side_effect
         result = self.api.bind_ports_to_host(
-            ctxt, inst, 'fake-host', 'normal', {'foo': 'bar'})
+            ctxt, inst, 'fake-host', {uuids.port: 'normal'},
+            {uuids.port: {'foo': 'bar'}})
         self.assertEqual(1, mock_client.return_value.post.call_count)
         self.assertDictEqual({uuids.port: binding['binding']}, result)
 
+    @mock.patch('nova.network.neutronv2.api._get_ksa_client')
+    def test_bind_ports_to_host_with_vif_profile_and_vnic(self, mock_client):
+        """Tests bind_ports_to_host with default/non-default parameters."""
+        def post_side_effect(*args, **kwargs):
+            self.assertDictEqual(binding, kwargs['json'])
+            return mock.DEFAULT
+
+        ctxt = context.get_context()
+        vif_profile = {'foo': 'default'}
+        nwinfo = model.NetworkInfo([model.VIF(id=uuids.port,
+                                              vnic_type="direct",
+                                              profile=vif_profile)])
+        inst = objects.Instance(
+            info_cache=objects.InstanceInfoCache(network_info=nwinfo))
+        binding = {'binding': {'host': 'fake-host',
+                               'vnic_type': 'direct',
+                               'profile': vif_profile}}
+        resp = fake_req.FakeResponse(200, content=jsonutils.dumps(binding))
+        mock_client.return_value.post.return_value = resp
+        mock_client.return_value.post.side_effect = post_side_effect
+        result = self.api.bind_ports_to_host(ctxt, inst, 'fake-host')
+        self.assertEqual(1, mock_client.return_value.post.call_count)
+        self.assertDictEqual({uuids.port: binding['binding']}, result)
+
+        # assert that that if vnic_type and profile are set in VIF object
+        # the provided vnic_type and profile take precedence.
+
+        nwinfo = model.NetworkInfo([model.VIF(id=uuids.port,
+                                              vnic_type='direct',
+                                              profile=vif_profile)])
+        inst = objects.Instance(
+            info_cache=objects.InstanceInfoCache(network_info=nwinfo))
+        vif_profile_per_port = {uuids.port: {'foo': 'overridden'}}
+        vnic_type_per_port = {uuids.port: "direct-overridden"}
+        binding = {'binding': {'host': 'fake-host',
+                               'vnic_type': 'direct-overridden',
+                               'profile': {'foo': 'overridden'}}}
+        resp = fake_req.FakeResponse(200, content=jsonutils.dumps(binding))
+        mock_client.return_value.post.return_value = resp
+        result = self.api.bind_ports_to_host(
+            ctxt, inst, 'fake-host', vnic_type_per_port, vif_profile_per_port)
+        self.assertEqual(2, mock_client.return_value.post.call_count)
+        self.assertDictEqual({uuids.port: binding['binding']}, result)
+
     @mock.patch('nova.network.neutronv2.api._get_ksa_client')
     def test_bind_ports_to_host_rollback(self, mock_client):
         """Tests a scenario where an instance has two ports, and binding the