fix #1168 take container name pattern into account when containers ar…

…e stopped (#1427)

Co-authored-by: Jens Thielscher <jens.thielscher@sabio.de>
Co-authored-by: Roland Huß <roland@ro14nd.de>

src/main/java/io/fabric8/maven/docker/util/ContainerNamingUtil.java

@@ -60,11 +60,21 @@ public static String formatContainerName(final ImageConfiguration image,
     }
 
     /**
-     * Keep only the entry with the higest index if an indexed naming scheme for container has been chosen.
+     * Keep only the entry with the higest index if an indexed naming scheme for container has been chosen or if the container name
+     * pattern doesn't contain any index placeholders then filter containers (analog
+     * {@link #formatContainerName(ImageConfiguration, String, Date, Collection)} but with stopping them in mind).<br>
+     *<br>
+     *The placeholders of the containerNamePattern are resolved as follows:<br>
+     *<ul>
+     *<li>%a is replaced with the image alias</li>
+     *<li>%n is replaced with the image name</li>
+     *<li>%t is replaced with any date (regex \d{10,})</li>
+     *<li>%i is replaced with any number (regex \d+)</li>
+     *</ul>
      * @param image the image from which to the the container pattern
      * @param buildTimestamp the timestamp for the build
      * @param containers the list of existing containers
-     * @return a list with potentially lower indexed entries removed
+     * @return filtered container instances, maybe empty but never null
      */
     public static Collection<Container> getContainersToStop(final ImageConfiguration image,
                                                             final String defaultContainerNamePattern,
@@ -73,22 +83,43 @@ public static Collection<Container> getContainersToStop(final ImageConfiguration
 
         String containerNamePattern = extractContainerNamePattern(image, defaultContainerNamePattern);
 
-        if (shouldUseEmptyName(containerNamePattern) || !containerNamePattern.contains(INDEX_PLACEHOLDER)) {
+        if (shouldUseEmptyName(containerNamePattern)) {
             return containers;
         }
 
-        final String partiallyApplied =
-                replacePlaceholders(
-                        containerNamePattern,
-                        image.getName(),
-                        image.getAlias(),
-                        buildTimestamp);
+        // if the pattern contains an index placeholder -> we have to stick to the old approach (backward compatibility)
+        if (containerNamePattern.contains(INDEX_PLACEHOLDER)) {
+            final String partiallyApplied =
+                    replacePlaceholders(
+                            containerNamePattern,
+                            image.getName(),
+                            image.getAlias(),
+                            buildTimestamp);
+
+            return keepOnlyLastIndexedContainer(containers, partiallyApplied);
+        }
 
-        return keepOnlyLastIndexedContainer(containers, partiallyApplied);
+        return replacePlaceholdersAndFilterContainers(containers, image, containerNamePattern);
     }
 
+
     // ========================================================================================================
 
+    private static Collection<Container> replacePlaceholdersAndFilterContainers(Collection<Container> containers, ImageConfiguration image,
+            String containerNamePattern) {
+        Map<String, FormatParameterReplacer.Lookup> lookups = new HashMap<>();
+        lookups.put("a", () -> image.getAlias());
+        lookups.put("n", () -> cleanImageName(image.getName()));
+        lookups.put("t", () -> "\\d{10,}");
+        lookups.put("i", () -> "\\d+");
+
+        String appliedContainerNamePattern = new FormatParameterReplacer(lookups).replace(containerNamePattern);
+
+        return containers.stream()
+                .filter(container -> container.getName().matches(appliedContainerNamePattern))
+                .collect(Collectors.toList());
+    }
+
     private static String replacePlaceholders(String containerNamePattern, String imageName, String nameAlias, Date buildTimestamp) {
 
         Map<String, FormatParameterReplacer.Lookup> lookups = new HashMap<>();

src/test/java/io/fabric8/maven/docker/BaseMojoTest.java

@@ -141,6 +141,16 @@ protected ImageConfiguration singleImageWithRun() {
                 .build();
     }
 
+    protected ImageConfiguration singleImageWithRunAndAlias(String alias) {
+        return new ImageConfiguration.Builder()
+                .name("example:latest")
+                .alias(alias)
+                .runConfig(new RunImageConfiguration.Builder()
+                        .cmd("echo")
+                        .build())
+                .build();
+    }
+
     protected ImageConfiguration singleImageWithCopy(List<CopyConfiguration.Entry> entries) {
         return singleImageWithCopyNamePatternAndCopyEntries(null, entries);
     }

src/test/java/io/fabric8/maven/docker/StopMojoTest.java

@@ -286,8 +286,7 @@ public void stopWithMultipleImagesIndexPatternMissingIndices(@Mocked Container r
      * Mock project with one image, query service indicates three running images, none labelled,
      * but allContainers is true.
      *
-     * The containers are named with ascending indices - but this doesn't match the container
-     * name pattern, so all are stopped.
+     * One container matches the name pattern and the others not, so that not all containers are stopped.
      *
      * @throws IOException
      * @throws MojoExecutionException
@@ -300,21 +299,57 @@ public void stopWithMultipleImagesNoIndexNamePattern(@Mocked Container running1,
 
         givenAllContainersIsTrue();
 
-        givenRunningContainer(running1, "container-id-1", "example-1");
+        givenRunningContainer(running1, "container-id-1", "example");
         givenRunningContainer(running2, "container-id-2", "example-2");
         givenRunningContainer(running3, "container-id-3", "example-3");
         givenContainersAreRunningForImage("example:latest", running1, running2, running3);
 
-        // If name pattern doesn't contain index placeholder, all containers are stopped
+        // If name pattern doesn't contain index placeholder then matching containers are stopped
         Deencapsulation.setField(stopMojo, "containerNamePattern", "%n");
 
         whenMojoExecutes();
 
         thenContainerLookupByImageOccurs("example:latest");
         thenListContainersIsNotCalled();
         thenContainerIsStopped("container-id-1", false, false);
+        thenContainerIsNotStopped("container-id-2");
+        thenContainerIsNotStopped("container-id-3");
+    }
+
+    /**
+     * Mock project with one image, query service indicates three running images, none labelled,
+     * but allContainers is true.
+     *
+     * The containers are named with ascending indices - but this doesn't match the container
+     * name pattern, so all are stopped.
+     *
+     * @throws IOException
+     * @throws MojoExecutionException
+     * @throws ExecException
+     */
+    @Test
+    public void stopWithAliasNamePattern(@Mocked Container running1, @Mocked Container running2,
+            @Mocked Container running3) throws IOException, MojoExecutionException, ExecException {
+        this.consoleLogger.setThreshold(0);
+        givenProjectWithResolvedImage(singleImageWithRunAndAlias("example-2"));
+
+        givenAllContainersIsTrue();
+
+        givenRunningContainer(running1, "container-id-1", "example-1");
+        givenRunningContainer(running2, "container-id-2", "example-2");
+        givenRunningContainer(running3, "container-id-3", "example-3");
+        givenContainersAreRunningForImage("example:latest", running1, running2, running3);
+
+        // If name pattern doesn't contain index placeholder, all containers are stopped
+        Deencapsulation.setField(stopMojo, "containerNamePattern", "%a");
+
+        whenMojoExecutes();
+
+        thenContainerLookupByImageOccurs("example:latest");
+        thenListContainersIsNotCalled();
+        thenContainerIsNotStopped("container-id-1");
         thenContainerIsStopped("container-id-2", false, false);
-        thenContainerIsStopped("container-id-3", false, false);
+        thenContainerIsNotStopped("container-id-3");
     }
 
     /**

src/test/java/io/fabric8/maven/docker/util/ContainerNamingUtilTest.java

@@ -139,6 +139,23 @@ public void testContainersToStop() {
         Assert.assertEquals(container2, filtered.iterator().next());
     }
 
+    @Test
+    public void testContainersToStopWithAlias() {
+        new Expectations() {{
+            container1.getName();result = "container-1";
+            container2.getName();result = "container-2";
+
+        }};
+        Collection<Container> containers = Arrays.asList(container1, container2);
+        Collection<Container> filtered = ContainerNamingUtil.getContainersToStop(
+                imageConfiguration("jolokia/jolokia_demo","container-2", "%a"),
+                null,
+                new Date(123456),
+                containers);
+        Assert.assertEquals(1, filtered.size());
+        Assert.assertEquals(container2, filtered.iterator().next());
+    }
+
     private ImageConfiguration imageConfiguration(String name, String alias, String containerNamePattern) {
         ImageConfiguration.Builder builder = new ImageConfiguration.Builder().name(name).alias(alias);
         if (containerNamePattern != null) {