openstack/nova
Code Issues Proposed changes

Merge "Pre-load expected attrs that the view builder needs for server details"

Browse Source
This commit is contained in:
Jenkins
2015-08-27 11:15:15 +00:00
committed by Gerrit Code Review
parent 83f972f8a7 2e68b2298e
commit 19810750b4
6 changed files with 98 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

40
nova/api/openstack/compute/legacy_v2/servers.py
View File

@@ -214,14 +214,19 @@ class Controller(wsgi.Controller):
sort_keys, sort_dirs = None, None
if self.ext_mgr.is_loaded('os-server-sort-keys'):
sort_keys, sort_dirs = common.get_sort_params(req.params)
expected_attrs = None
if is_detail:
# merge our expected attrs with what the view builder needs for
# showing details
expected_attrs = self._view_builder.get_show_expected_attrs(
expected_attrs)
try:
instance_list = self.compute_api.get_all(elevated or context,
search_opts=search_opts,
limit=limit,
marker=marker,
want_objects=True,
sort_keys=sort_keys,
sort_dirs=sort_dirs)
search_opts=search_opts, limit=limit, marker=marker,
want_objects=True, expected_attrs=expected_attrs,
sort_keys=sort_keys, sort_dirs=sort_dirs)
except exception.MarkerNotFound:
msg = _('marker [%s] not found') % marker
raise exc.HTTPBadRequest(explanation=msg)
@@ -237,11 +242,22 @@ class Controller(wsgi.Controller):
req.cache_db_instances(instance_list)
return response
def _get_server(self, context, req, instance_uuid):
"""Utility function for looking up an instance by uuid."""
def _get_server(self, context, req, instance_uuid, is_detail=False):
"""Utility function for looking up an instance by uuid.
:param context: request context for auth
:param req: HTTP request. The instance is cached in this request.
:param instance_uuid: UUID of the server instance to get
:param is_detail: True if you plan on showing the details of the
instance in the response, False otherwise.
"""
expected_attrs = ['flavor']
if is_detail:
expected_attrs = self._view_builder.get_show_expected_attrs(
expected_attrs)
instance = common.get_instance(self.compute_api, context,
instance_uuid,
expected_attrs=['flavor'])
expected_attrs=expected_attrs)
req.cache_db_instance(instance)
return instance
@@ -371,7 +387,7 @@ class Controller(wsgi.Controller):
def show(self, req, id):
"""Returns server details by server id."""
context = req.environ['nova.context']
instance = self._get_server(context, req, id)
instance = self._get_server(context, req, id, is_detail=True)
return self._view_builder.show(req, instance)
def _extract(self, server_dict, ext_name, key):
@@ -703,7 +719,7 @@ class Controller(wsgi.Controller):
msg = _("Personality cannot be updated.")
raise exc.HTTPBadRequest(explanation=msg)
instance = self._get_server(ctxt, req, id)
instance = self._get_server(ctxt, req, id, is_detail=True)
try:
policy.enforce(ctxt, 'compute:update', instance)
instance.update(update_dict)
@@ -1032,7 +1048,7 @@ class Controller(wsgi.Controller):
exception.AutoDiskConfigDisabledByImage) as error:
raise exc.HTTPBadRequest(explanation=error.format_message())
instance = self._get_server(context, req, id)
instance = self._get_server(context, req, id, is_detail=True)
view = self._view_builder.show(req, instance)

39
nova/api/openstack/compute/servers.py
View File

