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

stop and remove using regex names #900

Closed

Conversation

transamericamoon
Copy link
Contributor

@transamericamoon transamericamoon commented Nov 17, 2017

So we use the maven artifactID as the image name and the version number as the tag for naming our images and containers, the problem is once the version number changes, we can no longer stop or remove old images to deploy the new ones.

This was a quick fix we put together, i don't know how useful this would be to others, But thought i'd at least throw it out there. If it's not useful feel free to close.

Usage:
Enabled stop and remove using regex.

<properties>
        <docker.stopUsingRegex>true</docker.stopUsingRegex>
        <docker.removeUsingRegex>true</docker.removeUsingRegex>
...

Then under the image tags:

<image>
  <alias>${project.artifactId}_${project.version}</alias>
  <aliasRegex>${project.artifactId}.*${revision.branch}-${revision.type}</aliasRegex>
  <name>${project.artifactId}:${project.version}</name>
  <nameRegex>${project.artifactId}.*${revision.branch}-${revision.type}</nameRegex>
....

@rhuss rhuss force-pushed the regex-stop-remove branch from 08fe3ab to a4cbc30 Compare November 27, 2017 07:47
@codecov
Copy link

codecov bot commented Nov 30, 2017

Codecov Report

Merging #900 into master will decrease coverage by 1.56%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master     #900      +/-   ##
============================================
- Coverage     53.78%   52.21%   -1.57%     
+ Complexity     1617     1501     -116     
============================================
  Files           154      150       -4     
  Lines          8174     8034     -140     
  Branches       1251     1212      -39     
============================================
- Hits           4396     4195     -201     
- Misses         3342     3426      +84     
+ Partials        436      413      -23
Impacted Files Coverage Δ Complexity Δ
...8/maven/docker/assembly/DockerAssemblyManager.java 16.79% <ø> (ø) 11 <0> (ø) ⬇️
...abric8/maven/docker/config/ImageConfiguration.java 49.35% <0%> (-4.18%) 15 <0> (ø)
.../io/fabric8/maven/docker/service/QueryService.java 0% <0%> (ø) 0 <0> (ø) ⬇️
.../main/java/io/fabric8/maven/docker/RemoveMojo.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...ava/io/fabric8/maven/docker/access/UrlBuilder.java 65.28% <0%> (-1.1%) 21 <0> (ø)
...rc/main/java/io/fabric8/maven/docker/StopMojo.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...ven/docker/access/hc/DockerAccessWithHcClient.java 12.35% <0%> (-1.88%) 13 <0> (ø)
...rc/main/java/io/fabric8/maven/docker/SaveMojo.java 0% <0%> (-77.03%) 0% <0%> (-34%)
.../java/io/fabric8/maven/docker/util/AnsiLogger.java 41.98% <0%> (-10.29%) 17% <0%> (-10%)
...8/maven/docker/config/BuildImageConfiguration.java 83.23% <0%> (-1.47%) 62% <0%> (-10%)
... and 11 more

@transamericamoon transamericamoon force-pushed the regex-stop-remove branch 2 times, most recently from c5fe3f0 to 68e4c12 Compare November 30, 2017 21:34
@rhuss rhuss force-pushed the regex-stop-remove branch from 51ceadd to c7d2cc9 Compare January 19, 2018 09:53
rhuss added a commit to transamericamoon/docker-maven-plugin that referenced this pull request Jan 19, 2018
* Based on the work of fabric8io#900 (thanks !)
* Fixed conflicts
* Removed the top-level properties and just check whether the ImageConfiguration has the regular expression set.
* Not sure how useful `aliasRegex` is as I suppose that an alias is a fixed name without any version information and stable across releases. For now I removed it (and moved to a `cleanupRegexp` parameter in the image configuration which is the image regexp to use for stop/remove.
* Added some docs and streamlined the code a bit
@rhuss
Copy link
Collaborator

rhuss commented Jan 19, 2018

Thanks for the PR, I think this a perfectly valid use case.

I resolved the conflicts and made some updates to streamline a bit:

  • Removed the top-level properties and just check whether the ImageConfiguration has the regular expression set.
  • Not sure how useful aliasRegex is as I suppose that an alias is a fixed name without any version information and stable across releases. For now I removed it (and moved to a cleanupRegexp parameter in the image configuration which is the image regexp to use for stop/remove.
  • Added some docs and streamlined the code a bit

'hope this is ok for you. wrt to aliasRegex, do you have a more concrete use case why this would be required ?

@@ -81,7 +86,7 @@ public String inspectExecContainer(String containerId) {
}

public String listContainers(String ... filter) {
Builder builder = u("containers/json");
Builder builder = u("containers/json").p("all", true);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why do need "all" here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is so even if the container is stopped it can still be removed. Without "all" only running containers are returned.

@rhuss
Copy link
Collaborator

rhuss commented Feb 7, 2018

fyi, just did a rebase and pushed.

@transamericamoon
Copy link
Contributor Author

transamericamoon commented Apr 16, 2018

Sorry did not send feedback I gleaned over this a long time ago and thought it was accepted, it kinda fell off the radar...

Alias regex was just so it would be consistent if they are using alias naming. ie.

<alias>service-${project.version}</alias>

could be removed with
<aliasRegex>service-.*</aliasRegex>

@rhuss
Copy link
Collaborator

rhuss commented Apr 6, 2019

@rohanKanojia this is another PR which would need a bit of love. If you feel fancy, you can resolve the conflicts (shouldn't be hard, and then let's try to get this merged).

wrosenuance added a commit to wrosenuance/docker-maven-plugin that referenced this pull request Apr 19, 2019
@rhuss
Copy link
Collaborator

rhuss commented Apr 21, 2019

@rohanKanojia @wrosenuance I think this PR overlaps with #1215 so we should decide which one to continue.

@rhuss
Copy link
Collaborator

rhuss commented Aug 10, 2019

We added now #1215, so this should cover your use case, too. You can expect a release over the weekend.

Thanks a lot for the PR and sorry for being that late ;-)

@rhuss rhuss closed this Aug 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants