openstack/zun
Code Issues Proposed changes

Merge "Container logs is not good user experience"

Browse Source
This commit is contained in:
Jenkins 2017-03-01 06:11:58 +00:00 committed by Gerrit Code Review
commit 085e5dcf8b
12 changed files with 101 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
zun/api/controllers/v1/containers.py
View File

@ -376,21 +376,24 @@ class ContainersController(rest.RestController):
@pecan.expose('json')
@exception.wrap_pecan_controller_exception
@validation.validate_query_param(pecan.request, schema.query_param_logs)
def logs(self, container_id, stdout=True, stderr=True):
def logs(self, container_id, stdout=True, stderr=True,
timestamps=False, tail='all', since=None):
container = _get_container(container_id)
check_policy_on_container(container.as_dict(), "container:logs")
try:
stdout = strutils.bool_from_string(stdout, strict=True)
stderr = strutils.bool_from_string(stderr, strict=True)
timestamps = strutils.bool_from_string(timestamps, strict=True)
except ValueError:
msg = _('Valid stdout and stderr values are ''true'', '
msg = _('Valid stdout, stderr and timestamps values are ''true'', '
'"false", True, False, 0 and 1, yes and no')
raise exception.InvalidValue(msg)
LOG.debug('Calling compute.container_logs with %s' %
container.uuid)
context = pecan.request.context
compute_api = pecan.request.compute_api
return compute_api.container_logs(context, container, stdout, stderr)
return compute_api.container_logs(context, container, stdout, stderr,
timestamps, tail, since)
@pecan.expose('json')
@exception.wrap_pecan_controller_exception

5
zun/api/controllers/v1/schemas/containers.py
View File

@ -84,7 +84,10 @@ query_param_logs = {
'type': 'object',
'properties': {
'stdout': parameter_types.boolean_extended,
'stderr': parameter_types.boolean_extended
'stderr': parameter_types.boolean_extended,
'timestamps': parameter_types.boolean_extended,
'tail': parameter_types.str_and_int,
'since': parameter_types.logs_since
},
'additionalProperties': False
}

10
zun/common/validation/parameter_types.py
View File

@ -141,3 +141,13 @@ string_ps_args = {
'type': ['string'],
'pattern': '[a-zA-Z- ,+]*'
}
str_and_int = {
'type': ['string', 'integer', 'null'],
}
logs_since = {
'type': ['string', 'integer', 'null'],
'pattern': '(^[0-9]*$)|\
([0-9]{4}-[0-9]{2}-[0-9]{2} [0-9]{2}:[0-9]{2}:[0-9]{2},[0-9]{1,3})'
}

6
zun/compute/api.py
View File

@ -75,8 +75,10 @@ class API(object):
def container_unpause(self, context, container, *args):
return self.rpcapi.container_unpause(context, container, *args)
def container_logs(self, context, container, stdout, stderr):
return self.rpcapi.container_logs(context, container, stdout, stderr)
def container_logs(self, context, container, stdout, stderr,
timestamps, tail, since):
return self.rpcapi.container_logs(context, container, stdout, stderr,
timestamps, tail, since)
def container_exec(self, context, container, *args):
return self.rpcapi.container_exec(context, container, *args)

7
zun/compute/manager.py
View File

@ -294,11 +294,14 @@ class Manager(object):
utils.spawn_n(self._do_container_unpause, context, container)
@translate_exception
def container_logs(self, context, container, stdout, stderr):
def container_logs(self, context, container, stdout, stderr,
timestamps, tail, since):
LOG.debug('Showing container logs: %s', container.uuid)
try:
return self.driver.show_logs(container,
stdout=stdout, stderr=stderr)
stdout=stdout, stderr=stderr,
timestamps=timestamps, tail=tail,
since=since)
except exception.DockerError as e:
LOG.error(_LE("Error occurred while calling Docker logs API: %s"),
six.text_type(e))

6
zun/compute/rpcapi.py
View File

