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/<ID or NAME> 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
This commit is contained in:
prameswar 2016-12-26 21:24:53 +05:30
parent 9c2cbd1625
commit ec216e57d6
4 changed files with 40 additions and 73 deletions

View File

@ -18,7 +18,6 @@
"container:logs": "rule:admin_or_user", "container:logs": "rule:admin_or_user",
"container:execute": "rule:admin_or_user", "container:execute": "rule:admin_or_user",
"container:kill": "rule:admin_or_user", "container:kill": "rule:admin_or_user",
"container:run": "rule:default",
"image:pull": "rule:default", "image:pull": "rule:default",
"image:get_all": "rule:default", "image:get_all": "rule:default",

View File

@ -231,8 +231,7 @@ class ContainersController(rest.RestController):
'unpause': ['POST'], 'unpause': ['POST'],
'logs': ['GET'], 'logs': ['GET'],
'execute': ['POST'], 'execute': ['POST'],
'kill': ['POST'], 'kill': ['POST']
'run': ['POST']
} }
@pecan.expose('json') @pecan.expose('json')
@ -305,71 +304,43 @@ class ContainersController(rest.RestController):
@api_utils.enforce_content_types(['application/json']) @api_utils.enforce_content_types(['application/json'])
@exception.wrap_pecan_controller_exception @exception.wrap_pecan_controller_exception
@validation.validated(schema.container_create) @validation.validated(schema.container_create)
def post(self, **container_dict): def post(self, run=False, **container_dict):
"""Create a new container. """Create a new container.
:param container: a container within the request body. :param run: if true, starts the container
""" :param container: a container within the request body.
context = pecan.request.context """
policy.enforce(context, "container:create", context = pecan.request.context
action="container:create") policy.enforce(context, "container:create",
# NOTE(mkrai): Intent here is to check the existence of image action="container:create")
# before proceeding to create container. If image is not found, # NOTE(mkrai): Intent here is to check the existence of image
# container create will fail with 400 status. # before proceeding to create container. If image is not found,
images = pecan.request.rpcapi.image_search(context, # container create will fail with 400 status.
container_dict['image'], images = pecan.request.rpcapi.image_search(context,
exact_match=True) container_dict['image'],
if not images: exact_match=True)
raise exception.ImageNotFound(container_dict['image']) if not images:
container_dict['project_id'] = context.project_id raise exception.ImageNotFound(container_dict['image'])
container_dict['user_id'] = context.user_id container_dict['project_id'] = context.project_id
name = container_dict.get('name') or \ container_dict['user_id'] = context.user_id
self._generate_name_for_container() name = container_dict.get('name') or \
container_dict['name'] = name self._generate_name_for_container()
if container_dict.get('memory'): container_dict['name'] = name
container_dict['memory'] = \ if container_dict.get('memory'):
str(container_dict['memory']) + 'M' container_dict['memory'] = \
container_dict['status'] = fields.ContainerStatus.CREATING str(container_dict['memory']) + 'M'
new_container = objects.Container(context, **container_dict) container_dict['status'] = fields.ContainerStatus.CREATING
new_container.create(context) new_container = objects.Container(context, **container_dict)
pecan.request.rpcapi.container_create(context, new_container) new_container.create(context)
if run:
# Set the HTTP Location Header pecan.request.rpcapi.container_run(context, new_container)
pecan.response.location = link.build_url('containers', else:
new_container.uuid) pecan.request.rpcapi.container_create(context, new_container)
pecan.response.status = 202 # Set the HTTP Location Header
return Container.convert_with_links(new_container) pecan.response.location = link.build_url('containers',
new_container.uuid)
@pecan.expose('json') pecan.response.status = 202
@api_utils.enforce_content_types(['application/json']) return Container.convert_with_links(new_container)
@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)
@pecan.expose('json') @pecan.expose('json')
@exception.wrap_pecan_controller_exception @exception.wrap_pecan_controller_exception

View File

@ -10,8 +10,6 @@
# License for the specific language governing permissions and limitations # License for the specific language governing permissions and limitations
# under the License. # under the License.
import copy
from zun.common.validation import parameter_types from zun.common.validation import parameter_types
_container_properties = { _container_properties = {
@ -34,5 +32,3 @@ container_create = {
'required': ['image'], 'required': ['image'],
'additionalProperties': False 'additionalProperties': False
} }
container_run = copy.deepcopy(container_create)

View File

@ -26,13 +26,14 @@ from zun.tests.unit.objects import utils as obj_utils
class TestContainerController(api_base.FunctionalTest): class TestContainerController(api_base.FunctionalTest):
@patch('zun.compute.api.API.container_run') @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 mock_container_run.side_effect = lambda x, y: y
params = ('{"name": "MyDocker", "image": "ubuntu",' params = ('{"name": "MyDocker", "image": "ubuntu",'
'"command": "env", "memory": "512",' '"command": "env", "memory": "512",'
'"environment": {"key1": "val1", "key2": "val2"}}') '"environment": {"key1": "val1", "key2": "val2"}}')
response = self.app.post('/v1/containers/run', response = self.app.post('/v1/containers?run=true',
params=params, params=params,
content_type='application/json') content_type='application/json')