From e70cf0e5d1d88c23d0021daeeb4fe6a3e5975b02 Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Fri, 22 Oct 2021 07:29:31 -0700 Subject: [PATCH] Improve security group rule add performance and reliability This change leverages a new NSX client method, patch_entries. This method does not require all rules to be in the request body. We can therefore save a DB operation, and submit a much smaller payload. NSX responses are also much faster. In addition, this routine ensure the DB record for a security group rule is removed if the creation of the same rule fails at the NSX backend. Change-Id: I5c97c3042f8f740cac211314e11ce01e03beaa7e --- vmware_nsx/plugins/nsx_p/plugin.py | 33 ++++++++++--------- .../test_provider_security_groups.py | 2 +- vmware_nsx/tests/unit/nsx_p/test_plugin.py | 2 +- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/vmware_nsx/plugins/nsx_p/plugin.py b/vmware_nsx/plugins/nsx_p/plugin.py index f9e8f598eb..4c2134271f 100644 --- a/vmware_nsx/plugins/nsx_p/plugin.py +++ b/vmware_nsx/plugins/nsx_p/plugin.py @@ -4142,6 +4142,8 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base): sg = self.get_security_group(context, sg_id) self._prevent_non_admin_edit_provider_sg(context, sg_id) + # The DB process adds attributed that need to be used for creating + # NSX resources. Keep DB update before NSX operation with db_api.CONTEXT_WRITER.using(context): rules_db = (super(NsxPolicyPlugin, self).create_security_group_rule_bulk_native( @@ -4152,8 +4154,6 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base): is_provider_sg = sg.get(provider_sg.PROVIDER) secgroup_logging = self._is_security_group_logged(context, sg_id) - category = (NSX_P_PROVIDER_SECTION_CATEGORY if is_provider_sg - else NSX_P_REGULAR_SECTION_CATEGORY) # Create the NSX backend rules in a single transaction def _do_update_rules(): @@ -4164,20 +4164,23 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base): context, sg_id, rule_data, secgroup_logging, is_provider_sg=is_provider_sg) backend_rules.append(rule_entry) - # Add the old rules - for rule in sg['security_group_rules']: - rule_entry = self._create_security_group_backend_rule( - context, sg_id, rule, secgroup_logging, - is_provider_sg=is_provider_sg, - create_related_resource=True) - backend_rules.append(rule_entry) + # Update the policy with the new rules only + self.nsxpolicy.comm_map.patch_entries( + NSX_P_GLOBAL_DOMAIN_ID, sg_id, entries=backend_rules) - # Update the policy with all the rules. - self.nsxpolicy.comm_map.update_with_entries( - NSX_P_GLOBAL_DOMAIN_ID, sg_id, entries=backend_rules, - category=category, use_child_rules=False) - - self._run_under_transaction(_do_update_rules) + try: + self._run_under_transaction(_do_update_rules) + except Exception as e: + LOG.error("Unable to create securiy group rules on backend: %s", e) + # Rollback DB operation + with excutils.save_and_reraise_exception(): + rule_ids = [] + for rule_data in rules_db: + super(NsxPolicyPlugin, self).delete_security_group_rule( + context, rule_data['id']) + rule_ids.append(rule_data['id']) + LOG.info("Security group rules %s removed from Neutron DB " + "due to failed NSX transaction", ",".join(rule_ids)) return rules_db diff --git a/vmware_nsx/tests/unit/extensions/test_provider_security_groups.py b/vmware_nsx/tests/unit/extensions/test_provider_security_groups.py index a2974bf5a6..209cdb7b8c 100644 --- a/vmware_nsx/tests/unit/extensions/test_provider_security_groups.py +++ b/vmware_nsx/tests/unit/extensions/test_provider_security_groups.py @@ -402,7 +402,7 @@ class TestNSXpProviderSecurityGrp(test_nsxp_plugin.NsxPPluginTestCaseMixin, sg_id = provider_secgroup['security_group']['id'] with mock.patch("vmware_nsxlib.v3.policy.core_resources." - "NsxPolicyCommunicationMapApi.update_with_entries" + "NsxPolicyCommunicationMapApi.patch_entries" ) as entry_create,\ self.security_group_rule(security_group_id=sg_id): entry_create.assert_called_once() diff --git a/vmware_nsx/tests/unit/nsx_p/test_plugin.py b/vmware_nsx/tests/unit/nsx_p/test_plugin.py index e55660139a..f92c7161b1 100644 --- a/vmware_nsx/tests/unit/nsx_p/test_plugin.py +++ b/vmware_nsx/tests/unit/nsx_p/test_plugin.py @@ -1747,7 +1747,7 @@ class NsxPTestSecurityGroup(common_v3.FixExternalNetBaseTest, with self.security_group(name, description) as sg: sg_id = sg['security_group']['id'] with mock.patch("vmware_nsxlib.v3.policy.core_resources." - "NsxPolicyCommunicationMapApi.update_with_entries" + "NsxPolicyCommunicationMapApi.patch_entries" ) as update_policy,\ self.security_group_rule(sg_id, direction, protocol, port_range_min,