Merge lp:~tpatil/nova/bug838466 into lp:~hudson-openstack/nova/trunk

[Tushar Patil]

Instance record is not inserted in db if the security group passed to the RunInstances API doesn't exists.

[Vish Ishaya (vishvananda)]

lgtm

review: Approve

[Christopher MacGown (0x44)]

Hi Tushar,

I've created a failing test for the previous revision that passes with your code @ lp:~0x44/nova/bug838466 , please merge or write your own. Otherwise, lgtm.

review: Needs Fixing

[Tushar Patil (tpatil)]

I should have written this unit test at the first place. Anyway thank you very much for your help.

Merged your unit test, Please review again.

[Christopher MacGown (0x44)]

review: Approve

=== modified file 'nova/compute/api.py'
--- nova/compute/api.py	2011-08-31 23:07:29 +0000
+++ nova/compute/api.py	2011-09-03 00:13:26 +0000
@@ -383,10 +383,6 @@
         If you are changing this method, be sure to update both
         call paths.
         """
-        instance = dict(launch_index=num, **base_options)
-        instance = self.db.instance_create(context, instance)
-        instance_id = instance['id']
-
         elevated = context.elevated()
         if security_group is None:
             security_group = ['default']
@@ -400,6 +396,10 @@
                     security_group_name)
             security_groups.append(group['id'])
 
+        instance = dict(launch_index=num, **base_options)
+        instance = self.db.instance_create(context, instance)
+        instance_id = instance['id']
+
         for security_group_id in security_groups:
             self.db.instance_add_security_group(elevated,
                                                 instance_id,

=== modified file 'nova/tests/test_compute.py'
--- nova/tests/test_compute.py	2011-08-29 12:34:40 +0000
+++ nova/tests/test_compute.py	2011-09-03 00:13:26 +0000
@@ -161,6 +161,19 @@
             db.security_group_destroy(self.context, group['id'])
             db.instance_destroy(self.context, ref[0]['id'])
 
+    def test_create_instance_with_invalid_security_group_raises(self):
+        instance_type = instance_types.get_default_instance_type()
+
+        pre_build_len = len(db.instance_get_all(context.get_admin_context()))
+        self.assertRaises(exception.SecurityGroupNotFoundForProject,
+                          self.compute_api.create,
+                          self.context,
+                          instance_type=instance_type,
+                          image_href=None,
+                          security_group=['this_is_a_fake_sec_group'])
+        self.assertEqual(pre_build_len,
+                         len(db.instance_get_all(context.get_admin_context())))
+
     def test_create_instance_associates_config_drive(self):
         """Make sure create associates a config drive."""