seaworthy icon indicating copy to clipboard operation
seaworthy copied to clipboard

Wait patterns don't work with non-local log adapters

Open OlegSmelov opened this issue 6 years ago • 1 comments

I have a CI server that has --log-driver fluentd configuration parameter passed to dockerd, making Fluentd the default logging adapter. The side effect of using it is that docker logs command no longer works:

e = HTTPError('501 Server Error: Not Implemented for url: http+docker://localhost/v1.35/containers/9f1e70c461ef6a6111c9433f778dcce7a77b2e3a8dac31d20b03d44bdd2434df/logs?stderr=1&stdout=1&timestamps=0&follow=1&tail=all',)

    def create_api_error_from_http_exception(e):
        """
        Create a suitable APIError from requests.exceptions.HTTPError.
        """
        response = e.response
        try:
            explanation = response.json()['message']
        except ValueError:
            explanation = (response.content or '').strip()
        cls = APIError
        if response.status_code == 404:
            if explanation and ('No such image' in str(explanation) or
                                'not found: does not exist or no pull access'
                                in str(explanation) or
                                'repository does not exist' in str(explanation)):
                cls = ImageNotFound
            else:
                cls = NotFound
>       raise cls(e, response=response, explanation=explanation)
E       docker.errors.APIError: 501 Server Error: Not Implemented ("configured logging driver does not support reading")

As a workaround, I've set log_config to the JSON adapter (Docker default) explicitly:

from docker.types import LogConfig
from seaworthy.definitions import ContainerDefinition

container = ContainerDefinition(
    "my-service",
    "my-service:latest",
    wait_patterns=[r"Listening at: http://0\.0\.0\.0:9000"],
    create_kwargs={
        "ports": {"9000": None},
        "log_config": LogConfig(type=LogConfig.types.JSON),  # <------
    },
)

I think Seaworthy should set this parameter by default when using wait_patterns. What do you think?

OlegSmelov avatar Jan 14 '20 14:01 OlegSmelov

This is a tricky one. On the one hand, it would be nice to have stuff Just Work. On the other, I expect there are many combinations of config options that break Seaworthy's assumptions about docker's behaviour and trying to handle them all would add a lot of extra complexity on top of all the complexity already inherent in what we're trying to do.

In this case, I think it's reasonable for Seaworthy to assume that it's going to be used with a docker configuration that allows reading logs. There are also potentially cases where it would make sense to use a different log driver (maybe one that logs to a file for further analysis later in a CI/CD flow) that still supports the functionality Seaworthy requires.

For environments where docker is configured with problematic defaults, perhaps it would make sense to use a custom ContainerDefinition subclass (or wrapper) that adds the necessary parameters. There may also be some value in some kind of built-in mechanism for specifying common parameters across all definitions, but I'm not quite sure what that would look like.

jerith avatar Jan 16 '20 10:01 jerith