@@ -364,10 +364,18 @@ class ServersController(wsgi.Controller):
limit, marker = common.get_limit_and_marker(req)
sort_keys, sort_dirs = common.get_sort_params(req.params)
expected_attrs = ['pci_devices']
if is_detail:
# merge our expected attrs with what the view builder needs for
# showing details
expected_attrs = self._view_builder.get_show_expected_attrs(
expected_attrs)
try:
instance_list = self.compute_api.get_all(elevated or context,
search_opts=search_opts, limit=limit, marker=marker,
want_objects=True, expected_attrs=['pci_devices'],
want_objects=True, expected_attrs=expected_attrs,
sort_keys=sort_keys, sort_dirs=sort_dirs)
except exception.MarkerNotFound:
msg = _('marker [%s] not found') % marker
@@ -385,12 +393,22 @@ class ServersController(wsgi.Controller):
req.cache_db_instances(instance_list)
return response
def _get_server(self, context, req, instance_uuid):
"""Utility function for looking up an instance by uuid."""
def _get_server(self, context, req, instance_uuid, is_detail=False):
"""Utility function for looking up an instance by uuid.
:param context: request context for auth
:param req: HTTP request. The instance is cached in this request.
:param instance_uuid: UUID of the server instance to get
:param is_detail: True if you plan on showing the details of the
instance in the response, False otherwise.
"""
expected_attrs = ['flavor', 'pci_devices']
if is_detail:
expected_attrs = self._view_builder.get_show_expected_attrs(
expected_attrs)
instance = common.get_instance(self.compute_api, context,
instance_uuid,
expected_attrs=['pci_devices',
'flavor'])
expected_attrs=expected_attrs)
req.cache_db_instance(instance)
return instance
@@ -479,11 +497,8 @@ class ServersController(wsgi.Controller):
def show(self, req, id):
"""Returns server details by server id."""
context = req.environ['nova.context']
instance = common.get_instance(self.compute_api, context, id,
expected_attrs=['pci_devices',
'flavor'])
authorize(context, action="show")
req.cache_db_instance(instance)
instance = self._get_server(context, req, id, is_detail=True)
return self._view_builder.show(req, instance)
@wsgi.response(202)
@@ -750,12 +765,10 @@ class ServersController(wsgi.Controller):
self.update_extension_manager.map(self._update_extension_point,
body['server'], update_dict)
instance = common.get_instance(self.compute_api, ctxt, id,
expected_attrs=['pci_devices'])
instance = self._get_server(ctxt, req, id, is_detail=True)
try:
# NOTE(mikal): this try block needs to stay because save() still
# might throw an exception.
req.cache_db_instance(instance)
instance.update(update_dict)
instance.save()
return self._view_builder.show(req, instance,
@@ -991,7 +1004,7 @@ class ServersController(wsgi.Controller):
exception.AutoDiskConfigDisabledByImage) as error:
raise exc.HTTPBadRequest(explanation=error.format_message())
instance = self._get_server(context, req, id)
instance = self._get_server(context, req, id, is_detail=True)
view = self._view_builder.show(req, instance, extend_address=False)

25
nova/api/openstack/compute/views/servers.py
View File

@@ -50,6 +50,11 @@ class ViewBuilder(common.ViewBuilder):
"ERROR", "DELETED"
)
# These are the lazy-loadable instance attributes required for showing
# details about an instance. Add to this list as new things need to be
# shown.
_show_expected_attrs = ['flavor', 'info_cache', 'metadata']
def __init__(self):
"""Initialize view builder."""
super(ViewBuilder, self).__init__()
@@ -80,6 +85,26 @@ class ViewBuilder(common.ViewBuilder):
},
}
def get_show_expected_attrs(self, expected_attrs=None):
"""Returns a list of lazy-loadable expected attributes used by show
This should be used when getting the instances from the database so
that the necessary attributes are pre-loaded before needing to build
the show response where lazy-loading can fail if an instance was
deleted.
:param list expected_attrs: The list of expected attributes that will
be requested in addition to what this view builder requires. This
method will merge the two lists and return what should be
ultimately used when getting an instance from the database.
:returns: merged and sorted list of expected attributes
"""
if expected_attrs is None:
expected_attrs = []
# NOTE(mriedem): We sort the list so we can have predictable test
# results.
return sorted(list(set(self._show_expected_attrs + expected_attrs)))
def show(self, request, instance):
"""Detailed view of a single instance."""
ip_v4 = instance.get('access_ip_v4')

8
nova/tests/unit/api/openstack/compute/legacy_v2/test_servers.py
View File

