From 33e48cf84db0f6b51a0f981499bbd341860b6c1c Mon Sep 17 00:00:00 2001 From: Harald Jensas <harald.jensas@gmail.com> Date: Sun, 4 Jun 2017 23:41:19 +0200 Subject: [PATCH] Do not defer allocation if fixed-ips is in the port create request. Fix a usage regression, use case #2 in Nova Neutron Routed Networks spec https://specs.openstack.org/openstack/nova-specs/specs/newton/implemented/neutron-routed-networks.html Currently ip allocation is always deferred if binding:host_id is not specified when routed networks are used. This causes initial fixed-ips data provided by the user to be _lost_ unless the user also specify the host. Since the user specified the IP or Subnet to allocate in, there is no reason to defer the allocation. a) It is a common pattern, especially in Heat templates to: 1. Create a port with fixed-ips specifying a subnet. 2. Create a server and associate the existing port. b) It is also common to use Neutron IPAM as a source to get VIP addresses for clusters on provider networks. This change enables these use cases with routed networks. DocImpact: "The Networking service defers assignment of IP addresses to the port until the particular compute node becomes apparent." This is no longer true if fixed-ips is used in the port create request. Change-Id: I86d4aafa1f8cd425cb1eeecfeaf95226d6d248b4 Closes-Bug: #1695740 --- neutron/db/ipam_backend_mixin.py | 16 ++++++++++++---- neutron/db/ipam_pluggable_backend.py | 5 +++-- neutron/tests/unit/extensions/test_segment.py | 13 +++++++++++++ 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/neutron/db/ipam_backend_mixin.py b/neutron/db/ipam_backend_mixin.py index 562dfc90954..5990b06b9e9 100644 --- a/neutron/db/ipam_backend_mixin.py +++ b/neutron/db/ipam_backend_mixin.py @@ -657,14 +657,15 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon): # implementations. return host and validators.is_attr_set(host) - def _ipam_get_subnets(self, context, network_id, host, service_type=None): + def _ipam_get_subnets(self, context, network_id, host, service_type=None, + fixed_configured=False): """Return eligible subnets If no eligible subnets are found, determine why and potentially raise an appropriate error. """ - subnets = self._find_candidate_subnets( - context, network_id, host, service_type) + subnets = self._find_candidate_subnets(context, network_id, host, + service_type, fixed_configured) if subnets: subnet_dicts = [self._make_subnet_dict(subnet, context=context) for subnet in subnets] @@ -697,13 +698,20 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon): raise ipam_exceptions.IpAddressGenerationFailureNoMatchingSubnet() - def _find_candidate_subnets(self, context, network_id, host, service_type): + def _find_candidate_subnets(self, context, network_id, host, service_type, + fixed_configured): """Find canditate subnets for the network, host, and service_type""" query = self._query_subnets_on_network(context, network_id) query = self._query_filter_service_subnets(query, service_type) # Select candidate subnets and return them if not self.is_host_set(host): + if fixed_configured: + # If fixed_ips in request and host is not known all subnets on + # the network are candidates. Host/Segment will be validated + # on port update with binding:host_id set. Allocation _cannot_ + # be deferred as requested fixed_ips would then be lost. + return query.all() # If the host isn't known, we can't allocate on a routed network. # So, exclude any subnets attached to segments. return self._query_exclude_subnets_on_segments(query).all() diff --git a/neutron/db/ipam_pluggable_backend.py b/neutron/db/ipam_pluggable_backend.py index a8bd5ccfe77..dc26346bd60 100644 --- a/neutron/db/ipam_pluggable_backend.py +++ b/neutron/db/ipam_pluggable_backend.py @@ -200,15 +200,16 @@ class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin): a subnet_id then allocate an IP address accordingly. """ p = port['port'] + fixed_configured = p['fixed_ips'] is not constants.ATTR_NOT_SPECIFIED subnets = self._ipam_get_subnets(context, network_id=p['network_id'], host=p.get(portbindings.HOST_ID), - service_type=p.get('device_owner')) + service_type=p.get('device_owner'), + fixed_configured=fixed_configured) v4, v6_stateful, v6_stateless = self._classify_subnets( context, subnets) - fixed_configured = p['fixed_ips'] is not constants.ATTR_NOT_SPECIFIED if fixed_configured: ips = self._test_fixed_ips_for_port(context, p["network_id"], diff --git a/neutron/tests/unit/extensions/test_segment.py b/neutron/tests/unit/extensions/test_segment.py index 878f883b542..900fba4fa24 100644 --- a/neutron/tests/unit/extensions/test_segment.py +++ b/neutron/tests/unit/extensions/test_segment.py @@ -880,6 +880,19 @@ class TestSegmentAwareIpam(SegmentAwareIpamTestCase): # Don't allocate IPs in this case because we didn't give binding info self.assertEqual(0, len(res['port']['fixed_ips'])) + def test_port_create_fixed_ips_with_segment_subnets_no_binding_info(self): + """Fixed IP provided and no binding info, do not defer IP allocation""" + network, segment, subnet = self._create_test_segment_with_subnet() + response = self._create_port(self.fmt, + net_id=network['network']['id'], + tenant_id=network['network']['tenant_id'], + fixed_ips=[ + {'subnet_id': subnet['subnet']['id']} + ]) + res = self.deserialize(self.fmt, response) + # We gave fixed_ips, allocate IPs in this case despite no binding info + self._validate_immediate_ip_allocation(res['port']['id']) + def _assert_one_ip_in_subnet(self, response, cidr): res = self.deserialize(self.fmt, response) self.assertEqual(1, len(res['port']['fixed_ips']))