From ec216e57d66db2f4c37baf89cdb36ddae235fee6 Mon Sep 17 00:00:00 2001 From: prameswar Date: Mon, 26 Dec 2016 21:24:53 +0530 Subject: [PATCH] zun run URL changed to /v1/containers?run=true The "run" URL violating the OpenStack API guide. This is the "run" API: POST /v1/containers/run In particular, the guide said: It is often the case that an API will have URIs that represent collections for resources and individual members of that collection in a hierarchy. Per my understanding, it means the following: * /v1/containers represent collections of resources * /v1/containers/ represent individual members of that collection However, the URL /v1/containers/run might be interpreted as an individual member with name "run". This is not an effective URL design. According to the guide, it suggested to change it as following POST /v1/containers?run=true Closes-Bug: #1650820 Change-Id: I058a947afd4d4635fa8cc2f73ed2ff1fa25db13d --- etc/zun/policy.json | 1 - zun/api/controllers/v1/containers.py | 103 +++++++----------- zun/api/controllers/v1/schemas/containers.py | 4 - .../api/controllers/v1/test_containers.py | 5 +- 4 files changed, 40 insertions(+), 73 deletions(-) diff --git a/etc/zun/policy.json b/etc/zun/policy.json index 8f31b7e26..54feed83c 100644 --- a/etc/zun/policy.json +++ b/etc/zun/policy.json @@ -18,7 +18,6 @@ "container:logs": "rule:admin_or_user", "container:execute": "rule:admin_or_user", "container:kill": "rule:admin_or_user", - "container:run": "rule:default", "image:pull": "rule:default", "image:get_all": "rule:default", diff --git a/zun/api/controllers/v1/containers.py b/zun/api/controllers/v1/containers.py index c4f97bfd8..f9b252c65 100644 --- a/zun/api/controllers/v1/containers.py +++ b/zun/api/controllers/v1/containers.py @@ -231,8 +231,7 @@ class ContainersController(rest.RestController): 'unpause': ['POST'], 'logs': ['GET'], 'execute': ['POST'], - 'kill': ['POST'], - 'run': ['POST'] + 'kill': ['POST'] } @pecan.expose('json') @@ -305,71 +304,43 @@ class ContainersController(rest.RestController): @api_utils.enforce_content_types(['application/json']) @exception.wrap_pecan_controller_exception @validation.validated(schema.container_create) - def post(self, **container_dict): - """Create a new container. + def post(self, run=False, **container_dict): + """Create a new container. - :param container: a container within the request body. - """ - context = pecan.request.context - policy.enforce(context, "container:create", - action="container:create") - # NOTE(mkrai): Intent here is to check the existence of image - # before proceeding to create container. If image is not found, - # container create will fail with 400 status. - images = pecan.request.rpcapi.image_search(context, - container_dict['image'], - exact_match=True) - if not images: - raise exception.ImageNotFound(container_dict['image']) - container_dict['project_id'] = context.project_id - container_dict['user_id'] = context.user_id - name = container_dict.get('name') or \ - self._generate_name_for_container() - container_dict['name'] = name - if container_dict.get('memory'): - container_dict['memory'] = \ - str(container_dict['memory']) + 'M' - container_dict['status'] = fields.ContainerStatus.CREATING - new_container = objects.Container(context, **container_dict) - new_container.create(context) - pecan.request.rpcapi.container_create(context, new_container) - - # Set the HTTP Location Header - pecan.response.location = link.build_url('containers', - new_container.uuid) - pecan.response.status = 202 - return Container.convert_with_links(new_container) - - @pecan.expose('json') - @api_utils.enforce_content_types(['application/json']) - @exception.wrap_pecan_controller_exception - @validation.validated(schema.container_run) - def run(self, **container_dict): - """Create and starts a new container. - - :param container: a container within the request body. - """ - context = pecan.request.context - policy.enforce(context, "container:run", - action="container:run") - container_dict['project_id'] = context.project_id - container_dict['user_id'] = context.user_id - name = container_dict.get('name') or \ - self._generate_name_for_container() - if container_dict.get('memory'): - container_dict['memory'] = \ - str(container_dict['memory']) + 'M' - container_dict['name'] = name - container_dict['status'] = fields.ContainerStatus.CREATING - new_container = objects.Container(context, **container_dict) - new_container.create(context) - pecan.request.rpcapi.container_run(context, new_container) - - # Set the HTTP Location Header - pecan.response.location = link.build_url('containers', - new_container.uuid) - pecan.response.status = 202 - return Container.convert_with_links(new_container) + :param run: if true, starts the container + :param container: a container within the request body. + """ + context = pecan.request.context + policy.enforce(context, "container:create", + action="container:create") + # NOTE(mkrai): Intent here is to check the existence of image + # before proceeding to create container. If image is not found, + # container create will fail with 400 status. + images = pecan.request.rpcapi.image_search(context, + container_dict['image'], + exact_match=True) + if not images: + raise exception.ImageNotFound(container_dict['image']) + container_dict['project_id'] = context.project_id + container_dict['user_id'] = context.user_id + name = container_dict.get('name') or \ + self._generate_name_for_container() + container_dict['name'] = name + if container_dict.get('memory'): + container_dict['memory'] = \ + str(container_dict['memory']) + 'M' + container_dict['status'] = fields.ContainerStatus.CREATING + new_container = objects.Container(context, **container_dict) + new_container.create(context) + if run: + pecan.request.rpcapi.container_run(context, new_container) + else: + pecan.request.rpcapi.container_create(context, new_container) + # Set the HTTP Location Header + pecan.response.location = link.build_url('containers', + new_container.uuid) + pecan.response.status = 202 + return Container.convert_with_links(new_container) @pecan.expose('json') @exception.wrap_pecan_controller_exception diff --git a/zun/api/controllers/v1/schemas/containers.py b/zun/api/controllers/v1/schemas/containers.py index e2bfb815b..59948beb8 100644 --- a/zun/api/controllers/v1/schemas/containers.py +++ b/zun/api/controllers/v1/schemas/containers.py @@ -10,8 +10,6 @@ # License for the specific language governing permissions and limitations # under the License. -import copy - from zun.common.validation import parameter_types _container_properties = { @@ -34,5 +32,3 @@ container_create = { 'required': ['image'], 'additionalProperties': False } - -container_run = copy.deepcopy(container_create) diff --git a/zun/tests/unit/api/controllers/v1/test_containers.py b/zun/tests/unit/api/controllers/v1/test_containers.py index 8561658a0..4e71eb78c 100644 --- a/zun/tests/unit/api/controllers/v1/test_containers.py +++ b/zun/tests/unit/api/controllers/v1/test_containers.py @@ -26,13 +26,14 @@ from zun.tests.unit.objects import utils as obj_utils class TestContainerController(api_base.FunctionalTest): @patch('zun.compute.api.API.container_run') - def test_run_container(self, mock_container_run): + @patch('zun.compute.api.API.image_search') + def test_run_container(self, mock_search, mock_container_run): mock_container_run.side_effect = lambda x, y: y params = ('{"name": "MyDocker", "image": "ubuntu",' '"command": "env", "memory": "512",' '"environment": {"key1": "val1", "key2": "val2"}}') - response = self.app.post('/v1/containers/run', + response = self.app.post('/v1/containers?run=true', params=params, content_type='application/json')