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

Watch image #141

Merged
merged 2 commits into from
May 15, 2015
Merged

Watch image #141

merged 2 commits into from
May 15, 2015

Conversation

iocanel
Copy link
Member

@iocanel iocanel commented Apr 10, 2015

No description provided.

@rhuss
Copy link
Collaborator

rhuss commented May 13, 2015

Sorry for the delay. Will check this ASAP.

@iocanel
Copy link
Member Author

iocanel commented May 14, 2015

replaced single thread logic with scheduled executor and squashed changes.

@rhuss
Copy link
Collaborator

rhuss commented May 15, 2015

Thanks, I applied the PR, looks good to me.

However, I have somme comments :)

  • When two container share volumes and the one 'owning' the volume gets updated, the other doesn't get the updated data. So it probably had to be restarted as well. We could add an option like 'restartVolumeDependenciesin the watch configuration. E.g. in that way the sample indata-jolokia-demo` which contains of a "data" image and a "server" image (tomcat-7) where '/maven' is linked to the tomcat-7 container which deploys wars within it automatically during start would work when updating "data" by restarting "tomcat-7", too. However, the logic is a bit more involved then. For now, I will mention that in the documentation, but might be worth an extra issue.

  • When a container is named (with the namingStrategy=alias where the alias is used to name the container), the merely stopping the container and starting a new is not enough. It needs to be removed, too. I added this, since I think, thats save, too.

  • When an image is removed (which can be done for data containers which are already stopped), an error occurs periodically. I think, thats ok.

  • I wonder whether we should kill the containers (i.e. do a docker:stop) if the user kills maven when watching. I think that's the expected behaviour. What do you think ? Or should we simply document that in order to cleanup a docker:stop is needed afterwards ? (as it would be when docker:start is called without watching)

  • Also, I wonder how it works with network links.I think it's the same as for the shared volumes: If a the image other dependent is updated, the dependant images must be restarted, too.

  • I renamed docker.watch.enable to simply docker.watch so that the command line can look like

    mvn -Ddocker.watch docker:start
    

Since I want to cut a release pretty soon (and still have some PRs to look over), I suggest, that we leave the implementation as it is now, add some remarks in the doc (will do that) and open new issues for things to improve like auto-starting dependant images, too.

rhuss added a commit that referenced this pull request May 15, 2015
... and some minor tunings #141
@rhuss rhuss added the applied label May 15, 2015
@rhuss rhuss added this to the 0.11.4 milestone May 15, 2015
@rhuss rhuss merged commit 3b4574f into fabric8io:master May 15, 2015
leusonmario pushed a commit to leusonmario/docker-maven-plugin that referenced this pull request Aug 18, 2018
Write digest to image info file on push
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants