From f9f694f95f5338d62f75ce54677dd165dc87a8d5 Mon Sep 17 00:00:00 2001 From: ZHU ZHU Date: Fri, 5 Sep 2014 03:01:07 -0500 Subject: [PATCH] Reduce security group db calls to neutron server Within ovs agent daemon loop, prepare_devices_filter will impose heavy workloads to neutron server in order to retrieve the security groups message to apply firewall rules. If agent is configured to use Noopfirewall driver or security groups are disabled, there is no need for loading the rules from server and refreshing the firewalls. This will reduce the number of db calls and improve performance for neutron server in this case. Change-Id: Id244aab3cac37fc6ed3dc05cbee91cdf9e34d9cc Closes-Bug: #1365806 (cherry picked from commit 524981cce05a9b365036c0a1e9810036936d3d5b) --- neutron/agent/securitygroups_rpc.py | 41 ++++++++++++----- .../tests/unit/test_security_groups_rpc.py | 45 +++++++++++++++++++ 2 files changed, 74 insertions(+), 12 deletions(-) diff --git a/neutron/agent/securitygroups_rpc.py b/neutron/agent/securitygroups_rpc.py index 17b544502..3da4d347f 100644 --- a/neutron/agent/securitygroups_rpc.py +++ b/neutron/agent/securitygroups_rpc.py @@ -14,11 +14,14 @@ # under the License. # +import functools + from oslo.config import cfg from oslo import messaging +from neutron.agent import firewall from neutron.common import topics -from neutron.openstack.common.gettextutils import _LW +from neutron.openstack.common.gettextutils import _LI, _LW from neutron.openstack.common import importutils from neutron.openstack.common import log as logging @@ -74,9 +77,9 @@ def _disable_extension(extension, aliases): def disable_security_group_extension_by_config(aliases): if not is_firewall_enabled(): - LOG.info(_('Disabled security-group extension.')) + LOG.info(_LI('Disabled security-group extension.')) _disable_extension('security-group', aliases) - LOG.info(_('Disabled allowed-address-pairs extension.')) + LOG.info(_LI('Disabled allowed-address-pairs extension.')) _disable_extension('allowed-address-pairs', aliases) @@ -187,10 +190,23 @@ class SecurityGroupAgentRpcMixin(object): return False return True + def skip_if_noopfirewall_or_firewall_disabled(func): + @functools.wraps(func) + def decorated_function(self, *args, **kwargs): + if (isinstance(self.firewall, firewall.NoopFirewallDriver) or + not is_firewall_enabled()): + LOG.info(_LI("Skipping method %s as firewall is disabled " + "or configured as NoopFirewallDriver."), + func.__name__) + else: + return func(self, *args, **kwargs) + return decorated_function + + @skip_if_noopfirewall_or_firewall_disabled def prepare_devices_filter(self, device_ids): if not device_ids: return - LOG.info(_("Preparing filters for devices %s"), device_ids) + LOG.info(_LI("Preparing filters for devices %s"), device_ids) if self.use_enhanced_rpc: devices_info = self.plugin_rpc.security_group_info_for_devices( self.context, list(device_ids)) @@ -220,15 +236,15 @@ class SecurityGroupAgentRpcMixin(object): remote_sg_id, member_ips) def security_groups_rule_updated(self, security_groups): - LOG.info(_("Security group " - "rule updated %r"), security_groups) + LOG.info(_LI("Security group " + "rule updated %r"), security_groups) self._security_group_updated( security_groups, 'security_groups') def security_groups_member_updated(self, security_groups): - LOG.info(_("Security group " - "member updated %r"), security_groups) + LOG.info(_LI("Security group " + "member updated %r"), security_groups) self._security_group_updated( security_groups, 'security_group_source_groups') @@ -249,7 +265,7 @@ class SecurityGroupAgentRpcMixin(object): self.refresh_firewall(devices) def security_groups_provider_updated(self): - LOG.info(_("Provider rule updated")) + LOG.info(_LI("Provider rule updated")) if self.defer_refresh_firewall: # NOTE(salv-orlando): A 'global refresh' might not be # necessary if the subnet for which the provider rules @@ -261,7 +277,7 @@ class SecurityGroupAgentRpcMixin(object): def remove_devices_filter(self, device_ids): if not device_ids: return - LOG.info(_("Remove device filter for %r"), device_ids) + LOG.info(_LI("Remove device filter for %r"), device_ids) with self.firewall.defer_apply(): for device_id in device_ids: device = self.firewall.ports.get(device_id) @@ -269,12 +285,13 @@ class SecurityGroupAgentRpcMixin(object): continue self.firewall.remove_port_filter(device) + @skip_if_noopfirewall_or_firewall_disabled def refresh_firewall(self, device_ids=None): - LOG.info(_("Refresh firewall rules")) + LOG.info(_LI("Refresh firewall rules")) if not device_ids: device_ids = self.firewall.ports.keys() if not device_ids: - LOG.info(_("No ports here to refresh firewall")) + LOG.info(_LI("No ports here to refresh firewall")) return if self.use_enhanced_rpc: devices_info = self.plugin_rpc.security_group_info_for_devices( diff --git a/neutron/tests/unit/test_security_groups_rpc.py b/neutron/tests/unit/test_security_groups_rpc.py index 7f20c7d9e..3acc378b1 100644 --- a/neutron/tests/unit/test_security_groups_rpc.py +++ b/neutron/tests/unit/test_security_groups_rpc.py @@ -1021,6 +1021,7 @@ class BaseSecurityGroupAgentRpcTestCase(base.BaseTestCase): self.agent.root_helper = 'sudo' self.agent.plugin_rpc = mock.Mock() self.agent.init_firewall(defer_refresh_firewall=defer_refresh_firewall) + self.default_firewall = self.agent.firewall self.firewall = mock.Mock() firewall_object = firewall_base.FirewallDriver() self.firewall.defer_apply.side_effect = firewall_object.defer_apply @@ -1057,6 +1058,26 @@ class SecurityGroupAgentRpcTestCase(BaseSecurityGroupAgentRpcTestCase): self.fake_device), ]) + def test_prepare_devices_filter_with_noopfirewall(self): + self.agent.firewall = self.default_firewall + self.agent.plugin_rpc.security_group_info_for_devices = mock.Mock() + self.agent.plugin_rpc.security_group_rules_for_devices = mock.Mock() + self.agent.prepare_devices_filter(['fake_device']) + self.assertFalse(self.agent.plugin_rpc. + security_group_info_for_devices.called) + self.assertFalse(self.agent.plugin_rpc. + security_group_rules_for_devices.called) + + def test_prepare_devices_filter_with_firewall_disabled(self): + cfg.CONF.set_override('enable_security_group', False, 'SECURITYGROUP') + self.agent.plugin_rpc.security_group_info_for_devices = mock.Mock() + self.agent.plugin_rpc.security_group_rules_for_devices = mock.Mock() + self.agent.prepare_devices_filter(['fake_device']) + self.assertFalse(self.agent.plugin_rpc. + security_group_info_for_devices.called) + self.assertFalse(self.agent.plugin_rpc. + security_group_rules_for_devices.called) + def test_security_groups_rule_updated(self): self.agent.refresh_firewall = mock.Mock() self.agent.prepare_devices_filter(['fake_port_id']) @@ -1111,6 +1132,30 @@ class SecurityGroupAgentRpcTestCase(BaseSecurityGroupAgentRpcTestCase): self.agent.refresh_firewall([]) self.assertFalse(self.firewall.called) + def test_refresh_firewall_with_firewall_disabled(self): + cfg.CONF.set_override('enable_security_group', False, 'SECURITYGROUP') + self.agent.plugin_rpc.security_group_info_for_devices = mock.Mock() + self.agent.plugin_rpc.security_group_rules_for_devices = mock.Mock() + self.agent.firewall.defer_apply = mock.Mock() + self.agent.refresh_firewall([self.fake_device]) + self.assertFalse(self.agent.plugin_rpc. + security_group_info_for_devices.called) + self.assertFalse(self.agent.plugin_rpc. + security_group_rules_for_devices.called) + self.assertFalse(self.agent.firewall.defer_apply.called) + + def test_refresh_firewall_with_noopfirewall(self): + self.agent.firewall = self.default_firewall + self.agent.plugin_rpc.security_group_info_for_devices = mock.Mock() + self.agent.plugin_rpc.security_group_rules_for_devices = mock.Mock() + self.agent.firewall.defer_apply = mock.Mock() + self.agent.refresh_firewall([self.fake_device]) + self.assertFalse(self.agent.plugin_rpc. + security_group_info_for_devices.called) + self.assertFalse(self.agent.plugin_rpc. + security_group_rules_for_devices.called) + self.assertFalse(self.agent.firewall.defer_apply.called) + class SecurityGroupAgentEnhancedRpcTestCase( BaseSecurityGroupAgentRpcTestCase): -- 2.45.2