From 38a080aca37197ff9c5464a3bc9981e684decee6 Mon Sep 17 00:00:00 2001 From: Ryan Tidwell Date: Fri, 8 Apr 2016 16:16:25 -0700 Subject: [PATCH] Remove IP availability range recalculation logic This patch removes unreachable code that rebuilds IP address availability data and persists in the database. This is ultimately derived data that is now computed in memory and never persisted. The code being removed is dead code and does not include the contract migration for removal of the IPAvailabilityRange models. Change-Id: I96cb67396b8e0ebbe7f98353fad1607405944e44 Partial-Bug: #1543094 --- neutron/db/ipam_backend_mixin.py | 11 +-- neutron/db/ipam_non_pluggable_backend.py | 96 ------------------- neutron/ipam/drivers/neutrondb_ipam/driver.py | 53 ---------- .../tests/unit/db/test_db_base_plugin_v2.py | 7 +- .../db/test_ipam_non_pluggable_backend.py | 84 ---------------- 5 files changed, 2 insertions(+), 249 deletions(-) diff --git a/neutron/db/ipam_backend_mixin.py b/neutron/db/ipam_backend_mixin.py index 6092df932a5..fc6c1a7c513 100644 --- a/neutron/db/ipam_backend_mixin.py +++ b/neutron/db/ipam_backend_mixin.py @@ -51,12 +51,6 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon): # Tracks changes in ip allocation for port using namedtuple Changes = collections.namedtuple('Changes', 'add original remove') - @staticmethod - def _rebuild_availability_ranges(context, subnets): - """Should be redefined for non-ipam backend only - """ - pass - @staticmethod def _gateway_ip_str(subnet, cidr_net): if subnet.get('gateway_ip') is const.ATTR_NOT_SPECIFIED: @@ -176,10 +170,7 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon): subnet_id=subnet_id) for p in pools] context.session.add_all(new_pools) - # Call static method with self to redefine in child - # (non-pluggable backend) - if not ipv6_utils.is_ipv6_pd_enabled(s): - self._rebuild_availability_ranges(context, [s]) + # Gather new pools for result result_pools = [{'start': p[0], 'end': p[1]} for p in pools] del s['allocation_pools'] diff --git a/neutron/db/ipam_non_pluggable_backend.py b/neutron/db/ipam_non_pluggable_backend.py index dd0f9d4916a..ae4467e98cb 100644 --- a/neutron/db/ipam_non_pluggable_backend.py +++ b/neutron/db/ipam_non_pluggable_backend.py @@ -23,7 +23,6 @@ from neutron_lib import exceptions as n_exc from oslo_db import exception as db_exc from oslo_log import log as logging from sqlalchemy import and_ -from sqlalchemy import orm from sqlalchemy.orm import exc from neutron._i18n import _ @@ -97,97 +96,6 @@ class IpamNonPluggableBackend(ipam_backend_mixin.IpamBackendMixin): raise n_exc.IpAddressGenerationFailure( net_id=subnets[0]['network_id']) - @staticmethod - def _rebuild_availability_ranges(context, subnets): - """Rebuild availability ranges. - - This method is called only when there's no more IP available or by - _update_subnet_allocation_pools. Calling - _update_subnet_allocation_pools before calling this function deletes - the IPAllocationPools associated with the subnet that is updating, - which will result in deleting the IPAvailabilityRange too. - """ - ip_qry = context.session.query( - models_v2.IPAllocation).with_lockmode('update') - # PostgreSQL does not support select...for update with an outer join. - # No join is needed here. - pool_qry = context.session.query( - models_v2.IPAllocationPool).options( - orm.noload('available_ranges')).with_lockmode('update') - for subnet in sorted(subnets): - LOG.debug("Rebuilding availability ranges for subnet %s", - subnet) - - # Create a set of all currently allocated addresses - ip_qry_results = ip_qry.filter_by(subnet_id=subnet['id']) - allocations = netaddr.IPSet([netaddr.IPAddress(i['ip_address']) - for i in ip_qry_results]) - - for pool in pool_qry.filter_by(subnet_id=subnet['id']): - # Create a set of all addresses in the pool - poolset = netaddr.IPSet(netaddr.IPRange(pool['first_ip'], - pool['last_ip'])) - - # Use set difference to find free addresses in the pool - available = poolset - allocations - - # Generator compacts an ip set into contiguous ranges - def ipset_to_ranges(ipset): - first, last = None, None - for cidr in ipset.iter_cidrs(): - if last and last + 1 != cidr.first: - yield netaddr.IPRange(first, last) - first = None - first, last = first if first else cidr.first, cidr.last - if first: - yield netaddr.IPRange(first, last) - - # Write the ranges to the db - for ip_range in ipset_to_ranges(available): - available_range = models_v2.IPAvailabilityRange( - allocation_pool_id=pool['id'], - first_ip=str(netaddr.IPAddress(ip_range.first)), - last_ip=str(netaddr.IPAddress(ip_range.last))) - context.session.add(available_range) - - @staticmethod - def _allocate_specific_ip(context, subnet_id, ip_address): - """Allocate a specific IP address on the subnet.""" - ip = int(netaddr.IPAddress(ip_address)) - range_qry = context.session.query( - models_v2.IPAvailabilityRange).join( - models_v2.IPAllocationPool).with_lockmode('update') - results = range_qry.filter_by(subnet_id=subnet_id) - for ip_range in results: - first = int(netaddr.IPAddress(ip_range['first_ip'])) - last = int(netaddr.IPAddress(ip_range['last_ip'])) - if first <= ip <= last: - if first == last: - context.session.delete(ip_range) - return - elif first == ip: - new_first_ip = str(netaddr.IPAddress(ip_address) + 1) - ip_range['first_ip'] = new_first_ip - return - elif last == ip: - new_last_ip = str(netaddr.IPAddress(ip_address) - 1) - ip_range['last_ip'] = new_last_ip - return - else: - # Adjust the original range to end before ip_address - old_last_ip = ip_range['last_ip'] - new_last_ip = str(netaddr.IPAddress(ip_address) - 1) - ip_range['last_ip'] = new_last_ip - - # Create a new second range for after ip_address - new_first_ip = str(netaddr.IPAddress(ip_address) + 1) - new_ip_range = models_v2.IPAvailabilityRange( - allocation_pool_id=ip_range['allocation_pool_id'], - first_ip=new_first_ip, - last_ip=old_last_ip) - context.session.add(new_ip_range) - return - @staticmethod def _check_unique_ip(context, network_id, subnet_id, ip_address): """Validate that the IP address on the subnet is not in use.""" @@ -311,10 +219,6 @@ class IpamNonPluggableBackend(ipam_backend_mixin.IpamBackendMixin): subnet = self._get_subnet(context, fixed['subnet_id']) is_auto_addr = ipv6_utils.is_auto_address_subnet(subnet) if 'ip_address' in fixed: - if not is_auto_addr: - # Remove the IP address from the allocation pool - IpamNonPluggableBackend._allocate_specific_ip( - context, fixed['subnet_id'], fixed['ip_address']) allocated_ips.append(fixed['ip_address']) ips.append({'ip_address': fixed['ip_address'], 'subnet_id': fixed['subnet_id']}) diff --git a/neutron/ipam/drivers/neutrondb_ipam/driver.py b/neutron/ipam/drivers/neutrondb_ipam/driver.py index 43cd54f0f3c..9dea0a85628 100644 --- a/neutron/ipam/drivers/neutrondb_ipam/driver.py +++ b/neutron/ipam/drivers/neutrondb_ipam/driver.py @@ -154,59 +154,6 @@ class NeutronDbSubnet(ipam_base.Subnet): subnet_id=self.subnet_manager.neutron_id, ip=ip_address) - def _rebuild_availability_ranges(self, session): - """Rebuild availability ranges. - - This method should be called only when the availability ranges are - exhausted or when the subnet's allocation pools are updated, - which may trigger a deletion of the availability ranges. - - For this operation to complete successfully, this method uses a - locking query to ensure that no IP is allocated while the regeneration - of availability ranges is in progress. - - :param session: database session - """ - # List all currently allocated addresses, and prevent further - # allocations with a write-intent lock. - # NOTE: because of this driver's logic the write intent lock is - # probably unnecessary as this routine is called when the availability - # ranges for a subnet are exhausted and no further address can be - # allocated. - # TODO(salv-orlando): devise, if possible, a more efficient solution - # for building the IPSet to ensure decent performances even with very - # large subnets. - allocations = netaddr.IPSet( - [netaddr.IPAddress(allocation['ip_address']) for - allocation in self.subnet_manager.list_allocations( - session)]) - - # MEH MEH - # There should be no need to set a write intent lock on the allocation - # pool table. Indeed it is not important for the correctness of this - # operation if the allocation pools are updated by another operation, - # which will result in the generation of new availability ranges. - # NOTE: it might be argued that an allocation pool update should in - # theory preempt rebuilding the availability range. This is an option - # to consider for future developments. - LOG.debug("Rebuilding availability ranges for subnet %s", - self.subnet_manager.neutron_id) - - for pool in self.subnet_manager.list_pools(session): - # Create a set of all addresses in the pool - poolset = netaddr.IPSet(netaddr.IPRange(pool['first_ip'], - pool['last_ip'])) - # Use set difference to find free addresses in the pool - available = poolset - allocations - # Write the ranges to the db - for ip_range in available.iter_ipranges(): - av_range = self.subnet_manager.create_range( - session, - pool['id'], - netaddr.IPAddress(ip_range.first).format(), - netaddr.IPAddress(ip_range.last).format()) - session.add(av_range) - def _generate_ip(self, session, prefer_next=False): """Generate an IP address from the set of available addresses.""" ip_allocations = netaddr.IPSet() diff --git a/neutron/tests/unit/db/test_db_base_plugin_v2.py b/neutron/tests/unit/db/test_db_base_plugin_v2.py index 8d6e1c04046..6746ae31c3d 100644 --- a/neutron/tests/unit/db/test_db_base_plugin_v2.py +++ b/neutron/tests/unit/db/test_db_base_plugin_v2.py @@ -49,7 +49,6 @@ from neutron import context from neutron.db import api as db_api from neutron.db import db_base_plugin_common from neutron.db import ipam_backend_mixin -from neutron.db import ipam_non_pluggable_backend as non_ipam from neutron.db import l3_db from neutron.db import models_v2 from neutron.db import securitygroups_db as sgdb @@ -1927,10 +1926,7 @@ fixed_ips=ip_address%%3D%s&fixed_ips=ip_address%%3D%s&fixed_ips=subnet_id%%3D%s self.assertEqual(webob.exc.HTTPClientError.code, res.status_int) - @mock.patch.object(non_ipam.IpamNonPluggableBackend, - '_allocate_specific_ip') - def test_requested_fixed_ip_address_v6_slaac_router_iface( - self, alloc_specific_ip): + def test_requested_fixed_ip_address_v6_slaac_router_iface(self): with self.subnet(gateway_ip='fe80::1', cidr='fe80::/64', ip_version=6, @@ -1945,7 +1941,6 @@ fixed_ips=ip_address%%3D%s&fixed_ips=ip_address%%3D%s&fixed_ips=subnet_id%%3D%s self.assertEqual(len(port['port']['fixed_ips']), 1) self.assertEqual(port['port']['fixed_ips'][0]['ip_address'], 'fe80::1') - self.assertFalse(alloc_specific_ip.called) def test_requested_subnet_id_v6_slaac(self): with self.subnet(gateway_ip='fe80::1', diff --git a/neutron/tests/unit/db/test_ipam_non_pluggable_backend.py b/neutron/tests/unit/db/test_ipam_non_pluggable_backend.py index d16784ad158..9defbdd1ab5 100644 --- a/neutron/tests/unit/db/test_ipam_non_pluggable_backend.py +++ b/neutron/tests/unit/db/test_ipam_non_pluggable_backend.py @@ -22,96 +22,12 @@ from neutron.common import ipv6_utils from neutron.db import db_base_plugin_v2 from neutron.db import ipam_backend_mixin from neutron.db import ipam_non_pluggable_backend as non_ipam -from neutron.db import models_v2 from neutron.tests import base class TestIpamNonPluggableBackend(base.BaseTestCase): """Unit Tests for non pluggable IPAM Logic.""" - def _validate_rebuild_availability_ranges(self, pools, allocations, - expected): - ip_qry = mock.Mock() - ip_qry.with_lockmode.return_value = ip_qry - ip_qry.filter_by.return_value = allocations - - pool_qry = mock.Mock() - pool_qry.options.return_value = pool_qry - pool_qry.with_lockmode.return_value = pool_qry - pool_qry.filter_by.return_value = pools - - def return_queries_side_effect(*args, **kwargs): - if args[0] == models_v2.IPAllocation: - return ip_qry - if args[0] == models_v2.IPAllocationPool: - return pool_qry - - context = mock.Mock() - context.session.query.side_effect = return_queries_side_effect - subnets = [mock.MagicMock()] - - non_ipam.IpamNonPluggableBackend._rebuild_availability_ranges( - context, subnets) - - actual = [[args[0].allocation_pool_id, - args[0].first_ip, args[0].last_ip] - for _name, args, _kwargs in context.session.add.mock_calls] - self.assertEqual(expected, actual) - - def test_rebuild_availability_ranges(self): - pools = [{'id': 'a', - 'first_ip': '192.168.1.3', - 'last_ip': '192.168.1.10'}, - {'id': 'b', - 'first_ip': '192.168.1.100', - 'last_ip': '192.168.1.120'}] - - allocations = [{'ip_address': '192.168.1.3'}, - {'ip_address': '192.168.1.78'}, - {'ip_address': '192.168.1.7'}, - {'ip_address': '192.168.1.110'}, - {'ip_address': '192.168.1.11'}, - {'ip_address': '192.168.1.4'}, - {'ip_address': '192.168.1.111'}] - - expected = [['a', '192.168.1.5', '192.168.1.6'], - ['a', '192.168.1.8', '192.168.1.10'], - ['b', '192.168.1.100', '192.168.1.109'], - ['b', '192.168.1.112', '192.168.1.120']] - - self._validate_rebuild_availability_ranges(pools, allocations, - expected) - - def test_rebuild_ipv6_availability_ranges(self): - pools = [{'id': 'a', - 'first_ip': '2001::1', - 'last_ip': '2001::50'}, - {'id': 'b', - 'first_ip': '2001::100', - 'last_ip': '2001::ffff:ffff:ffff:fffe'}] - - allocations = [{'ip_address': '2001::10'}, - {'ip_address': '2001::45'}, - {'ip_address': '2001::60'}, - {'ip_address': '2001::111'}, - {'ip_address': '2001::200'}, - {'ip_address': '2001::ffff:ffff:ffff:ff10'}, - {'ip_address': '2001::ffff:ffff:ffff:f2f0'}] - - expected = [['a', '2001::1', '2001::f'], - ['a', '2001::11', '2001::44'], - ['a', '2001::46', '2001::50'], - ['b', '2001::100', '2001::110'], - ['b', '2001::112', '2001::1ff'], - ['b', '2001::201', '2001::ffff:ffff:ffff:f2ef'], - ['b', '2001::ffff:ffff:ffff:f2f1', - '2001::ffff:ffff:ffff:ff0f'], - ['b', '2001::ffff:ffff:ffff:ff11', - '2001::ffff:ffff:ffff:fffe']] - - self._validate_rebuild_availability_ranges(pools, allocations, - expected) - def _test__allocate_ips_for_port(self, subnets, port, expected): # this test is incompatible with pluggable ipam, because subnets # were not actually created, so no ipam_subnet exists