allow linking to containers not managed by the plugin

[jgangemi]

i thought i had seen an issue regarding this before, but perhaps it was for referencing volumes from other containers.

in any case, when starting a container via the plugin, i'd like to be able to link to an existing container that has been started via some other mechanism.

in an effort not to break existing configuration, my thought is to just add a new xml block to the run section:

<linkExternal>
  <link>container_name:env_var_prefix</link>
</linkExternal>

thoughts? i'm open to different element names, etc.

[rhuss]

I thought linking already works also with external container names (and internal aliases). Is this not the case ?

[jgangemi]

unless i'm doing something wrong, it doesn't. i get this error message:

Cannot resolve image dependencies for start order

and w/o looking at the code, my guess is the plugin is looking for the image configuration so it can be started.

if you think we should just use the existing configuration elements and make the plugin smarter to check for an already running container if it can't find the image in the xml, that would work as well.

[rhuss]

Ah, ok. Good point, but this is more a bug than a missing feature. Any externally linked container (i.e. any container which is not in the realm of the plugin) should be assumed to be up and running and hence don't need to be considered when calculating the start order.

[jgangemi]

hrm - how would you compare this w/ #73 b/c they are related in the sense that you want to reference something from an existing container.

should that code be made smarter to check for a running image if it's not found in the plugin configuration or should we use separate tags?

[rhuss]

Yeah, you are right. Personally I think we shouldn't clutter the configuration namesspace to much, so I'm against having new tags without a super good reason ;-)

Maybe we can really be that smart to (a) first check whether it's an internal container (that we already do) and if not (b) check if another container with this name is running and than use that one. If neither (a) nor (b) then --> error.

What do you think ?

[jgangemi]

sounds good to me!

[rhuss]

ok, then let's go this route and fix #73 in a similar way. I think we already have a DockerAccess.hasContainer() (and if not, should not hard to add).

If you like you can take it, otherwise I can go for it sometimes later (backlog is already quite long ;-)

[jgangemi]

i'll take care of it - i need this for something else i'm doing so that will motivate me to get it done.

[rhuss]

Thanks a lot !

[rhuss]

Any news on that one ? I want to cut a release on Friday and wonder whether it could be included.

No pressure, though ;-P (really)

[jgangemi]

unfortunately no, i'm still working on it - other things have gotten in the way.

i will try and get it finished over the weekend.

[rhuss]

No hurry, there will always be a next release ;-P

[jgangemi]

i started working on this and i've got a question...

DockerAccess is becoming a little 'crowded' and i was going to pull some of the fine grained methods out of that class into a QueryService similar to what was done w/ the RunSevice (although that was spawned from code in the mojos) but i wanted to have a better understanding of why you made it a component before i did my implementation.

what does that get us verse just creating an instead of the class in the mojo, like how the LogDispatcher is handled, and just working w/ it that way? one thing it seems to prevent is the ability to inject dependencies into the component itself - i will admit i'm not a fan of passing the DockerAccess instance to every method, so i was trying to see if i could inject it into the service, but given how the docs for plexus are rather sparse now that codehaus has shutdown, i didn't have much luck.

worst case i figured i could just inject it w/ a setter at the start of the execute method which lead me to wonder why not just call new in the mojo and pass the client as a constructor arg.

[rhuss]

Yeah, you are right. DockerAccess indeed is a bit too large. I also already thought about splitting it up.

The story behind RunService: My use case here was that for docker:watch I need to build images and to restart containers. These functionality was included in BuildMojo and RunMojo respectively. These Mojos were also too large. In order to avoid code duplication I made two things:

• Introduced a AbstractDockerBuildMojo which is inherited by BuildMojo and WhatchMojo.
• Introduced a new layer (I call it 'service layer') between the DAO (DockerAccess) and the Mojos. In that case it is the RunService which is both used by RunMojo and WatchMojo.

Of course ideally we would also have a BuildService similar to RunService since composition is nearly always better than inheritance. The problem with Maven is, although we have Plexus ad DI container, there are certain parts which can be only injected into Mojos (like the MavenProject I think). So sometimes you need inheritance in the Mojo hierarchy, but I have to revisit this again.

The point about RunService is that it carries state about the containers stareted. This state is shared by start, stop and watch. So I need a singleton which can be easily achieved via plexus and this singleton is shared by all mojos.

DockerAcess can not really be injected because the creation is a bit more complex than a simple constructor and needs mojo parameters, too, which not available if using injection. So we can't use Plexus alone. Moving it down is indeed no so nice (but on the other not so super problematic), we could either consider the setter approach you propose or some thread local stuff with static methods for getting to DockerAccess.

new RunService(DockerAccess access) is not possible because you want to share this singleton across multiple instances as said before.

Altough DockerAccess is a bit lengthy, we could split it up into multiple DAOs (QueryDao, ImageDao, ...). The difference of a 'Service' is that it is on top of the DAO. Ideally we have a complete layer like Mojo -> Service -> Dao, so that no Mojo accesses the DockerAccess directly and mostly serves only as a (stupid) entry point.

[jgangemi]

thanks for that explanation! it makes sense for shared state to be handled that way. i don't think a query service will need to share state, but i think it will probably be best to follow the component regardless.

i think a setter method could be added to RunService to inject DockerAccess just like you do for the logger which would simplify things.

ok...i'll finish this and put together an initial pull request and we can review. i think what i have in mind should offer some good flexibility and also puts us in a good position to offer a docker:ps mojo.

[jgangemi]

fixed in 0.13.2