Adds quotas for fixed IPs on ports
RM11643 Implements quota checking the number of fixed IP addresses allowed on a port on create and update port, as well as create and update IP Address.
This commit is contained in:
		| @@ -54,7 +54,9 @@ quark_resources = [ | |||||||
|     quota.BaseResource('v4_subnets_per_network', |     quota.BaseResource('v4_subnets_per_network', | ||||||
|                        'quota_v4_subnets_per_network'), |                        'quota_v4_subnets_per_network'), | ||||||
|     quota.BaseResource('v6_subnets_per_network', |     quota.BaseResource('v6_subnets_per_network', | ||||||
|                        'quota_v6_subnets_per_network') |                        'quota_v6_subnets_per_network'), | ||||||
|  |     quota.BaseResource('fixed_ips_per_port', | ||||||
|  |                        'quota_fixed_ips_per_port') | ||||||
| ] | ] | ||||||
|  |  | ||||||
| quark_quota_opts = [ | quark_quota_opts = [ | ||||||
| @@ -81,7 +83,10 @@ quark_quota_opts = [ | |||||||
|                help=_('Maximum v4 subnets per network')), |                help=_('Maximum v4 subnets per network')), | ||||||
|     cfg.IntOpt('quota_v6_subnets_per_network', |     cfg.IntOpt('quota_v6_subnets_per_network', | ||||||
|                default=1, |                default=1, | ||||||
|                help=_('Maximum v6 subnets per network')) |                help=_('Maximum v6 subnets per network')), | ||||||
|  |     cfg.IntOpt('quota_fixed_ips_per_port', | ||||||
|  |                default=5, | ||||||
|  |                help=_('Maximum number of fixed IPs per port')) | ||||||
| ] | ] | ||||||
|  |  | ||||||
|  |  | ||||||
|   | |||||||
| @@ -14,6 +14,7 @@ | |||||||
| #    under the License. | #    under the License. | ||||||
|  |  | ||||||
| from neutron.common import exceptions | from neutron.common import exceptions | ||||||
|  | from neutron import quota | ||||||
| from oslo.config import cfg | from oslo.config import cfg | ||||||
| from oslo_log import log as logging | from oslo_log import log as logging | ||||||
| import webob | import webob | ||||||
| @@ -65,6 +66,13 @@ def validate_ports_on_network_and_same_segment(ports, network_id): | |||||||
|                                             msg="Segment id's do not match.") |                                             msg="Segment id's do not match.") | ||||||
|  |  | ||||||
|  |  | ||||||
|  | def validate_port_ip_quotas(context, ports): | ||||||
|  |     for port in ports: | ||||||
|  |         addresses = port.get("ip_addresses", []) | ||||||
|  |         quota.QUOTAS.limit_check(context, context.tenant_id, | ||||||
|  |                                  fixed_ips_per_port=len(addresses) + 1) | ||||||
|  |  | ||||||
|  |  | ||||||
| def _shared_ip_request(ip_address): | def _shared_ip_request(ip_address): | ||||||
|     port_ids = ip_address.get('ip_address', {}).get('port_ids', []) |     port_ids = ip_address.get('ip_address', {}).get('port_ids', []) | ||||||
|     return len(port_ids) > 1 |     return len(port_ids) > 1 | ||||||
| @@ -117,6 +125,7 @@ def create_ip_address(context, body): | |||||||
|                                           net_id=network_id) |                                           net_id=network_id) | ||||||
|  |  | ||||||
|     validate_ports_on_network_and_same_segment(ports, network_id) |     validate_ports_on_network_and_same_segment(ports, network_id) | ||||||
|  |     validate_port_ip_quotas(context, ports) | ||||||
|  |  | ||||||
|     # Shared Ips are only new IPs. Two use cases: if we got device_id |     # Shared Ips are only new IPs. Two use cases: if we got device_id | ||||||
|     # or if we got port_ids. We should check the case where we got port_ids |     # or if we got port_ids. We should check the case where we got port_ids | ||||||
| @@ -185,6 +194,7 @@ def update_ip_address(context, id, ip_address): | |||||||
|  |  | ||||||
|             validate_ports_on_network_and_same_segment(ports, |             validate_ports_on_network_and_same_segment(ports, | ||||||
|                                                        address["network_id"]) |                                                        address["network_id"]) | ||||||
|  |             validate_port_ip_quotas(context, ports) | ||||||
|  |  | ||||||
|             LOG.info("Updating IP address, %s, to only be used by the" |             LOG.info("Updating IP address, %s, to only be used by the" | ||||||
|                      "following ports:  %s" % (address.address_readable, |                      "following ports:  %s" % (address.address_readable, | ||||||
|   | |||||||
| @@ -91,6 +91,11 @@ def create_port(context, port): | |||||||
|                 resource="port", msg="This device is already connected to the " |                 resource="port", msg="This device is already connected to the " | ||||||
|                 "requested network via another port") |                 "requested network via another port") | ||||||
|  |  | ||||||
|  |     # Try to fail early on quotas and save ourselves some db overhead | ||||||
|  |     if fixed_ips: | ||||||
|  |         quota.QUOTAS.limit_check(context, context.tenant_id, | ||||||
|  |                                  fixed_ips_per_port=len(fixed_ips)) | ||||||
|  |  | ||||||
|     if not STRATEGY.is_parent_network(net_id): |     if not STRATEGY.is_parent_network(net_id): | ||||||
|         # We don't honor segmented networks when they aren't "shared" |         # We don't honor segmented networks when they aren't "shared" | ||||||
|         segment_id = None |         segment_id = None | ||||||
| @@ -261,6 +266,13 @@ def update_port(context, id, port): | |||||||
|     utils.filter_body(context, port_dict, admin_only=admin_only, |     utils.filter_body(context, port_dict, admin_only=admin_only, | ||||||
|                       always_filter=always_filter) |                       always_filter=always_filter) | ||||||
|  |  | ||||||
|  |     # Pre-check the requested fixed_ips before making too many db trips. | ||||||
|  |     # Note that this is the only check we need, since this call replaces | ||||||
|  |     # the entirety of the IP addresses document if fixed_ips are provided. | ||||||
|  |     if fixed_ips: | ||||||
|  |         quota.QUOTAS.limit_check(context, context.tenant_id, | ||||||
|  |                                  fixed_ips_per_port=len(fixed_ips)) | ||||||
|  |  | ||||||
|     # TODO(anyone): security groups are not currently supported on port create, |     # TODO(anyone): security groups are not currently supported on port create, | ||||||
|     #               nor on isolated networks today. Please see RM8615 |     #               nor on isolated networks today. Please see RM8615 | ||||||
|     new_security_groups = utils.pop_param(port_dict, "security_groups") |     new_security_groups = utils.pop_param(port_dict, "security_groups") | ||||||
|   | |||||||
| @@ -146,6 +146,45 @@ class TestIpAddresses(test_quark_plugin.TestQuarkPlugin): | |||||||
|                 self.plugin.create_ip_address(self.context, ip_address) |                 self.plugin.create_ip_address(self.context, ip_address) | ||||||
|  |  | ||||||
|  |  | ||||||
|  | class TestCreateIpAddressQuotaCheck(test_quark_plugin.TestQuarkPlugin): | ||||||
|  |     @contextlib.contextmanager | ||||||
|  |     def _stubs(self, port, addresses): | ||||||
|  |         port_model = models.Port() | ||||||
|  |         port_model.update(port) | ||||||
|  |  | ||||||
|  |         for addr in addresses: | ||||||
|  |             addr_model = models.IPAddress() | ||||||
|  |             addr_model.update(addr) | ||||||
|  |             port_model["ip_addresses"].append(addr_model) | ||||||
|  |  | ||||||
|  |         with contextlib.nested( | ||||||
|  |             mock.patch("quark.db.api.port_find"), | ||||||
|  |             mock.patch("quark.plugin_modules.ip_addresses.ipam_driver"), | ||||||
|  |             mock.patch("quark.plugin_modules.ip_addresses.db_api" | ||||||
|  |                        ".port_associate_ip"), | ||||||
|  |             mock.patch("quark.plugin_modules.ip_addresses" | ||||||
|  |                        ".validate_ports_on_network_and_same_segment") | ||||||
|  |         ) as (port_find, mock_ipam, mock_port_associate_ip, validate): | ||||||
|  |             port_find.return_value = port_model | ||||||
|  |             yield | ||||||
|  |  | ||||||
|  |     def test_create_ip_address_with_port_over_quota(self): | ||||||
|  |         addresses = [{"id": ip, "address": ip} for ip in xrange(5)] | ||||||
|  |         port = dict(id=1, network_id=2, ip_addresses=[]) | ||||||
|  |  | ||||||
|  |         ip = dict(id=1, address=3232235876, address_readable="192.168.1.100", | ||||||
|  |                   subnet_id=1, network_id=2, version=4) | ||||||
|  |  | ||||||
|  |         with self._stubs(port=port, addresses=addresses): | ||||||
|  |             ip_address = dict(port_ids=[port["id"]]) | ||||||
|  |             ip_address['version'] = 4 | ||||||
|  |             ip_address['network_id'] = 2 | ||||||
|  |  | ||||||
|  |             with self.assertRaises(exceptions.OverQuota): | ||||||
|  |                 self.plugin.create_ip_address( | ||||||
|  |                     self.context, dict(ip_address=ip_address)) | ||||||
|  |  | ||||||
|  |  | ||||||
| @mock.patch("quark.plugin_modules.ip_addresses.v") | @mock.patch("quark.plugin_modules.ip_addresses.v") | ||||||
| @mock.patch("quark.plugin_modules.ip_addresses" | @mock.patch("quark.plugin_modules.ip_addresses" | ||||||
|             ".validate_ports_on_network_and_same_segment") |             ".validate_ports_on_network_and_same_segment") | ||||||
| @@ -514,6 +553,55 @@ class TestQuarkUpdateIPAddress(test_quark_plugin.TestQuarkPlugin): | |||||||
|             self.assertEqual(response['port_ids'], []) |             self.assertEqual(response['port_ids'], []) | ||||||
|  |  | ||||||
|  |  | ||||||
|  | class TestQuarkUpdateIPAddressQuotaCheck(test_quark_plugin.TestQuarkPlugin): | ||||||
|  |     @contextlib.contextmanager | ||||||
|  |     def _stubs(self, port, addresses): | ||||||
|  |         port_models = [] | ||||||
|  |         addr_model = None | ||||||
|  |  | ||||||
|  |         port_model = models.Port() | ||||||
|  |         port_model.update(port) | ||||||
|  |         port_models.append(port_model) | ||||||
|  |  | ||||||
|  |         for addr in addresses: | ||||||
|  |             addr_model = models.IPAddress() | ||||||
|  |             addr_model.update(addr) | ||||||
|  |             port_model["ip_addresses"].append(addr_model) | ||||||
|  |  | ||||||
|  |         db_mod = "quark.db.api" | ||||||
|  |         with contextlib.nested( | ||||||
|  |             mock.patch("%s.port_find" % db_mod), | ||||||
|  |             mock.patch("%s.ip_address_find" % db_mod), | ||||||
|  |             mock.patch("%s.port_associate_ip" % db_mod), | ||||||
|  |             mock.patch("%s.port_disassociate_ip" % db_mod), | ||||||
|  |             mock.patch("quark.plugin_modules.ip_addresses" | ||||||
|  |                        ".validate_ports_on_network_and_same_segment"), | ||||||
|  |             mock.patch("quark.plugin_modules.ip_addresses.ipam_driver") | ||||||
|  |         ) as (port_find, ip_find, port_associate_ip, port_disassociate_ip, val, | ||||||
|  |               mock_ipam): | ||||||
|  |             port_find.return_value = port_models | ||||||
|  |             ip_find.return_value = addr_model | ||||||
|  |             port_associate_ip.side_effect = _port_associate_stub | ||||||
|  |             port_disassociate_ip.side_effect = _port_disassociate_stub | ||||||
|  |             mock_ipam.deallocate_ip_address.side_effect = ( | ||||||
|  |                 _ip_deallocate_stub) | ||||||
|  |             yield | ||||||
|  |  | ||||||
|  |     def test_update_ip_address_port_over_quota(self): | ||||||
|  |         addresses = [{"id": ip, "address": ip} for ip in xrange(5)] | ||||||
|  |  | ||||||
|  |         port = dict(id=1, network_id=2) | ||||||
|  |         ip = dict(id=1, address=3232235876, address_readable="192.168.1.100", | ||||||
|  |                   subnet_id=1, network_id=2, version=4, deallocated=1, | ||||||
|  |                   deallocated_at='2020-01-01 00:00:00') | ||||||
|  |  | ||||||
|  |         with self._stubs(port=port, addresses=addresses): | ||||||
|  |             ip_address = {'ip_address': {"port_ids": [port]}} | ||||||
|  |             with self.assertRaises(exceptions.OverQuota): | ||||||
|  |                 self.plugin.update_ip_address(self.admin_context, ip['id'], | ||||||
|  |                                               ip_address) | ||||||
|  |  | ||||||
|  |  | ||||||
| class TestQuarkGetIpAddress(test_quark_plugin.TestQuarkPlugin): | class TestQuarkGetIpAddress(test_quark_plugin.TestQuarkPlugin): | ||||||
|     @contextlib.contextmanager |     @contextlib.contextmanager | ||||||
|     def _stubs(self, ips, ports): |     def _stubs(self, ips, ports): | ||||||
|   | |||||||
| @@ -712,6 +712,26 @@ class TestQuarkPortCreateQuota(test_quark_plugin.TestQuarkPlugin): | |||||||
|                 self.plugin.create_port(self.context, port) |                 self.plugin.create_port(self.context, port) | ||||||
|  |  | ||||||
|  |  | ||||||
|  | class TestQuarkPortCreateFixedIpsQuota(test_quark_plugin.TestQuarkPlugin): | ||||||
|  |     @contextlib.contextmanager | ||||||
|  |     def _stubs(self, network): | ||||||
|  |         network["network_plugin"] = "BASE" | ||||||
|  |         network["ipam_strategy"] = "ANY" | ||||||
|  |         with mock.patch("quark.db.api.network_find") as net_find: | ||||||
|  |             net_find.return_value = network | ||||||
|  |             yield | ||||||
|  |  | ||||||
|  |     def test_create_port_fixed_ips_over_quota(self): | ||||||
|  |         network = {"id": 1, "tenant_id": self.context.tenant_id} | ||||||
|  |         fixed_ips = [{"subnet_id": 1}, {"subnet_id": 1}, {"subnet_id": 1}, | ||||||
|  |                      {"subnet_id": 1}, {"subnet_id": 1}, {"subnet_id": 1}] | ||||||
|  |         port = {"port": {"network_id": 1, "tenant_id": self.context.tenant_id, | ||||||
|  |                          "device_id": 2, "fixed_ips": fixed_ips}} | ||||||
|  |         with self._stubs(network=network): | ||||||
|  |             with self.assertRaises(exceptions.OverQuota): | ||||||
|  |                 self.plugin.create_port(self.context, port) | ||||||
|  |  | ||||||
|  |  | ||||||
| class TestQuarkUpdatePort(test_quark_plugin.TestQuarkPlugin): | class TestQuarkUpdatePort(test_quark_plugin.TestQuarkPlugin): | ||||||
|     @contextlib.contextmanager |     @contextlib.contextmanager | ||||||
|     def _stubs(self, port, new_ips=None, parent_net=False): |     def _stubs(self, port, new_ips=None, parent_net=False): | ||||||
| @@ -728,8 +748,7 @@ class TestQuarkUpdatePort(test_quark_plugin.TestQuarkPlugin): | |||||||
|             mock.patch("quark.db.api.port_update"), |             mock.patch("quark.db.api.port_update"), | ||||||
|             mock.patch("quark.ipam.QuarkIpam.allocate_ip_address"), |             mock.patch("quark.ipam.QuarkIpam.allocate_ip_address"), | ||||||
|             mock.patch("quark.ipam.QuarkIpam.deallocate_ips_by_port"), |             mock.patch("quark.ipam.QuarkIpam.deallocate_ips_by_port"), | ||||||
|             mock.patch("neutron.quota.QuotaEngine.limit_check"), |         ) as (port_find, port_update, alloc_ip, dealloc_ip): | ||||||
|         ) as (port_find, port_update, alloc_ip, dealloc_ip, limit_check): |  | ||||||
|             port_find.return_value = port_model |             port_find.return_value = port_model | ||||||
|             port_update.return_value = port_model |             port_update.return_value = port_model | ||||||
|             if new_ips: |             if new_ips: | ||||||
| @@ -854,6 +873,20 @@ class TestQuarkUpdatePort(test_quark_plugin.TestQuarkPlugin): | |||||||
|             self.assertEqual(port_res["fixed_ips"][1]["ip_address"], |             self.assertEqual(port_res["fixed_ips"][1]["ip_address"], | ||||||
|                              str(ip2.ipv6())) |                              str(ip2.ipv6())) | ||||||
|  |  | ||||||
|  |     def test_update_port_goes_over_quota(self): | ||||||
|  |         fixed_ips = {"fixed_ips": [{"subnet_id": 1}, | ||||||
|  |                                    {"subnet_id": 1}, | ||||||
|  |                                    {"subnet_id": 1}, | ||||||
|  |                                    {"subnet_id": 1}, | ||||||
|  |                                    {"subnet_id": 1}, | ||||||
|  |                                    {"subnet_id": 1}]} | ||||||
|  |         with self._stubs( | ||||||
|  |             port=dict(id=1, name="myport", mac_address="0:0:0:0:0:1") | ||||||
|  |         ) as (port_find, port_update, alloc_ip, dealloc_ip): | ||||||
|  |             new_port = {"port": fixed_ips} | ||||||
|  |             with self.assertRaises(exceptions.OverQuota): | ||||||
|  |                 self.plugin.update_port(self.context, 1, new_port) | ||||||
|  |  | ||||||
|  |  | ||||||
| class TestQuarkUpdatePortSecurityGroups(test_quark_plugin.TestQuarkPlugin): | class TestQuarkUpdatePortSecurityGroups(test_quark_plugin.TestQuarkPlugin): | ||||||
|     @contextlib.contextmanager |     @contextlib.contextmanager | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Matt Dietz
					Matt Dietz