Check Neutron port quota during validate_networks in API

Unless ports are passed into Nova it will create ports on the
requested networks as part of the network allocation in
the compute manager. However if the user exceeds their
port quota the instance will end up in an Error state, having
first been re-scheduled a number of times.

It would be much better if the quota failure was detected as
part of the network validation in the API server, so that an
error can be reported to the user and the creation failed.

A full fix would include reserving or creating the ports at
this stage, but there is no reservation mechanism in the
Neutron API, and port creation depends in some cases
on mac addresses only available on the compute manager.

Instead this change just validates the quota and adjusts the
max_count to be consistent with that quota, which doesn't
guarantee that the create will work, but does catch the
majority of cases.

Refs bug: 1172808

Change-Id: Iaaee059a6746fad68049712f94b2f8cfea6ab8dc

nova/api/openstack/compute/plugins/v3/servers.py

@@ -803,7 +803,8 @@ class ServersController(wsgi.Controller):
                             admin_password=password,
                             requested_networks=requested_networks,
                             **create_kwargs)
-        except exception.QuotaError as error:
+        except (exception.QuotaError,
+                exception.PortLimitExceeded) as error:
             raise exc.HTTPRequestEntityTooLarge(
                 explanation=error.format_message(),
                 headers={'Retry-After': 0})

nova/api/openstack/compute/servers.py

@@ -938,7 +938,8 @@ class Controller(wsgi.Controller):
                             auto_disk_config=auto_disk_config,
                             scheduler_hints=scheduler_hints,
                             legacy_bdm=legacy_bdm)
-        except exception.QuotaError as error:
+        except (exception.QuotaError,
+                exception.PortLimitExceeded) as error:
             raise exc.HTTPRequestEntityTooLarge(
                 explanation=error.format_message(),
                 headers={'Retry-After': 0})

nova/compute/api.py

@@ -444,13 +444,15 @@ class API(base.Base):
                 raise exception.SecurityGroupNotFoundForProject(
                     project_id=context.project_id, security_group_id=secgroup)
 
-    def _check_requested_networks(self, context, requested_networks):
+    def _check_requested_networks(self, context, requested_networks,
+                                  max_count):
         """
         Check if the networks requested belongs to the project
         and the fixed IP address for each network provided is within
         same the network block
         """
-        self.network_api.validate_networks(context, requested_networks)
+        return self.network_api.validate_networks(context, requested_networks,
+                                                  max_count)
 
     @staticmethod
     def _handle_kernel_and_ramdisk(context, kernel_id, ramdisk_id, image):
@@ -663,7 +665,8 @@ class API(base.Base):
                                          access_ip_v4, access_ip_v6,
                                          requested_networks, config_drive,
                                          block_device_mapping,
-                                         auto_disk_config, reservation_id):
+                                         auto_disk_config, reservation_id,
+                                         max_count):
         """Verify all the input parameters regardless of the provisioning
         strategy being performed.
         """
@@ -695,7 +698,12 @@ class API(base.Base):
                 instance_type, metadata, injected_files)
 
         self._check_requested_secgroups(context, security_groups)
-        self._check_requested_networks(context, requested_networks)
+
+        # Note:  max_count is the number of instances requested by the user,
+        # max_network_count is the maximum number of instances taking into
+        # account any network quotas
+        max_network_count = self._check_requested_networks(context,
+                                     requested_networks, max_count)
 
         kernel_id, ramdisk_id = self._handle_kernel_and_ramdisk(
                 context, kernel_id, ramdisk_id, boot_meta)
@@ -748,7 +756,9 @@ class API(base.Base):
 
         base_options.update(options_from_image)
 
-        return base_options
+        # return the validated options and maximum number of instances allowed
+        # by the network quotas
+        return base_options, max_network_count
 
     def _build_filter_properties(self, context, scheduler_hints, forced_host,
             forced_node, instance_type):
@@ -875,13 +885,27 @@ class API(base.Base):
         availability_zone, forced_host, forced_node = handle_az(context,
                                                             availability_zone)
 
