-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Upgrade client to fix build, authentication, and image issues #26
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
Conversation
Conflicts: CONTRIBUTING.md
| @@ -1,4 +1,6 @@ | |||
| FROM ubuntu:14.04 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A previous commit had set the base image to ubuntu:14:04 as a workaround for incorrectly packaged dependencies because ubuntu contains many of those dependencies (and much more besides). Proper use of golang-builder to package all of watchtower's dependencies into a single binary executable allows us to revert to a minimal base image.
Load authentication credentials for available credential stores in order of preference: 1. Environment variables REPO_USER, REPO_PASS 2. Docker config files Request image pull with authentication header. Wait until pull request is complete before exiting function.
… on environment variables
…ays used. This causes authentication failures on registries that don't match, including public registries. Fallback to no-authentication to handle the case of public registries.
|
I found a couple of issues still remained with authentication. Authentication was failing silently for private registries. The upgraded docker api client was not loading authentication credentials from the docker config by default as I had thought.
I realised afterwards that the default docker config credential store cannot be supported since the host's docker config files are not available to the container at runtime (and cannot be mounted into a |
|
Any chance of you posting your version of the watchtower till it is merged in? Really need the config.json support |
|
@rosscado It would be awesome if you created a temp hub publish |
|
@seertenedos It is available in his own repo. Or did I get your message wrong? |
|
@dolanor @rosscado I meant it would be great if it could be put on Docker Hub. Also why is the config.jon not possible? Couldn't the user just copy it to a folder on the host and use volume mapping or something to pass it in at a particular location? Or maybe a small web Ui to configure it. In my case i have 2 private repositories. A dockerhub one and canister.io since it allows multi repositories for free. That said it is really the canister.io that i care about with watchtower |
|
@rosscado thanks so much it works well. One question. Is there a way to specify repository the password is for like for cloud.canister.io so that it only uses the username and password for containers from that repository and will not try to user the username and password for the containers from other repositories? I know i can just run 2 copies of watch tower and list every container each one supports but i was wondering if i could get away with one watchtower that could support authenticated and un authenticated |
|
@seertenedos there is not, but I appreciate the multiple-registry use case. The There might be a solution if I could mount the |
|
@rosscado putting the file into the container at any path would not be hard i would think using the -v param when creating the docker container like they do for the docker socket but i assume you are having an issue telling docker where the config file is and to use it. Is that the issue or is it that there is no home directory or even the possibility to create one on the scratch image. I thought it would just use the /root folder as the home directory as it would be running as root by default right so maybe if we mount it under there it would work? An alternative is couldn't we set $HOME using the -e command when creating the container and then put the file in $HOME/.docker/config.json? Also one other option is to set the config folder with --config param on the docker commandline (https://docs.docker.com/engine/reference/commandline/cli/) |
|
I'm very interested in that patch because I wanted to do it myself to see if it would magically support multiple network from libnetwork by using the official client. |
The circle.yml version on this branch will automatically trigger a Circle CI build and push the resulting Docker image to the DockerHub repo rosscado/watchtower. This is a temporary DockerHub repo for the rosscado/watchtower GitHub repo, and can be used to pull or test unofficial watchtower builds before they are merged into the official centurylink/watchtower repos.
|
@seertenedos @dolanor I have added config.json support as discussed. Watchtower will load private registry authentication credentials from the host's docker config file when mapped to the root ( Or to a different location specified by the If |
|
@rosscado I hit a new issue i did not have before. did you update the docker client version? I get the error below now and since i run on a QNAP NAS i can't change docker versions till they do if that is the issue. watchtower outputs that and dies right away |
|
@seertenedos a solution is available using the ExampleI refered to the API version problem in my opening post. I don't much like this default behaviour and am open to improvements if you can suggest any? |
|
@rosscado thanks that worked. I did not realise docker did that. Also i can confirm the config.json is working perfectly! one more question when it is checking to see if there is a new version how much data does it use? as in does it pull just a few kb to check or does it pull the whole image to find out there is no difference? At the minute i have watchtower on default check of every 5 min but wonder if i need to make them further apart |
|
@seertenedos afaict it just pulls a small amount of data to check for changes and pulls the full image only if a difference is found. But that's legacy functionality that I haven't modified in this pull request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was all changes pushed to rosscado/watchtower?
i can't seem to get this work by giving
docker run --name watchtower -v /var/run/docker.sock:/var/run/docker.sock -v ~/.docker/config.json:/config.json rosscado/watchtower
for private registry
EDIT:
Sorry my bad this works !!! thanks :D
|
So, I checked if my container joined the correct networks after a restart, and it is still not the case. So I guess I have to find where the code is and patch it. |
dolanor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code should be passed through go fmt.
| @@ -110,92 +115,93 @@ func (client dockerClient) StopContainer(c Container, timeout time.Duration) err | |||
| } | |||
|
|
|||
| func (client dockerClient) StartContainer(c Container) error { | |||
| bg := context.Background(); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not put some semicolon. It's one of my favourite feature of this language 😛
| c := Container{containerInfo: containerInfo, imageInfo: imageInfo} | ||
| if fn(c) { | ||
| c := Container{containerInfo: &containerInfo, imageInfo: &imageInfo} | ||
| if (fn(c)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove those parenthesis.
Can you pass your code through go fmt?
Removed hub_mirror deployment, came in with PR #26.
The watchtower build has been broken for several months since a failed attempt to introduce support for private docker registry authentication.
I have addressed these and related issues with the following changes.
Client Upgrade
Migrated the watchtower api client from the deprecated samalba/dockerclient to the officially supported docker/engine-api. The upgraded client achieves support for a private docker registry authentication use-case as described in issue #6, while leveraging
.docker/config.jsonfor authentication credentials as described in issue #3.Dependency Management
watchtower had been using a deprecated version of godep to manage its dependencies on external go packages. I tried migrating godep but godep failed to resolve the project's dependencies, so I abandoned it in favour of the simpler
go getcommand.Revert Base Image
Pull request #13 had also inflated the size of the watchtower image from <6MB to >190MB by replacing the minimal base image with
ubuntu:14.04. This had been introduced as a fix for package dependency errors and was unsuccessfully addressed by pull request #21.Resolving the dependency management errors allowed me to package the application into a small, self-contained binary with golang-builder as intended, and to revert the base image to
centurylink/ca-certs. This reduced the total image size to ~8MB.Security Changes
Changes in the docker api client have enabled additional use cases such as private repo authentication while simplifying configuration. As a result of the latter I have removed some of the TLS options, e.g. paths to CA keys. It is possible that some more complex use cases are no longer supported as a result. I have not tested this area thoroughly and would appreciate contributions to identify/resolve any lost functionality.
Other Changes
A new cli option
--apiversionsignals the minimum server api version to work with. This information is required by the new client library and I added the cli option as a way for users with older docker servers to avoid version incompatibility errors. Others may have better suggestions for how to handle this.