Remove security_group_handler

Now that nova's security groups are more plugable and decoupled from the
database the security_group_handler code is not very useful so this patch
removes it.

A little info about the security_group_handler:
The security_group_handle code was added to provide a hook into nova security
groups so that one could get security group add/delete/update notification
and proxy them somewhere else (i.e quantum). Trying to actually using this
method opens one up to several transactional issues because in the current
implemenation the security group is commited to the nova database before
security_group_handler is called.

Implements blueprint: remove-security-group-handler

Change-Id: I45047fe8dbb81555505e03309838910113080c37

9 files changed, 5 insertions, 263 deletions

diff --git a/etc/nova/nova.conf.sample b/etc/nova/nova.conf.sample
index 762576e84..1897d9a65 100644
--- a/etc/nova/nova.conf.sample
+++ b/etc/nova/nova.conf.sample
@@ -1210,11 +1210,6 @@
 # The full class name of the security API class (string value)
 #security_group_api=nova
 
-# The full class name of the security group handler class
-# (string value)
-#security_group_handler=nova.network.sg.NullSecurityGroupHandler
-
-
 #
 # Options defined in bin.nova-clear-rabbit-queues
 #
diff --git a/nova/compute/api.py b/nova/compute/api.py
index 454fd90d9..512b991d7 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -191,7 +191,6 @@ class API(base.Base):
         self.volume_api = volume_api or volume.API()
         self.security_group_api = (security_group_api or
             openstack_driver.get_openstack_security_group_driver())
-        self.sgh = openstack_driver.get_security_group_handler()
         self.consoleauth_rpcapi = consoleauth_rpcapi.ConsoleAuthAPI()
         self.scheduler_rpcapi = scheduler_rpcapi.SchedulerAPI()
         self.compute_rpcapi = compute_rpcapi.ComputeAPI()
@@ -921,9 +920,6 @@ class API(base.Base):
         self._populate_instance_shutdown_terminate(instance, image,
                                                    block_device_mapping)
 
-        # ensure_default security group is called before the instance
-        # is created so the creation of the default security group is
-        # proxied to the sgh.
         self.security_group_api.ensure_default(context)
         instance = self.db.instance_create(context, instance)
 
@@ -2833,7 +2829,6 @@ class SecurityGroupAPI(base.Base, security_group_base.SecurityGroupBase):
     def __init__(self, **kwargs):
         super(SecurityGroupAPI, self).__init__(**kwargs)
         self.security_group_rpcapi = compute_rpcapi.SecurityGroupAPI()
-        self.sgh = openstack_driver.get_security_group_handler()
 
     def validate_property(self, value, property, allowed):
         """
@@ -2876,9 +2871,7 @@ class SecurityGroupAPI(base.Base, security_group_base.SecurityGroupBase):
 
         :param context: the security context
         """
-        existed, group = self.db.security_group_ensure_default(context)
-        if not existed:
-            self.sgh.trigger_security_group_create_refresh(context, group)
+        self.db.security_group_ensure_default(context)
 
     def create_security_group(self, context, name, description):
         try:
@@ -2902,7 +2895,6 @@ class SecurityGroupAPI(base.Base, security_group_base.SecurityGroupBase):
                      'name': name,
                      'description': description}
             group_ref = self.db.security_group_create(context, group)
-            self.sgh.trigger_security_group_create_refresh(context, group)
             # Commit the reservation
             QUOTAS.commit(context, reservations)
         except Exception:
@@ -2979,9 +2971,6 @@ class SecurityGroupAPI(base.Base, security_group_base.SecurityGroupBase):
                   context=context)
         self.db.security_group_destroy(context, security_group['id'])
 
-        self.sgh.trigger_security_group_destroy_refresh(context,
-                                                        security_group['id'])
-
         # Commit the reservations
         if reservations:
             QUOTAS.commit(context, reservations)
@@ -3027,9 +3016,6 @@ class SecurityGroupAPI(base.Base, security_group_base.SecurityGroupBase):
         self.security_group_rpcapi.refresh_security_group_rules(context,
                 security_group['id'], host=instance['host'])
 
-        self.trigger_handler('instance_add_security_group',
-                context, instance, security_group_name)
-
     @wrap_check_security_groups_policy
     def remove_from_instance(self, context, instance, security_group_name):
         """Remove the security group associated with the instance."""
