From bd54462ad61f789a3e3a35021bd4879900397c32 Mon Sep 17 00:00:00 2001 From: asarfaty Date: Thu, 27 Feb 2020 09:34:38 +0200 Subject: [PATCH] Fix firewall section add rule/s retry Stale revision retry should include the revison number Change-Id: Ibad52cca60131e970447536fd22c4f4440c66d34 --- .../tests/unit/v3/nsxlib_testcase.py | 15 -------- vmware_nsxlib/tests/unit/v3/test_security.py | 38 +++++++++++++++++++ vmware_nsxlib/v3/security.py | 36 +++++++++++------- 3 files changed, 60 insertions(+), 29 deletions(-) diff --git a/vmware_nsxlib/tests/unit/v3/nsxlib_testcase.py b/vmware_nsxlib/tests/unit/v3/nsxlib_testcase.py index 27c01fe0..c63c69bc 100644 --- a/vmware_nsxlib/tests/unit/v3/nsxlib_testcase.py +++ b/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"), diff --git a/vmware_nsxlib/tests/unit/v3/test_security.py b/vmware_nsxlib/tests/unit/v3/test_security.py index f23c581c..f869e537 100644 --- a/vmware_nsxlib/tests/unit/v3/test_security.py +++ b/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""" diff --git a/vmware_nsxlib/v3/security.py b/vmware_nsxlib/v3/security.py index b4b8c506..99508e5d 100644 --- a/vmware_nsxlib/v3/security.py +++ b/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) - params = '?operation=%s' % operation - if (version.LooseVersion(self.nsxlib.get_version()) >= - version.LooseVersion(consts.NSX_VERSION_2_4_0)): - rule['_revision'] = self.get(section_id)['_revision'] - return self._create_with_retry(resource + params, rule) + @utils.retry_upon_exception(exceptions.StaleRevision, + max_attempts=self.client.max_attempts) + def _do_add_rule(): + resource = '%s/rules' % self.get_path(section_id) + params = '?operation=%s' % operation + 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) - params = '?action=create_multiple&operation=%s' % operation - if (version.LooseVersion(self.nsxlib.get_version()) >= - version.LooseVersion(consts.NSX_VERSION_2_4_0)): - rev_id = self.get(section_id)['_revision'] - for rule in rules: - rule['_revision'] = rev_id - return self._create_with_retry(resource + params, {'rules': rules}) + @utils.retry_upon_exception(exceptions.StaleRevision, + max_attempts=self.client.max_attempts) + def _do_add_rules(): + resource = '%s/rules' % self.get_path(section_id) + params = '?action=create_multiple&operation=%s' % operation + if (version.LooseVersion(self.nsxlib.get_version()) >= + version.LooseVersion(consts.NSX_VERSION_2_4_0)): + 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)