-        base_options = self._validate_and_build_base_options(context,
+        base_options, max_net_count = self._validate_and_build_base_options(
+                context,
                 instance_type, boot_meta, image_href, image_id, kernel_id,
                 ramdisk_id, display_name, display_description,
                 key_name, key_data, security_groups, availability_zone,
                 forced_host, user_data, metadata, injected_files, access_ip_v4,
                 access_ip_v6, requested_networks, config_drive,
-                block_device_mapping, auto_disk_config, reservation_id)
+                block_device_mapping, auto_disk_config, reservation_id,
+                max_count)
+
+        # max_net_count is the maximum number of instances requested by the
+        # user adjusted for any network quota constraints, including
+        # considertaion of connections to each requested network
+        if max_net_count == 0:
+            raise exception.PortLimitExceeded()
+        elif max_net_count < max_count:
+            LOG.debug(_("max count reduced from %(max_count)d to "
+                        "%(max_net_count)d due to network port quota"),
+                       {'max_count': max_count,
+                        'max_net_count': max_net_count})
+            max_count = max_net_count
 
         block_device_mapping = self._check_and_transform_bdm(
             base_options, boot_meta, min_count, max_count,

nova/compute/cells_api.py

@@ -149,11 +149,12 @@ class ComputeCellsAPI(compute_api.API):
         return self.cells_rpcapi.call_compute_api_method(context, cell_name,
                 method, instance_uuid, *args, **kwargs)
 
-    def _check_requested_networks(self, context, requested_networks):
+    def _check_requested_networks(self, context, requested_networks,
+                                  max_count):
         """Override compute API's checking of this.  It'll happen in
         child cell
         """
-        return
+        return max_count
 
     def create(self, *args, **kwargs):
         """We can use the base functionality, but I left this here just

nova/network/api.py

@@ -410,15 +410,22 @@ class API(base.Base):
         return network_model.NetworkInfo.hydrate(nw_info)
 
     @wrap_check_policy
-    def validate_networks(self, context, requested_networks):
+    def validate_networks(self, context, requested_networks, num_instances):
         """validate the networks passed at the time of creating
-        the server
-        """
-        if not requested_networks:
-            return
+        the server.
 
-        return self.network_rpcapi.validate_networks(context,
-                                                     requested_networks)
+        Return the number of instances that can be successfully allocated
+        with the requested network configuration.
+        """
+        if requested_networks:
+            self.network_rpcapi.validate_networks(context,
+                                                  requested_networks)
+
+        # Neutron validation checks and returns how many of num_instances
+        # instances can be supported by the quota.  For Nova network
+        # this is part of the subsequent quota check, so we just return
+        # the requested number in this case.
+        return num_instances
 
     @wrap_check_policy
     def get_instance_uuids_by_ip_filter(self, context, filters):

nova/network/neutronv2/api.py

@@ -117,12 +117,13 @@ class API(base.Base):
         """Setup or teardown the network structures."""
 
     def _get_available_networks(self, context, project_id,
-                                net_ids=None):
+                                net_ids=None, neutron=None):
         """Return a network list available for the tenant.
         The list contains networks owned by the tenant and public networks.
         If net_ids specified, it searches networks with requested IDs only.
         """
-        neutron = neutronv2.get_client(context)
+        if not neutron:
+            neutron = neutronv2.get_client(context)
 
         if net_ids:
             # If user has specified to attach instance only to specific
@@ -521,13 +522,21 @@ class API(base.Base):
         raise exception.FixedIpNotFoundForSpecificInstance(
                 instance_uuid=instance['uuid'], ip=address)
 
-    def validate_networks(self, context, requested_networks):
-        """Validate that the tenant can use the requested networks."""
+    def validate_networks(self, context, requested_networks, num_instances):
+        """Validate that the tenant can use the requested networks.
+
+        Return the number of instances than can be successfully allocated
+        with the requested network configuration.
+        """
         LOG.debug(_('validate_networks() for %s'),
                   requested_networks)
 
+        neutron = neutronv2.get_client(context)
+        ports_needed_per_instance = 0
+
         if not requested_networks:
-            nets = self._get_available_networks(context, context.project_id)
+            nets = self._get_available_networks(context, context.project_id,
+                                                neutron=neutron)
             if len(nets) > 1:
                 # Attaching to more than one network by default doesn't
                 # make sense, as the order will be arbitrary and the guest OS
@@ -535,42 +544,64 @@ class API(base.Base):
                 msg = _("Multiple possible networks found, use a Network "
                          "ID to be more specific.")
                 raise exception.NetworkAmbiguous(msg)
-            return
+            else:
+                ports_needed_per_instance = 1
 
-        net_ids = []
+        else:
+            net_ids = []
 
