Reject networks with QoS policy

When nova needs to create ports in Neutron in a network that has minimum
bandwidth policy Nova would need to create allocation for the bandwidth
resources. The port creation happens in the compute manager after the
scheduling and resource claiming. Supporting for this is considered out
of scope for the first iteration of this feature.

To avoid resource allocation inconsistencies this patch propose to
reject such request. This rejection does not break any existing use
case as minimum bandwidth policy rule only supported by the SRIOV
Neutron backend but Nova only support booting with SRIOV port if those
ports are precreated in Neutron.

Co-Authored-By: Elod Illes <elod.illes@ericsson.com>

Change-Id: I7e1edede827cf8469771c0496b1dce55c627cf5d
blueprint: bandwidth-resource-provider

nova/api/openstack/compute/attach_interfaces.py

@@ -137,7 +137,8 @@ class InterfaceAttachmentController(wsgi.Controller):
                 exception.PortNotUsable,
                 exception.AttachInterfaceNotSupported,
                 exception.SecurityGroupCannotBeApplied,
-                exception.NetworkInterfaceTaggedAttachNotSupported) as e:
+                exception.NetworkInterfaceTaggedAttachNotSupported,
+                exception.NetworksWithQoSPolicyNotSupported) as e:
             raise exc.HTTPBadRequest(explanation=e.format_message())
         except (exception.InstanceIsLocked,
                 exception.FixedIpAlreadyInUse,

nova/compute/manager.py

@@ -2170,7 +2170,8 @@ class ComputeManager(manager.Manager):
         except (exception.VirtualInterfaceCreateException,
                 exception.VirtualInterfaceMacAddressException,
                 exception.FixedIpInvalidOnHost,
-                exception.UnableToAutoAllocateNetwork) as e:
+                exception.UnableToAutoAllocateNetwork,
+                exception.NetworksWithQoSPolicyNotSupported) as e:
             LOG.exception('Failed to allocate network(s)',
                           instance=instance)
             self._notify_about_instance_usage(context, instance,

nova/exception.py

@@ -2148,6 +2148,11 @@ class AttachInterfaceWithQoSPolicyNotSupported(AttachInterfaceNotSupported):
                 "instance %(instance_uuid)s.")
 
 
