nova-net: Make the security group API a module

We're wrestling with multiple imports for this thing and have introduced
a cache to avoid having to load the thing repeatedly. However, Python
already has a way to ensure this doesn't happen: the use of a module.
Given that we don't have any state, we can straight up drop the class
and just call functions directly. Along the way, we drop the
'ensure_default' function, which is a no-op for neutron and switch all
the mocks over, where necessary.

Change-Id: Ia8dbe8ba61ec6d1b8498918a53a103a6eff4d488
Signed-off-by: Stephen Finucane <[email protected]>

Author: stephenfin

nova/api/metadata/base.py

@@ -33,7 +33,7 @@
 from nova import context
 from nova import exception
 from nova.network import neutron
-from nova.network.security_group import openstack_driver
+from nova.network import security_group_api
 from nova import objects
 from nova.objects import virt_device_metadata as metadata_obj
 from nova import utils
@@ -140,8 +140,7 @@ def __init__(self, instance, address=None, content=None, extra_md=None,
 
         self.availability_zone = instance.get('availability_zone')
 
-        secgroup_api = openstack_driver.get_openstack_security_group_driver()
-        self.security_groups = secgroup_api.get_instance_security_groups(
+        self.security_groups = security_group_api.get_instance_security_groups(
             ctxt, instance)
 
         self.mappings = _format_instance_mapping(ctxt, instance)

nova/api/openstack/compute/security_groups.py

@@ -28,7 +28,7 @@
 from nova.compute import api as compute
 from nova import exception
 from nova.i18n import _
-from nova.network.security_group import openstack_driver
+from nova.network import security_group_api
 from nova.policies import security_groups as sg_policies
 from nova.virt import netutils
 
@@ -48,10 +48,7 @@ class SecurityGroupControllerBase(object):
 
     def __init__(self):
         super(SecurityGroupControllerBase, self).__init__()
-        self.security_group_api = (
-            openstack_driver.get_openstack_security_group_driver())
-        self.compute_api = compute.API(
-            security_group_api=self.security_group_api)
+        self.compute_api = compute.API()
 
     def _format_security_group_rule(self, context, rule, group_rule_data=None):
         """Return a security group rule in desired API response format.
@@ -71,7 +68,7 @@ def _format_security_group_rule(self, context, rule, group_rule_data=None):
             sg_rule['group'] = group_rule_data
         elif rule['group_id']:
             try:
-                source_group = self.security_group_api.get(
+                source_group = security_group_api.get(
                     context, id=rule['group_id'])
             except exception.SecurityGroupNotFound:
                 # NOTE(arosen): There is a possible race condition that can
@@ -127,7 +124,7 @@ def _get_group_rule_data_by_rule_group_id(self, context, groups):
                 if (rule_group_id and
                         rule_group_id not in group_rule_data_by_rule_group_id):
                     try:
-                        source_group = self.security_group_api.get(
+                        source_group = security_group_api.get(
                             context, id=rule['group_id'])
                         group_rule_data_by_rule_group_id[rule_group_id] = {
                             'name': source_group.get('name'),
@@ -161,9 +158,9 @@ def show(self, req, id):
         context = _authorize_context(req)
 
         try:
-            id = self.security_group_api.validate_id(id)
-            security_group = self.security_group_api.get(context, None, id,
-                                                         map_exception=True)
+            id = security_group_api.validate_id(id)
+            security_group = security_group_api.get(
+                context, None, id, map_exception=True)
         except exception.SecurityGroupNotFound as exp:
             raise exc.HTTPNotFound(explanation=exp.format_message())
         except exception.Invalid as exp:
@@ -180,10 +177,10 @@ def delete(self, req, id):
         context = _authorize_context(req)
 
         try:
-            id = self.security_group_api.validate_id(id)
-            security_group = self.security_group_api.get(context, None, id,
-                                                         map_exception=True)
-            self.security_group_api.destroy(context, security_group)
+            id = security_group_api.validate_id(id)
+            security_group = security_group_api.get(
+                context, None, id, map_exception=True)
+            security_group_api.destroy(context, security_group)
         except exception.SecurityGroupNotFound as exp:
             raise exc.HTTPNotFound(explanation=exp.format_message())
         except exception.Invalid as exp:
@@ -200,9 +197,8 @@ def index(self, req):
         search_opts.update(req.GET)
 
         project_id = context.project_id
-        raw_groups = self.security_group_api.list(context,
-                                                  project=project_id,
-                                                  search_opts=search_opts)
+        raw_groups = security_group_api.list(
+            context, project=project_id, search_opts=search_opts)
 
         limited_list = common.limited(raw_groups, req)
         result = [self._format_security_group(context, group)
@@ -224,10 +220,10 @@ def create(self, req, body):
         group_description = security_group.get('description', None)
 
         try:
-            self.security_group_api.validate_property(group_name, 'name', None)
-            self.security_group_api.validate_property(group_description,
+            security_group_api.validate_property(group_name, 'name', None)
+            security_group_api.validate_property(group_description,
                                                       'description', None)
-            group_ref = self.security_group_api.create_security_group(
+            group_ref = security_group_api.create_security_group(
                 context, group_name, group_description)
         except exception.Invalid as exp:
             raise exc.HTTPBadRequest(explanation=exp.format_message())
@@ -244,9 +240,9 @@ def update(self, req, id, body):
         context = _authorize_context(req)
 
         try:
-            id = self.security_group_api.validate_id(id)
-            security_group = self.security_group_api.get(context, None, id,
-                                                         map_exception=True)
+            id = security_group_api.validate_id(id)
+            security_group = security_group_api.get(
+                context, None, id, map_exception=True)
         except exception.SecurityGroupNotFound as exp:
             raise exc.HTTPNotFound(explanation=exp.format_message())
         except exception.Invalid as exp:
@@ -257,10 +253,10 @@ def update(self, req, id, body):
         group_description = security_group_data.get('description', None)
 
         try:
-            self.security_group_api.validate_property(group_name, 'name', None)
-            self.security_group_api.validate_property(group_description,
-                                                      'description', None)
-            group_ref = self.security_group_api.update_security_group(
+            security_group_api.validate_property(group_name, 'name', None)
+            security_group_api.validate_property(
+                group_description, 'description', None)
+            group_ref = security_group_api.update_security_group(
                 context, security_group, group_name, group_description)
         except exception.SecurityGroupNotFound as exp:
             raise exc.HTTPNotFound(explanation=exp.format_message())
@@ -284,15 +280,14 @@ def create(self, req, body):
         source_group = {}
 
         try:
-            parent_group_id = self.security_group_api.validate_id(
+            parent_group_id = security_group_api.validate_id(
                 sg_rule.get('parent_group_id'))
-            security_group = self.security_group_api.get(context, None,
-                                                         parent_group_id,
-                                                         map_exception=True)
+            security_group = security_group_api.get(
+                context, None, parent_group_id, map_exception=True)
             if group_id is not None:
-                group_id = self.security_group_api.validate_id(group_id)
+                group_id = security_group_api.validate_id(group_id)
 
-                source_group = self.security_group_api.get(
+                source_group = security_group_api.get(
                     context, id=group_id)
             new_rule = self._rule_args_to_dict(context,
                               to_port=sg_rule.get('to_port'),
@@ -324,7 +319,7 @@ def create(self, req, body):
                                    'tenant_id': source_group.get('project_id')}
 
             security_group_rule = (
-                self.security_group_api.create_security_group_rule(
+                security_group_api.create_security_group_rule(
                     context, security_group, new_rule))
         except exception.Invalid as exp:
             raise exc.HTTPBadRequest(explanation=exp.format_message())
@@ -342,12 +337,12 @@ def _rule_args_to_dict(self, context, to_port=None, from_port=None,
                            ip_protocol=None, cidr=None, group_id=None):
 
         if group_id is not None:
-            return self.security_group_api.new_group_ingress_rule(
-                                    group_id, ip_protocol, from_port, to_port)
+            return security_group_api.new_group_ingress_rule(
+                group_id, ip_protocol, from_port, to_port)
         else:
-            cidr = self.security_group_api.parse_cidr(cidr)
-            return self.security_group_api.new_cidr_ingress_rule(
-                                        cidr, ip_protocol, from_port, to_port)
+            cidr = security_group_api.parse_cidr(cidr)
+            return security_group_api.new_cidr_ingress_rule(
+                cidr, ip_protocol, from_port, to_port)
 
     @wsgi.Controller.api_version("2.1", MAX_PROXY_API_SUPPORT_VERSION)
     @wsgi.expected_errors((400, 404, 409))
@@ -356,14 +351,13 @@ def delete(self, req, id):
         context = _authorize_context(req)
 
         try:
-            id = self.security_group_api.validate_id(id)
-            rule = self.security_group_api.get_rule(context, id)
+            id = security_group_api.validate_id(id)
+            rule = security_group_api.get_rule(context, id)
             group_id = rule['parent_group_id']
-            security_group = self.security_group_api.get(context, None,
-                                                         group_id,
-                                                         map_exception=True)
-            self.security_group_api.remove_rules(context, security_group,
-                                                 [rule['id']])
+            security_group = security_group_api.get(
+                context, None, group_id, map_exception=True)
+            security_group_api.remove_rules(
+                context, security_group, [rule['id']])
         except exception.SecurityGroupNotFound as exp:
             raise exc.HTTPNotFound(explanation=exp.format_message())
         except exception.NoUniqueMatch as exp:
@@ -379,11 +373,9 @@ def index(self, req, server_id):
         """Returns a list of security groups for the given instance."""
         context = _authorize_context(req)
 
-        self.security_group_api.ensure_default(context)
-
         instance = common.get_instance(self.compute_api, context, server_id)
         try:
-            groups = self.security_group_api.get_instance_security_groups(
+            groups = security_group_api.get_instance_security_groups(
                 context, instance, True)
         except (exception.SecurityGroupNotFound,
                 exception.InstanceNotFound) as exp:
@@ -408,10 +400,7 @@ def index(self, req, server_id):
 class SecurityGroupActionController(wsgi.Controller):
     def __init__(self):
         super(SecurityGroupActionController, self).__init__()
-        self.security_group_api = (
-            openstack_driver.get_openstack_security_group_driver())
-        self.compute_api = compute.API(
-            security_group_api=self.security_group_api)
+        self.compute_api = compute.API()
 
     def _parse(self, body, action):
         try:
@@ -443,7 +432,7 @@ def _addSecurityGroup(self, req, id, body):
 
         group_name = self._parse(body, 'addSecurityGroup')
         try:
-            return self._invoke(self.security_group_api.add_to_instance,
+            return self._invoke(security_group_api.add_to_instance,
                                 context, id, group_name)
         except (exception.SecurityGroupNotFound,
                 exception.InstanceNotFound) as exp:
@@ -464,7 +453,7 @@ def _removeSecurityGroup(self, req, id, body):
         group_name = self._parse(body, 'removeSecurityGroup')
 
         try:
-            return self._invoke(self.security_group_api.remove_from_instance,
+            return self._invoke(security_group_api.remove_from_instance,
                                 context, id, group_name)
         except (exception.SecurityGroupNotFound,
                 exception.InstanceNotFound) as exp:

nova/api/openstack/compute/views/servers.py

@@ -27,7 +27,7 @@
 from nova.compute import vm_states
 from nova import context as nova_context
 from nova import exception
-from nova.network.security_group import openstack_driver
+from nova.network import security_group_api
 from nova import objects
 from nova.objects import fields
 from nova.objects import virtual_interface
@@ -70,8 +70,6 @@ def __init__(self):
         self._image_builder = views_images.ViewBuilder()
         self._flavor_builder = views_flavors.ViewBuilder()
         self.compute_api = compute.API()
-        self.security_group_api = (
-            openstack_driver.get_openstack_security_group_driver())
 
     def create(self, request, instance):
         """View that should be returned when an instance is created."""
@@ -657,9 +655,8 @@ def _add_security_grps(self, req, servers, instances,
         if not create_request:
             context = req.environ['nova.context']
             sg_instance_bindings = (
-                self.security_group_api
-                .get_instances_security_groups_bindings(context,
-                                                        servers))
+                security_group_api.get_instances_security_groups_bindings(
+                    context, servers))
             for server in servers:
                 groups = sg_instance_bindings.get(server['id'])
                 if groups:

nova/compute/api.py

@@ -62,7 +62,7 @@
 from nova.network import constants
 from nova.network import model as network_model
 from nova.network import neutron
-from nova.network.security_group import openstack_driver
+from nova.network import security_group_api
 from nova import objects
 from nova.objects import base as obj_base
 from nova.objects import block_device as block_device_obj
@@ -287,13 +287,11 @@ class API(base.Base):
     """API for interacting with the compute manager."""
 
     def __init__(self, image_api=None, network_api=None, volume_api=None,
-                 security_group_api=None, **kwargs):
+                 **kwargs):
         self.image_api = image_api or image.API()
         self.network_api = network_api or neutron.API()
         self.volume_api = volume_api or cinder.API()
         self._placementclient = None  # Lazy-load on first access.
-        self.security_group_api = (security_group_api or
-            openstack_driver.get_openstack_security_group_driver())
         self.compute_rpcapi = compute_rpcapi.ComputeAPI()
         self.compute_task_api = conductor.ComputeTaskAPI()
         self.servicegroup_api = servicegroup.API()
@@ -399,7 +397,7 @@ def _check_requested_secgroups(self, context, secgroups):
             if secgroup == "default":
                 security_groups.append(secgroup)
                 continue
-            secgroup_dict = self.security_group_api.get(context, secgroup)
+            secgroup_dict = security_group_api.get(context, secgroup)
             if not secgroup_dict:
                 raise exception.SecurityGroupNotFoundForProject(
                     project_id=context.project_id, security_group_id=secgroup)
@@ -1167,9 +1165,8 @@ def _provision_instances(self, context, instance_type, min_count,
         # Check quotas
         num_instances = compute_utils.check_num_instances_quota(
                 context, instance_type, min_count, max_count)
-        security_groups = self.security_group_api.populate_security_groups(
+        security_groups = security_group_api.populate_security_groups(
                 security_groups)
-        self.security_group_api.ensure_default(context)
         port_resource_requests = base_options.pop('port_resource_requests')
         instances_to_build = []
         # We could be iterating over several instances with several BDMs per