Merge "Fixes port device_id/device_owner change in failed operation"
This commit is contained in:
commit
9fd6b5841d
|
@ -633,15 +633,7 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
|
|||
raise n_exc.BadRequest(resource='router', msg=msg)
|
||||
return port
|
||||
|
||||
def _add_interface_by_port(self, context, router, port_id, owner):
|
||||
# Update owner before actual process in order to avoid the
|
||||
# case where a port might get attached to a router without the
|
||||
# owner successfully updating due to an unavailable backend.
|
||||
self._check_router_port(context, port_id, '')
|
||||
self._core_plugin.update_port(
|
||||
context, port_id, {'port': {'device_id': router.id,
|
||||
'device_owner': owner}})
|
||||
|
||||
def _validate_router_port_info(self, context, router, port_id):
|
||||
with context.session.begin(subtransactions=True):
|
||||
# check again within transaction to mitigate race
|
||||
port = self._check_router_port(context, port_id, router.id)
|
||||
|
@ -678,6 +670,23 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
|
|||
raise n_exc.BadRequest(resource='router', msg=msg)
|
||||
return port, subnets
|
||||
|
||||
def _add_interface_by_port(self, context, router, port_id, owner):
|
||||
# Update owner before actual process in order to avoid the
|
||||
# case where a port might get attached to a router without the
|
||||
# owner successfully updating due to an unavailable backend.
|
||||
port = self._check_router_port(context, port_id, '')
|
||||
prev_owner = port['device_owner']
|
||||
self._core_plugin.update_port(
|
||||
context, port_id, {'port': {'device_id': router.id,
|
||||
'device_owner': owner}})
|
||||
try:
|
||||
return self._validate_router_port_info(context, router, port_id)
|
||||
except Exception:
|
||||
with excutils.save_and_reraise_exception():
|
||||
self._core_plugin.update_port(
|
||||
context, port_id, {'port': {'device_id': '',
|
||||
'device_owner': prev_owner}})
|
||||
|
||||
def _port_has_ipv6_address(self, port):
|
||||
for fixed_ip in port['fixed_ips']:
|
||||
if netaddr.IPNetwork(fixed_ip['ip_address']).version == 6:
|
||||
|
|
|
@ -1241,6 +1241,30 @@ class L3NatTestCaseBase(L3NatTestCaseMixin):
|
|||
None,
|
||||
p['port']['id'])
|
||||
|
||||
def _assert_body_port_id_and_update_port(self, body, mock_update_port,
|
||||
port_id, device_id):
|
||||
self.assertNotIn('port_id', body)
|
||||
expected_port_update_before_update = {
|
||||
'device_owner': l3_constants.DEVICE_OWNER_ROUTER_INTF,
|
||||
'device_id': device_id}
|
||||
expected_port_update_after_fail = {
|
||||
'device_owner': '',
|
||||
'device_id': ''}
|
||||
mock_update_port.assert_has_calls(
|
||||
[mock.call(
|
||||
mock.ANY,
|
||||
port_id,
|
||||
{'port': expected_port_update_before_update}),
|
||||
mock.call(
|
||||
mock.ANY,
|
||||
port_id,
|
||||
{'port': expected_port_update_after_fail})],
|
||||
any_order=False)
|
||||
# fetch port and confirm device_id and device_owner
|
||||
body = self._show('ports', port_id)
|
||||
self.assertEqual('', body['port']['device_owner'])
|
||||
self.assertEqual('', body['port']['device_id'])
|
||||
|
||||
def test_router_add_interface_multiple_ipv4_subnet_port_returns_400(self):
|
||||
"""Test adding router port with multiple IPv4 subnets fails.
|
||||
|
||||
|
@ -1252,13 +1276,17 @@ class L3NatTestCaseBase(L3NatTestCaseMixin):
|
|||
self.subnet(network=n, cidr='10.0.1.0/24')) as s2:
|
||||
fixed_ips = [{'subnet_id': s1['subnet']['id']},
|
||||
{'subnet_id': s2['subnet']['id']}]
|
||||
with self.port(subnet=s1, fixed_ips=fixed_ips) as p:
|
||||
orig_update_port = self.plugin.update_port
|
||||
with self.port(subnet=s1, fixed_ips=fixed_ips) as p, (
|
||||
mock.patch.object(self.plugin,
|
||||
'update_port')) as update_port:
|
||||
update_port.side_effect = orig_update_port
|
||||
exp_code = exc.HTTPBadRequest.code
|
||||
self._router_interface_action('add',
|
||||
r['router']['id'],
|
||||
None,
|
||||
p['port']['id'],
|
||||
expected_code=exp_code)
|
||||
body = self._router_interface_action(
|
||||
'add', r['router']['id'], None, p['port']['id'],
|
||||
expected_code=exp_code)
|
||||
self._assert_body_port_id_and_update_port(
|
||||
body, update_port, p['port']['id'], r['router']['id'])
|
||||
|
||||
def test_router_add_interface_ipv6_port_existing_network_returns_400(self):
|
||||
"""Ensure unique IPv6 router ports per network id.
|
||||
|
@ -1273,17 +1301,21 @@ class L3NatTestCaseBase(L3NatTestCaseMixin):
|
|||
ip_version=6) as s1, (
|
||||
self.subnet(network=n, cidr='fd01::/64',
|
||||
ip_version=6)) as s2:
|
||||
with self.port(subnet=s1) as p:
|
||||
orig_update_port = self.plugin.update_port
|
||||
with self.port(subnet=s1) as p, (
|
||||
mock.patch.object(self.plugin,
|
||||
'update_port')) as update_port:
|
||||
update_port.side_effect = orig_update_port
|
||||
self._router_interface_action('add',
|
||||
r['router']['id'],
|
||||
s2['subnet']['id'],
|
||||
None)
|
||||
exp_code = exc.HTTPBadRequest.code
|
||||
self._router_interface_action('add',
|
||||
r['router']['id'],
|
||||
None,
|
||||
p['port']['id'],
|
||||
expected_code=exp_code)
|
||||
body = self._router_interface_action(
|
||||
'add', r['router']['id'], None, p['port']['id'],
|
||||
expected_code=exp_code)
|
||||
self._assert_body_port_id_and_update_port(
|
||||
body, update_port, p['port']['id'], r['router']['id'])
|
||||
self._router_interface_action('remove',
|
||||
r['router']['id'],
|
||||
s2['subnet']['id'],
|
||||
|
@ -1378,18 +1410,22 @@ class L3NatTestCaseBase(L3NatTestCaseMixin):
|
|||
with self.router() as r:
|
||||
with self.subnet() as s1, self.subnet(cidr='1.0.0.0/24') as s2:
|
||||
with self.port(subnet=s1) as p1, self.port(subnet=s2) as p2:
|
||||
with self.port(subnet=s1) as p3:
|
||||
orig_update_port = self.plugin.update_port
|
||||
with self.port(subnet=s1) as p3, (
|
||||
mock.patch.object(self.plugin,
|
||||
'update_port')) as update_port:
|
||||
update_port.side_effect = orig_update_port
|
||||
for p in [p1, p2]:
|
||||
self._router_interface_action('add',
|
||||
r['router']['id'],
|
||||
None,
|
||||
p['port']['id'])
|
||||
self._router_interface_action('add',
|
||||
r['router']['id'],
|
||||
None,
|
||||
p3['port']['id'],
|
||||
expected_code=exc.
|
||||
HTTPBadRequest.code)
|
||||
body = self._router_interface_action(
|
||||
'add', r['router']['id'], None, p3['port']['id'],
|
||||
expected_code=exc.HTTPBadRequest.code)
|
||||
self._assert_body_port_id_and_update_port(
|
||||
body, update_port, p3['port']['id'],
|
||||
r['router']['id'])
|
||||
|
||||
def test_router_add_interface_overlapped_cidr_returns_400(self):
|
||||
with self.router() as r:
|
||||
|
|
Loading…
Reference in New Issue