From 6df752e3b4aa64802c6b5d9dd465032f0a4f4d8e Mon Sep 17 00:00:00 2001 From: Hongbin Lu Date: Sun, 28 Aug 2016 15:40:19 -0500 Subject: [PATCH] Consolidate multiple controllers into one * In before, container requests are handle by multiple controllers. This commit consolidates them into one to reduce the complexity. * Change the method of the following endpoints from PUT to POST. Both Nova and Docker uses POST in similiar cases. - /containers/%id/pause - /containers/%id/unpause - /containers/%id/execute - /containers/%id/reboot - /containers/%id/start - /containers/%id/stop Change-Id: I9307038e5b78574bd4fa1867c5355d0681cfc4db --- zun/api/controllers/v1/containers.py | 322 ++++++------------ .../api/controllers/v1/test_containers.py | 14 +- 2 files changed, 120 insertions(+), 216 deletions(-) diff --git a/zun/api/controllers/v1/containers.py b/zun/api/controllers/v1/containers.py index 7b4c8fdda..f35b05871 100644 --- a/zun/api/controllers/v1/containers.py +++ b/zun/api/controllers/v1/containers.py @@ -16,6 +16,7 @@ from oslo_log import log as logging from oslo_utils import timeutils import pecan +from pecan import rest from zun.api.controllers import base from zun.api.controllers import link @@ -166,158 +167,29 @@ class ContainerCollection(collection.Collection): return sample -class StartController(object): - - def __init__(self, container_id): - self.container_id = container_id - - @pecan.expose(generic=True) - def index(self, **kwargs): - pecan.abort(405) - - @index.when(method='PUT', template='json') - @exception.wrap_pecan_controller_exception - def on_put(self, **kw): - container = _get_container(self.container_id) - check_policy_on_container(container, "container:start") - LOG.debug('Calling compute.container_start with %s', - container.uuid) - context = pecan.request.context - pecan.request.rpcapi.container_start(context, container) - return Container.convert_with_links(container) - - -class StopController(object): - - def __init__(self, container_id): - self.container_id = container_id - - @pecan.expose(generic=True) - def index(self, **kwargs): - pecan.abort(405) - - @index.when(method='PUT', template='json') - @exception.wrap_pecan_controller_exception - def on_put(self, **kw): - container = _get_container(self.container_id) - check_policy_on_container(container, "container:stop") - LOG.debug('Calling compute.container_stop with %s' % - container.uuid) - context = pecan.request.context - pecan.request.rpcapi.container_stop(context, container) - return Container.convert_with_links(container) - - -class RebootController(object): - - def __init__(self, container_id): - self.container_id = container_id - - @pecan.expose(generic=True) - def index(self, **kwargs): - pecan.abort(405) - - @index.when(method='PUT', template='json') - @exception.wrap_pecan_controller_exception - def on_put(self, **kw): - container = _get_container(self.container_id) - check_policy_on_container(container, "container:reboot") - LOG.debug('Calling compute.container_reboot with %s' % - container.uuid) - context = pecan.request.context - pecan.request.rpcapi.container_reboot(context, container) - return Container.convert_with_links(container) - - -class PauseController(object): - - def __init__(self, container_id): - self.container_id = container_id - - @pecan.expose(generic=True) - def index(self, **kwargs): - pecan.abort(405) - - @index.when(method='PUT', template='json') - @exception.wrap_pecan_controller_exception - def on_put(self, **kw): - container = _get_container(self.container_id) - check_policy_on_container(container, "container:pause") - LOG.debug('Calling compute.container_pause with %s' % - container.uuid) - context = pecan.request.context - pecan.request.rpcapi.container_pause(context, container) - return Container.convert_with_links(container) - - -class UnpauseController(object): - - def __init__(self, container_id): - self.container_id = container_id - - @pecan.expose(generic=True) - def index(self, **kwargs): - pecan.abort(405) - - @index.when(method='PUT', template='json') - @exception.wrap_pecan_controller_exception - def on_put(self, **kw): - container = _get_container(self.container_id) - check_policy_on_container(container, "container:unpause") - LOG.debug('Calling compute.container_unpause with %s' % - container.uuid) - context = pecan.request.context - pecan.request.rpcapi.container_unpause(context, container) - return Container.convert_with_links(container) - - -class LogsController(object): - - def __init__(self, container_id): - self.container_id = container_id - - @pecan.expose(generic=True) - def index(self, **kwargs): - pecan.abort(405) # HTTP 405 Method Not Allowed as default - - @index.when(method='GET', template='json') - @exception.wrap_pecan_controller_exception - def on_get(self, **kwargs): - container = _get_container(self.container_id) - check_policy_on_container(container, "container:logs") - LOG.debug('Calling compute.container_logs with %s' % - container.uuid) - context = pecan.request.context - return pecan.request.rpcapi.container_logs(context, container) - - -class ExecuteController(object): - - def __init__(self, container_id): - self.container_id = container_id - - @pecan.expose(generic=True) - def index(self, **kwargs): - pecan.abort(405) - - @index.when(method='PUT', template='json') - @exception.wrap_pecan_controller_exception - def on_put(self, **kw): - container = _get_container(self.container_id) - check_policy_on_container(container, "container:execute") - LOG.debug('Calling compute.container_exec with %s command %s' - % (container.uuid, kw['command'])) - context = pecan.request.context - return pecan.request.rpcapi.container_exec(context, container, - kw['command']) - - -class ContainersController(object): +class ContainersController(rest.RestController): """Controller for Containers.""" - @pecan.expose() - def _lookup(self, container_id, *remainder): - return ContainerController(container_id), remainder + _custom_actions = { + 'start': ['POST'], + 'stop': ['POST'], + 'reboot': ['POST'], + 'pause': ['POST'], + 'unpause': ['POST'], + 'logs': ['GET'], + 'execute': ['POST'] + } + + @pecan.expose('json') + @exception.wrap_pecan_controller_exception + def get_all(self, **kwargs): + """Retrieve a list of containers. + + """ + context = pecan.request.context + policy.enforce(context, "container:get_all", + action="container:get_all") + return self._get_containers_collection(**kwargs) def _get_containers_collection(self, **kwargs): context = pecan.request.context @@ -355,25 +227,21 @@ class ContainersController(object): sort_key=sort_key, sort_dir=sort_dir) - @pecan.expose(generic=True) - def index(self, **kwargs): - pecan.abort(405) # HTTP 405 Method Not Allowed as default - - @index.when(method='GET', template='json') + @pecan.expose('json') @exception.wrap_pecan_controller_exception - def on_get(self, **kwargs): - """Retrieve a list of containers. + def get_one(self, container_id): + """Retrieve information about the given container. + :param container_ident: UUID or name of a container. """ - context = pecan.request.context - policy.enforce(context, "container:get_all", - action="container:get_all") - return self._get_containers_collection(**kwargs) + container = _get_container(container_id) + check_policy_on_container(container, "container:get") + return Container.convert_with_links(container) - @index.when(method='POST', template='json') + @pecan.expose('json') @api_utils.enforce_content_types(['application/json']) @exception.wrap_pecan_controller_exception - def on_post(self, **container_dict): + def post(self, **container_dict): """Create a new container. :param container: a container within the request body. @@ -395,54 +263,14 @@ class ContainersController(object): pecan.response.status = 202 return Container.convert_with_links(new_container) - -class ContainerController(object): - - def __init__(self, container_id): - self.container_id = container_id - - @pecan.expose() - def _lookup(self, sub_resource, *remainder): - if sub_resource == 'start': - return StartController(self.container_id), remainder - elif sub_resource == 'stop': - return StopController(self.container_id), remainder - elif sub_resource == 'pause': - return PauseController(self.container_id), remainder - elif sub_resource == 'unpause': - return UnpauseController(self.container_id), remainder - elif sub_resource == 'reboot': - return RebootController(self.container_id), remainder - elif sub_resource == 'logs': - return LogsController(self.container_id), remainder - elif sub_resource == 'execute': - return ExecuteController(self.container_id), remainder - else: - pecan.abort(405) - - @pecan.expose(generic=True) - def index(self, **kwargs): - pecan.abort(405) # HTTP 405 Method Not Allowed as default - - @index.when(method='GET', template='json') + @pecan.expose('json') @exception.wrap_pecan_controller_exception - def on_get(self, **kwargs): - """Retrieve information about the given container. - - :param container_ident: UUID or name of a container. - """ - container = _get_container(self.container_id) - check_policy_on_container(container, "container:get") - return Container.convert_with_links(container) - - @index.when(method='PATCH', template='json') - @exception.wrap_pecan_controller_exception - def on_patch(self, **kwargs): + def patch(self, container_id, **kwargs): """Update an existing container. :param patch: a json PATCH document to apply to this container. """ - container = _get_container(self.container_id) + container = _get_container(container_id) check_policy_on_container(container, "container:update") try: patch = kwargs.get('patch') @@ -465,16 +293,92 @@ class ContainerController(object): container.save() return Container.convert_with_links(container) - @index.when(method='DELETE', template='json') + @pecan.expose('json') @exception.wrap_pecan_controller_exception - def on_delete(self, **kwargs): + def delete(self, container_id): """Delete a container. :param container_ident: UUID or Name of a container. """ - container = _get_container(self.container_id) + container = _get_container(container_id) check_policy_on_container(container, "container:delete") context = pecan.request.context pecan.request.rpcapi.container_delete(context, container) container.destroy() pecan.response.status = 204 + + @pecan.expose('json') + @exception.wrap_pecan_controller_exception + def start(self, container_id, **kw): + container = _get_container(container_id) + check_policy_on_container(container, "container:start") + LOG.debug('Calling compute.container_start with %s', + container.uuid) + context = pecan.request.context + pecan.request.rpcapi.container_start(context, container) + return Container.convert_with_links(container) + + @pecan.expose('json') + @exception.wrap_pecan_controller_exception + def stop(self, container_id, **kw): + container = _get_container(container_id) + check_policy_on_container(container, "container:stop") + LOG.debug('Calling compute.container_stop with %s' % + container.uuid) + context = pecan.request.context + pecan.request.rpcapi.container_stop(context, container) + return Container.convert_with_links(container) + + @pecan.expose('json') + @exception.wrap_pecan_controller_exception + def reboot(self, container_id, **kw): + container = _get_container(container_id) + check_policy_on_container(container, "container:reboot") + LOG.debug('Calling compute.container_reboot with %s' % + container.uuid) + context = pecan.request.context + pecan.request.rpcapi.container_reboot(context, container) + return Container.convert_with_links(container) + + @pecan.expose('json') + @exception.wrap_pecan_controller_exception + def pause(self, container_id, **kw): + container = _get_container(container_id) + check_policy_on_container(container, "container:pause") + LOG.debug('Calling compute.container_pause with %s' % + container.uuid) + context = pecan.request.context + pecan.request.rpcapi.container_pause(context, container) + return Container.convert_with_links(container) + + @pecan.expose('json') + @exception.wrap_pecan_controller_exception + def unpause(self, container_id, **kw): + container = _get_container(container_id) + check_policy_on_container(container, "container:unpause") + LOG.debug('Calling compute.container_unpause with %s' % + container.uuid) + context = pecan.request.context + pecan.request.rpcapi.container_unpause(context, container) + return Container.convert_with_links(container) + + @pecan.expose('json') + @exception.wrap_pecan_controller_exception + def logs(self, container_id): + container = _get_container(container_id) + check_policy_on_container(container, "container:logs") + LOG.debug('Calling compute.container_logs with %s' % + container.uuid) + context = pecan.request.context + return pecan.request.rpcapi.container_logs(context, container) + + @pecan.expose('json') + @exception.wrap_pecan_controller_exception + def execute(self, container_id, **kw): + container = _get_container(container_id) + check_policy_on_container(container, "container:execute") + LOG.debug('Calling compute.container_exec with %s command %s' + % (container.uuid, kw['command'])) + context = pecan.request.context + return pecan.request.rpcapi.container_exec(context, container, + kw['command']) diff --git a/zun/tests/unit/api/controllers/v1/test_containers.py b/zun/tests/unit/api/controllers/v1/test_containers.py index 1c2f8e6ce..c267ae720 100644 --- a/zun/tests/unit/api/controllers/v1/test_containers.py +++ b/zun/tests/unit/api/controllers/v1/test_containers.py @@ -336,8 +336,8 @@ class TestContainerController(api_base.FunctionalTest): get_by_ident_loc = 'zun.objects.Container.get_by_%s' % ident_field with patch(get_by_ident_loc) as mock_get_by_indent: mock_get_by_indent.return_value = test_container_obj - response = self.app.put('/v1/containers/%s/%s/' % (ident, - action)) + response = self.app.post('/v1/containers/%s/%s/' % (ident, + action)) self.assertEqual(200, response.status_int) # Only PUT should work, others like GET should fail @@ -455,7 +455,7 @@ class TestContainerController(api_base.FunctionalTest): mock_get_by_uuid.return_value = test_container_obj container_uuid = test_container.get('uuid') - self.assertRaises(AppError, self.app.put, + self.assertRaises(AppError, self.app.post, '/v1/containers/%s/logs/' % container_uuid) self.assertFalse(mock_container_logs.called) @@ -471,7 +471,7 @@ class TestContainerController(api_base.FunctionalTest): container_uuid = test_container.get('uuid') url = '/v1/containers/%s/%s/' % (container_uuid, 'execute') cmd = {'command': 'ls'} - response = self.app.put(url, cmd) + response = self.app.post(url, cmd) self.assertEqual(200, response.status_int) mock_container_exec.assert_called_once_with( mock.ANY, test_container_obj, cmd['command']) @@ -488,7 +488,7 @@ class TestContainerController(api_base.FunctionalTest): container_name = test_container.get('name') url = '/v1/containers/%s/%s/' % (container_name, 'execute') cmd = {'command': 'ls'} - response = self.app.put(url, cmd) + response = self.app.post(url, cmd) self.assertEqual(200, response.status_int) mock_container_exec.assert_called_once_with( mock.ANY, test_container_obj, cmd['command']) @@ -622,7 +622,7 @@ class TestContainerEnforcement(api_base.FunctionalTest): def test_policy_only_owner_execute(self): container = obj_utils.create_test_container(self.context, user_id='another') - self._owner_check("container:execute", self.put_json, + self._owner_check("container:execute", self.post_json, '/containers/%s/execute/' % container.uuid, params={'command': 'ls'}, expect_errors=True) @@ -631,6 +631,6 @@ class TestContainerEnforcement(api_base.FunctionalTest): container = obj_utils.create_test_container(self.context, user_id='another') for action in actions: - self._owner_check('container:%s' % action, self.put_json, + self._owner_check('container:%s' % action, self.post_json, '/containers/%s/%s/' % (container.uuid, action), {}, expect_errors=True)