Revert "Revise log when create port failed"

After this patch, during delete subnets with fixed ip, there are high
chances a subnet can't be found(because it just deleted) while
deleting another subnet and Raise BadRequest instead of Notfound.
The behavior change led to heat gate failure, since Heat expect to
see NotFound exception instead of BadRequest. 
Also the error message is really confusing while delete subnets
(you can check out more detail in
https://storyboard.openstack.org/#!/story/2006962)

hence, I think to revert this patch will make sense to maintain the
behavior before a proper solution comes out. 

Story: 2006962
Task: 37652
This reverts commit cae66a4d8d.

Change-Id: I9fc200de376668b662cd8c2bfba01483dc9bb677
This commit is contained in:
Rico Lin 2019-11-28 04:16:12 +00:00 committed by Slawek Kaplonski
parent 8375f4cc1c
commit 22d2bb94f7
5 changed files with 17 additions and 16 deletions

View File

@ -379,7 +379,7 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon):
if segment.is_dynamic: if segment.is_dynamic:
raise segment_exc.SubnetCantAssociateToDynamicSegment() raise segment_exc.SubnetCantAssociateToDynamicSegment()
def _get_subnet_for_fixed_ip(self, fixed, subnets, network_id): def _get_subnet_for_fixed_ip(self, context, fixed, subnets):
# Subnets are all the subnets belonging to the same network. # Subnets are all the subnets belonging to the same network.
if not subnets: if not subnets:
msg = _('IP allocation requires subnets for network') msg = _('IP allocation requires subnets for network')
@ -392,10 +392,12 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon):
return subnet return subnet
subnet = get_matching_subnet() subnet = get_matching_subnet()
if not subnet: if not subnet:
subnet_obj = self._get_subnet_object(context,
fixed['subnet_id'])
msg = (_("Failed to create port on network %(network_id)s" msg = (_("Failed to create port on network %(network_id)s"
", because fixed_ips included invalid subnet " ", because fixed_ips included invalid subnet "
"%(subnet_id)s") % "%(subnet_id)s") %
{'network_id': network_id, {'network_id': subnet_obj.network_id,
'subnet_id': fixed['subnet_id']}) 'subnet_id': fixed['subnet_id']})
raise exc.InvalidInput(error_message=msg) raise exc.InvalidInput(error_message=msg)
# Ensure that the IP is valid on the subnet # Ensure that the IP is valid on the subnet

View File

@ -240,7 +240,8 @@ class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin):
context, subnets) context, subnets)
if fixed_configured: if fixed_configured:
ips = self._test_fixed_ips_for_port(p["network_id"], ips = self._test_fixed_ips_for_port(context,
p["network_id"],
p['fixed_ips'], p['fixed_ips'],
p['device_owner'], p['device_owner'],
subnets) subnets)
@ -274,7 +275,7 @@ class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin):
'mac': port['mac_address']}) 'mac': port['mac_address']})
return ips return ips
def _test_fixed_ips_for_port(self, network_id, fixed_ips, def _test_fixed_ips_for_port(self, context, network_id, fixed_ips,
device_owner, subnets): device_owner, subnets):
"""Test fixed IPs for port. """Test fixed IPs for port.
@ -288,7 +289,7 @@ class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin):
fixed_ip_list = [] fixed_ip_list = []
for fixed in fixed_ips: for fixed in fixed_ips:
fixed['device_owner'] = device_owner fixed['device_owner'] = device_owner
subnet = self._get_subnet_for_fixed_ip(fixed, subnets, network_id) subnet = self._get_subnet_for_fixed_ip(context, fixed, subnets)
is_auto_addr_subnet = ipv6_utils.is_auto_address_subnet(subnet) is_auto_addr_subnet = ipv6_utils.is_auto_address_subnet(subnet)
if ('ip_address' in fixed and if ('ip_address' in fixed and
@ -350,7 +351,7 @@ class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin):
# Check if the IP's to add are OK # Check if the IP's to add are OK
to_add = self._test_fixed_ips_for_port( to_add = self._test_fixed_ips_for_port(
port['network_id'], changes.add, context, port['network_id'], changes.add,
port['device_owner'], subnets) port['device_owner'], subnets)
if port['device_owner'] not in constants.ROUTER_INTERFACE_OWNERS: if port['device_owner'] not in constants.ROUTER_INTERFACE_OWNERS:

View File

@ -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'] net_id = port['port']['network_id']
res = self._create_port(self.fmt, net_id=net_id, **kwargs) res = self._create_port(self.fmt, net_id=net_id, **kwargs)
port2 = self.deserialize(self.fmt, res) port2 = self.deserialize(self.fmt, res)
self.assertEqual(webob.exc.HTTPBadRequest.code, res.status_int) self.assertEqual(webob.exc.HTTPNotFound.code, res.status_int)
# Test invalid IP address on specified subnet_id # Test invalid IP address on specified subnet_id
kwargs = {"fixed_ips": kwargs = {"fixed_ips":

View File

@ -368,6 +368,7 @@ class TestDbBasePluginIpam(test_db_base.NeutronDbPluginV2TestCase):
mocks['subnets'].allocate.assert_called_once_with(mock.ANY) mocks['subnets'].allocate.assert_called_once_with(mock.ANY)
def test_test_fixed_ips_for_port_pd_gateway(self): def test_test_fixed_ips_for_port_pd_gateway(self):
context = mock.Mock()
pluggable_backend = ipam_pluggable_backend.IpamPluggableBackend() pluggable_backend = ipam_pluggable_backend.IpamPluggableBackend()
with self.subnet(cidr=constants.PROVISIONAL_IPV6_PD_PREFIX, with self.subnet(cidr=constants.PROVISIONAL_IPV6_PD_PREFIX,
ip_version=constants.IP_VERSION_6) as subnet: ip_version=constants.IP_VERSION_6) as subnet:
@ -375,7 +376,7 @@ class TestDbBasePluginIpam(test_db_base.NeutronDbPluginV2TestCase):
fixed_ips = [{'subnet_id': subnet['id'], fixed_ips = [{'subnet_id': subnet['id'],
'ip_address': '::1'}] 'ip_address': '::1'}]
filtered_ips = (pluggable_backend. filtered_ips = (pluggable_backend.
_test_fixed_ips_for_port( _test_fixed_ips_for_port(context,
subnet['network_id'], subnet['network_id'],
fixed_ips, fixed_ips,
constants.DEVICE_OWNER_ROUTER_INTF, constants.DEVICE_OWNER_ROUTER_INTF,

View File

@ -720,15 +720,12 @@ class TestMl2SubnetsV2(test_plugin.TestSubnetsV2,
req = self.new_delete_request('subnets', req = self.new_delete_request('subnets',
s1['subnet']['id']) s1['subnet']['id'])
res = req.get_response(self.api) res = req.get_response(self.api)
self.assertEqual(webob.exc.HTTPBadRequest.code, self.assertEqual(204, res.status_int)
res.status_int) # ensure port only has 1 IP on s3
# ensure port has IP on s1 and s3
port = self._show('ports', p['port']['id'])['port'] port = self._show('ports', p['port']['id'])['port']
self.assertEqual(2, len(port['fixed_ips'])) self.assertEqual(1, len(port['fixed_ips']))
port_subnet_id = [port['fixed_ips'][0]['subnet_id'], self.assertEqual(s3['subnet']['id'],
port['fixed_ips'][1]['subnet_id']] port['fixed_ips'][0]['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): def test_update_subnet_with_empty_body(self):
with self.subnet() as subnet: with self.subnet() as subnet: