From ba2106b05d157666931a0d9959dda344c4d74bbc Mon Sep 17 00:00:00 2001 From: Hongbin Lu Date: Thu, 23 Mar 2017 14:40:37 -0500 Subject: [PATCH] Add "run" parameter to the exec API The "run" parameter determines whether to run the command after creating the exec instance. If "run" is true, Zun will create and start the exec instance, and await for the execution of the command to finish. Finally, inspect the instance to retrieve the status of the command execution. If "run" is false, Zun will create an exec instance without running the command. Instead, the ID of the exec instance will return to users and the returned ID could be used to trigger command execution at client side. NOTE: We might add support for interactive mode in the future and it requires Zun server to return the exec ID to zunclient, which is a use case for setting "run" to false. Change-Id: I05432fd150db89f80b14fab68012048c09d1cf36 --- zun/api/controllers/v1/containers.py | 10 ++++++++-- zun/compute/manager.py | 8 ++++++-- zun/compute/rpcapi.py | 4 ++-- zun/container/docker/driver.py | 6 +++++- zun/container/driver.py | 8 ++++++-- .../api/controllers/v1/test_containers.py | 2 +- .../unit/compute/test_compute_manager.py | 19 +++++++++++-------- .../container/docker/test_docker_driver.py | 14 +++++++++----- 8 files changed, 48 insertions(+), 23 deletions(-) diff --git a/zun/api/controllers/v1/containers.py b/zun/api/controllers/v1/containers.py index fd47bd74f..3b5abf8ec 100644 --- a/zun/api/controllers/v1/containers.py +++ b/zun/api/controllers/v1/containers.py @@ -410,15 +410,21 @@ class ContainersController(rest.RestController): @pecan.expose('json') @exception.wrap_pecan_controller_exception - def execute(self, container_id, **kw): + def execute(self, container_id, run=True, **kw): container = _get_container(container_id) check_policy_on_container(container.as_dict(), "container:execute") + try: + run = strutils.bool_from_string(run, strict=True) + except ValueError: + msg = _('Valid run values are true, false, 0, 1, yes and no') + raise exception.InvalidValue(msg) utils.validate_container_state(container, 'execute') LOG.debug('Calling compute.container_exec with %s command %s' % (container.uuid, kw['command'])) context = pecan.request.context compute_api = pecan.request.compute_api - return compute_api.container_exec(context, container, kw['command']) + return compute_api.container_exec(context, container, kw['command'], + run) @pecan.expose('json') @exception.wrap_pecan_controller_exception diff --git a/zun/compute/manager.py b/zun/compute/manager.py index eef958fe4..426561ee6 100755 --- a/zun/compute/manager.py +++ b/zun/compute/manager.py @@ -330,11 +330,15 @@ class Manager(object): raise @translate_exception - def container_exec(self, context, container, command): + def container_exec(self, context, container, command, run): # TODO(hongbin): support exec command interactively LOG.debug('Executing command in container: %s', container.uuid) try: - return self.driver.execute(container, command) + exec_id = self.driver.execute_create(container, command) + if run: + return self.driver.execute_run(exec_id) + else: + return exec_id except exception.DockerError as e: LOG.error("Error occurred while calling Docker exec API: %s", six.text_type(e)) diff --git a/zun/compute/rpcapi.py b/zun/compute/rpcapi.py index e2f82fbc8..899021d85 100644 --- a/zun/compute/rpcapi.py +++ b/zun/compute/rpcapi.py @@ -75,9 +75,9 @@ class API(rpc_service.API): stdout=stdout, stderr=stderr, timestamps=timestamps, tail=tail, since=since) - def container_exec(self, context, container, command): + def container_exec(self, context, container, command, run): return self._call(container.host, 'container_exec', - container=container, command=command) + container=container, command=command, run=run) def container_kill(self, context, container, signal): self._cast(container.host, 'container_kill', container=container, diff --git a/zun/container/docker/driver.py b/zun/container/docker/driver.py index b86f4e2f4..8cb5ead84 100644 --- a/zun/container/docker/driver.py +++ b/zun/container/docker/driver.py @@ -287,11 +287,15 @@ class DockerDriver(driver.ContainerDriver): timestamps, tail, since) @check_container_id - def execute(self, container, command): + def execute_create(self, container, command): with docker_utils.docker_client() as docker: create_res = docker.exec_create( container.container_id, command, True, True, False) exec_id = create_res['Id'] + return exec_id + + def execute_run(self, exec_id): + with docker_utils.docker_client() as docker: output = docker.exec_start(exec_id, False, False, False) inspect_res = docker.exec_inspect(exec_id) return {"output": output, "exit_code": inspect_res['ExitCode']} diff --git a/zun/container/driver.py b/zun/container/driver.py index e44e59be5..83c182b77 100644 --- a/zun/container/driver.py +++ b/zun/container/driver.py @@ -100,8 +100,12 @@ class ContainerDriver(object): """Show logs of a container.""" raise NotImplementedError() - def execute(self, container, command): - """Execute a command in a running container.""" + def execute_create(self, container, command): + """Create an execute instance for running a command.""" + raise NotImplementedError() + + def execute_run(self, exec_id): + """Run the command specified by an execute instance.""" raise NotImplementedError() def kill(self, container, signal): diff --git a/zun/tests/unit/api/controllers/v1/test_containers.py b/zun/tests/unit/api/controllers/v1/test_containers.py index 73497bc55..55941755f 100644 --- a/zun/tests/unit/api/controllers/v1/test_containers.py +++ b/zun/tests/unit/api/controllers/v1/test_containers.py @@ -807,7 +807,7 @@ class TestContainerController(api_base.FunctionalTest): 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']) + mock.ANY, test_container_obj, cmd['command'], True) def test_exec_command_by_uuid_invalid_state(self): uuid = uuidutils.generate_uuid() diff --git a/zun/tests/unit/compute/test_compute_manager.py b/zun/tests/unit/compute/test_compute_manager.py index a281ff548..2a89d48ac 100755 --- a/zun/tests/unit/compute/test_compute_manager.py +++ b/zun/tests/unit/compute/test_compute_manager.py @@ -390,20 +390,23 @@ class TestManager(base.TestCase): self.context, container, True, True, False, 'all', None) - @mock.patch.object(fake_driver, 'execute') - def test_container_execute(self, mock_execute): + @mock.patch.object(fake_driver, 'execute_run') + @mock.patch.object(fake_driver, 'execute_create') + def test_container_execute(self, mock_execute_create, mock_execute_run): + mock_execute_create.return_value = 'fake_exec_id' container = Container(self.context, **utils.get_test_container()) self.compute_manager.container_exec( - self.context, container, 'fake_cmd') - mock_execute.assert_called_once_with(container, 'fake_cmd') + self.context, container, 'fake_cmd', True) + mock_execute_create.assert_called_once_with(container, 'fake_cmd') + mock_execute_run.assert_called_once_with('fake_exec_id') - @mock.patch.object(fake_driver, 'execute') - def test_container_execute_failed(self, mock_execute): + @mock.patch.object(fake_driver, 'execute_create') + def test_container_execute_failed(self, mock_execute_create): container = Container(self.context, **utils.get_test_container()) - mock_execute.side_effect = exception.DockerError + mock_execute_create.side_effect = exception.DockerError self.assertRaises(exception.DockerError, self.compute_manager.container_exec, - self.context, container, 'fake_cmd') + self.context, container, 'fake_cmd', True) @mock.patch.object(fake_driver, 'kill') def test_container_kill(self, mock_kill): diff --git a/zun/tests/unit/container/docker/test_docker_driver.py b/zun/tests/unit/container/docker/test_docker_driver.py index ed60e17b2..e21345392 100644 --- a/zun/tests/unit/container/docker/test_docker_driver.py +++ b/zun/tests/unit/container/docker/test_docker_driver.py @@ -238,15 +238,19 @@ class TestDockerDriver(base.DriverTestCase): mock_container.container_id, True, True, False, False, 'all', None) - def test_execute(self): + def test_execute_create(self): self.mock_docker.exec_create = mock.Mock(return_value={'Id': 'test'}) + mock_container = mock.MagicMock() + exec_id = self.driver.execute_create(mock_container, 'ls') + self.assertEqual('test', exec_id) + self.mock_docker.exec_create.assert_called_once_with( + mock_container.container_id, 'ls', True, True, False) + + def test_execute_run(self): self.mock_docker.exec_start = mock.Mock(return_value='test') self.mock_docker.exec_inspect = mock.Mock( return_value={u'ExitCode': 0}) - mock_container = mock.MagicMock() - self.driver.execute(mock_container, 'ls') - self.mock_docker.exec_create.assert_called_once_with( - mock_container.container_id, 'ls', True, True, False) + self.driver.execute_run('test') self.mock_docker.exec_start.assert_called_once_with('test', False, False, False) self.mock_docker.exec_inspect.assert_called_once()