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
This commit is contained in:
parent
f3be2be987
commit
e70cf0e5d1
@ -4142,6 +4142,8 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base):
|
|||||||
sg = self.get_security_group(context, sg_id)
|
sg = self.get_security_group(context, sg_id)
|
||||||
self._prevent_non_admin_edit_provider_sg(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):
|
with db_api.CONTEXT_WRITER.using(context):
|
||||||
rules_db = (super(NsxPolicyPlugin,
|
rules_db = (super(NsxPolicyPlugin,
|
||||||
self).create_security_group_rule_bulk_native(
|
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)
|
is_provider_sg = sg.get(provider_sg.PROVIDER)
|
||||||
secgroup_logging = self._is_security_group_logged(context, sg_id)
|
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
|
# Create the NSX backend rules in a single transaction
|
||||||
|
|
||||||
def _do_update_rules():
|
def _do_update_rules():
|
||||||
@ -4164,20 +4164,23 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base):
|
|||||||
context, sg_id, rule_data, secgroup_logging,
|
context, sg_id, rule_data, secgroup_logging,
|
||||||
is_provider_sg=is_provider_sg)
|
is_provider_sg=is_provider_sg)
|
||||||
backend_rules.append(rule_entry)
|
backend_rules.append(rule_entry)
|
||||||
# Add the old rules
|
# Update the policy with the new rules only
|
||||||
for rule in sg['security_group_rules']:
|
self.nsxpolicy.comm_map.patch_entries(
|
||||||
rule_entry = self._create_security_group_backend_rule(
|
NSX_P_GLOBAL_DOMAIN_ID, sg_id, entries=backend_rules)
|
||||||
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 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)
|
|
||||||
|
|
||||||
|
try:
|
||||||
self._run_under_transaction(_do_update_rules)
|
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
|
return rules_db
|
||||||
|
|
||||||
|
@ -402,7 +402,7 @@ class TestNSXpProviderSecurityGrp(test_nsxp_plugin.NsxPPluginTestCaseMixin,
|
|||||||
sg_id = provider_secgroup['security_group']['id']
|
sg_id = provider_secgroup['security_group']['id']
|
||||||
|
|
||||||
with mock.patch("vmware_nsxlib.v3.policy.core_resources."
|
with mock.patch("vmware_nsxlib.v3.policy.core_resources."
|
||||||
"NsxPolicyCommunicationMapApi.update_with_entries"
|
"NsxPolicyCommunicationMapApi.patch_entries"
|
||||||
) as entry_create,\
|
) as entry_create,\
|
||||||
self.security_group_rule(security_group_id=sg_id):
|
self.security_group_rule(security_group_id=sg_id):
|
||||||
entry_create.assert_called_once()
|
entry_create.assert_called_once()
|
||||||
|
@ -1747,7 +1747,7 @@ class NsxPTestSecurityGroup(common_v3.FixExternalNetBaseTest,
|
|||||||
with self.security_group(name, description) as sg:
|
with self.security_group(name, description) as sg:
|
||||||
sg_id = sg['security_group']['id']
|
sg_id = sg['security_group']['id']
|
||||||
with mock.patch("vmware_nsxlib.v3.policy.core_resources."
|
with mock.patch("vmware_nsxlib.v3.policy.core_resources."
|
||||||
"NsxPolicyCommunicationMapApi.update_with_entries"
|
"NsxPolicyCommunicationMapApi.patch_entries"
|
||||||
) as update_policy,\
|
) as update_policy,\
|
||||||
self.security_group_rule(sg_id, direction,
|
self.security_group_rule(sg_id, direction,
|
||||||
protocol, port_range_min,
|
protocol, port_range_min,
|
||||||
|
Loading…
x
Reference in New Issue
Block a user