neutron: actually populate list in populate_security_groups

We're going to store the list of requested security group objects
in the request spec so the neutron security group driver code
needs to actually populate that list now for use in the compute API.

As the nova-network and neutron security group API code is basically
the same for populating that list, the backend-specific implementations
are removed and pulled up into the base security group driver API class.

Since we don't want to actually try and persist this list with the
instance object in the nova database we need to add a check in the
compute API to store an empty list, like the old behavior, when
creating the instance. A later change will use the populated list
to store in the RequestSpec.

Part of blueprint cells-scheduling-interaction

Change-Id: I6139eb92f534f010565fde31661382d33463ae58

nova/compute/api.py

@@ -73,7 +73,6 @@ from nova.objects import block_device as block_device_obj
 from nova.objects import fields as fields_obj
 from nova.objects import keypair as keypair_obj
 from nova.objects import quotas as quotas_obj
-from nova.objects import security_group as security_group_obj
 from nova.pci import request as pci_request
 import nova.policy
 from nova import rpc
@@ -1447,7 +1446,13 @@ class API(base.Base):
 
         instance.system_metadata.update(system_meta)
 
-        instance.security_groups = security_groups
+        if CONF.use_neutron:
+            # For Neutron we don't actually store anything in the database, we
+            # proxy the security groups on the instance from the ports
+            # attached to the instance.
+            instance.security_groups = objects.SecurityGroupList()
+        else:
+            instance.security_groups = security_groups
 
         self._populate_instance_names(instance, num_instances)
         instance.shutdown_terminate = shutdown_terminate
@@ -4800,9 +4805,3 @@ class SecurityGroupAPI(base.Base, security_group_base.SecurityGroupBase):
             return self.db.security_group_get_by_instance(context,
                                                           instance.uuid)
         return [{'name': group.name} for group in instance.security_groups]
-
-    def populate_security_groups(self, security_groups):
-        if not security_groups:
-            # Make sure it's an empty SecurityGroupList and not None
-            return objects.SecurityGroupList()
-        return security_group_obj.make_secgroup_list(security_groups)

nova/network/security_group/neutron_driver.py

@@ -27,7 +27,6 @@ from nova import exception
 from nova.i18n import _, _LE, _LI, _LW
 from nova.network.neutronv2 import api as neutronapi
 from nova.network.security_group import security_group_base
-from nova import objects
 from nova import utils
 
 
@@ -542,11 +541,6 @@ class SecurityGroupAPI(security_group_base.SecurityGroupBase):
                     'instance': instance.uuid})
             self.raise_not_found(msg)
 
-    def populate_security_groups(self, security_groups):
-        # Returning an empty list since we do not want to populate this field
-        # in the nova database if using the neutron driver
-        return objects.SecurityGroupList()
-
     def get_default_rule(self, context, id):
         msg = _("Network driver does not support this function.")
         raise exc.HTTPNotImplemented(explanation=msg)

nova/network/security_group/security_group_base.py

@@ -21,6 +21,7 @@ from six.moves import urllib
 
 from nova import exception
 from nova.i18n import _
+from nova.objects import security_group as security_group_obj
 from nova import utils
 
 
@@ -167,10 +168,16 @@ class SecurityGroupBase(object):
         pass
 
     def populate_security_groups(self, security_groups):
-        """Called when populating the database for an instances
-        security groups.
+        """Build and return a SecurityGroupList.
+
+        :param security_groups: list of requested security group names or uuids
+        :type security_groups: list
+        :returns: nova.objects.security_group.SecurityGroupList
         """
-        raise NotImplementedError()
+        if not security_groups:
+            # Make sure it's an empty SecurityGroupList and not None
+            return security_group_obj.SecurityGroupList()
+        return security_group_obj.make_secgroup_list(security_groups)
 
     def create_security_group(self, context, name, description):
         raise NotImplementedError()

nova/objects/security_group.py

@@ -12,6 +12,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from oslo_utils import uuidutils
 from oslo_utils import versionutils
 
 from nova import db
@@ -123,7 +124,7 @@ class SecurityGroupList(base.ObjectListBase, base.NovaObject):
 
 
 def make_secgroup_list(security_groups):
-    """A helper to make security group objects from a list of names.
+    """A helper to make security group objects from a list of names or uuids.
 
     Note that this does not make them save-able or have the rest of the
     attributes they would normally have, but provides a quick way to fill,
@@ -131,8 +132,14 @@ def make_secgroup_list(security_groups):
     """
     secgroups = objects.SecurityGroupList()
     secgroups.objects = []
-    for name in security_groups:
+    for sg in security_groups:
         secgroup = objects.SecurityGroup()
-        secgroup.name = name
+        if uuidutils.is_uuid_like(sg):
+            # This is a neutron security group uuid so store in the uuid field.
+            secgroup.uuid = sg
+        else:
+            # This is either a nova-network security group name, or it's the
+            # special 'default' security group in the case of neutron.
+            secgroup.name = sg
         secgroups.objects.append(secgroup)
     return secgroups

nova/tests/unit/compute/test_compute_api.py

@@ -43,6 +43,7 @@ from nova.objects import base as obj_base
 from nova.objects import block_device as block_device_obj
 from nova.objects import fields as fields_obj
 from nova.objects import quotas as quotas_obj
+from nova.objects import security_group as secgroup_obj
 from nova import quota
 from nova import test
 from nova.tests import fixtures
@@ -4718,6 +4719,23 @@ class _ComputeAPIUnitTestMixIn(object):
                           self.compute_api.update_instance,
                           self.context, instance, updates)
 
+    def test_populate_instance_for_create_neutron_secgroups(self):
+        """Tests that a list of security groups passed in do not actually get
+        stored on with the instance when using neutron.
+        """
+        self.flags(use_neutron=True)
+        flavor = self._create_flavor()
+        params = {'display_name': 'fake-instance'}
+        instance = self._create_instance_obj(params, flavor)
+        security_groups = objects.SecurityGroupList()
+        security_groups.objects = [
+            secgroup_obj.SecurityGroup(uuid=uuids.secgroup_id)
+        ]
+        instance = self.compute_api._populate_instance_for_create(
+            self.context, instance, {}, 0, security_groups, flavor, 1,
+            False)
+        self.assertEqual(0, len(instance.security_groups))
+
 
 class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
     def setUp(self):

nova/tests/unit/network/security_group/test_neutron_driver.py

@@ -410,6 +410,8 @@ class TestNeutronDriverWithoutMock(test.NoDBTestCase):
 
     def test_populate_security_groups(self):
         sg_api = neutron_driver.SecurityGroupAPI()
-        r = sg_api.populate_security_groups('ignore')
+        r = sg_api.populate_security_groups(['default', uuids.secgroup_uuid])
         self.assertIsInstance(r, objects.SecurityGroupList)
-        self.assertEqual(0, len(r))
+        self.assertEqual(2, len(r))
+        self.assertEqual('default', r[0].name)
+        self.assertEqual(uuids.secgroup_uuid, r[1].uuid)