@@ -3053,9 +3039,6 @@ class SecurityGroupAPI(base.Base, security_group_base.SecurityGroupBase):
         self.security_group_rpcapi.refresh_security_group_rules(context,
                 security_group['id'], host=instance['host'])
 
-        self.trigger_handler('instance_remove_security_group',
-                context, instance, security_group_name)
-
     def get_rule(self, context, id):
         self.ensure_default(context)
         try:
@@ -3086,8 +3069,6 @@ class SecurityGroupAPI(base.Base, security_group_base.SecurityGroupBase):
         rules = [self.db.security_group_rule_create(context, v) for v in vals]
 
         self.trigger_rules_refresh(context, id=id)
-        self.trigger_handler('security_group_rule_create', context,
-                             [r['id'] for r in rules])
         return rules
 
     def remove_rules(self, context, security_group, rule_ids):
@@ -3099,7 +3080,6 @@ class SecurityGroupAPI(base.Base, security_group_base.SecurityGroupBase):
 
         # NOTE(vish): we removed some rules, so refresh
         self.trigger_rules_refresh(context, id=security_group['id'])
-        self.trigger_handler('security_group_rule_destroy', context, rule_ids)
 
     def remove_default_rules(self, context, rule_ids):
         for rule_id in rule_ids:
@@ -3148,10 +3128,6 @@ class SecurityGroupAPI(base.Base, security_group_base.SecurityGroupBase):
             msg = _("Security group id should be integer")
             self.raise_invalid_property(msg)
 
-    def trigger_handler(self, event, *args):
-        handle = getattr(self.sgh, 'trigger_%s_refresh' % event)
-        handle(*args)
-
     def trigger_rules_refresh(self, context, id):
         """Called when a rule is added to or removed from a security_group."""
 
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index 72ade9857..d789b04a5 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -1441,7 +1441,7 @@ def instance_create(context, values):
 
     def _get_sec_group_models(session, security_groups):
         models = []
-        _existed, default_group = security_group_ensure_default(context,
+        default_group = security_group_ensure_default(context,
             session=session)
         if 'default' in security_groups:
             models.append(default_group)
@@ -3227,17 +3227,11 @@ def security_group_create(context, values, session=None):
 
 
 def security_group_ensure_default(context, session=None):
-    """Ensure default security group exists for a project_id.
-
-    Returns a tuple with the first element being a bool indicating
-    if the default security group previously existed. Second
-    element is the dict used to create the default security group.
-    """
+    """Ensure default security group exists for a project_id."""
     try:
         default_group = security_group_get_by_name(context,
                 context.project_id, 'default',
                 columns_to_join=[], session=session)
-        return (True, default_group)
     except exception.NotFound:
         values = {'name': 'default',
                   'description': 'default',
@@ -3255,7 +3249,7 @@ def security_group_ensure_default(context, session=None):
                            'parent_group_id': default_group.id,
             }
             security_group_rule_create(context, rule_values)
-        return (False, default_group)
+    return default_group
 
 
 @require_context
diff --git a/nova/network/manager.py b/nova/network/manager.py
index f8959dceb..54c079245 100644
--- a/nova/network/manager.py
+++ b/nova/network/manager.py
@@ -387,21 +387,6 @@ class NetworkManager(manager.Manager):
 
         self.security_group_api.trigger_members_refresh(admin_context,
                                                         group_ids)
-        self.security_group_api.trigger_handler('security_group_members',
-                                                admin_context, group_ids)
-
-    def _do_trigger_security_group_handler(self, handler, instance_id):
-        admin_context = context.get_admin_context(read_deleted="yes")
-        if uuidutils.is_uuid_like(instance_id):
-            instance_ref = self.db.instance_get_by_uuid(admin_context,
-                                                        instance_id)
-        else:
-            instance_ref = self.db.instance_get(admin_context,
-                                                instance_id)
-        for group_name in [group['name'] for group
-                in instance_ref['security_groups']]:
-            self.security_group_api.trigger_handler(handler, admin_context,
-                    instance_ref, group_name)
 
     def get_floating_ips_by_fixed_address(self, context, fixed_address):
         # NOTE(jkoelker) This is just a stub function. Managers supporting
@@ -856,8 +841,6 @@ class NetworkManager(manager.Manager):
                         context.elevated(), network['id'], instance_id)
                 self._do_trigger_security_group_members_refresh_for_instance(
                     instance_id)
