NSX: Optionally not enforce nat rule match length check
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
(cherry picked from commit 138d977400
)
This commit is contained in:
parent
d57e3b345c
commit
3a8594624a
|
@ -544,6 +544,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)
|
||||
|
@ -557,14 +558,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):
|
||||
|
|
|
@ -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(
|
||||
|
@ -1682,6 +1683,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):
|
||||
|
@ -1786,6 +1788,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 "
|
||||
|
|
|
@ -21,6 +21,7 @@ from oslo.config import cfg
|
|||
from neutron.common import exceptions
|
||||
from neutron.openstack.common import uuidutils
|
||||
from neutron.plugins.vmware.api_client import exception as api_exc
|
||||
from neutron.plugins.vmware.api_client import version as version_module
|
||||
from neutron.plugins.vmware.api_client.version import Version
|
||||
from neutron.plugins.vmware.common import exceptions as nsx_exc
|
||||
from neutron.plugins.vmware.common import utils
|
||||
|
@ -920,3 +921,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)
|
||||
|
|
|
@ -14,6 +14,7 @@
|
|||
# limitations under the License.
|
||||
|
||||
import contextlib
|
||||
import uuid
|
||||
|
||||
import mock
|
||||
import netaddr
|
||||
|
@ -1150,12 +1151,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},
|
||||
|
@ -1163,7 +1173,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},
|
||||
|
@ -1172,6 +1182,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)
|
||||
|
|
Loading…
Reference in New Issue