-        for (net_id, _i, port_id) in requested_networks:
-            if port_id:
-                try:
-                    port = (neutronv2.get_client(context)
-                                     .show_port(port_id)
-                                     .get('port'))
-                except neutronv2.exceptions.NeutronClientException as e:
-                    if e.status_code == 404:
-                        port = None
-                    else:
-                        raise
-                if not port:
-                    raise exception.PortNotFound(port_id=port_id)
-                if port.get('device_id', None):
-                    raise exception.PortInUse(port_id=port_id)
-                net_id = port['network_id']
-            if net_id in net_ids:
-                raise exception.NetworkDuplicated(network_id=net_id)
-            net_ids.append(net_id)
+            for (net_id, _i, port_id) in requested_networks:
+                if port_id:
+                    try:
+                        port = neutron.show_port(port_id).get('port')
+                    except neutronv2.exceptions.NeutronClientException as e:
+                        if e.status_code == 404:
+                            port = None
+                        else:
+                            raise
+                    if not port:
+                        raise exception.PortNotFound(port_id=port_id)
+                    if port.get('device_id', None):
+                        raise exception.PortInUse(port_id=port_id)
+                    net_id = port['network_id']
+                else:
+                    ports_needed_per_instance += 1
 
-        # Now check to see if all requested networks exist
-        nets = self._get_available_networks(context,
-                                context.project_id, net_ids)
+                if net_id in net_ids:
+                    raise exception.NetworkDuplicated(network_id=net_id)
+                net_ids.append(net_id)
 
