client.containers.run returns None if none of json-file or journald logging drivers used

Signed-off-by: Artem Bolshakov <either.free@gmail.com>

5 files changed, 80 insertions, 4 deletions

diff --git a/docker/errors.py b/docker/errors.py
index 0da97f4..1f8ac23 100644
--- a/docker/errors.py
+++ b/docker/errors.py
@@ -127,8 +127,14 @@ class ContainerError(DockerException):
         self.command = command
         self.image = image
         self.stderr = stderr
-        msg = ("Command '{}' in image '{}' returned non-zero exit status {}: "
-               "{}").format(command, image, exit_status, stderr)
+
+        if stderr is None:
+            msg = ("Command '{}' in image '{}' returned non-zero exit "
+                   "status {}").format(command, image, exit_status, stderr)
+        else:
+            msg = ("Command '{}' in image '{}' returned non-zero exit "
+                   "status {}: {}").format(command, image, exit_status, stderr)
+
         super(ContainerError, self).__init__(msg)
 
 
diff --git a/docker/models/containers.py b/docker/models/containers.py
index cf01b27..a3598f2 100644
--- a/docker/models/containers.py
+++ b/docker/models/containers.py
@@ -667,6 +667,13 @@ class ContainerCollection(Collection):
             The container logs, either ``STDOUT``, ``STDERR``, or both,
             depending on the value of the ``stdout`` and ``stderr`` arguments.
 
+            ``STDOUT`` and ``STDERR`` may be read only if either ``json-file``
+            or ``journald`` logging driver used. Thus, if you are using none of
+            these drivers, a ``None`` object is returned instead. See the
+            `Engine API documentation
+            <https://docs.docker.com/engine/api/v1.30/#operation/ContainerLogs/>`_
+            for full details.
+
             If ``detach`` is ``True``, a :py:class:`Container` object is
             returned instead.
 
@@ -709,7 +716,14 @@ class ContainerCollection(Collection):
         if exit_status != 0:
             stdout = False
             stderr = True
-        out = container.logs(stdout=stdout, stderr=stderr)
+
+        logging_driver = container.attrs['HostConfig']['LogConfig']['Type']
+
+        if logging_driver == 'json-file' or logging_driver == 'journald':
+            out = container.logs(stdout=stdout, stderr=stderr)
+        else:
+            out = None
+
         if remove:
             container.remove()
         if exit_status != 0:
diff --git a/tests/integration/models_containers_test.py b/tests/integration/models_containers_test.py
index b76a88f..ce3349b 100644
--- a/tests/integration/models_containers_test.py
+++ b/tests/integration/models_containers_test.py
@@ -88,6 +88,24 @@ class ContainerCollectionTest(BaseIntegrationTest):
         assert 'Networks' in attrs['NetworkSettings']
         assert list(attrs['NetworkSettings']['Networks'].keys()) == [net_name]
 
+    def test_run_with_none_driver(self):
+        client = docker.from_env(version=TEST_API_VERSION)
+
+        out = client.containers.run(
+            "alpine", "echo hello",
+            log_config=dict(type='none')
+        )
+        self.assertEqual(out, None)
+
+    def test_run_with_json_file_driver(self):
+        client = docker.from_env(version=TEST_API_VERSION)
+
+        out = client.containers.run(
+            "alpine", "echo hello",
+            log_config=dict(type='json-file')
+        )
+        self.assertEqual(out, b'hello\n')
+
     def test_get(self):
         client = docker.from_env(version=TEST_API_VERSION)
         container = client.containers.run("alpine", "sleep 300", detach=True)
diff --git a/tests/unit/errors_test.py b/tests/unit/errors_test.py
index b78af4e..9678669 100644
--- a/tests/unit/errors_test.py
+++ b/tests/unit/errors_test.py
@@ -2,8 +2,10 @@ import unittest
 
 import requests
 
-from docker.errors import (APIError, DockerException,
+from docker.errors import (APIError, ContainerError, DockerException,
                            create_unexpected_kwargs_error)
+from .fake_api import FAKE_CONTAINER_ID, FAKE_IMAGE_ID
+from .fake_api_client import make_fake_client
 
 
 class APIErrorTest(unittest.TestCase):
@@ -77,6 +79,36 @@ class APIErrorTest(unittest.TestCase):
         assert err.is_client_error() is True
 
 
+class ContainerErrorTest(unittest.TestCase):
+    def test_container_without_stderr(self):
+        """The massage does not contain stderr"""
+        client = make_fake_client()
+        container = client.containers.get(FAKE_CONTAINER_ID)
+        command = "echo Hello World"
+        exit_status = 42
+        image = FAKE_IMAGE_ID
+        stderr = None
+
+        err = ContainerError(container, exit_status, command, image, stderr)
+        msg = ("Command '{}' in image '{}' returned non-zero exit status {}"
+               ).format(command, image, exit_status, stderr)
+        assert str(err) == msg
+
+    def test_container_with_stderr(self):
+        """The massage contains stderr"""
+        client = make_fake_client()
+        container = client.containers.get(FAKE_CONTAINER_ID)
+        command = "echo Hello World"
+        exit_status = 42
+        image = FAKE_IMAGE_ID
+        stderr = "Something went wrong"
+
+        err = ContainerError(container, exit_status, command, image, stderr)
+        msg = ("Command '{}' in image '{}' returned non-zero exit status {}: "
+               "{}").format(command, image, exit_status, stderr)
+        assert str(err) == msg
+
+
 class CreateUnexpectedKwargsErrorTest(unittest.TestCase):
     def test_create_unexpected_kwargs_error_single(self):
         e = create_unexpected_kwargs_error('f', {'foo': 'bar'})
diff --git a/tests/unit/fake_api.py b/tests/unit/fake_api.py
index ff0f1b6..2ba85bb 100644
--- a/tests/unit/fake_api.py
+++ b/tests/unit/fake_api.py
@@ -146,6 +146,12 @@ def get_fake_inspect_container(tty=False):
             "StartedAt": "2013-09-25T14:01:18.869545111+02:00",
             "Ghost": False
         },
+        "HostConfig": {
+            "LogConfig": {
+                "Type": "json-file",
+                "Config": {}
+            },
+        },
         "MacAddress": "02:42:ac:11:00:0a"
     }
     return status_code, response