+class NetworksWithQoSPolicyNotSupported(Invalid):
+    msg_fmt = _("Using networks with QoS policy is not supported for "
+                "instance %(instance_uuid)s. (Network ID is %(network_id)s)")
+
+
 class InvalidReservedMemoryPagesOption(Invalid):
     msg_fmt = _("The format of the option 'reserved_huge_pages' is invalid. "
                 "(found '%(conf)s') Please refer to the nova "

nova/network/neutronv2/api.py

@@ -472,6 +472,15 @@ class API(base_api.NetworkAPI):
 
         return nets
 
+    def _cleanup_created_port(self, port_client, port_id, instance):
+        try:
+            port_client.delete_port(port_id)
+        except neutron_client_exc.NeutronClientException:
+            LOG.exception(
+                'Failed to delete port %(port_id)s while cleaning up after an '
+                'error.', {'port_id': port_id},
+                instance=instance)
+
     def _create_port_minimal(self, port_client, instance, network_id,
                              fixed_ip=None, security_group_ids=None):
         """Attempts to create a port for the instance on the given network.
@@ -487,6 +496,8 @@ class API(base_api.NetworkAPI):
         :raises NoMoreFixedIps: If neutron fails with
             IpAddressGenerationFailure error.
         :raises: PortBindingFailed: If port binding failed.
+        :raises NetworksWithQoSPolicyNotSupported: if the created port has
+                resource request.
         """
         # Set the device_id so it's clear who this port was created for,
         # and to stop other instances trying to use it
@@ -505,6 +516,19 @@ class API(base_api.NetworkAPI):
 
             port = port_response['port']
             port_id = port['id']
+
+            # NOTE(gibi): Checking if the created port has resource request as
+            # such ports are currently not supported as they would at least
+            # need resource allocation manipulation in placement but might also
+            # need a new scheduling if resource on this host is not available.
+            if port.get('resource_request', None):
+                self._cleanup_created_port(port_client, port_id, instance)
+                # NOTE(gibi): This limitation regarding server create can be
+                # removed when the port creation is moved to the conductor. But
+                # this code also limits attaching a network that has QoS
+                # minimum bandwidth rule.
+                raise exception.NetworksWithQoSPolicyNotSupported(
+                    instance_uuid=instance.uuid, network_id=network_id)
             try:
                 _ensure_no_port_binding_failure(port)
             except exception.PortBindingFailed:

nova/tests/functional/test_servers.py

@@ -14,6 +14,7 @@
 #    under the License.
 
 import collections
+import copy
 import datetime
 import time
 import zlib
@@ -5395,3 +5396,49 @@ class PortResourceRequestBasedSchedulingTest(
         self.assertIn('Attaching interfaces with QoS policy is '
                       'not supported for instance',
                       six.text_type(ex))
+
+    @mock.patch('nova.tests.fixtures.NeutronFixture.create_port')
+    def test_interface_attach_with_network_create_port_has_resource_request(
+            self, mock_neutron_create_port):
+        # create a server
+        server = self._create_server(
+            flavor=self.flavor,
+            networks=[{'port': self.neutron.port_1['id']}])
+        self._wait_for_state_change(self.admin_api, server, 'ACTIVE')
+
+        # the interfaceAttach operation below will result in a new port being
+        # created in the network that is attached. Make sure that neutron
+        # returns a port that has resource request.
+        mock_neutron_create_port.return_value = (
+            {'port': copy.deepcopy(self.neutron.port_with_resource_request)})
+
+        # try to attach a network
+        post = {
+            'interfaceAttachment': {
+                'net_id': self.neutron.network_1['id']
+        }}
+        ex = self.assertRaises(client.OpenStackApiException,
+                               self.api.attach_interface,
+                               server['id'], post)
+        self.assertEqual(400, ex.response.status_code)
+        self.assertIn('Using networks with QoS policy is not supported for '
+                      'instance',
+                      six.text_type(ex))
+
+    @mock.patch('nova.tests.fixtures.NeutronFixture.create_port')
+    def test_create_server_with_network_create_port_has_resource_request(
+            self, mock_neutron_create_port):
+        # the server create operation below will result in a new port being
+        # created in the network. Make sure that neutron returns a port that
+        # has resource request.
+        mock_neutron_create_port.return_value = (
+            {'port': copy.deepcopy(self.neutron.port_with_resource_request)})
+
+        server = self._create_server(
+            flavor=self.flavor,
+            networks=[{'uuid': self.neutron.network_1['id']}])
+        server = self._wait_for_state_change(self.admin_api, server, 'ERROR')
+
+        self.assertEqual(500, server['fault']['code'])
+        self.assertIn('Failed to allocate the network',
+                      server['fault']['message'])

nova/tests/unit/network/test_neutronv2.py

@@ -3846,6 +3846,49 @@ class TestNeutronv2WithMock(_TestNeutronv2Common):
         self.assertEqual(expected_exception_msg, str(exc))
         self.assertTrue(create_port_mock.called)
 
+    def test_create_port_minimal_raise_qos_not_supported(self):
+        instance = fake_instance.fake_instance_obj(self.context)
+        mock_client = mock.MagicMock()
+        mock_client.create_port.return_value = {'port': {
+            'id': uuids.port_id,
+            'resource_request': {'resources': {'CUSTOM_RESOURCE_CLASS': 42}}
+        }}
+
+        exc = self.assertRaises(exception.NetworksWithQoSPolicyNotSupported,
+                                self.api._create_port_minimal,
+                                mock_client, instance, uuids.my_netid1)
+        expected_exception_msg = ('Using networks with QoS policy is not '
+                                  'supported for instance %(instance)s. '
+                                  '(Network ID is %(net_id)s)' %
+                                  {'instance': instance.uuid,
+                                   'net_id': uuids.my_netid1})
+        self.assertEqual(expected_exception_msg, six.text_type(exc))
+        mock_client.delete_port.assert_called_once_with(uuids.port_id)
+
+    @mock.patch('nova.network.neutronv2.api.LOG')
+    def test_create_port_minimal_raise_qos_not_supported_cleanup_fails(
+            self, mock_log):
+        instance = fake_instance.fake_instance_obj(self.context)
+        mock_client = mock.MagicMock()
+        mock_client.create_port.return_value = {'port': {
+            'id': uuids.port_id,
+            'resource_request': {'resources': {'CUSTOM_RESOURCE_CLASS': 42}}
+        }}
+        mock_client.delete_port.side_effect = \
+            exceptions.NeutronClientException()
+
+        exc = self.assertRaises(exception.NetworksWithQoSPolicyNotSupported,
+                                self.api._create_port_minimal,
+                                mock_client, instance, uuids.my_netid1)
+        expected_exception_msg = ('Using networks with QoS policy is not '
+                                  'supported for instance %(instance)s. '
+                                  '(Network ID is %(net_id)s)' %
+                                  {'instance': instance.uuid,
+                                   'net_id': uuids.my_netid1})
+        self.assertEqual(expected_exception_msg, six.text_type(exc))
+        mock_client.delete_port.assert_called_once_with(uuids.port_id)
+        self.assertTrue(mock_log.exception.called)
+
     def test_get_network_detail_not_found(self):
         api = neutronapi.API()
         expected_exc = exceptions.NetworkNotFoundClient()

nova/virt/fake.py

@@ -191,6 +191,12 @@ class FakeDriver(driver.ComputeDriver):
     def spawn(self, context, instance, image_meta, injected_files,
               admin_password, allocations, network_info=None,
               block_device_info=None):
+
+        # simulate a real driver triggering the async network allocation as it
+        # might cause an error
+        if network_info:
+            [ip for vif in network_info for ip in vif.fixed_ips()]
+
         uuid = instance.uuid
         state = power_state.RUNNING
         flavor = instance.flavor

releasenotes/notes/reject-networks-with-qos-policy-2746c74fd1f3ff26.yaml

@@ -0,0 +1,9 @@
+---
+other:
+  - |
+    The ``POST /servers/{server_id}/os-interface`` request and the
+    ``POST /servers`` request will be rejected with HTTP 400 if the Neutron
+    network referenced in the request body has `QoS minimum bandwidth rule`_
+    attached as Nova currently cannot support such operations.
+
+    .. _QoS minimum bandwidth rule: https://docs.openstack.org/neutron/latest/admin/config-qos.html