diff --git a/nova/api/openstack/compute/attach_interfaces.py b/nova/api/openstack/compute/attach_interfaces.py index 717b85f132f6..891747650ab6 100644 --- a/nova/api/openstack/compute/attach_interfaces.py +++ b/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, diff --git a/nova/compute/manager.py b/nova/compute/manager.py index a24743c2044c..0306415d4eb3 100644 --- a/nova/compute/manager.py +++ b/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, diff --git a/nova/exception.py b/nova/exception.py index b5ee5a4a4fad..27761383400c 100644 --- a/nova/exception.py +++ b/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 " diff --git a/nova/network/neutronv2/api.py b/nova/network/neutronv2/api.py index 2ebf8f2c9d69..bfcc5de8061c 100644 --- a/nova/network/neutronv2/api.py +++ b/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: diff --git a/nova/tests/functional/test_servers.py b/nova/tests/functional/test_servers.py index 9fc03758c8e3..92764a722f3f 100644 --- a/nova/tests/functional/test_servers.py +++ b/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']) diff --git a/nova/tests/unit/network/test_neutronv2.py b/nova/tests/unit/network/test_neutronv2.py index 41fed9875233..96e00708005e 100644 --- a/nova/tests/unit/network/test_neutronv2.py +++ b/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() diff --git a/nova/virt/fake.py b/nova/virt/fake.py index b4e222217d35..6b0f59b11783 100644 --- a/nova/virt/fake.py +++ b/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 diff --git a/releasenotes/notes/reject-networks-with-qos-policy-2746c74fd1f3ff26.yaml b/releasenotes/notes/reject-networks-with-qos-policy-2746c74fd1f3ff26.yaml new file mode 100644 index 000000000000..94c7ec2266c0 --- /dev/null +++ b/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