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

fix stop mojo by taking container name pattern into account #1427

Merged
merged 3 commits into from
Jul 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 41 additions & 10 deletions src/main/java/io/fabric8/maven/docker/util/ContainerNamingUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it correct that this changes the current behaviour in a non-backwards compatible way? (i.e. that all containers are stopped now by default, not the latest started)

If so we would need to be more careful to keep the old behaviour (at least for a certain deprecation period) and introduce this new behaviour either by a certain indication in the container name pattern or if this is not possible, with a new flag for docker:stop.

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, that's true. I have changed it so that the new approach is only applied if the container name pattern doesn't contain an index placeholder. This way it should be backward compatible again.


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<>();
Expand Down
10 changes: 10 additions & 0 deletions src/test/java/io/fabric8/maven/docker/BaseMojoTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
45 changes: 40 additions & 5 deletions src/test/java/io/fabric8/maven/docker/StopMojoTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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");
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down