x/vmware-nsx
Code Issues Proposed changes

NSX: Optionally not enforce nat rule match length check

Browse Source 
This patch adds the 'raise_on_len_mismatch' parameter to the
'delete_nat_rules_by_match' function. The plugin then leverages
this parameter for ensuring NAT rules deletion operations
are completed successfully even when duplicate rules are found
or no corresponding rules are found at all.

With this change, the 'remove_router_interface' operation will
correctly complete even in cases when NAT rules in Neutron and
the NSX backend are out of sync.

This patch also changes a check in delete_nat_rules_by_match in
order to make it less expensive.

Closes-Bug: 1328181

Change-Id: I856d67ef5ff6264374cb8f2569668da4c205ad9f
This commit is contained in:
Salvatore Orlando 2014-06-09 09:53:47 -07:00
parent 019444170b
commit 529bc090e9
4 changed files with 81 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
neutron/plugins/vmware/nsxlib/router.py
View File

@ -551,6 +551,7 @@ def create_lrouter_dnat_rule_v3(cluster, router_id, dst_ip, to_dst_port=None,
def delete_nat_rules_by_match(cluster, router_id, rule_type,
max_num_expected,
min_num_expected=0,
raise_on_len_mismatch=True,
**kwargs):
# remove nat rules
nat_rules = query_nat_rules(cluster, router_id)
@ -564,14 +565,26 @@ def delete_nat_rules_by_match(cluster, router_id, rule_type,
break
else:
to_delete_ids.append(r['uuid'])
if not (len(to_delete_ids) in
range(min_num_expected, max_num_expected + 1)):
raise nsx_exc.NatRuleMismatch(actual_rules=len(to_delete_ids),
min_rules=min_num_expected,
max_rules=max_num_expected)
num_rules_to_delete = len(to_delete_ids)
if (num_rules_to_delete < min_num_expected or
num_rules_to_delete > max_num_expected):
if raise_on_len_mismatch:
raise nsx_exc.NatRuleMismatch(actual_rules=num_rules_to_delete,
min_rules=min_num_expected,
max_rules=max_num_expected)
else:
LOG.warn(_("Found %(actual_rule_num)d matching NAT rules, which "
"is not in the expected range (%(min_exp_rule_num)d,"
"%(max_exp_rule_num)d)"),
{'actual_rule_num': num_rules_to_delete,
'min_exp_rule_num': min_num_expected,
'max_exp_rule_num': max_num_expected})
for rule_id in to_delete_ids:
delete_router_nat_rule(cluster, router_id, rule_id)
# Return number of deleted rules - useful at least for
# testing purposes
return num_rules_to_delete
def delete_router_nat_rule(cluster, router_id, rule_id):

3
neutron/plugins/vmware/plugins/base.py
View File

@ -298,6 +298,7 @@ class NsxPluginV2(addr_pair_db.AllowedAddressPairsMixin,
routerlib.delete_nat_rules_by_match(
self.cluster, nsx_router_id, "SourceNatRule",
max_num_expected=1, min_num_expected=0,
raise_on_len_mismatch=False,
source_ip_addresses=cidr)
if add_snat_rules:
ip_addresses = self._build_ip_address_list(
@ -1703,6 +1704,7 @@ class NsxPluginV2(addr_pair_db.AllowedAddressPairsMixin,
routerlib.delete_nat_rules_by_match(
self.cluster, nsx_router_id, "SourceNatRule",
max_num_expected=1, min_num_expected=1,
raise_on_len_mismatch=False,
source_ip_addresses=subnet['cidr'])
def add_router_interface(self, context, router_id, interface_info):
@ -1806,6 +1808,7 @@ class NsxPluginV2(addr_pair_db.AllowedAddressPairsMixin,
routerlib.delete_nat_rules_by_match(
self.cluster, nsx_router_id, "NoSourceNatRule",
max_num_expected=1, min_num_expected=0,
raise_on_len_mismatch=False,
destination_ip_addresses=subnet['cidr'])
except n_exc.NotFound:
LOG.error(_("Logical router resource %s not found "

26
neutron/tests/unit/vmware/nsxlib/test_router.py
View File

@ -920,3 +920,29 @@ class TestLogicalRouters(base.NsxlibTestCase):
routerlib.delete_nat_rules_by_match,
self.fake_cluster, lrouter['uuid'],
'SomeWeirdType', 1, 1)
def test_delete_nat_rules_by_match_len_mismatch_does_not_raise(self):
lrouter = self._prepare_nat_rules_for_delete_tests()
rules = routerlib.query_nat_rules(self.fake_cluster, lrouter['uuid'])
self.assertEqual(len(rules), 3)
deleted_rules = routerlib.delete_nat_rules_by_match(
self.fake_cluster, lrouter['uuid'],
'DestinationNatRule',
max_num_expected=1, min_num_expected=1,
raise_on_len_mismatch=False,
destination_ip_addresses='99.99.99.99')
self.assertEqual(0, deleted_rules)
# add an extra rule to emulate a duplicate one
with mock.patch.object(self.fake_cluster.api_client,
'get_version',
new=lambda: version_module.Version('2.0')):
routerlib.create_lrouter_snat_rule(
self.fake_cluster, lrouter['uuid'],
'10.0.0.2', '10.0.0.2', order=220,
match_criteria={'source_ip_addresses': '192.168.0.0/24'})
deleted_rules_2 = routerlib.delete_nat_rules_by_match(
self.fake_cluster, lrouter['uuid'], 'SourceNatRule',
min_num_expected=1, max_num_expected=1,
raise_on_len_mismatch=False,
source_ip_addresses='192.168.0.0/24')
self.assertEqual(2, deleted_rules_2)

37
neutron/tests/unit/vmware/test_nsx_plugin.py
View File

@ -14,6 +14,7 @@
# limitations under the License.
import contextlib
import uuid
import mock
import netaddr
@ -1165,12 +1166,21 @@ class NeutronNsxOutOfSync(NsxPluginV2TestCase,
res = req.get_response(self.ext_api)
self.assertEqual(res.status_int, 200)
def test_remove_router_interface_not_in_nsx(self):
def _test_remove_router_interface_nsx_out_of_sync(self, unsync_action):
# Create external network and subnet
ext_net_id = self._create_network_and_subnet('1.1.1.0/24', True)[0]
# Create internal network and subnet
int_sub_id = self._create_network_and_subnet('10.0.0.0/24')[1]
res = self._create_router('json', 'tenant')
router = self.deserialize('json', res)
# Add interface to router (needed to generate NAT rule)
# Set gateway and add interface to router (needed to generate NAT rule)
req = self.new_update_request(
'routers',
{'router': {'external_gateway_info':
{'network_id': ext_net_id}}},
router['router']['id'])
res = req.get_response(self.ext_api)
self.assertEqual(res.status_int, 200)
req = self.new_action_request(
'routers',
{'subnet_id': int_sub_id},
@ -1178,7 +1188,7 @@ class NeutronNsxOutOfSync(NsxPluginV2TestCase,
"add_router_interface")
res = req.get_response(self.ext_api)
self.assertEqual(res.status_int, 200)
self.fc._fake_lrouter_dict.clear()
unsync_action()
req = self.new_action_request(
'routers',
{'subnet_id': int_sub_id},
@ -1187,6 +1197,27 @@ class NeutronNsxOutOfSync(NsxPluginV2TestCase,
res = req.get_response(self.ext_api)
self.assertEqual(res.status_int, 200)
def test_remove_router_interface_not_in_nsx(self):
def unsync_action():
self.fc._fake_lrouter_dict.clear()
self.fc._fake_lrouter_nat_dict.clear()
self._test_remove_router_interface_nsx_out_of_sync(unsync_action)
def test_remove_router_interface_nat_rule_not_in_nsx(self):
self._test_remove_router_interface_nsx_out_of_sync(
self.fc._fake_lrouter_nat_dict.clear)
def test_remove_router_interface_duplicate_nat_rules_in_nsx(self):
def unsync_action():
# duplicate every entry in the nat rule dict
for (_rule_id, rule) in self.fc._fake_lrouter_nat_dict.items():
self.fc._fake_lrouter_nat_dict[uuid.uuid4()] = rule
self._test_remove_router_interface_nsx_out_of_sync(unsync_action)
def test_update_router_not_in_nsx(self):
res = self._create_router('json', 'tenant')
router = self.deserialize('json', res)