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

Container name pattern ignored when stopping #1168

Closed
jakub-bochenski opened this issue Feb 28, 2019 · 9 comments · Fixed by #1427
Closed

Container name pattern ignored when stopping #1168

jakub-bochenski opened this issue Feb 28, 2019 · 9 comments · Fixed by #1427
Labels

Comments

@jakub-bochenski
Copy link
Contributor

jakub-bochenski commented Feb 28, 2019

Description

I want to run concurrent builds on a single docker host (for CI).

I started two builds with -Ddocker.containerNamePattern="$BUILD_TAG-%n-%i" (e.g. -Ddocker.containerNamePattern=build-1-%n-%i and -Ddocker.containerNamePattern=build-2-%n-%i).

I expected the builds to ignore each other's containers, only stopping the ones that match their respective build pattern, but instead they stop each other's containers.

This results in errors because containers being stopped mid-use.

12:59:54 [ERROR] DOCKER> [acme-all:latest] "acme-all": Container stopped with exit code 137 unexpectedly after 35529 ms while waiting on healthcheck '"CMD-SHELL", "wget --server-response -q -O /dev/null http://localhost:8080/acme/status || exit 1"'
12:59:54 [ERROR] DOCKER> Error occurred during container startup, shutting down...
12:59:54 [ERROR] DOCKER> Unable to remove container [b097fd97dc80] : removal of container b097fd97dc80 is already in progress (Conflict: 409) [removal of container b097fd97dc80 is already in progress (Conflict: 409)]

While in the other build log there is

12:59:54 [INFO] DOCKER> [acme-all:latest]: Stop and removed container b097fd97dc80 after 0 ms

Info

@jakub-bochenski
Copy link
Contributor Author

I've created a sample project to demonstrate the issue: https://github.com/jakub-bochenski/demp-concurrent-stop

This is a major blocker for me, so I would appreciate at least some hints on how to work around this.

@jakub-bochenski jakub-bochenski changed the title Container name pattern ignored when stopping? Container name pattern ignored when stopping Mar 1, 2019
@jakub-bochenski
Copy link
Contributor Author

Looking at https://github.com/fabric8io/docker-maven-plugin/blob/master/src/main/java/io/fabric8/maven/docker/util/ContainerNamingUtil.java#L66 there is no code to differentiate images created with different name patterns.

Seems like the only thing that matters is GAV and the image used for the container. The name pattern is not taken into account to differentiate containers. This is not what the documentation suggests

You can combine the placeholders in any combination and will be resolved during docker:start, docker:stop and docker:watch.

Would you accept a PR that would change it, so that only containers started with the same name pattern would be considered?

@rhuss
Copy link
Collaborator

rhuss commented Apr 6, 2019

Sorry for the long delay, but yes, it sounds like a very valid bug. If you would send a PR, that would be awesome. Currently I have a bit of a time window to work on d-m-p, so happy to get a fix in quickly.

@rhuss rhuss added the bug label Apr 6, 2019
@jakub-bochenski
Copy link
Contributor Author

OK, thanks
I'll get round to do this eventually, for now I have a workaround in place and some other priorities

@rhuss
Copy link
Collaborator

rhuss commented Apr 12, 2019

thanks, I totally understand and happy that you found a solution. I keep that bug open, maybe someone else wants to jump on it.

@j3t
Copy link
Contributor

j3t commented Jan 14, 2021

We have a similar issue but in our case the containerNamePattern is just %a. I can provide a fix for that and maybe I can also try to fix the original issue.

@j3t
Copy link
Contributor

j3t commented Jan 14, 2021

Regarding the original issue, in case the containerNamePattern contains the %i or %t placeholder then we probably cannot fix it easily.

@jakub-bochenski
Copy link
Contributor Author

I think that would be appreciated. Unfortunately I think I won't be able to work on it any time soon

@j3t
Copy link
Contributor

j3t commented Jan 15, 2021

The current implementation treats index placeholders specially. All containers are stopped except the one with the highest index (see https://github.com/fabric8io/docker-maven-plugin/blob/master/src/main/java/io/fabric8/maven/docker/util/ContainerNamingUtil.java#L85). Why is this necessary? I don't get it.

However, for me it's not clear how we should handle the stopping process when the containerNamePattern contains %i or %t. Both parameters are automatically generated (see io.fabric8.maven.docker.util.ContainerNamingUtil#formatContainerName) and they are not available when the stop mojo is executed.

Proposal:

  • all containers where the name matches the containerNamePattern are stopped
  • %a is replaced with the alias of the image configuration (equivalent to the start mojo)
  • %n is replaced with the name of the image configuration (equivalent to the start mojo)
  • %i is replaced with any number (\d+)
  • %t is replaced with any date (\d{10,})

@rohanKanojia rohanKanojia added this to the 0.35.0 milestone Mar 7, 2021
@rohanKanojia rohanKanojia removed this from the 0.35.0 milestone Apr 4, 2021
j3t pushed a commit to j3t/docker-maven-plugin that referenced this issue Apr 19, 2021
rhuss added a commit that referenced this issue Jul 20, 2021
…e stopped (#1427)

Co-authored-by: Jens Thielscher <jens.thielscher@sabio.de>
Co-authored-by: Roland Huß <roland@ro14nd.de>
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 a pull request may close this issue.

4 participants