@ -66,10 +66,12 @@ class API(rpc_service.API):
def container_unpause(self, context, container):
self._cast(container.host, 'container_unpause', container=container)
def container_logs(self, context, container, stdout, stderr):
def container_logs(self, context, container, stdout, stderr,
timestamps, tail, since):
host = container.host
return self._call(host, 'container_logs', container=container,
stdout=stdout, stderr=stderr)
stdout=stdout, stderr=stderr,
timestamps=timestamps, tail=tail, since=since)
def container_exec(self, context, container, command):
return self._call(container.host, 'container_exec',

27
zun/container/docker/driver.py
View File

@ -250,10 +250,31 @@ class DockerDriver(driver.ContainerDriver):
return container
@check_container_id
def show_logs(self, container, stdout=True, stderr=True):
def show_logs(self, container, stdout=True, stderr=True,
timestamps=False, tail='all', since=None):
with docker_utils.docker_client() as docker:
return docker.get_container_logs(container.container_id,
stdout, stderr)
try:
tail = int(tail)
except ValueError:
tail = 'all'
if since is None or since == 'None':
return docker.get_container_logs(container.container_id,
stdout, stderr, False,
timestamps, tail, None)
else:
try:
since = int(since)
except ValueError:
try:
since = \
datetime.datetime.strptime(since,
'%Y-%m-%d %H:%M:%S,%f')
except Exception:
raise
return docker.get_container_logs(container.container_id,
stdout, stderr, False,
timestamps, tail, since)
@check_container_id
def execute(self, container, command):

6
zun/container/docker/utils.py
View File

@ -105,6 +105,8 @@ class DockerHTTPClient(client.Client):
container = container.container_id
super(DockerHTTPClient, self).unpause(container)
def get_container_logs(self, docker_id, stdout, stderr):
def get_container_logs(self, docker_id, stdout, stderr, stream,
timestamps, tail, since):
"""Fetch the logs of a container."""
return self.logs(docker_id, stdout, stderr)
return self.logs(docker_id, stdout, stderr, False,
timestamps, tail, since)

3
zun/container/driver.py
View File

@ -97,7 +97,8 @@ class ContainerDriver(object):
"""Pause a container."""
raise NotImplementedError()
def show_logs(self, container, stdout=True, stderr=True):
def show_logs(self, container, stdout=True, stderr=True,
timestamps=False, tail='all', since=None):
"""Show logs of a container."""
raise NotImplementedError()

38
zun/tests/unit/api/controllers/v1/test_containers.py
View File

@ -873,7 +873,7 @@ class TestContainerController(api_base.FunctionalTest):
self.assertEqual(200, response.status_int)
mock_container_logs.assert_called_once_with(
mock.ANY, test_container_obj, True, True)
mock.ANY, test_container_obj, True, True, False, 'all', None)
@patch('zun.compute.api.API.container_logs')
@patch('zun.objects.Container.get_by_name')
@ -888,7 +888,7 @@ class TestContainerController(api_base.FunctionalTest):
self.assertEqual(200, response.status_int)
mock_container_logs.assert_called_once_with(
mock.ANY, test_container_obj, True, True)
mock.ANY, test_container_obj, True, True, False, 'all', None)
@patch('zun.compute.api.API.container_logs')
@patch('zun.objects.Container.get_by_uuid')
@ -901,12 +901,11 @@ class TestContainerController(api_base.FunctionalTest):
container_uuid = test_container.get('uuid')
response = self.app.get(
'/v1/containers/%s/logs?stderr=False&stdout=True' %
container_uuid)
'/v1/containers/%s/logs?stderr=True&stdout=True'
'&timestamps=False&tail=1&since=100000000' % container_uuid)
self.assertEqual(200, response.status_int)
mock_container_logs.assert_called_once_with(
mock.ANY, test_container_obj, True, False)
mock.ANY, test_container_obj, True, True, False, '1', '100000000')
@patch('zun.compute.api.API.container_logs')
@patch('zun.objects.Container.get_by_name')
@ -919,12 +918,14 @@ class TestContainerController(api_base.FunctionalTest):
container_name = test_container.get('name')
response = self.app.get(
'/v1/containers/%s/logs?stderr=False&stdout=True' %
container_name)
'/v1/containers/%s/logs?stderr=False&stdout=True'
'&timestamps=False&tail=all&since=2000-01-01 01:01:01,000'
% container_name)
self.assertEqual(200, response.status_int)
mock_container_logs.assert_called_once_with(
mock.ANY, test_container_obj, True, False)
mock.ANY, test_container_obj, True, False,
False, 'all', '2000-01-01 01:01:01,000')
@patch('zun.compute.api.API.container_logs')
@patch('zun.objects.Container.get_by_uuid')
@ -938,6 +939,25 @@ class TestContainerController(api_base.FunctionalTest):
'/v1/containers/%s/logs/' % container_uuid)
self.assertFalse(mock_container_logs.called)
@patch('zun.compute.api.API.container_logs')
@patch('zun.objects.Container.get_by_uuid')
def test_get_logs_with_invalid_since(self, mock_get_by_uuid,
mock_container_logs):
invalid_sinces = ['x11', '11x', '2000-01-01 01:01:01']
for value in invalid_sinces:
test_container = utils.get_test_container()
test_container_obj = objects.Container(self.context,
**test_container)
mock_get_by_uuid.return_value = test_container_obj
container_uuid = test_container.get('uuid')
params = {'since': value}
self.assertRaises(AppError, self.app.post,
'/v1/containers/%s/logs' %
container_uuid, params)
self.assertFalse(mock_container_logs.called)
@patch('zun.common.utils.validate_container_state')
@patch('zun.compute.api.API.container_exec')
@patch('zun.objects.Container.get_by_uuid')

10
zun/tests/unit/compute/test_compute_manager.py
View File

@ -323,8 +323,11 @@ class TestManager(base.TestCase):
def test_container_logs(self, mock_logs):
container = Container(self.context, **utils.get_test_container())
self.compute_manager.container_logs(self.context,
container, True, True)
mock_logs.assert_called_once_with(container, stderr=True, stdout=True)
container, True, True,
False, 'all', None)
mock_logs.assert_called_once_with(container, stderr=True, stdout=True,
timestamps=False, tail='all',
since=None)
@mock.patch.object(fake_driver, 'show_logs')
def test_container_logs_failed(self, mock_logs):
@ -332,7 +335,8 @@ class TestManager(base.TestCase):
mock_logs.side_effect = exception.DockerError
self.assertRaises(exception.DockerError,
self.compute_manager.container_logs,
self.context, container, True, True)
self.context, container, True, True,
False, 'all', None)
@mock.patch.object(fake_driver, 'execute')
def test_container_execute(self, mock_execute):

3
zun/tests/unit/container/docker/test_docker_driver.py
View File

@ -281,7 +281,8 @@ class TestDockerDriver(base.DriverTestCase):
mock_container = mock.MagicMock()
self.driver.show_logs(mock_container)
self.mock_docker.get_container_logs.assert_called_once_with(
mock_container.container_id, True, True)
mock_container.container_id, True, True, False, False,
'all', None)
def test_execute(self):
self.mock_docker.exec_create = mock.Mock(return_value='test')