diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py index 6622776f5b7..e35e8990875 100644 --- a/neutron/db/db_base_plugin_v2.py +++ b/neutron/db/db_base_plugin_v2.py @@ -1297,8 +1297,23 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon, new_port['dns_name'] = '' new_mac = new_port.get('mac_address') self._validate_port_for_update(context, db_port, new_port, new_mac) - changes = self.ipam.update_port_with_ips(context, db_port, - new_port, new_mac) + # Note: _make_port_dict is called here to load extension data + # (specifically host binding). The IPAM plugin is separate from + # the core plugin, so extensions are not loaded. + # + # The IPAM code could cheat and get it directly from db_port but it + # would have to know about the implementation (remember ml2 has its + # own port binding schema that differs from the generic one) + # + # This code could extract just the port binding host here and pass + # that in. The problem is that db_base_plugin_common shouldn't + # know anything about port binding. This compromise sends IPAM a + # port_dict with all of the extension data loaded. + changes = self.ipam.update_port( + context, + old_port_db=db_port, + old_port=self._make_port_dict(db_port), + new_port=new_port) if 'dns-integration' in self.supported_extension_aliases: dns_assignment = self._get_dns_names_for_updated_port( context, original_ips, original_dns_name, diff --git a/neutron/db/ipam_backend_mixin.py b/neutron/db/ipam_backend_mixin.py index 00254e8fdb7..3d7ac16033b 100644 --- a/neutron/db/ipam_backend_mixin.py +++ b/neutron/db/ipam_backend_mixin.py @@ -14,6 +14,7 @@ # under the License. import collections +import copy import itertools import netaddr @@ -34,6 +35,7 @@ from neutron.common import utils as common_utils from neutron.db import db_base_plugin_common from neutron.db import models_v2 from neutron.db import segments_db +from neutron.extensions import portbindings from neutron.extensions import segment from neutron.ipam import utils as ipam_utils from neutron.services.segments import db as segment_svc_db @@ -594,3 +596,41 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon): if validators.is_attr_set(subnet.get(segment.SEGMENT_ID)): args['segment_id'] = subnet[segment.SEGMENT_ID] return args + + def update_port(self, context, old_port_db, old_port, new_port): + """Update the port IPs + + Updates the port's IPs based on any new fixed_ips passed in or if + deferred IP allocation is in effect because allocation requires host + binding information that wasn't provided until port update. + + :param old_port_db: The port database record + :param old_port: A port dict created by calling _make_port_dict. This + must be called before calling this method in order to + load data from extensions, specifically host binding. + :param new_port: The new port data passed through the API. + """ + old_host = old_port.get(portbindings.HOST_ID) + new_host = new_port.get(portbindings.HOST_ID) + host = new_host if validators.is_attr_set(new_host) else old_host + + changes = self.update_port_with_ips(context, + host, + old_port_db, + new_port, + new_port.get('mac_address')) + + fixed_ips_requested = validators.is_attr_set(new_port.get('fixed_ips')) + deferred_ip_allocation = (host and not old_host + and not old_port.get('fixed_ips') + and not fixed_ips_requested) + if not deferred_ip_allocation: + return changes + + # Allocate as if this were the port create. + port_copy = copy.deepcopy(old_port) + port_copy['fixed_ips'] = const.ATTR_NOT_SPECIFIED + port_copy.update(new_port) + ips = self.allocate_ips_for_port_and_store( + context, {'port': port_copy}, port_copy['id']) + return self.Changes(add=ips, original=[], remove=[]) diff --git a/neutron/db/ipam_non_pluggable_backend.py b/neutron/db/ipam_non_pluggable_backend.py index 3d8fbaac472..6ad064e3b49 100644 --- a/neutron/db/ipam_non_pluggable_backend.py +++ b/neutron/db/ipam_non_pluggable_backend.py @@ -211,14 +211,14 @@ class IpamNonPluggableBackend(ipam_backend_mixin.IpamBackendMixin): subnet_id, port_id) return ips - def update_port_with_ips(self, context, db_port, new_port, new_mac): + def update_port_with_ips(self, context, host, db_port, new_port, new_mac): changes = self.Changes(add=[], original=[], remove=[]) # Check if the IPs need to be updated network_id = db_port['network_id'] if 'fixed_ips' in new_port: original = self._make_port_dict(db_port, process_extensions=False) changes = self._update_ips_for_port( - context, network_id, + context, network_id, host, original["fixed_ips"], new_port['fixed_ips'], original['mac_address'], db_port['device_owner']) @@ -313,14 +313,14 @@ class IpamNonPluggableBackend(ipam_backend_mixin.IpamBackendMixin): 'subnet_id': result['subnet_id']}) return ips - def _update_ips_for_port(self, context, network_id, original_ips, + def _update_ips_for_port(self, context, network_id, host, original_ips, new_ips, mac_address, device_owner): """Add or remove IPs from the port.""" added = [] changes = self._get_changed_ips_for_port(context, original_ips, new_ips, device_owner) subnets = self._ipam_get_subnets( - context, network_id=network_id, host=None) + context, network_id=network_id, host=host) # Check if the IP's to add are OK to_add = self._test_fixed_ips_for_port(context, network_id, changes.add, device_owner, diff --git a/neutron/db/ipam_pluggable_backend.py b/neutron/db/ipam_pluggable_backend.py index 8b9581b0560..df3df176b63 100644 --- a/neutron/db/ipam_pluggable_backend.py +++ b/neutron/db/ipam_pluggable_backend.py @@ -263,7 +263,7 @@ class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin): self._validate_max_ips_per_port(fixed_ip_list, device_owner) return fixed_ip_list - def _update_ips_for_port(self, context, port, + def _update_ips_for_port(self, context, port, host, original_ips, new_ips, mac): """Add or remove IPs from the port. IPAM version""" added = [] @@ -271,7 +271,7 @@ class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin): changes = self._get_changed_ips_for_port( context, original_ips, new_ips, port['device_owner']) subnets = self._ipam_get_subnets( - context, network_id=port['network_id'], host=None) + context, network_id=port['network_id'], host=host) # Check if the IP's to add are OK to_add = self._test_fixed_ips_for_port( context, port['network_id'], changes.add, @@ -301,7 +301,7 @@ class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin): last_ip=last_ip) context.session.add(ip_pool) - def update_port_with_ips(self, context, db_port, new_port, new_mac): + def update_port_with_ips(self, context, host, db_port, new_port, new_mac): changes = self.Changes(add=[], original=[], remove=[]) if 'fixed_ips' in new_port: @@ -309,6 +309,7 @@ class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin): process_extensions=False) changes = self._update_ips_for_port(context, db_port, + host, original["fixed_ips"], new_port['fixed_ips'], new_mac) diff --git a/neutron/tests/unit/db/test_ipam_pluggable_backend.py b/neutron/tests/unit/db/test_ipam_pluggable_backend.py index 04c942b8457..57a02e69f2f 100644 --- a/neutron/tests/unit/db/test_ipam_pluggable_backend.py +++ b/neutron/tests/unit/db/test_ipam_pluggable_backend.py @@ -617,7 +617,7 @@ class TestDbBasePluginIpam(test_db_base.NeutronDbPluginV2TestCase): port_dict = {'device_owner': uuidutils.generate_uuid(), 'network_id': uuidutils.generate_uuid()} - mocks['ipam']._update_ips_for_port(context, port_dict, + mocks['ipam']._update_ips_for_port(context, port_dict, None, original_ips, new_ips, mac) mocks['driver'].get_address_request_factory.assert_called_once_with() mocks['ipam']._ipam_get_subnets.assert_called_once_with( @@ -752,6 +752,7 @@ class TestDbBasePluginIpam(test_db_base.NeutronDbPluginV2TestCase): self.assertRaises(db_exc.DBDeadlock, mocks['ipam'].update_port_with_ips, context, + None, db_port, new_port, mock.Mock()) diff --git a/neutron/tests/unit/extensions/test_segment.py b/neutron/tests/unit/extensions/test_segment.py index 9f989c549c1..0d2be909ea5 100644 --- a/neutron/tests/unit/extensions/test_segment.py +++ b/neutron/tests/unit/extensions/test_segment.py @@ -22,6 +22,7 @@ from neutron.api.v2 import attributes from neutron import context from neutron.db import agents_db from neutron.db import db_base_plugin_v2 +from neutron.db import portbindings_db from neutron.db import segments_db from neutron.extensions import portbindings from neutron.extensions import segment as ext_segment @@ -100,6 +101,7 @@ class SegmentTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase): class SegmentTestPlugin(db_base_plugin_v2.NeutronDbPluginV2, + portbindings_db.PortBindingMixin, db.SegmentDbMixin): __native_pagination_support = True __native_sorting_support = True @@ -112,6 +114,12 @@ class SegmentTestPlugin(db_base_plugin_v2.NeutronDbPluginV2, def get_plugin_type(self): return "segments" + def create_port(self, context, port): + port_dict = super(SegmentTestPlugin, self).create_port(context, port) + self._process_portbindings_create_and_update( + context, port['port'], port_dict) + return port_dict + class TestSegment(SegmentTestCase): @@ -534,6 +542,13 @@ class TestSegmentAwareIpam(SegmentTestCase): # Don't allocate IPs in this case because we didn't give binding info self.assertEqual(0, len(res['port']['fixed_ips'])) + def _assert_one_ip_in_subnet(self, response, cidr): + res = self.deserialize(self.fmt, response) + self.assertEqual(1, len(res['port']['fixed_ips'])) + ip = res['port']['fixed_ips'][0]['ip_address'] + ip_net = netaddr.IPNetwork(cidr) + self.assertIn(ip, ip_net) + def test_port_create_with_binding_information(self): """Binding information is provided, subnets are on segments""" network, segments, subnets = self._create_test_segments_with_subnets(3) @@ -549,12 +564,9 @@ class TestSegmentAwareIpam(SegmentTestCase): tenant_id=network['network']['tenant_id'], arg_list=(portbindings.HOST_ID,), **{portbindings.HOST_ID: 'fakehost'}) - res = self.deserialize(self.fmt, response) # Since host mapped to middle segment, IP must come from middle subnet - ip = res['port']['fixed_ips'][0]['ip_address'] - ip_net = netaddr.IPNetwork(subnets[1]['subnet']['cidr']) - self.assertIn(ip, ip_net) + self._assert_one_ip_in_subnet(response, subnets[1]['subnet']['cidr']) def test_port_create_with_binding_and_no_subnets(self): """Binding information is provided, no subnets.""" @@ -594,12 +606,9 @@ class TestSegmentAwareIpam(SegmentTestCase): tenant_id=network['network']['tenant_id'], arg_list=(portbindings.HOST_ID,), **{portbindings.HOST_ID: 'fakehost'}) - res = self.deserialize(self.fmt, response) # Since the subnet is not on a segment, fall back to it - ip = res['port']['fixed_ips'][0]['ip_address'] - ip_net = netaddr.IPNetwork(subnet['subnet']['cidr']) - self.assertIn(ip, ip_net) + self._assert_one_ip_in_subnet(response, subnet['subnet']['cidr']) def test_port_create_on_unconnected_host(self): """Binding information provided, host not connected to any segment""" @@ -646,3 +655,189 @@ class TestSegmentAwareIpam(SegmentTestCase): self.assertEqual(webob.exc.HTTPConflict.code, response.status_int) self.assertEqual(segment_exc.HostConnectedToMultipleSegments.__name__, res['NeutronError']['type']) + + def test_port_update_excludes_hosts_on_segments(self): + """No binding information is provided, subnets on segments""" + with self.network() as network: + segment = self._test_create_segment( + network_id=network['network']['id'], + physical_network='physnet') + + # Create a port with no IP address (since there is no subnet) + port = self._create_deferred_ip_port(network) + + # Create the subnet and try to update the port to get an IP + with self.subnet(network=network, + segment_id=segment['segment']['id']) as subnet: + # Try requesting an IP (but the only subnet is on a segment) + data = {'port': { + 'fixed_ips': [{'subnet_id': subnet['subnet']['id']}]}} + port_id = port['port']['id'] + port_req = self.new_update_request('ports', data, port_id) + response = port_req.get_response(self.api) + + # Gets bad request because there are no eligible subnets. + self.assertEqual(webob.exc.HTTPBadRequest.code, response.status_int) + + def test_port_update_is_host_aware(self): + """Binding information is provided, subnets on segments""" + with self.network() as network: + segment = self._test_create_segment( + network_id=network['network']['id'], + physical_network='physnet') + + # Map the host to the segment + self._setup_host_mappings([(segment['segment']['id'], 'fakehost')]) + + # Create a bound port with no IP address (since there is no subnet) + response = self._create_port(self.fmt, + net_id=network['network']['id'], + tenant_id=network['network']['tenant_id'], + arg_list=(portbindings.HOST_ID,), + **{portbindings.HOST_ID: 'fakehost'}) + port = self.deserialize(self.fmt, response) + + # Create the subnet and try to update the port to get an IP + with self.subnet(network=network, + segment_id=segment['segment']['id']) as subnet: + # Try requesting an IP (but the only subnet is on a segment) + data = {'port': { + 'fixed_ips': [{'subnet_id': subnet['subnet']['id']}]}} + port_id = port['port']['id'] + port_req = self.new_update_request('ports', data, port_id) + response = port_req.get_response(self.api) + + # Since port is bound and there is a mapping to segment, it succeeds. + self.assertEqual(webob.exc.HTTPOk.code, response.status_int) + self._assert_one_ip_in_subnet(response, subnet['subnet']['cidr']) + + def _create_deferred_ip_port(self, network): + response = self._create_port(self.fmt, + net_id=network['network']['id'], + tenant_id=network['network']['tenant_id']) + port = self.deserialize(self.fmt, response) + ips = port['port']['fixed_ips'] + self.assertEqual(0, len(ips)) + return port + + def test_port_update_deferred_allocation(self): + """Binding information is provided on update, subnets on segments""" + network, segment, subnet = self._create_test_segment_with_subnet() + + # Map the host to the segment + self._setup_host_mappings([(segment['segment']['id'], 'fakehost')]) + + port = self._create_deferred_ip_port(network) + + # Try requesting an IP (but the only subnet is on a segment) + data = {'port': {portbindings.HOST_ID: 'fakehost'}} + port_id = port['port']['id'] + port_req = self.new_update_request('ports', data, port_id) + response = port_req.get_response(self.api) + + # Port update succeeds and allocates a new IP address. + self.assertEqual(webob.exc.HTTPOk.code, response.status_int) + self._assert_one_ip_in_subnet(response, subnet['subnet']['cidr']) + + def test_port_update_deferred_allocation_no_segments(self): + """Binding information is provided, subnet created after port""" + with self.network() as network: + pass + + port = self._create_deferred_ip_port(network) + + # Create the subnet and try to update the port to get an IP + with self.subnet(network=network) as subnet: + data = {'port': {portbindings.HOST_ID: 'fakehost'}} + port_id = port['port']['id'] + port_req = self.new_update_request('ports', data, port_id) + response = port_req.get_response(self.api) + + self.assertEqual(webob.exc.HTTPOk.code, response.status_int) + self._assert_one_ip_in_subnet(response, subnet['subnet']['cidr']) + + def test_port_update_deferred_allocation_no_segments_manual_alloc(self): + """Binding information is provided, subnet created after port""" + with self.network() as network: + pass + + port = self._create_deferred_ip_port(network) + + # Create the subnet and try to update the port to get an IP + with self.subnet(network=network) as subnet: + data = {'port': { + portbindings.HOST_ID: 'fakehost', + 'fixed_ips': [{'subnet_id': subnet['subnet']['id']}]}} + port_id = port['port']['id'] + port_req = self.new_update_request('ports', data, port_id) + response = port_req.get_response(self.api) + + self.assertEqual(webob.exc.HTTPOk.code, response.status_int) + self._assert_one_ip_in_subnet(response, subnet['subnet']['cidr']) + + # Do a show to be sure that only one IP is recorded + port_req = self.new_show_request('ports', port_id) + response = port_req.get_response(self.api) + self.assertEqual(webob.exc.HTTPOk.code, response.status_int) + self._assert_one_ip_in_subnet(response, subnet['subnet']['cidr']) + + def test_port_update_deferred_allocation_no_segments_empty_alloc(self): + """Binding information is provided, subnet created after port""" + with self.network() as network: + pass + + port = self._create_deferred_ip_port(network) + + # Create the subnet and update the port but specify no IPs + with self.subnet(network=network): + data = {'port': { + portbindings.HOST_ID: 'fakehost', + 'fixed_ips': []}} + port_id = port['port']['id'] + port_req = self.new_update_request('ports', data, port_id) + response = port_req.get_response(self.api) + + self.assertEqual(webob.exc.HTTPOk.code, response.status_int) + res = self.deserialize(self.fmt, response) + # Since I specifically requested no IP addresses, I shouldn't get one. + self.assertEqual(0, len(res['port']['fixed_ips'])) + + def test_port_update_deferred_allocation_no_host_mapping(self): + """Binding information is provided on update, subnets on segments""" + network, segment, subnet = self._create_test_segment_with_subnet() + + port = self._create_deferred_ip_port(network) + + # Try requesting an IP (but the only subnet is on a segment) + data = {'port': {portbindings.HOST_ID: 'fakehost'}} + port_id = port['port']['id'] + port_req = self.new_update_request('ports', data, port_id) + response = port_req.get_response(self.api) + res = self.deserialize(self.fmt, response) + + # Gets conflict because it can't map the host to a segment + self.assertEqual(webob.exc.HTTPConflict.code, response.status_int) + self.assertEqual(segment_exc.HostNotConnectedToAnySegment.__name__, + res['NeutronError']['type']) + + def test_port_update_deferred_allocation_multiple_host_mapping(self): + """Binding information is provided on update, subnets on segments""" + network, segments, _s = self._create_test_segments_with_subnets(2) + + port = self._create_deferred_ip_port(network) + + # This host is bound to multiple segments + self._setup_host_mappings([(segments[0]['segment']['id'], 'fakehost'), + (segments[1]['segment']['id'], 'fakehost')]) + + # Try requesting an IP (but the only subnet is on a segment) + data = {'port': {portbindings.HOST_ID: 'fakehost'}} + port_id = port['port']['id'] + port_req = self.new_update_request('ports', data, port_id) + response = port_req.get_response(self.api) + res = self.deserialize(self.fmt, response) + + # Gets conflict because it can't map the host to a segment + self.assertEqual(webob.exc.HTTPConflict.code, response.status_int) + self.assertEqual(segment_exc.HostConnectedToMultipleSegments.__name__, + res['NeutronError']['type'])