diff options
author | Brian Lamar <brian.lamar@rackspace.com> | 2011-10-26 14:16:01 -0500 |
---|---|---|
committer | Brian Lamar <brian.lamar@rackspace.com> | 2011-10-31 17:00:38 -0400 |
commit | a225fa6acf9ea8689f66b5e415c4680795bac465 (patch) | |
tree | 8fc3faf798da71d1f5b08a44c0cd81b7382e8fdc | |
parent | 24298bb882ce8f8572e90fb59718398a921e10ff (diff) | |
download | nova-a225fa6acf9ea8689f66b5e415c4680795bac465.tar.gz nova-a225fa6acf9ea8689f66b5e415c4680795bac465.tar.xz nova-a225fa6acf9ea8689f66b5e415c4680795bac465.zip |
Too much information is returned from POST /servers
Currently we're returning a lot of information in the POST /servers
response. This isn't according to spec and is misleading to other
projects that might try and implement the OpenStack API specification.
The example on docs.openstack.com for the v1.1 API says:
"Note that when creating a server only the server ID, its links, and
the admin password are guaranteed to be returned in the request.
Additional attributes may be retrieved by performing subsequent GETs
on the server."
We're returning too much right now and this patch addresses that.
Further patches should be submitted to refactor the 'view builder'
concept as using keyword arguments like this patch does is not ideal
and I recognize that.
(Patch Set 2) Merged with master + conflict fix
(Patch Set 3) Reverted non-nova-standard superclass call
Change-Id: If246e51dbf84f1db3d2905694235692ab027859d
-rw-r--r-- | nova/api/openstack/contrib/createserverext.py | 10 | ||||
-rw-r--r-- | nova/api/openstack/servers.py | 8 | ||||
-rw-r--r-- | nova/api/openstack/views/servers.py | 10 | ||||
-rw-r--r-- | nova/tests/api/openstack/contrib/test_volumes.py | 3 | ||||
-rw-r--r-- | nova/tests/api/openstack/test_servers.py | 41 | ||||
-rw-r--r-- | nova/tests/integrated/test_servers.py | 4 |
6 files changed, 21 insertions, 55 deletions
diff --git a/nova/api/openstack/contrib/createserverext.py b/nova/api/openstack/contrib/createserverext.py index da95164e8..ab5037304 100644 --- a/nova/api/openstack/contrib/createserverext.py +++ b/nova/api/openstack/contrib/createserverext.py @@ -21,10 +21,12 @@ from nova.api.openstack import wsgi class CreateServerController(servers.Controller): - def _build_view(self, req, instance, is_detail=False): - server = super(CreateServerController, self)._build_view(req, - instance, - is_detail) + def _build_view(self, req, instance, is_detail=False, is_create=False): + server = super(CreateServerController, self).\ + _build_view(req, + instance, + is_detail=is_detail, + is_create=is_create) if is_detail: self._build_security_groups(server['server'], instance) return server diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 29160c5ce..8c1eaae9d 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -497,7 +497,7 @@ class Controller(object): instance['instance_type'] = inst_type instance['image_ref'] = image_href - server = self._build_view(req, instance, is_detail=True) + server = self._build_view(req, instance, is_create=True) if '_is_precooked' in server['server']: del server['server']['_is_precooked'] else: @@ -748,7 +748,7 @@ class Controller(object): return common.get_id_from_href(flavor_ref) - def _build_view(self, req, instance, is_detail=False): + def _build_view(self, req, instance, is_detail=False, is_create=False): context = req.environ['nova.context'] project_id = getattr(context, 'project_id', '') base_url = req.application_url @@ -757,7 +757,9 @@ class Controller(object): addresses_builder = views_addresses.ViewBuilder() builder = views_servers.ViewBuilder(context, addresses_builder, flavor_builder, image_builder, base_url, project_id) - return builder.build(instance, is_detail=is_detail) + return builder.build(instance, + is_detail=is_detail, + is_create=is_create) def _build_list(self, req, instances, is_detail=False): params = req.GET.copy() diff --git a/nova/api/openstack/views/servers.py b/nova/api/openstack/views/servers.py index 288730efe..4a0be46c1 100644 --- a/nova/api/openstack/views/servers.py +++ b/nova/api/openstack/views/servers.py @@ -41,13 +41,15 @@ class ViewBuilder(object): self.base_url = base_url self.project_id = project_id - def build(self, inst, is_detail=False): + def build(self, inst, is_detail=False, is_create=False): """Return a dict that represenst a server.""" if inst.get('_is_precooked', False): server = dict(server=inst) else: if is_detail: server = self._build_detail(inst) + elif is_create: + server = self._build_create(inst) else: server = self._build_simple(inst) @@ -59,6 +61,12 @@ class ViewBuilder(object): """Return a simple model of a server.""" return dict(server=dict(id=inst['uuid'], name=inst['display_name'])) + def _build_create(self, inst): + """Return data that should be returned from a server create.""" + server = dict(server=dict(id=inst['uuid'])) + self._build_links(server['server'], inst) + return server + def _build_detail(self, inst): """Returns a detailed model of a server.""" vm_state = inst.get('vm_state', vm_states.BUILDING) diff --git a/nova/tests/api/openstack/contrib/test_volumes.py b/nova/tests/api/openstack/contrib/test_volumes.py index 0a3023e48..a130d1140 100644 --- a/nova/tests/api/openstack/contrib/test_volumes.py +++ b/nova/tests/api/openstack/contrib/test_volumes.py @@ -81,9 +81,6 @@ class BootFromVolumeTest(test.TestCase): self.assertEqual(res.status_int, 200) server = json.loads(res.body)['server'] self.assertEqual(FAKE_UUID, server['id']) - self.assertEqual(2, int(server['flavor']['id'])) - self.assertEqual(u'test_server', server['name']) - self.assertEqual(IMAGE_UUID, server['image']['id']) self.assertEqual(FLAGS.password_length, len(server['adminPass'])) self.assertEqual(len(_block_device_mapping_seen), 1) self.assertEqual(_block_device_mapping_seen[0]['volume_id'], 1) diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index e6e528535..4518ef8e2 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -1441,10 +1441,7 @@ class ServersControllerCreateTest(test.TestCase): server = self.controller.create(req, body)['server'] self.assertEqual(FLAGS.password_length, len(server['adminPass'])) - self.assertEqual('server_test', server['name']) self.assertEqual(FAKE_UUID, server['id']) - self.assertEqual('2', server['flavor']['id']) - self.assertEqual(image_uuid, server['image']['id']) def test_create_multiple_instances(self): """Test creating multiple instances but not asking for @@ -1613,12 +1610,6 @@ class ServersControllerCreateTest(test.TestCase): server = res['server'] self.assertEqual(FLAGS.password_length, len(server['adminPass'])) self.assertEqual(FAKE_UUID, server['id']) - self.assertEqual(0, server['progress']) - self.assertEqual('server_test', server['name']) - self.assertEqual(expected_flavor, server['flavor']) - self.assertEqual(expected_image, server['image']) - self.assertEqual(access_ipv4, server['accessIPv4']) - self.assertEqual(access_ipv6, server['accessIPv6']) def test_create_instance(self): # proper local hrefs must start with 'http://localhost/v1.1/' @@ -1670,13 +1661,6 @@ class ServersControllerCreateTest(test.TestCase): server = res['server'] self.assertEqual(FLAGS.password_length, len(server['adminPass'])) self.assertEqual(FAKE_UUID, server['id']) - self.assertEqual("BUILD", server["status"]) - self.assertEqual(0, server['progress']) - self.assertEqual('server_test', server['name']) - self.assertEqual(expected_flavor, server['flavor']) - self.assertEqual(expected_image, server['image']) - self.assertEqual('1.2.3.4', server['accessIPv4']) - self.assertEqual('fead::1234', server['accessIPv6']) def test_create_instance_invalid_key_name(self): image_href = 'http://localhost/v1.1/images/2' @@ -1777,7 +1761,6 @@ class ServersControllerCreateTest(test.TestCase): server = res['server'] self.assertEqual(FAKE_UUID, server['id']) - self.assertTrue(server['config_drive']) def test_create_instance_with_config_drive_as_id(self): self.config_drive = 2 @@ -1805,8 +1788,6 @@ class ServersControllerCreateTest(test.TestCase): server = res['server'] self.assertEqual(FAKE_UUID, server['id']) - self.assertTrue(server['config_drive']) - self.assertEqual(2, server['config_drive']) def test_create_instance_with_bad_config_drive(self): self.config_drive = "asdf" @@ -1859,7 +1840,6 @@ class ServersControllerCreateTest(test.TestCase): server = res['server'] self.assertEqual(FAKE_UUID, server['id']) - self.assertFalse(server['config_drive']) def test_create_instance_bad_href(self): image_href = 'asdf' @@ -1879,24 +1859,6 @@ class ServersControllerCreateTest(test.TestCase): def test_create_instance_local_href(self): image_uuid = '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6' flavor_ref = 'http://localhost/v1.1/flavors/3' - expected_flavor = { - "id": "3", - "links": [ - { - "rel": "bookmark", - "href": 'http://localhost/fake/flavors/3', - }, - ], - } - expected_image = { - "id": image_uuid, - "links": [ - { - "rel": "bookmark", - "href": 'http://localhost/fake/images/%s' % image_uuid, - }, - ], - } body = { 'server': { 'name': 'server_test', @@ -1912,8 +1874,7 @@ class ServersControllerCreateTest(test.TestCase): res = self.controller.create(req, body) server = res['server'] - self.assertEqual(expected_flavor, server['flavor']) - self.assertEqual(expected_image, server['image']) + self.assertEqual(FAKE_UUID, server['id']) def test_create_instance_admin_pass(self): image_uuid = '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6' diff --git a/nova/tests/integrated/test_servers.py b/nova/tests/integrated/test_servers.py index 36f62ac01..58aca5778 100644 --- a/nova/tests/integrated/test_servers.py +++ b/nova/tests/integrated/test_servers.py @@ -292,10 +292,6 @@ class ServersTest(integrated_helpers._IntegratedTestBase): self.assertTrue(created_server['id']) created_server_id = created_server['id'] - # Reenable when bug fixed - self.assertEqual(metadata, created_server.get('metadata')) - # Check it's there - found_server = self.api.get_server(created_server_id) self.assertEqual(created_server_id, found_server['id']) self.assertEqual(metadata, found_server.get('metadata')) |