Merge "Return HTTP 400 for invalid server-group uuid"

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

@@ -84,6 +84,7 @@ CREATE_EXCEPTIONS = {
     exception.InstanceExists: exc.HTTPConflict,
     exception.NoUniqueMatch: exc.HTTPConflict,
     exception.Invalid: exc.HTTPBadRequest,
+    exception.InstanceGroupNotFound: exc.HTTPBadRequest,
 }
 
 CREATE_EXCEPTIONS_MSGS = {

nova/api/openstack/compute/schemas/scheduler_hints.py

@@ -19,9 +19,8 @@ _hints = {
     'type': 'object',
     'properties': {
         'group': {
-            # NOTE: The value of 'group' is stored to value which is
+            'type': 'string',
-            # defined as varchar(255) in instance_system_metadata table.
+            'format': 'uuid'
-            'type': 'string', 'maxLength': 255,
         },
         'different_host': {
             # NOTE: The value of 'different_host' is the set of server

nova/api/openstack/compute/servers.py

@@ -689,7 +689,8 @@ class ServersController(wsgi.Controller):
                 exception.ImageNUMATopologyCPUOutOfRange,
                 exception.ImageNUMATopologyCPUDuplicates,
                 exception.ImageNUMATopologyCPUsUnassigned,
-                exception.ImageNUMATopologyMemoryOutOfRange) as error:
+                exception.ImageNUMATopologyMemoryOutOfRange,
+                exception.InstanceGroupNotFound) as error:
             raise exc.HTTPBadRequest(explanation=error.format_message())
         except (exception.PortInUse,
                 exception.InstanceExists,

nova/compute/api.py

@@ -1031,6 +1031,8 @@ class API(base.Base):
         if not group_hint:
             return
 
+        # TODO(gibi): We need to remove the following validation code when
+        # removing legacy v2 code.
         if not uuidutils.is_uuid_like(group_hint):
             msg = _('Server group scheduler hint must be a UUID.')
             raise exception.InvalidInput(reason=msg)

nova/tests/unit/api/openstack/compute/test_scheduler_hints.py

@@ -15,6 +15,7 @@
 
 import datetime
 
+import mock
 from oslo_config import cfg
 from oslo_serialization import jsonutils
 
@@ -25,6 +26,7 @@ from nova.api.openstack.compute import servers as servers_v21
 from nova.api.openstack import extensions
 import nova.compute.api
 from nova.compute import flavors
+from nova import exception
 from nova import test
 from nova.tests.unit.api.openstack import fakes
 from nova.tests.unit import fake_instance
@@ -97,7 +99,11 @@ class SchedulerHintsTestCaseV21(test.TestCase):
         self.assertEqual(202, res.status_int)
 
     def test_create_server_with_group_hint(self):
-        self._test_create_server_with_hint({'group': 'foo'})
+        self._test_create_server_with_hint({'group': UUID})
+
+    def test_create_server_with_non_uuid_group_hint(self):
+        self._create_server_with_scheduler_hints_bad_request(
+                {'group': 'non-uuid'})
 
     def test_create_server_with_different_host_hint(self):
         self._test_create_server_with_hint(
@@ -160,6 +166,13 @@ class SchedulerHintsTestCaseV2(SchedulerHintsTestCaseV21):
         # We skip this test for v2.0.
         pass
 
+    @mock.patch(
+            'nova.api.openstack.compute.legacy_v2.servers.Controller.create')
+    def test_create_server_with_non_uuid_group_hint(self, mock_create):
+        mock_create.side_effect = exception.InvalidInput(reason='')
+        self._create_server_with_scheduler_hints_bad_request(
+                {'group': 'non-uuid'})
+
 
 class ServersControllerCreateTestV21(test.TestCase):
 

nova/tests/unit/api/openstack/compute/test_serversV21.py

@@ -3116,6 +3116,24 @@ class ServersControllerCreateTest(test.TestCase):
         test_group = objects.InstanceGroup.get_by_uuid(ctxt, test_group.uuid)
         self.assertIn(server['id'], test_group.members)
 
+    def test_create_instance_with_group_hint_group_not_found(self):
+        def fake_instance_destroy(context, uuid, constraint):
+            return fakes.stub_instance(1)
+
+        self.stub_out('nova.db.instance_destroy', fake_instance_destroy)
+        self.body['os:scheduler_hints'] = {
+            'group': '5b674f73-c8cf-40ef-9965-3b6fe4b304b1'}
+        self.req.body = jsonutils.dump_as_bytes(self.body)
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self.controller.create, self.req, body=self.body)
+
+    def test_create_instance_with_group_hint_wrong_uuid_format(self):
+        self.body['os:scheduler_hints'] = {
+            'group': 'non-uuid'}
+        self.req.body = jsonutils.dump_as_bytes(self.body)
+        self.assertRaises(exception.ValidationError,
+                          self.controller.create, self.req, body=self.body)
+
     def test_create_instance_with_neutronv2_port_in_use(self):
         network = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa'
         port = 'eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee'

nova/tests/unit/compute/test_compute.py

@@ -7832,9 +7832,27 @@ class ComputeAPITestCase(BaseTestCase):
         self.stubs.Set(fake_image._FakeImageService, 'show', self.fake_show)
 
         inst_type = flavors.get_default_flavor()
-        self.assertRaises(exception.InvalidInput, self.compute_api.create,
+        self.assertRaises(
-            self.context, inst_type, self.fake_image['id'],
+                exception.InvalidInput,
-            scheduler_hints={'group': 'groupname'})
+                self.compute_api.create,
+                self.context,
+                inst_type,
+                self.fake_image['id'],
+                scheduler_hints={'group': 'non-uuid'})
+
+    def test_instance_create_with_group_uuid_fails_group_not_exist(self):
+        self.stub_out('nova.tests.unit.image.fake._FakeImageService.show',
+                      self.fake_show)
+
+        inst_type = flavors.get_default_flavor()
+        self.assertRaises(
+                exception.InstanceGroupNotFound,
+                self.compute_api.create,
+                self.context,
+                inst_type,
+                self.fake_image['id'],
+                scheduler_hints={'group':
+                                     '5b674f73-c8cf-40ef-9965-3b6fe4b304b1'})
 
     def test_destroy_instance_disassociates_security_groups(self):
         # Make sure destroying disassociates security groups.