-        if len(nets) != len(net_ids):
-            requsted_netid_set = set(net_ids)
-            returned_netid_set = set([net['id'] for net in nets])
-            lostid_set = requsted_netid_set - returned_netid_set
-            id_str = ''
-            for _id in lostid_set:
-                id_str = id_str and id_str + ', ' + _id or _id
-            raise exception.NetworkNotFound(network_id=id_str)
+            # Now check to see if all requested networks exist
+            nets = self._get_available_networks(context,
+                                    context.project_id, net_ids,
+                                    neutron=neutron)
+
+            if len(nets) != len(net_ids):
+                requsted_netid_set = set(net_ids)
+                returned_netid_set = set([net['id'] for net in nets])
+                lostid_set = requsted_netid_set - returned_netid_set
+                id_str = ''
+                for _id in lostid_set:
+                    id_str = id_str and id_str + ', ' + _id or _id
+                raise exception.NetworkNotFound(network_id=id_str)
+
+        # Note(PhilD): Ideally Nova would create all required ports as part of
+        # network validation, but port creation requires some details
+        # from the hypervisor.  So we just check the quota and return
+        # how many of the requested number of instances can be created
+
+        ports = neutron.list_ports(tenant_id=context.project_id)['ports']
+        quotas = neutron.show_quota(tenant_id=context.project_id)['quota']
+        if quotas.get('port') == -1:
+            # Unlimited Port Quota
+            return num_instances
+        else:
+            free_ports = quotas.get('port') - len(ports)
+            ports_needed = ports_needed_per_instance * num_instances
+            if free_ports >= ports_needed:
+                return num_instances
+            else:
+                return free_ports // ports_needed_per_instance
 
     def _get_instance_uuids_by_ip(self, context, address):
         """Retrieve instance uuids associated with the given ip address.

nova/tests/api/openstack/fakes.py

@@ -241,8 +241,8 @@ def stub_out_nw_api(stubs, cls=None, private=None, publics=None):
         def get_floating_ips_by_fixed_address(*args, **kwargs):
             return publics
 
-        def validate_networks(*args, **kwargs):
-            pass
+        def validate_networks(self, context, networks, max_count):
+            return max_count
 
     if cls is None:
         cls = Fake

nova/tests/network/test_neutronv2.py

@@ -1009,19 +1009,29 @@ class TestNeutronv2(TestNeutronv2Base):
         self.moxed_client.list_networks(
             id=mox.SameElementsAs(ids)).AndReturn(
                 {'networks': self.nets2})
+        self.moxed_client.list_ports(tenant_id='my_tenantid').AndReturn(
+                    {'ports': []})
+        self.moxed_client.show_quota(
+            tenant_id='my_tenantid').AndReturn(
+                    {'quota': {'port': 50}})
         self.mox.ReplayAll()
         api = neutronapi.API()
-        api.validate_networks(self.context, requested_networks)
+        api.validate_networks(self.context, requested_networks, 1)
 
     def test_validate_networks_ex_1(self):
         requested_networks = [('my_netid1', 'test', None)]
         self.moxed_client.list_networks(
             id=mox.SameElementsAs(['my_netid1'])).AndReturn(
                 {'networks': self.nets1})
+        self.moxed_client.list_ports(tenant_id='my_tenantid').AndReturn(
+                    {'ports': []})
+        self.moxed_client.show_quota(
+            tenant_id='my_tenantid').AndReturn(
+                    {'quota': {'port': 50}})
         self.mox.ReplayAll()
         api = neutronapi.API()
         try:
-            api.validate_networks(self.context, requested_networks)
+            api.validate_networks(self.context, requested_networks, 1)
         except exception.NetworkNotFound as ex:
             self.assertIn("my_netid2", str(ex))
 
@@ -1036,7 +1046,7 @@ class TestNeutronv2(TestNeutronv2Base):
         self.mox.ReplayAll()
         api = neutronapi.API()
         try:
-            api.validate_networks(self.context, requested_networks)
+            api.validate_networks(self.context, requested_networks, 1)
         except exception.NetworkNotFound as ex:
             self.assertIn("my_netid2, my_netid3", str(ex))
 
@@ -1052,7 +1062,7 @@ class TestNeutronv2(TestNeutronv2Base):
         api = neutronapi.API()
         self.assertRaises(exception.NetworkDuplicated,
                           api.validate_networks,
-                          self.context, requested_networks)
+                          self.context, requested_networks, 1)
 
     def test_validate_networks_not_specified(self):
         requested_networks = []
@@ -1067,7 +1077,7 @@ class TestNeutronv2(TestNeutronv2Base):
         api = neutronapi.API()
         self.assertRaises(exception.NetworkAmbiguous,
                           api.validate_networks,
-                          self.context, requested_networks)
+                          self.context, requested_networks, 1)
 
     def test_validate_networks_port_not_found(self):
         # Verify that the correct exception is thrown when a non existent
@@ -1085,7 +1095,7 @@ class TestNeutronv2(TestNeutronv2Base):
         api = neutronapi.API()
         self.assertRaises(exception.PortNotFound,
                           api.validate_networks,
-                          self.context, requested_networks)
+                          self.context, requested_networks, 1)
 
     def test_validate_networks_port_show_rasies_non404(self):
         # Verify that the correct exception is thrown when a non existent
@@ -1103,7 +1113,7 @@ class TestNeutronv2(TestNeutronv2Base):
         api = neutronapi.API()
         self.assertRaises(neutronv2.exceptions.NeutronClientException,
                           api.validate_networks,
-                          self.context, requested_networks)
+                          self.context, requested_networks, 1)
 
     def test_validate_networks_port_in_use(self):
         requested_networks = [(None, None, self.port_data3[0]['id'])]
@@ -1115,7 +1125,7 @@ class TestNeutronv2(TestNeutronv2Base):
         api = neutronapi.API()
         self.assertRaises(exception.PortInUse,
                           api.validate_networks,
-                          self.context, requested_networks)
+                          self.context, requested_networks, 1)
 
     def test_validate_networks_ports_in_same_network(self):
         port_a = self.port_data3[0]
@@ -1135,7 +1145,7 @@ class TestNeutronv2(TestNeutronv2Base):
         api = neutronapi.API()
         self.assertRaises(exception.NetworkDuplicated,
                           api.validate_networks,
-                          self.context, requested_networks)
+                          self.context, requested_networks, 1)
 
     def test_validate_networks_ports_not_in_same_network(self):
         port_a = self.port_data3[0]
@@ -1153,11 +1163,112 @@ class TestNeutronv2(TestNeutronv2Base):
         search_opts = {'id': [port_a['network_id'], port_b['network_id']]}
         self.moxed_client.list_networks(
             **search_opts).AndReturn({'networks': self.nets2})
+        self.moxed_client.list_ports(tenant_id='my_tenantid').AndReturn(
+                    {'ports': []})
+        self.moxed_client.show_quota(
+            tenant_id='my_tenantid').AndReturn(
+                    {'quota': {'port': 50}})
 
         self.mox.ReplayAll()
 
         api = neutronapi.API()
-        api.validate_networks(self.context, requested_networks)
+        api.validate_networks(self.context, requested_networks, 1)
+
+    def test_validate_networks_no_quota(self):
+        # Test validation for a request for one instance needing
+        # two ports, where the quota is 2 and 2 ports are in use
+        #  => instances which can be created = 0
+        requested_networks = [('my_netid1', 'test', None),
+                              ('my_netid2', 'test2', None)]
+        ids = ['my_netid1', 'my_netid2']
+        self.moxed_client.list_networks(
+            id=mox.SameElementsAs(ids)).AndReturn(
+                {'networks': self.nets2})
+        self.moxed_client.list_ports(tenant_id='my_tenantid').AndReturn(
+                    {'ports': self.port_data2})
+        self.moxed_client.show_quota(
+            tenant_id='my_tenantid').AndReturn(
+                    {'quota': {'port': 2}})
+        self.mox.ReplayAll()
+        api = neutronapi.API()
+        max_count = api.validate_networks(self.context,
+                                          requested_networks, 1)
+        self.assertEqual(max_count, 0)
+
+    def test_validate_networks_some_quota(self):
+        # Test validation for a request for two instance needing
+        # two ports each, where the quota is 5 and 2 ports are in use
+        #  => instances which can be created = 1
+        requested_networks = [('my_netid1', 'test', None),
+                              ('my_netid2', 'test2', None)]
+        ids = ['my_netid1', 'my_netid2']
+        self.moxed_client.list_networks(
+            id=mox.SameElementsAs(ids)).AndReturn(
+                {'networks': self.nets2})
+        self.moxed_client.list_ports(tenant_id='my_tenantid').AndReturn(
+                    {'ports': self.port_data2})
+        self.moxed_client.show_quota(
+            tenant_id='my_tenantid').AndReturn(
+                    {'quota': {'port': 5}})
+        self.mox.ReplayAll()
+        api = neutronapi.API()
+        max_count = api.validate_networks(self.context,
+                                          requested_networks, 2)
+        self.assertEqual(max_count, 1)
+
+    def test_validate_networks_unlimited_quota(self):
+        # Test validation for a request for two instance needing
+        # two ports each, where the quota is -1 (unlimited)
+        #  => instances which can be created = 1
+        requested_networks = [('my_netid1', 'test', None),
+                              ('my_netid2', 'test2', None)]
+        ids = ['my_netid1', 'my_netid2']
+        self.moxed_client.list_networks(
+            id=mox.SameElementsAs(ids)).AndReturn(
+                {'networks': self.nets2})
+        self.moxed_client.list_ports(tenant_id='my_tenantid').AndReturn(
+                    {'ports': self.port_data2})
+        self.moxed_client.show_quota(
+            tenant_id='my_tenantid').AndReturn(
+                    {'quota': {'port': -1}})
+        self.mox.ReplayAll()
+        api = neutronapi.API()
+        max_count = api.validate_networks(self.context,
+                                          requested_networks, 2)
+        self.assertEqual(max_count, 2)
+
+    def test_validate_networks_no_quota_but_ports_supplied(self):
+        # Test validation for a request for one instance needing
+        # two ports, where the quota is 2 and 2 ports are in use
+        # but the request includes a port to be used
+        #  => instances which can be created = 1
+        port_a = self.port_data3[0]
+        port_b = self.port_data2[1]
+        self.assertNotEqual(port_a['network_id'], port_b['network_id'])
+        for port in [port_a, port_b]:
+            port['device_id'] = None
+            port['device_owner'] = None
+
+        requested_networks = [(None, None, port_a['id']),
+                              (None, None, port_b['id'])]
+        self.moxed_client.show_port(port_a['id']).AndReturn({'port': port_a})
+        self.moxed_client.show_port(port_b['id']).AndReturn({'port': port_b})
+
+        search_opts = {'id': [port_a['network_id'], port_b['network_id']]}
+        self.moxed_client.list_networks(
+            **search_opts).AndReturn({'networks': self.nets2})
+        self.moxed_client.list_ports(tenant_id='my_tenantid').AndReturn(
+                    {'ports': self.port_data2})
+        self.moxed_client.show_quota(
+            tenant_id='my_tenantid').AndReturn(
+                    {'quota': {'port': 2}})
+
+        self.mox.ReplayAll()
+
+        api = neutronapi.API()
+        max_count = api.validate_networks(self.context,
+                                          requested_networks, 1)
+        self.assertEqual(max_count, 1)
 
     def _mock_list_ports(self, port_data=None):
         if port_data is None: