Fix firewall section add rule/s retry

Stale revision retry should include the revison number

Change-Id: Ibad52cca60131e970447536fd22c4f4440c66d34

vmware_nsxlib/tests/unit/v3/nsxlib_testcase.py

@@ -56,17 +56,6 @@ def _mock_nsxlib():
     def _return_id_key(*args, **kwargs):
         return {'id': uuidutils.generate_uuid()}
 
-    def _mock_add_rules_in_section(*args):
-        # NOTE(arosen): the code in the neutron plugin expects the
-        # neutron rule id as the display_name.
-        rules = args[0]
-        return {
-            'rules': [
-                {'display_name': rule['display_name'],
-                 'id': uuidutils.generate_uuid()}
-                for rule in rules
-            ]}
-
     def _mock_limits(*args):
         return utils.TagLimits(20, 40, 15)
 
@@ -90,10 +79,6 @@ def _mock_nsxlib():
     mocking.append(mock.patch(
         "vmware_nsxlib.v3.security.NsxLibNsGroup.list"))
 
-    mocking.append(mock.patch(
-        "vmware_nsxlib.v3.security.NsxLibFirewallSection.add_rules",
-        side_effect=_mock_add_rules_in_section))
-
     mocking.append(mock.patch(
         ("vmware_nsxlib.v3.core_resources."
          "NsxLibTransportZone.get_id_by_name_or_id"),

vmware_nsxlib/tests/unit/v3/test_security.py

@@ -299,6 +299,44 @@ class TestNsxLibFirewallSection(nsxlib_testcase.NsxLibTestCase):
                 data = {'tags': fws_tags}
                 update.assert_called_with(resource, data, headers=None)
 
+    def test_create_rules_using_add_rules(self):
+        revision = 5
+        with mock.patch("vmware_nsxlib.v3.NsxLib.get_version",
+                        return_value='2.5.0'),\
+            mock.patch.object(self.nsxlib.client, 'get',
+                              return_value={'_revision': revision}),\
+            mock.patch.object(self.nsxlib.client, 'create') as create:
+            rule_id = uuidutils.generate_uuid()
+            rule = {'id': rule_id,
+                    'ethertype': 'IPv4',
+                    'protocol': 'ipip',
+                    'direction': 'ingress',
+                    'remote_ip_prefix': None}
+            rules = [rule]
+            section_id = 'section-id'
+            group_id = 'nsgroup-id'
+            target_id = 'dummy'
+            self.nsxlib.firewall_section.create_rules(
+                None, section_id, group_id, False,
+                "ALLOW", rules, {rule_id: target_id})
+            expected_rule = {'display_name': mock.ANY,
+                             'ip_protocol': 'IPV4',
+                             'direction': 'IN',
+                             'services': [{'service': {
+                                 'resource_type': 'IPProtocolNSService',
+                                 'protocol_number': 4}}],
+                             '_revision': revision,
+                             'disabled': False,
+                             'sources': [{'target_id': target_id,
+                                          'target_type': 'NSGroup'}],
+                             'destinations': [{'target_id': group_id,
+                                               'target_type': 'NSGroup'}],
+                             'logged': False, 'action': 'ALLOW'}
+            create.assert_called_once_with(
+                'firewall/sections/%s/rules?action=create_multiple&'
+                'operation=insert_bottom' % section_id,
+                {'rules': [expected_rule]})
+
 
 class TestNsxLibIPSet(nsxlib_testcase.NsxClientTestCase):
     """Tests for vmware_nsxlib.v3.security.NsxLibIPSet"""

vmware_nsxlib/v3/security.py

@@ -469,22 +469,30 @@ class NsxLibFirewallSection(utils.NsxLibApiBase):
         return rule_dict
 
     def add_rule(self, rule, section_id, operation=consts.FW_INSERT_BOTTOM):
-        resource = '%s/rules' % self.get_path(section_id)
+        @utils.retry_upon_exception(exceptions.StaleRevision,
-        params = '?operation=%s' % operation
+                                    max_attempts=self.client.max_attempts)
-        if (version.LooseVersion(self.nsxlib.get_version()) >=
+        def _do_add_rule():
-            version.LooseVersion(consts.NSX_VERSION_2_4_0)):
+            resource = '%s/rules' % self.get_path(section_id)
-            rule['_revision'] = self.get(section_id)['_revision']
+            params = '?operation=%s' % operation
-        return self._create_with_retry(resource + params, rule)
+            if (version.LooseVersion(self.nsxlib.get_version()) >=
+                version.LooseVersion(consts.NSX_VERSION_2_4_0)):
+                rule['_revision'] = self.get(section_id)['_revision']
+            return self.client.create(resource + params, rule)
+        return _do_add_rule()
 
     def add_rules(self, rules, section_id, operation=consts.FW_INSERT_BOTTOM):
-        resource = '%s/rules' % self.get_path(section_id)
+        @utils.retry_upon_exception(exceptions.StaleRevision,
-        params = '?action=create_multiple&operation=%s' % operation
+                                    max_attempts=self.client.max_attempts)
-        if (version.LooseVersion(self.nsxlib.get_version()) >=
+        def _do_add_rules():
-            version.LooseVersion(consts.NSX_VERSION_2_4_0)):
+            resource = '%s/rules' % self.get_path(section_id)
-            rev_id = self.get(section_id)['_revision']
+            params = '?action=create_multiple&operation=%s' % operation
-            for rule in rules:
+            if (version.LooseVersion(self.nsxlib.get_version()) >=
-                rule['_revision'] = rev_id
+                version.LooseVersion(consts.NSX_VERSION_2_4_0)):
-        return self._create_with_retry(resource + params, {'rules': rules})
+                rev_id = self.get(section_id)['_revision']
+                for rule in rules:
+                    rule['_revision'] = rev_id
+            return self.client.create(resource + params, {'rules': rules})
+        return _do_add_rules()
 
     def delete_rule(self, section_id, rule_id):
         resource = '%s/rules/%s' % (section_id, rule_id)