diff --git a/neutron/db/ipam_backend_mixin.py b/neutron/db/ipam_backend_mixin.py index a8d7222850e..39dc546e747 100644 --- a/neutron/db/ipam_backend_mixin.py +++ b/neutron/db/ipam_backend_mixin.py @@ -426,44 +426,69 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon): # the new_ips contain all of the fixed_ips that are to be updated self._validate_max_ips_per_port(ips, device_owner) - add_ips = [] - remove_ips = [] + add_ips, prev_ips, remove_candidates = [], [], [] - ips_map = {ip['ip_address']: ip - for ip in itertools.chain(new_ips, original_ips) - if 'ip_address' in ip} + # Consider fixed_ips that specify a specific address first to see if + # they already existed in original_ips or are completely new. + orig_by_ip = {ip['ip_address']: ip for ip in original_ips} + for ip in ips: + if 'ip_address' not in ip: + continue - new = set() - for ip in new_ips: - if ip.get('subnet_id') not in delete_subnet_ids: - if 'ip_address' in ip: - new.add(ip['ip_address']) - else: - add_ips.append(ip) + original = orig_by_ip.pop(ip['ip_address'], None) + if original: + prev_ips.append(original) + else: + add_ips.append(ip) - # Convert original ip addresses to sets - orig = set(ip['ip_address'] for ip in original_ips) + # Consider fixed_ips that don't specify ip_address. Try to match them + # up with originals to see if they can be reused. Create a new map of + # the remaining, unmatched originals for this step. + orig_by_subnet = collections.defaultdict(list) + for ip in orig_by_ip.values(): + orig_by_subnet[ip['subnet_id']].append(ip) - add = new - orig - unchanged = new & orig - remove = orig - new + for ip in ips: + if 'ip_address' in ip: + continue - # Convert results back to list of dicts - add_ips += [ips_map[ip] for ip in add] - prev_ips = [ips_map[ip] for ip in unchanged] + orig = orig_by_subnet.get(ip['subnet_id']) + if not orig: + add_ips.append(ip) + continue + + # Try to match this new request up with an existing IP + orig_ip = orig.pop() + if ipv6_utils.is_eui64_address(orig_ip['ip_address']): + # In case of EUI64 address, the prefix may have changed so + # we want to make sure IPAM gets a chance to re-allocate + # it. This is safe in general because EUI-64 addresses + # always come out the same given the prefix doesn't change. + add_ips.append(ip) + remove_candidates.append(orig_ip) + else: + # Reuse the existing address on this subnet. + prev_ips.append(orig_ip) + + # Iterate through any unclaimed original ips (orig_by_subnet) *and* the + # remove_candidates with this compound chain. + maybe_remove = itertools.chain( + itertools.chain.from_iterable(orig_by_subnet.values()), + remove_candidates) # Mark ip for removing if it is not found in new_ips # and subnet requires ip to be set manually. # For auto addressed subnet leave ip unchanged # unless it is explicitly marked for delete. - for ip in remove: - subnet_id = ips_map[ip]['subnet_id'] + remove_ips = [] + for ip in maybe_remove: + subnet_id = ip['subnet_id'] ip_required = self._is_ip_required_by_subnet(context, subnet_id, device_owner) if ip_required or subnet_id in delete_subnet_ids: - remove_ips.append(ips_map[ip]) + remove_ips.append(ip) else: - prev_ips.append(ips_map[ip]) + prev_ips.append(ip) return self.Changes(add=add_ips, original=prev_ips, diff --git a/neutron/tests/unit/db/test_ipam_backend_mixin.py b/neutron/tests/unit/db/test_ipam_backend_mixin.py index b281a7f6352..7ad0ecc13b4 100644 --- a/neutron/tests/unit/db/test_ipam_backend_mixin.py +++ b/neutron/tests/unit/db/test_ipam_backend_mixin.py @@ -71,13 +71,24 @@ class TestIpamBackendMixin(base.BaseTestCase): self.mixin._get_subnet = mock.Mock(side_effect=_get_subnet) - def _test_get_changed_ips_for_port(self, expected_change, original_ips, + def _test_get_changed_ips_for_port(self, expected, original_ips, new_ips, owner): change = self.mixin._get_changed_ips_for_port(self.ctx, original_ips, new_ips, owner) - self.assertEqual(expected_change, change) + + def assertUnorderedListOfDictEqual(a, b): + # Dicts are unorderable in py34. Return something orderable. + def key(d): + return sorted(d.items()) + + # Compare the sorted lists since the order isn't deterministic + self.assertEqual(sorted(a, key=key), sorted(b, key=key)) + + assertUnorderedListOfDictEqual(expected.add, change.add) + assertUnorderedListOfDictEqual(expected.original, change.original) + assertUnorderedListOfDictEqual(expected.remove, change.remove) def test__get_changed_ips_for_port(self): new_ips = self._prepare_ips(self.default_new_ips) @@ -167,6 +178,86 @@ class TestIpamBackendMixin(base.BaseTestCase): self._mock_slaac_subnet_on() self._test_get_changed_ips_for_port_no_ip_address() + def test__get_changed_ips_for_port_subnet_id_no_ip(self): + # If a subnet is specified without an IP address only allocate a new + # address if one doesn't exist + self._mock_slaac_subnet_off() + new_ips = [{'subnet_id': 'id-3'}] + original_ips = [{'subnet_id': 'id-3', 'ip_address': '4.3.2.1'}] + + expected_change = self.mixin.Changes( + add=[], + original=[{'subnet_id': 'id-3', 'ip_address': '4.3.2.1'}], + remove=[]) + self._test_get_changed_ips_for_port(expected_change, original_ips, + new_ips, self.owner_non_router) + + def test__get_changed_ips_for_port_multiple_ips_one_subnet_add_third(self): + # If a subnet is specified without an IP address only allocate a new + # address if one doesn't exist + self._mock_slaac_subnet_off() + new_ips = [{'subnet_id': 'id-3', 'ip_address': '4.3.2.1'}, + {'subnet_id': 'id-3'}, + {'subnet_id': 'id-3', 'ip_address': '4.3.2.10'}] + original_ips = [{'subnet_id': 'id-3', 'ip_address': '4.3.2.1'}, + {'subnet_id': 'id-3', 'ip_address': '4.3.2.10'}] + + expected_change = self.mixin.Changes( + add=[{'subnet_id': 'id-3'}], + original=[{'subnet_id': 'id-3', 'ip_address': '4.3.2.1'}, + {'subnet_id': 'id-3', 'ip_address': '4.3.2.10'}], + remove=[]) + self._test_get_changed_ips_for_port(expected_change, original_ips, + new_ips, self.owner_non_router) + + def test__get_changed_ips_for_port_multiple_ips_one_subnet_noip(self): + # If a subnet is specified without an IP address only allocate a new + # address if one doesn't exist + self._mock_slaac_subnet_off() + new_ips = [{'subnet_id': 'id-3'}, + {'subnet_id': 'id-3'}] + original_ips = [{'subnet_id': 'id-3', 'ip_address': '4.3.2.1'}, + {'subnet_id': 'id-3', 'ip_address': '4.3.2.10'}] + + expected_change = self.mixin.Changes( + add=[], + original=[{'subnet_id': 'id-3', 'ip_address': '4.3.2.1'}, + {'subnet_id': 'id-3', 'ip_address': '4.3.2.10'}], + remove=[]) + self._test_get_changed_ips_for_port(expected_change, original_ips, + new_ips, self.owner_non_router) + + def test__get_changed_ips_for_port_subnet_id_no_ip_ipv6(self): + # If a subnet is specified without an IP address only allocate a new + # address if one doesn't exist + self._mock_slaac_subnet_off() + new_ips = [{'subnet_id': 'id-3'}] + original_ips = [{'subnet_id': 'id-3', 'ip_address': '2001:db8::8'}] + + expected_change = self.mixin.Changes( + add=[], + original=[{'subnet_id': 'id-3', 'ip_address': '2001:db8::8'}], + remove=[]) + self._test_get_changed_ips_for_port(expected_change, original_ips, + new_ips, self.owner_non_router) + + def test__get_changed_ips_for_port_subnet_id_no_ip_eui64(self): + # If a subnet is specified without an IP address allocate a new address + # if the address is eui-64. This supports changing prefix when prefix + # delegation is in use. + self._mock_slaac_subnet_off() + new_ips = [{'subnet_id': 'id-3'}] + original_ips = [{'subnet_id': 'id-3', + 'ip_address': '2001::eeb1:d7ff:fe2c:9c5f'}] + + expected_change = self.mixin.Changes( + add=[{'subnet_id': 'id-3'}], + original=[], + remove=[{'subnet_id': 'id-3', + 'ip_address': '2001::eeb1:d7ff:fe2c:9c5f'}]) + self._test_get_changed_ips_for_port(expected_change, original_ips, + new_ips, self.owner_non_router) + def test__is_ip_required_by_subnet_for_router_port(self): # Owner -> router: # _get_subnet should not be called, diff --git a/neutron/tests/unit/plugins/ml2/extensions/test_dns_integration.py b/neutron/tests/unit/plugins/ml2/extensions/test_dns_integration.py index e0ac55aa40a..8459502dd42 100644 --- a/neutron/tests/unit/plugins/ml2/extensions/test_dns_integration.py +++ b/neutron/tests/unit/plugins/ml2/extensions/test_dns_integration.py @@ -369,37 +369,6 @@ class DNSIntegrationTestCase(test_plugin.Ml2PluginV2TestCase): previous_dns_name=DNSNAME, original_ips=original_ips) - def test_update_port_fixed_ips_with_subnet_ids(self, *mocks): - net, port, dns_data_db = self._create_port_for_test() - original_ips = [ip['ip_address'] for ip in port['fixed_ips']] - ctx = context.get_admin_context() - kwargs = {'fixed_ips': []} - for ip in port['fixed_ips']: - # Since this tests using an "any" IP allocation to update the port - # IP address, change the allocation pools so that IPAM won't ever - # give us back the IP address we originally had. - subnet = self.plugin.get_subnet(ctx, ip['subnet_id']) - ip_net = netaddr.IPNetwork(subnet['cidr']) - subnet_size = ip_net.last - ip_net.first - subnet_mid_point = int(ip_net.first + subnet_size / 2) - start, end = (netaddr.IPAddress(subnet_mid_point + 1), - netaddr.IPAddress(ip_net.last - 1)) - allocation_pools = [{'start': str(start), 'end': str(end)}] - body = {'allocation_pools': allocation_pools} - req = self.new_update_request('subnets', {'subnet': body}, - ip['subnet_id']) - req.get_response(self.api) - kwargs['fixed_ips'].append( - {'subnet_id': ip['subnet_id']}) - - port, dns_data_db = self._update_port_for_test(port, - new_dns_name=None, - **kwargs) - self._verify_port_dns(net, port, dns_data_db, delete_records=True, - current_dns_name=DNSNAME, - previous_dns_name=DNSNAME, - original_ips=original_ips) - def test_update_port_fixed_ips_with_new_dns_name(self, *mocks): net, port, dns_data_db = self._create_port_for_test() original_ips = [ip['ip_address'] for ip in port['fixed_ips']]