#333: Some tuning and cleanup for new 'docker:stop' implementation

* Label is renamed to 'dmp.coordinates'
* `runId` removed from label
* renamed property for stopping all containers from the images to 'docker.allContainers' so that a 'mvn docker:stop -Ddocker.allContainers' will reproduce the old behaviour.
* Some minor refactorings

See also #87.

README.md

@@ -6,7 +6,7 @@
 [![Flattr](http://api.flattr.com/button/flattr-badge-large.png)](http://flattr.com/thing/73919/Jolokia-JMX-on-Capsaicin)
 
 This is a Maven plugin for managing Docker images and containers for your builds.
-The current version is **0.14.0** and works with Maven 3.2.1 and Docker 1.6.0 or later.
+The current version is **0.13.7** and works with Maven 3.2.1 and Docker 1.6.0 or later.
 
 The current Docker API version used is `v1.18` (which is the minimal required API version). If you want to use the 
 copy mode for `docker:watch` you need `v1.20` or greater (Docker 1.8.1). See the **[User Manual](https://rhuss.github.io/docker-maven-plugin)** 

doc/changelog.md

@@ -8,7 +8,7 @@
   - Move VOLUME to the end of the Dockerfile to allow initialization via RUN commands (#341)
   - Allow multiple configurations with different Docker hosts again (#320) 
   - docker:start blocks now only when system property docker.follow is given (#249) 
-  - `docker:stop` no longer acts like a sledgehammer (#87)
+  - `docker:stop` only stops containers started by this plugin by default (#87)
 
 * **0.13.6**
   - Don't use user from image when pulling base images ([#147](https://github.com/rhuss/docker-maven-plugin/issues/147))

doc/manual/docker-stop.md

@@ -14,13 +14,13 @@ container it finds whose image is defined in the project's configuration.
 Any existing containers found running whose image name matches but was not 
 started by the plugin will not be affected.
 
-It should be noted that any containers created prior to version `0.14.0` of the
-plugin may not be stopped correctly by the plugin b/c the label needed to tie
+It should be noted that any containers created prior to version `0.13.7` of the
+plugin may not be stopped correctly by the plugin because the label needed to tie
 the container to the project may not exist. Should this happen, you will need to
 use the Docker CLI to clean up the containers and/or use the `docker.sledgehammer`
 option listed below. 
 
-For tuning what should happen when stopping there are three global
+For tuning what should happen when stopping there are four global
 parameters which are typically used as system properties:
 
 * **keepContainer** (`docker.keepContainer`) If given will not destroy
@@ -34,10 +34,9 @@ parameters which are typically used as system properties:
 * **removeVolumes** (`docker.removeVolumes`) If given will remove any
   volumes associated to the container as well. This option will be ignored
   if either `keepContainer` or `keepRunning` are true.
-* **sledgehammer** (`docker.sledgehammer`) Stops and removes any container that
+* **allContainers** (`docker.allContainers`) Stops and removes any container that
   matches an image defined in the current project's configuration. This was the
-  default behavior of the plugin prior to [issue #87](https://github.com/rhuss/docker-maven-plugin/issues/87)
-  being resolved.
+  default behavior of the plugin prior up to version 0.13.6
 
 Example: 
 

pom.xml

@@ -6,7 +6,7 @@
 
   <groupId>org.jolokia</groupId>
   <artifactId>docker-maven-plugin</artifactId>
-  <version>0.14.0-SNAPSHOT</version>
+  <version>0.13.7-SNAPSHOT</version>
   <packaging>maven-plugin</packaging>
 
   <name>maven-docker-plugin</name>

src/main/java/org/jolokia/docker/maven/StartMojo.java

@@ -14,7 +14,6 @@
 import java.util.regex.Pattern;
 
 import org.apache.commons.lang3.text.StrSubstitutor;
-import org.apache.http.client.methods.HttpUriRequest;
 import org.apache.maven.plugin.MojoExecutionException;
 import org.codehaus.plexus.util.StringUtils;
 import org.jolokia.docker.maven.access.*;
@@ -23,11 +22,6 @@
 import org.jolokia.docker.maven.config.*;
 import org.jolokia.docker.maven.log.LogDispatcher;
 import org.jolokia.docker.maven.service.*;
-import org.jolokia.docker.maven.util.StartOrderResolver;
-import org.jolokia.docker.maven.util.Timestamp;
-import org.jolokia.docker.maven.util.WaitUtil;
-import org.jolokia.docker.maven.service.QueryService;
-import org.jolokia.docker.maven.service.RunService;
 import org.jolokia.docker.maven.util.*;
 
 

src/main/java/org/jolokia/docker/maven/StopMojo.java

@@ -43,50 +43,51 @@ protected void executeInternal(ServiceHub hub) throws MojoExecutionException, Do
             if (invokedViaDockerStart()) {
                 runService.stopStartedContainers(keepContainer, removeVolumes);
             } else {
-                stopAndRemoveContainers(queryService, runService);
+                stopContainers(queryService, runService);
             }
         }
 
         // Switch off all logging
         LogDispatcher dispatcher = getLogDispatcher(hub);
         dispatcher.untrackAllContainerLogs();
     }
-
-    private boolean invokedViaDockerStart() {
-        Boolean startCalled = (Boolean) getPluginContext().get(CONTEXT_KEY_START_CALLED);
-        return (startCalled != null && startCalled == true);
-    }
-
-    private boolean isSledgeHammerEnabled() {
-        return Boolean.valueOf(System.getProperty("docker.sledgehammer", Boolean.FALSE.toString()));
+
+    private void stopContainers(QueryService queryService, RunService runService) throws
+                                                                                  DockerAccessException {
+        PomLabel pomLabel = getPomLabel();
+
+        for (ImageConfiguration image : getImages()) {
+            String imageName = image.getName();
+
+            for (Container container : queryService.getContainersForImage(imageName)) {
+                if (shouldStopContainer(container, pomLabel)) {
+                    runService.stopContainer(container.getId(), image, keepContainer, removeVolumes);
+                }
+            }
+        }
     }
-    
+
     private boolean shouldStopContainer(Container container, PomLabel pomLabel) {
         boolean stopContainer = true;
 
-        if (!isSledgeHammerEnabled()) {
-            String key = pomLabel.getKey();        
+        if (!isStopAllContainers()) {
+            String key = pomLabel.getKey();
             Map<String, String> labels = container.getLabels();
 
             if (labels.containsKey(key)) {
-                stopContainer = pomLabel.matches(new PomLabel(labels.get(key)), false);
+                stopContainer = pomLabel.matches(new PomLabel(labels.get(key)));
             }
         }
 
         return stopContainer;
     }
-
-    private void stopAndRemoveContainers(QueryService queryService, RunService runService) throws DockerAccessException { 
-        PomLabel pomLabel = getPomLabel();
 
-        for (ImageConfiguration image : getImages()) {
-            String imageName = image.getName();
-
-            for (Container container : queryService.getContainersForImage(imageName)) {
-                if (shouldStopContainer(container, pomLabel)) {
-                    runService.stopContainer(container.getId(), image, keepContainer, removeVolumes);
-                }
-            }
-        }
+    private boolean isStopAllContainers() {
+        return Boolean.valueOf(System.getProperty("docker.allContainers", Boolean.FALSE.toString()));
+    }
+
+    private boolean invokedViaDockerStart() {
+        Boolean startCalled = (Boolean) getPluginContext().get(CONTEXT_KEY_START_CALLED);
+        return startCalled != null && startCalled;
     }
 }

src/main/java/org/jolokia/docker/maven/access/DockerAccess.java

@@ -7,7 +7,6 @@
 import org.jolokia.docker.maven.access.log.LogGetHandle;
 import org.jolokia.docker.maven.config.Arguments;
 import org.jolokia.docker.maven.log.LogOutputSpec;
-import org.jolokia.docker.maven.model.*;
 import org.jolokia.docker.maven.model.Container;
 import org.jolokia.docker.maven.util.PomLabel;
 
@@ -101,16 +100,6 @@ public interface DockerAccess {
      */
     void stopContainer(String containerId, int killWait) throws DockerAccessException;
 
-    /**
-     * Get all containers matching a certain label. This might not be a cheap operation especially if many containers
-     * are running. Use with care.
-     *
-     * @param label label which the container must match
-     * @return list of container names matching the label.
-     * @see PomLabel#matches(PomLabel)
-     */
-    List<String> getContainersWithLabel(PomLabel label) throws DockerAccessException;
-
     /** Copy an archive (must be a tar) into a running container
      * Get all containers matching a certain label. This might not be a cheap operation especially if many containers
      * are running. Use with care.
@@ -123,7 +112,6 @@ public interface DockerAccess {
     void copyArchive(String containerId, File archive, String targetPath)
             throws DockerAccessException;
 
-     /**
     /**
      * Get logs for a container up to now synchronously.
      *

src/main/java/org/jolokia/docker/maven/access/hc/DockerAccessWithHcClient.java

@@ -12,24 +12,13 @@
 import org.jolokia.docker.maven.access.hc.unix.UnixSocketClientBuilder;
 import org.jolokia.docker.maven.access.log.*;
 import org.jolokia.docker.maven.config.Arguments;
-import org.jolokia.docker.maven.model.*;
-import org.jolokia.docker.maven.util.*;
-import org.jolokia.docker.maven.log.LogOutputSpec;
 import org.jolokia.docker.maven.log.DefaultLogCallback;
-import org.jolokia.docker.maven.model.Container;
-import org.jolokia.docker.maven.model.ContainerDetails;
-import org.jolokia.docker.maven.model.ContainersListElement;
+import org.jolokia.docker.maven.log.LogOutputSpec;
+import org.jolokia.docker.maven.model.*;
 import org.jolokia.docker.maven.util.*;
 import org.json.JSONArray;
 import org.json.JSONObject;
 
-import java.io.*;
-import java.net.URI;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
-import java.util.Map;
-
 import static java.net.HttpURLConnection.*;
 import static org.jolokia.docker.maven.access.hc.ApacheHttpClientDelegate.*;
 
@@ -198,12 +187,6 @@ public void stopContainer(String containerId, int killWait) throws DockerAccessE
         }
     }
 
-    @Override
-    public List<String> getContainersWithLabel(PomLabel label) throws DockerAccessException {
-        // TODO: Implementation
-        return null;
-    }
-
     @Override
     public void buildImage(String image, File dockerArchive, boolean forceRemove)
             throws DockerAccessException {

src/main/java/org/jolokia/docker/maven/model/ContainerDetails.java

@@ -51,11 +51,10 @@ public String getImage() {
 
     @Override
     public Map<String, String> getLabels() {
-        if (!json.getJSONObject(CONFIG).has(LABELS)) {
-            return Collections.emptyMap();
-        }
-
-         return mapLabels(json.getJSONObject(CONFIG).getJSONObject(LABELS));
+        JSONObject config = json.getJSONObject(CONFIG);
+        return config.has(LABELS) ?
+                mapLabels(config.getJSONObject(LABELS)) :
+                Collections.<String, String>emptyMap();
     }
 
     @Override

src/main/java/org/jolokia/docker/maven/util/PomLabel.java

@@ -16,8 +16,6 @@
  * limitations under the License.
  */
 
-import java.util.UUID;
-
 /**
  * Label used to mark a container belonging to a certain build.
  *
@@ -26,11 +24,7 @@
  */
 public class PomLabel {
 
-    // Environment variable used to label containers
-    public static final String CONTAINER_LABEL_NAME = "docker.maven.plugin.container";
-
     private String mavenCoordinates;
-    private String runId;
 
     /**
      * Construct from a given label
@@ -39,22 +33,11 @@ public class PomLabel {
      */
     public PomLabel(String label) {
         String[] parts = label.split(":");
-        if (parts.length != 4 && parts.length != 3) {
+        if (parts.length != 3) {
             throw new IllegalArgumentException("Label '" + label +
-                                               "' has not the format <group>:<artifact>:<version>[:<runId>]");
+                                               "' has not the format <group>:<artifact>:<version>");
         }
         mavenCoordinates = parts[0] + ":" + parts[1] + ":" + parts[2];
-        runId = parts.length == 4 ? parts[3] : null;
-    }
-
-    /**
-     * Construct from maven coordinates. A random run-ID is created automatically.
-     * @param groupId Maven group
-     * @param artifactId Maven artifact
-     * @param version version
-     */
-    public PomLabel(String groupId, String artifactId, String version) {
-        this(groupId, artifactId, version, UUID.randomUUID().toString());
     }
 
     /**
@@ -64,11 +47,9 @@ public PomLabel(String groupId, String artifactId, String version) {
      * @param groupId Maven group
      * @param artifactId Maven artifact
      * @param version version
-     * @param runId a run id or <code>null</code>.
      */
-    public PomLabel(String groupId, String artifactId, String version, String runId) {
+    public PomLabel(String groupId, String artifactId, String version) {
         mavenCoordinates = groupId + ":" + artifactId + ":" + version;
-        this.runId = runId;
     }
 
     /**
@@ -77,21 +58,17 @@ public PomLabel(String groupId, String artifactId, String version, String runId)
      * @return the label name to use to mark a container belonging to this build
      */
     public String getKey() {
-        return "docker.maven.plugin.container";
+        return "dmp.coordinates";
     }
 
     /**
      * Get this label in string representation
      * @return this label as string
      */
     public String getValue() {
-        return mavenCoordinates + (runId != null ? ":" + runId : "");
+        return mavenCoordinates;
     }
 
-    public boolean matches(PomLabel other) {
-        return matches(other, true);
-    }
-
     /**
      * Check whether this label matches another.
      * <p>
@@ -100,14 +77,9 @@ public boolean matches(PomLabel other) {
      * </p>
      *
      * @param other label to match
-     * @param incRunId <code>true<code> if the <code>runId</code> should be considered during the match, <code>false<code> otherwise.
      * @return true for a match
      */
-    public boolean matches(PomLabel other, boolean incRunId) {
-        boolean matches = other.mavenCoordinates.equals(mavenCoordinates);
-        if (incRunId) {
-            matches = matches && (runId == null || runId.equals(other.runId));
-        }
-        return matches;
+    public boolean matches(PomLabel other) {
+        return other.mavenCoordinates.equals(mavenCoordinates);
     }
 }

src/test/java/org/jolokia/docker/maven/model/ContainerDetailsTest.java

@@ -43,6 +43,7 @@ public void testContainerWithoutPorts() {
         thenPortBindingSizeIs(0);
     }
 
+    @Test
     public void testContainerWithLabels() {
         givenAContainerWithLabels();
         whenCreateContainer();
@@ -60,8 +61,11 @@ private void givenAContainerWithLabels() {
         JSONObject labels = new JSONObject();
         labels.put("key1", "value1");
         labels.put("key2", "value2");
-
-        json.put(ContainerDetails.LABELS, labels);
+
+        JSONObject config = new JSONObject();
+        config.put(ContainerDetails.LABELS, labels);
+
+        json.put("Config",config);
     }
 
     @Test