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

diff --git a/neutron/plugins/vmware/nsxlib/router.py b/neutron/plugins/vmware/nsxlib/router.py
index 52d34299f20cca2149434fa036ff21f32c9679eb..254b4b6a5177f1a70a87be95ce509bbcf031a3be 100644 (file)
--- a/neutron/plugins/vmware/nsxlib/router.py
+++ b/neutron/plugins/vmware/nsxlib/router.py
@@ -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):

diff --git a/neutron/plugins/vmware/plugins/base.py b/neutron/plugins/vmware/plugins/base.py
index 072fd20eba27affbde5c08dda8f25925142e4a74..3895a042baecef5e4c7514cd91860ff95d762162 100644 (file)
--- a/neutron/plugins/vmware/plugins/base.py
+++ b/neutron/plugins/vmware/plugins/base.py
@@ -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 "

diff --git a/neutron/tests/unit/vmware/nsxlib/test_router.py b/neutron/tests/unit/vmware/nsxlib/test_router.py
index 6d4063fe5127064bac69444f03e99336979615d0..39aa2e4a29f27553567394191b256679c7edc59c 100644 (file)
--- a/neutron/tests/unit/vmware/nsxlib/test_router.py
+++ b/neutron/tests/unit/vmware/nsxlib/test_router.py
@@ -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)

diff --git a/neutron/tests/unit/vmware/test_nsx_plugin.py b/neutron/tests/unit/vmware/test_nsx_plugin.py
index 21b28513aaf4d69932e3182dc05e0676f7fea264..d898159bbae9f0d8f232e2e2a24ce42345d92276 100644 (file)
--- a/neutron/tests/unit/vmware/test_nsx_plugin.py
+++ b/neutron/tests/unit/vmware/test_nsx_plugin.py
@@ -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)