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

Creating a container is not offering options for linking container #42

Closed
MartinAhrer opened this issue Apr 1, 2015 · 13 comments
Closed
Assignees
Milestone

Comments

@MartinAhrer
Copy link
Contributor

Are there any plans to include container linking along with container creation? The Java API seems to provide that capability: https://github.com/docker-java/docker-java/blob/master/src/main/java/com/github/dockerjava/api/model/HostConfig.java

@bmuschko
Copy link
Owner

bmuschko commented Apr 1, 2015

Generally speaking, yes. Do you know with which version of Docker Java this was added? The plugin is still on version 0.10.5. I am waiting for 1.0.1 which is still not release. 1.0.0 had an issue.

Would you be interested in working on a pull request for this?

@MartinAhrer
Copy link
Contributor Author

To me it looks like this feature is already in 0.10.5: https://github.com/docker-java/docker-java/blob/docker-java-0.10.5/src/main/java/com/github/dockerjava/api/model/HostConfig.java. HostConfig is encapsulating much of the more advanced settings.

Ok, yes I can try working on a pull request. It shouldn't be too hard to pass through a collection of links to the Java API.

@bmuschko
Copy link
Owner

bmuschko commented Apr 3, 2015

As a side note: You'll need to create HostConfig via reflection. See DockerThreadContextClassLoader.groovy for more info.

@MartinAhrer
Copy link
Contributor Author

I'm already working on it and have already fleshed out the factory for the HostConfig, Links and Link types of the Docker Java API. However I seem to be running into a issue introduced with DockerThreadContextClassLoader. The Links type unlike most other container types does not offer a constructor accepting a List but only a Link[].

    def createLinks(List<Object> links) {
        def linkClass = loadClass('com.github.dockerjava.api.model.Link')
        def linksClass = loadClass('com.github.dockerjava.api.model.Links')
        def linksArray=Array.newInstance(linkClass, 0)
        Constructor constructor = linksClass.getConstructor(linksArray.class)
        constructor.newInstance((Object)links.toArray(linksArray))
    }

Doing the same with classes loaded with Class.fromString in some extra test project works fine. The easiest thing to do will be asking the Docker Java API maintainer to add the extra constructor to Links. But this will probably take much more time - so I would appreciate your input.

@bmuschko
Copy link
Owner

bmuschko commented Apr 3, 2015

I asked for something similar before. I'd say open an issue with them and see if they can incorporate it quickly. Optimally, you'd provide a pull request right away.

@MartinAhrer
Copy link
Contributor Author

Ok, my change was accepted in docker-java through issue 191. Also, version 1.1.0 of docker java has been released ;)

@paclat
Copy link

paclat commented Apr 8, 2015

It is great that you guys are on this as I was just needing it today. Is there an ETA on when it will get merged and released?

@bmuschko
Copy link
Owner

@MartinAhrer I just released a new version of the plugin that uses the latest version of the Docker Java plugin.

@MartinAhrer
Copy link
Contributor Author

Thanks for the update. I just double checked and I'm sorry that the docker-java guy did the release 30 minutes before he accepted the pull request with the constructor addition. So I fear I have to wait for his next release.

@bmuschko
Copy link
Owner

@MartinAhrer Try to push the guy a little bit if you want this to become part of a Docker Java release. His release cycle is very infrequent (maybe once every 2 months).

@MartinAhrer
Copy link
Contributor Author

A short update, docker-java 1.2 has been released on my request. So expect a pull request soon for container linking.

@bmuschko
Copy link
Owner

@MartinAhrer Cool. I'd actually prefer two different pull requests if possible. Easier to merge and verify.

@MartinAhrer
Copy link
Contributor Author

I'm going to provide that with a separated pull-request anyway. My current implementation and integration tests already work. Just for the Workflow test I need the container name feature pull-request in master so I can rebase my container-linking branch and implement the Workflow test.
So I hope soon you can consider the container name pull request #44 to be ready for a merge.

MartinAhrer added a commit to MartinAhrer/gradle-docker-plugin that referenced this issue Apr 16, 2015
MartinAhrer added a commit to MartinAhrer/gradle-docker-plugin that referenced this issue Apr 16, 2015
@bmuschko bmuschko self-assigned this Apr 18, 2015
@bmuschko bmuschko added this to the v2.3 milestone Apr 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants