From cae66a4d8d13deda74db806d733c2ac39ea2e849 Mon Sep 17 00:00:00 2001 From: zhengyong Date: Mon, 11 Nov 2019 10:15:49 +0800 Subject: [PATCH] Revise log when create port failed Log network_id passed from api rather than subnet['network_id'] Change-Id: Ia36635014e827b4a321dbdce22f605c76cc88390 Closes-Bug: #1844607 --- neutron/db/ipam_backend_mixin.py | 6 ++---- neutron/db/ipam_pluggable_backend.py | 9 ++++----- neutron/tests/unit/db/test_db_base_plugin_v2.py | 2 +- .../tests/unit/db/test_ipam_pluggable_backend.py | 3 +-- neutron/tests/unit/plugins/ml2/test_plugin.py | 13 ++++++++----- 5 files changed, 16 insertions(+), 17 deletions(-) diff --git a/neutron/db/ipam_backend_mixin.py b/neutron/db/ipam_backend_mixin.py index 15656d1880c..6f67ad5ac02 100644 --- a/neutron/db/ipam_backend_mixin.py +++ b/neutron/db/ipam_backend_mixin.py @@ -379,7 +379,7 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon): if segment.is_dynamic: raise segment_exc.SubnetCantAssociateToDynamicSegment() - def _get_subnet_for_fixed_ip(self, context, fixed, subnets): + def _get_subnet_for_fixed_ip(self, fixed, subnets, network_id): # Subnets are all the subnets belonging to the same network. if not subnets: msg = _('IP allocation requires subnets for network') @@ -392,12 +392,10 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon): return subnet subnet = get_matching_subnet() if not subnet: - subnet_obj = self._get_subnet_object(context, - fixed['subnet_id']) msg = (_("Failed to create port on network %(network_id)s" ", because fixed_ips included invalid subnet " "%(subnet_id)s") % - {'network_id': subnet_obj.network_id, + {'network_id': network_id, 'subnet_id': fixed['subnet_id']}) raise exc.InvalidInput(error_message=msg) # Ensure that the IP is valid on the subnet diff --git a/neutron/db/ipam_pluggable_backend.py b/neutron/db/ipam_pluggable_backend.py index 7282ef80ed2..b22c324feff 100644 --- a/neutron/db/ipam_pluggable_backend.py +++ b/neutron/db/ipam_pluggable_backend.py @@ -240,8 +240,7 @@ class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin): context, subnets) if fixed_configured: - ips = self._test_fixed_ips_for_port(context, - p["network_id"], + ips = self._test_fixed_ips_for_port(p["network_id"], p['fixed_ips'], p['device_owner'], subnets) @@ -275,7 +274,7 @@ class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin): 'mac': port['mac_address']}) return ips - def _test_fixed_ips_for_port(self, context, network_id, fixed_ips, + def _test_fixed_ips_for_port(self, network_id, fixed_ips, device_owner, subnets): """Test fixed IPs for port. @@ -289,7 +288,7 @@ class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin): fixed_ip_list = [] for fixed in fixed_ips: fixed['device_owner'] = device_owner - subnet = self._get_subnet_for_fixed_ip(context, fixed, subnets) + subnet = self._get_subnet_for_fixed_ip(fixed, subnets, network_id) is_auto_addr_subnet = ipv6_utils.is_auto_address_subnet(subnet) if ('ip_address' in fixed and @@ -351,7 +350,7 @@ class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin): # Check if the IP's to add are OK to_add = self._test_fixed_ips_for_port( - context, port['network_id'], changes.add, + port['network_id'], changes.add, port['device_owner'], subnets) if port['device_owner'] not in constants.ROUTER_INTERFACE_OWNERS: 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 ea3cdd288a6..bc4e96c83b1 100644 --- a/neutron/tests/unit/db/test_db_base_plugin_v2.py +++ b/neutron/tests/unit/db/test_db_base_plugin_v2.py @@ -2466,7 +2466,7 @@ fixed_ips=ip_address%%3D%s&fixed_ips=ip_address%%3D%s&fixed_ips=subnet_id%%3D%s net_id = port['port']['network_id'] res = self._create_port(self.fmt, net_id=net_id, **kwargs) port2 = self.deserialize(self.fmt, res) - self.assertEqual(webob.exc.HTTPNotFound.code, res.status_int) + self.assertEqual(webob.exc.HTTPBadRequest.code, res.status_int) # Test invalid IP address on specified subnet_id kwargs = {"fixed_ips": diff --git a/neutron/tests/unit/db/test_ipam_pluggable_backend.py b/neutron/tests/unit/db/test_ipam_pluggable_backend.py index c013176e471..13ad3642d59 100644 --- a/neutron/tests/unit/db/test_ipam_pluggable_backend.py +++ b/neutron/tests/unit/db/test_ipam_pluggable_backend.py @@ -368,7 +368,6 @@ class TestDbBasePluginIpam(test_db_base.NeutronDbPluginV2TestCase): mocks['subnets'].allocate.assert_called_once_with(mock.ANY) def test_test_fixed_ips_for_port_pd_gateway(self): - context = mock.Mock() pluggable_backend = ipam_pluggable_backend.IpamPluggableBackend() with self.subnet(cidr=constants.PROVISIONAL_IPV6_PD_PREFIX, ip_version=constants.IP_VERSION_6) as subnet: @@ -376,7 +375,7 @@ class TestDbBasePluginIpam(test_db_base.NeutronDbPluginV2TestCase): fixed_ips = [{'subnet_id': subnet['id'], 'ip_address': '::1'}] filtered_ips = (pluggable_backend. - _test_fixed_ips_for_port(context, + _test_fixed_ips_for_port( subnet['network_id'], fixed_ips, constants.DEVICE_OWNER_ROUTER_INTF, diff --git a/neutron/tests/unit/plugins/ml2/test_plugin.py b/neutron/tests/unit/plugins/ml2/test_plugin.py index b4c2fd5ae93..b4b0811e854 100644 --- a/neutron/tests/unit/plugins/ml2/test_plugin.py +++ b/neutron/tests/unit/plugins/ml2/test_plugin.py @@ -720,12 +720,15 @@ class TestMl2SubnetsV2(test_plugin.TestSubnetsV2, req = self.new_delete_request('subnets', s1['subnet']['id']) res = req.get_response(self.api) - self.assertEqual(204, res.status_int) - # ensure port only has 1 IP on s3 + self.assertEqual(webob.exc.HTTPBadRequest.code, + res.status_int) + # ensure port has IP on s1 and s3 port = self._show('ports', p['port']['id'])['port'] - self.assertEqual(1, len(port['fixed_ips'])) - self.assertEqual(s3['subnet']['id'], - port['fixed_ips'][0]['subnet_id']) + self.assertEqual(2, len(port['fixed_ips'])) + port_subnet_id = [port['fixed_ips'][0]['subnet_id'], + port['fixed_ips'][1]['subnet_id']] + self.assertIn(s1['subnet']['id'], port_subnet_id) + self.assertIn(s3['subnet']['id'], port_subnet_id) def test_update_subnet_with_empty_body(self): with self.subnet() as subnet: