DockerLogsContainer implementation

[double16]

This task implements docker logs. Fixes #132

[cdancy]

@double16 thanks for the PR! A few things:

1. Any particular reason you created a separate functional test file? We've been putting all functional tests inside DockerWorkFlowFunctionalTest.
2. For loading classes from docker-java please follow the model we set within ThreadContextClassLoader and it's impl DockerThreadContextClassLoader.
3. Consider creating an inner class for the tail variables. Having 1 variable mean potentially N number of things will be confusing for users. Take a look at DockerCreateContainer to see how we've done similar things in the past.

[cdancy]

Should make these variables and default to true. If it's an option then it should be exposed to users to toggle whichever they may need.

[double16]

I'll address these and one more thing ... I forgot to add documentation. 😊

[cdancy]

On further thinking: lets keep the functional tests where you have them now. Our all-in-one functional test class is getting quite big and I think making it bigger, if only to keep tests in one place, is not the way to go.

[double16]

I've addressed your comments. For the tail, I opted to go with two tail options similar to the docker-java API and throw an exception if both are specified.

[cdancy]

Throw GradleException instead. It's standard to expect as much from a gradle plugin.

[double16]

InvalidUserDataException extends GradleException, so being more specific isn't hurting handling the more generic case, right? According to Gradle API, InvalidUserDataException is appropriate here:

A InvalidUserDataException is thrown, if a user is providing illegal data for the build.

[cdancy]

Ahhhh gotcha. I assumed it was java exception. Ok lets keep it. The standard we've been keeping thus far is to use the generic GradleException in all places. Having something a bit more specific can't hurt.

[cdancy]

@double16 just a couple minor things related to logging and I think we are set. Have you run the integration tests yet?

[double16]

@cdancy I have run all of the tests for the project and they pass, except for three that push to DockerHub and private repos, which I didn't configure.

[cdancy]

@double16 lgtm. Squash and rebase then I'll merge.

[double16]

@cdancy squashed and ready to go

[cdancy]

Thanks @double16!

[cdancy]

@double16 have you used this task at all? I'm more wondering how you are using it assuming you're doing so. My thoughts surround the capturing of the logs themselves and if you're doing so through some mechanism. I'm thinking of putting together some sort of sink for this task or some way to capture the output so folks can process it and/or make some decision further downstream. With all that in mind I'm wondering how you are going about things and if you have any thoughts surrounding such a thing.

[double16]

I am currently using this task in a Bamboo plan. Bamboo captures the Gradle output and saves the logs so if something goes wrong (or it succeeds), the logs can be inspected. I've really only used the logs to review when something goes wrong.

I avoid making decisions based on log output. I rely on output files with a specific purpose such as JUnit XML, or JSON, etc.

Allowing a sink to be specified sounds like a good idea, as long as it's optional. I'd really like to keep the functionality to output to stdout/stderr so CI tools like Bamboo or Jenkins can capture the output.

[cdancy]

Aaahhhh a fellow Bamboo user! Our use case here is more for checking when a given container is "live" by waiting for our app to print out an "init is complete" message.

I'd definitely keep the functionality as-is, as we've already released and folks will have come to depend on it. Any additions would be purely optional at this point.

Thanks for the info.