Merge "Reject networks with QoS policy"

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

@@ -2205,7 +2205,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

@@ -2154,6 +2154,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
@@ -5417,3 +5418,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

@@ -3855,6 +3855,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