-                self._do_trigger_security_group_handler(
-                    'instance_add_security_group', instance_id)
                 get_vif = self.db.virtual_interface_get_by_instance_and_network
                 vif = get_vif(context, instance_id, network['id'])
                 values = {'allocated': True,
@@ -898,8 +881,6 @@ class NetworkManager(manager.Manager):
 
         self._do_trigger_security_group_members_refresh_for_instance(
             instance_uuid)
-        self._do_trigger_security_group_handler(
-            'instance_remove_security_group', instance_uuid)
 
         # NOTE(vish) This db query could be removed if we pass az and name
         #            (or the whole instance object).
diff --git a/nova/network/quantumv2/api.py b/nova/network/quantumv2/api.py
index 6b2cac9bd..688918739 100644
--- a/nova/network/quantumv2/api.py
+++ b/nova/network/quantumv2/api.py
@@ -280,9 +280,6 @@ class API(base.Base):
                             LOG.debug(msg, {'portid': port_id,
                                             'exception': ex})
 
-        self.trigger_security_group_members_refresh(context, instance)
-        self.trigger_instance_add_security_group_refresh(context, instance)
-
         nw_info = self._get_instance_nw_info(context, instance, networks=nets)
         # NOTE(danms): Only return info about ports we created in this run.
         # In the initial allocation case, this will be everything we created,
@@ -329,8 +326,6 @@ class API(base.Base):
             except Exception as ex:
                 LOG.exception(_("Failed to delete quantum port %(portid)s ")
                               % {'portid': port['id']})
-        self.trigger_security_group_members_refresh(context, instance)
-        self.trigger_instance_remove_security_group_refresh(context, instance)
 
     @refresh_cache
     def allocate_port_for_instance(self, context, instance, port_id,
@@ -354,9 +349,6 @@ class API(base.Base):
             LOG.exception(_("Failed to delete quantum port %(port_id)s ") %
                           locals())
 
-        self.trigger_security_group_members_refresh(context, instance)
-        self.trigger_instance_remove_security_group_refresh(context, instance)
-
         return self._get_instance_nw_info(context, instance)
 
     def list_ports(self, context, **search_opts):
@@ -507,32 +499,6 @@ class API(base.Base):
         ip = ip.replace('\\.', '.')
         return self._get_instance_uuids_by_ip(context, ip)
 
-    def trigger_instance_add_security_group_refresh(self, context,
-                                                    instance_ref):
-        """Refresh and add security groups given an instance reference."""
-        admin_context = context.elevated()
-        for group in instance_ref['security_groups']:
-            self.conductor_api.security_groups_trigger_handler(context,
-                'instance_add_security_group', instance_ref, group['name'])
-
-    def trigger_instance_remove_security_group_refresh(self, context,
-                                                       instance_ref):
-        """Refresh and remove security groups given an instance reference."""
-        admin_context = context.elevated()
-        for group in instance_ref['security_groups']:
-            self.conductor_api.security_groups_trigger_handler(context,
-                'instance_remove_security_group', instance_ref, group['name'])
-
-    def trigger_security_group_members_refresh(self, context, instance_ref):
-        """Refresh security group members."""
-        admin_context = context.elevated()
-        group_ids = [group['id'] for group in instance_ref['security_groups']]
-
-        self.conductor_api.security_groups_trigger_members_refresh(
-            admin_context, group_ids)
-        self.conductor_api.security_groups_trigger_handler(admin_context,
-            'security_group_members', group_ids)
-
     def _get_port_id_by_fixed_address(self, client,
                                       instance, address):
         """Return port_id from a fixed address."""
diff --git a/nova/network/security_group/openstack_driver.py b/nova/network/security_group/openstack_driver.py
index 46f3f3491..152ed5841 100644
--- a/nova/network/security_group/openstack_driver.py
+++ b/nova/network/security_group/openstack_driver.py
@@ -25,9 +25,6 @@ security_group_opts = [
     cfg.StrOpt('security_group_api',
                default='nova',
                help='The full class name of the security API class'),
-    cfg.StrOpt('security_group_handler',
-               default='nova.network.sg.NullSecurityGroupHandler',
-               help='The full class name of the security group handler class'),
 ]
 
 CONF = cfg.CONF
@@ -48,10 +45,6 @@ def get_openstack_security_group_driver():
         return importutils.import_object(CONF.security_group_api)
 
 
-def get_security_group_handler():
-    return importutils.import_object(CONF.security_group_handler)
-
-
 def is_quantum_security_groups():
     if CONF.security_group_api.lower() == "quantum":
         return True
diff --git a/nova/network/sg.py b/nova/network/sg.py
deleted file mode 100644
index 1eac9aa97..000000000
--- a/nova/network/sg.py
+++ /dev/null
@@ -1,154 +0,0 @@
-# vim: tabstop=4 shiftwidth=4 softtabstop=4
-
-# Copyright 2012 Nicira Networks, Inc
-# All Rights Reserved.
-#
-#    Licensed under the Apache License, Version 2.0 (the "License"); you may
-#    not use this file except in compliance with the License. You may obtain
-#    a copy of the License at
-#
-#         http://www.apache.org/licenses/LICENSE-2.0
-#
-#    Unless required by applicable law or agreed to in writing, software
-#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
-#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
-#    License for the specific language governing permissions and limitations
-#    under the License.
-
-'''Implement Security Groups abstraction and API.
-
-The nova security_group_handler flag specifies which class is to be used
-to implement the security group calls.
-
-The NullSecurityGroupHandler provides a "no-op" plugin that is loaded
-by default and has no impact on current system behavior.  In the future,
-special purposes classes that inherit from SecurityGroupHandlerBase
-will provide enhanced functionality and will be loadable via the
-security_group_handler flag.
-'''
-
-
-class SecurityGroupHandlerBase(object):
-
-    def __init__(self):
-        raise NotImplementedError()
-
-    def trigger_security_group_create_refresh(self, context, group):
-        '''Called when a security group is created
-
-        :param context: the security context.
-        :param group: the new group added. group is a dictionary that contains
-            the following: user_id, project_id, name, description).'''
-        raise NotImplementedError()
-
-    def trigger_security_group_destroy_refresh(self, context,
-                                               security_group_id):
-        '''Called when a security group is deleted
-
-        :param context: the security context.
-        :param security_group_id: the security group identifier.'''
-        raise NotImplementedError()
-
-    def trigger_security_group_rule_create_refresh(self, context,
-                                                   rule_ids):
-        '''Called when a rule is added to a security_group.
-
-        :param context: the security context.
-        :param rule_ids: a list of rule ids that have been affected.'''
-        raise NotImplementedError()
-
-    def trigger_security_group_rule_destroy_refresh(self, context,
-                                                     rule_ids):
-        '''Called when a rule is removed from a security_group.
-
-        :param context: the security context.
-        :param rule_ids: a list of rule ids that have been affected.'''
-        raise NotImplementedError()
-
-    def trigger_instance_add_security_group_refresh(self, context, instance,
-                                                    group_name):
-        '''Called when a security group gains a new member.
-
-        :param context: the security context.
-        :param instance: the instance to be associated.
-        :param group_name: the name of the security group to be associated.'''
-        raise NotImplementedError()
-
-    def trigger_instance_remove_security_group_refresh(self, context, instance,
-                                                       group_name):
-        '''Called when a security group loses a member.
-
-        :param context: the security context.
-        :param instance: the instance to be associated.
-        :param group_name: the name of the security group to be associated.'''
-        raise NotImplementedError()
-
-    def trigger_security_group_members_refresh(self, context, group_ids):
-        '''Called when a security group gains or loses a member.
-
-        :param context: the security context.
-        :param group_ids: a list of security group identifiers.'''
-        raise NotImplementedError()
-
-
-class NullSecurityGroupHandler(SecurityGroupHandlerBase):
-
-    def __init__(self):
-        pass
-
-    def trigger_security_group_create_refresh(self, context, group):
-        '''Called when a rule is added to a security_group.
-
-        :param context: the security context.
-        :param group: the new group added. group is a dictionary that contains
-            the following: user_id, project_id, name, description).'''
-        pass
-
-    def trigger_security_group_destroy_refresh(self, context,
-                                               security_group_id):
-        '''Called when a rule is added to a security_group.
-
-        :param context: the security context.
-        :param security_group_id: the security group identifier.'''
-        pass
-
-    def trigger_security_group_rule_create_refresh(self, context,
-                                                   rule_ids):
-        '''Called when a rule is added to a security_group.
-
-        :param context: the security context.
-        :param rule_ids: a list of rule ids that have been affected.'''
-        pass
-
-    def trigger_security_group_rule_destroy_refresh(self, context,
-                                                     rule_ids):
-        '''Called when a rule is removed from a security_group.
-
-        :param context: the security context.
-        :param rule_ids: a list of rule ids that have been affected.'''
-        pass
-
-    def trigger_instance_add_security_group_refresh(self, context, instance,
-                                                    group_name):
-        '''Called when a security group gains a new member.
-
-        :param context: the security context.
-        :param instance: the instance to be associated.
-        :param group_name: the name of the security group to be associated.'''
-        pass
-
-    def trigger_instance_remove_security_group_refresh(self, context, instance,
-                                                       group_name):
-        '''Called when a security group loses a member.
-
-        :param context: the security context.
-        :param instance: the instance to be associated.
-        :param group_name: the name of the security group to be associated.'''
-        pass
-
-    def trigger_security_group_members_refresh(self, context, group_ids):
-        '''Called when a security group gains or loses a member.
-
-        :param context: the security context.
-        :param group_ids: a list of security group identifiers.'''
-        pass
diff --git a/nova/tests/api/openstack/compute/contrib/test_security_group_default_rules.py b/nova/tests/api/openstack/compute/contrib/test_security_group_default_rules.py
index 2b4145764..0528c09f5 100644
--- a/nova/tests/api/openstack/compute/contrib/test_security_group_default_rules.py
+++ b/nova/tests/api/openstack/compute/contrib/test_security_group_default_rules.py
@@ -282,7 +282,7 @@ class TestSecurityGroupDefaultRules(test.TestCase):
 
         setattr(ctxt, 'project_id', 'new_project_id')
 
-        _, sg = nova.db.security_group_ensure_default(ctxt)
+        sg = nova.db.security_group_ensure_default(ctxt)
         rules = nova.db.security_group_rule_get_by_security_group(ctxt, sg.id)
         security_group_rule = rules[0]
         self.assertEqual(sgr['id'], security_group_rule.id)
diff --git a/nova/tests/network/test_manager.py b/nova/tests/network/test_manager.py
index b47987b52..3fe2d6c74 100644
--- a/nova/tests/network/test_manager.py
+++ b/nova/tests/network/test_manager.py
@@ -318,9 +318,6 @@ class FlatNetworkTestCase(test.TestCase):
 
         db.instance_get_by_uuid(mox.IgnoreArg(),
                 mox.IgnoreArg()).AndReturn({'security_groups':
-                                            [{'id': 0}]})
-        db.instance_get_by_uuid(mox.IgnoreArg(),
-                mox.IgnoreArg()).AndReturn({'security_groups':
                                             [{'id': 0, 'name': 'test'}]})
 
         db.virtual_interface_get_by_instance_and_network(mox.IgnoreArg(),
@@ -364,9 +361,6 @@ class FlatNetworkTestCase(test.TestCase):
 
         db.instance_get_by_uuid(mox.IgnoreArg(),
                 mox.IgnoreArg()).AndReturn({'security_groups':
-                                            [{'id': 0}]})
-        db.instance_get_by_uuid(mox.IgnoreArg(),
-                mox.IgnoreArg()).AndReturn({'security_groups':
                                             [{'id': 0, 'name': 'test'}]})
 
         db.virtual_interface_get_by_instance_and_network(mox.IgnoreArg(),
@@ -454,9 +448,6 @@ class FlatNetworkTestCase(test.TestCase):
 
         db.instance_get_by_uuid(mox.IgnoreArg(),
                 mox.IgnoreArg()).AndReturn({'security_groups':
-                                            [{'id': 0}]})
-        db.instance_get_by_uuid(mox.IgnoreArg(),
-                mox.IgnoreArg()).AndReturn({'security_groups':
                                             [{'id': 0, 'name': 'test'}]})
 
         db.virtual_interface_get_by_instance_and_network(mox.IgnoreArg(),