@@ -279,6 +279,9 @@ class ServersControllerTest(ControllerTest):
"""
def return_instance_with_host(context, *args, **kwargs):
project_id = str(uuid.uuid4())
self.assertIn('expected_attrs', kwargs)
self.assertEqual(['flavor', 'info_cache', 'metadata'],
kwargs['expected_attrs'])
return fakes.stub_instance_obj(context, id=1, uuid=FAKE_UUID,
project_id=project_id,
host='fake_host')
@@ -907,6 +910,7 @@ class ServersControllerTest(ControllerTest):
get_all_mock.assert_called_once_with(mock.ANY,
search_opts=expected_search_opts, limit=mock.ANY,
marker=mock.ANY, want_objects=mock.ANY,
expected_attrs=None,
sort_keys=mock.ANY, sort_dirs=mock.ANY)
@mock.patch.object(compute_api.API, 'get_all')
@@ -933,6 +937,7 @@ class ServersControllerTest(ControllerTest):
get_all_mock.assert_called_once_with(mock.ANY,
search_opts=expected_search_opts, limit=mock.ANY,
marker=mock.ANY, want_objects=mock.ANY,
expected_attrs=None,
sort_keys=mock.ANY, sort_dirs=mock.ANY)
@mock.patch.object(compute_api.API, 'get_all')
@@ -965,6 +970,7 @@ class ServersControllerTest(ControllerTest):
get_all_mock.assert_called_once_with(mock.ANY,
search_opts=expected_search_opts, limit=mock.ANY,
marker=mock.ANY, want_objects=mock.ANY,
expected_attrs=None,
sort_keys=mock.ANY, sort_dirs=mock.ANY)
def test_get_servers_allows_task_status(self):
@@ -1073,6 +1079,7 @@ class ServersControllerTest(ControllerTest):
mock_get_all.assert_called_once_with(
mock.ANY, search_opts=expected_search_opts, limit=mock.ANY,
marker=mock.ANY, want_objects=mock.ANY,
expected_attrs=['flavor', 'info_cache', 'metadata'],
sort_keys=mock.ANY, sort_dirs=mock.ANY)
@mock.patch.object(compute_api.API, 'get_all')
@@ -1096,6 +1103,7 @@ class ServersControllerTest(ControllerTest):
mock_get_all.assert_called_once_with(
mock.ANY, search_opts=expected_search_opts, limit=mock.ANY,
marker=mock.ANY, want_objects=mock.ANY,
expected_attrs=['flavor', 'info_cache', 'metadata'],
sort_keys=mock.ANY, sort_dirs=mock.ANY)
def test_get_servers_allows_name(self):

2
nova/tests/unit/api/openstack/compute/test_server_actions.py
View File

@@ -129,7 +129,7 @@ class ServerActionsControllerTestV21(test.TestCase):
self.context, objects.Instance(), instance)
self.compute_api.get(self.context, uuid,
expected_attrs=['pci_devices', 'flavor'],
expected_attrs=['flavor', 'pci_devices'],
want_objects=True).AndReturn(instance)
return instance

22
nova/tests/unit/api/openstack/compute/test_serversV21.py
View File

@@ -314,21 +314,20 @@ class ServersControllerTest(ControllerTest):
self.assertEqual(res_dict['server']['id'], FAKE_UUID)
def test_get_server_joins_pci_devices(self):
self.expected_attrs = None
def fake_get(_self, *args, **kwargs):
self.expected_attrs = kwargs['expected_attrs']
expected_attrs = kwargs['expected_attrs']
self.assertEqual(['flavor', 'info_cache', 'metadata',
'pci_devices'], expected_attrs)
ctxt = context.RequestContext('fake', 'fake')
return fake_instance.fake_instance_obj(
ctxt, expected_attrs=['metadata', 'info_cache'])
ctxt, expected_attrs=expected_attrs)
self.stubs.Set(compute_api.API, 'get', fake_get)
req = self.req('/servers/%s' % FAKE_UUID)
self.controller.show(req, FAKE_UUID)
self.assertIn('pci_devices', self.expected_attrs)
def test_unique_host_id(self):
"""Create two servers with the same host and different
project_ids and check that the host_id's are unique.
@@ -1025,8 +1024,8 @@ class ServersControllerTest(ControllerTest):
# while calling get_all() method.
expected_search_opts = {'deleted': True, 'project_id': 'fake'}
mock_get_all.assert_called_once_with(
mock.ANY, search_opts=expected_search_opts,
limit=mock.ANY, expected_attrs=mock.ANY,
mock.ANY, search_opts=expected_search_opts, limit=mock.ANY,
expected_attrs=['flavor', 'info_cache', 'metadata', 'pci_devices'],
marker=mock.ANY, want_objects=mock.ANY,
sort_keys=mock.ANY, sort_dirs=mock.ANY)
@@ -1049,8 +1048,8 @@ class ServersControllerTest(ControllerTest):
# False while calling get_all() method.
expected_search_opts = {'deleted': False, 'project_id': 'fake'}
mock_get_all.assert_called_once_with(
mock.ANY, search_opts=expected_search_opts,
limit=mock.ANY, expected_attrs=mock.ANY,
mock.ANY, search_opts=expected_search_opts, limit=mock.ANY,
expected_attrs=['flavor', 'info_cache', 'metadata', 'pci_devices'],
marker=mock.ANY, want_objects=mock.ANY,
sort_keys=mock.ANY, sort_dirs=mock.ANY)
@@ -1063,6 +1062,7 @@ class ServersControllerTest(ControllerTest):
self.assertIsNotNone(search_opts)
self.assertIn('name', search_opts)
self.assertEqual(search_opts['name'], 'whee.*')
self.assertEqual(['pci_devices'], expected_attrs)
return objects.InstanceList(
objects=[fakes.stub_instance_obj(100, uuid=server_uuid)])
@@ -1307,19 +1307,17 @@ class ServersControllerTest(ControllerTest):
self.assertEqual(s['name'], 'server%d' % (i + 1))
def test_get_servers_joins_pci_devices(self):
self.expected_attrs = None
def fake_get_all(compute_self, context, search_opts=None,
limit=None, marker=None, want_objects=False,
expected_attrs=None, sort_keys=None, sort_dirs=None):
self.expected_attrs = expected_attrs
self.assertEqual(['pci_devices'], expected_attrs)
return []
self.stubs.Set(compute_api.API, 'get_all', fake_get_all)
req = self.req('/servers', use_admin_context=True)
self.assertIn('servers', self.controller.index(req))
self.assertIn('pci_devices', self.expected_attrs)
class ServersControllerTestV29(ServersControllerTest):