Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
OlegSmelov opened this issue Jan 14, 2020 · 1 comment
Open

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

OlegSmelov opened this issue Jan 14, 2020 · 1 comment

Comments

@OlegSmelov
Copy link

OlegSmelov commented Jan 14, 2020

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?

@jerith
Copy link
Member

jerith commented Jan 16, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants