#513 : Removed the fetch limit completely

Now when fetching images they are fetched all. This should happen now only when running `docker:stop` or `docker:watch` to find containers for a particular image and when running Docker < 1.11.
fabric8io · Jul 13, 2016 · 5a60b90 · 5a60b90
1 parent cec36fe
commit 5a60b90
Show file tree

Hide file tree

Showing 8 changed files with 9 additions and 41 deletions.
diff --git a/doc/changelog.md b/doc/changelog.md
@@ -2,7 +2,8 @@
 
 * **0.15-SNAPSHOT**
   - Dont do redirect when waiting on an HTTP port (#499)
-
+  - Removed the container fetch limit of 100 and optimized getting containers by name and image (#513)
+
 * **0.15.9** (2016-06-28)
   - Fixed issue when target directory does not exist yet ([#497](https://github.com/fabric8io/docker-maven-plugin/issues/497))
 

diff --git a/src/main/asciidoc/inc/_global-configuration.adoc b/src/main/asciidoc/inc/_global-configuration.adoc
@@ -61,10 +61,6 @@ the URL is:
 . `unix:///var/run/docker.sock` if it is a readable socket.
 | `docker.host`
 
-| *fetchLimit*
-| Number of running and stopped containers to return. A value of 0 returns all running and stopped containers. Default is 100.
-| `docker.fetchLimit`
-
 | *image*
 | In order to temporarily restrict the operation of plugin goals this configuration option can be used. Typically this will be set via the system property `docker.image` when Maven is called. The value can be a single image name (either its alias or full name) or it can be a comma separated list with multiple image names. Any name which doesn't refer an image in the configuration will be ignored.
 | `docker.image`

diff --git a/src/main/java/io/fabric8/maven/docker/AbstractDockerMojo.java b/src/main/java/io/fabric8/maven/docker/AbstractDockerMojo.java
@@ -119,10 +119,6 @@ public abstract class AbstractDockerMojo extends AbstractMojo implements Context
     @Parameter(property = "docker.skip", defaultValue = "false")
     private boolean skip;
 
-    // Max number of containers to fetch when stopping/removing
-    @Parameter(property = "docker.fetchLimit", defaultValue = "100")
-    protected int fetchLimit;
-
     /**
      * Whether the usage of docker machine should be skipped competely
      */
@@ -276,7 +272,6 @@ private DockerAccess createDockerAccess(String minimalVersion) throws MojoExecut
                 access = new DockerAccessWithHcClient("v" + version, dockerUrl,
                                                       dockerConnectionDetector.getCertPath(certPath),
                                                       maxConnections,
-                                                      fetchLimit,
                                                       log);
                 access.start();
                 setDockerHostAddressProperty(dockerUrl);

diff --git a/src/main/java/io/fabric8/maven/docker/access/UrlBuilder.java b/src/main/java/io/fabric8/maven/docker/access/UrlBuilder.java
@@ -76,11 +76,8 @@ public String inspectContainer(String containerId) {
                 .build();
     }
 
-    public String listContainers(int limit, String ... filter) {
+    public String listContainers(String ... filter) {
         Builder builder = u("containers/json");
-        if (limit != 0) {
-            builder.p("limit", limit);
-        }
         if (filter.length > 0) {
             if (filter.length % 2 != 0) {
                 throw new IllegalArgumentException("Filters must be given as key value pairs and not " +Arrays.asList(filter));

diff --git a/src/main/java/io/fabric8/maven/docker/access/hc/DockerAccessWithHcClient.java b/src/main/java/io/fabric8/maven/docker/access/hc/DockerAccessWithHcClient.java
@@ -57,29 +57,23 @@ public class DockerAccessWithHcClient implements DockerAccess {
     private final ApacheHttpClientDelegate delegate;
     private final UrlBuilder urlBuilder;
 
-    // limit how many containers to fetch
-    private final int fetchLimit;
-
     /**
      * Create a new access for the given URL
      *
      * @param baseUrl  base URL for accessing the docker Daemon
      * @param certPath used to build up a keystore with the given keys and certificates found in this
      *                 directory
      * @param maxConnections maximum parallel connections allowed to docker daemon (if a pool is used)
-     * @param fetchLimit how many containers to fetch with a list operation
      * @param log      a log handler for printing out logging information
      * @paran usePool  whether to use a connection bool or not
      */
     public DockerAccessWithHcClient(String apiVersion,
                                     String baseUrl,
                                     String certPath,
                                     int maxConnections,
-                                    int fetchLimit,
                                     Logger log)
             throws IOException {
         this.log = log;
-        this.fetchLimit = fetchLimit;
         URI uri = URI.create(baseUrl);
         if (uri.getScheme() == null) {
             throw new IllegalArgumentException("The docker access url '" + baseUrl + "' must contain a schema tcp:// or unix://");
@@ -261,10 +255,10 @@ public List<Container> getContainersForImage(String image) throws DockerAccessEx
         String serverApiVersion = getServerApiVersion();
         if (EnvUtil.greaterOrEqualsVersion(serverApiVersion, "1.23")) {
             // For Docker >= 1.11 we can use a new filter when listing containers
-            url = urlBuilder.listContainers(0,"ancestor",image);
+            url = urlBuilder.listContainers("ancestor",image);
         } else {
             // For older versions (< Docker 1.11) we need to iterate over the containers.
-            url = urlBuilder.listContainers(fetchLimit);
+            url = urlBuilder.listContainers();
         }
 
         try {

diff --git a/src/main/java/io/fabric8/maven/docker/config/RunImageConfiguration.java b/src/main/java/io/fabric8/maven/docker/config/RunImageConfiguration.java
@@ -109,9 +109,6 @@ public class RunImageConfiguration {
     @Parameter
     private WaitConfiguration wait;
 
-    @Parameter
-    private Integer fetchLimit;
-
     @Parameter
     private LogConfiguration log;
 
@@ -200,10 +197,6 @@ public WaitConfiguration getWaitConfiguration() {
         return wait;
     }
 
-    public Integer getFetchLimit() {
-        return fetchLimit;
-    }
-
     public LogConfiguration getLogConfiguration() {
         return log;
     }
@@ -394,11 +387,6 @@ public Builder wait(WaitConfiguration wait) {
             return this;
         }
 
-        public Builder fetchLimit(Integer fetchLimit) {
-            config.fetchLimit = fetchLimit;
-            return this;
-        }
-
         public Builder log(LogConfiguration log) {
             config.log = log;
             return this;

diff --git a/src/test/java/integration/DockerAccessIT.java b/src/test/java/integration/DockerAccessIT.java
@@ -87,7 +87,7 @@ public void testPullStartStopRemove() throws DockerAccessException {
     private DockerAccessWithHcClient createClient(String baseUrl, Logger logger) {
         try {
             String certPath = createDockerConnectionDetector(logger).getCertPath(null);
-            return new DockerAccessWithHcClient(AbstractDockerMojo.API_VERSION, baseUrl, certPath, 20, 100, logger);
+            return new DockerAccessWithHcClient(AbstractDockerMojo.API_VERSION, baseUrl, certPath, 20, logger);
         } catch (@SuppressWarnings("unused") IOException e) {
             // not using ssl, so not going to happen
             logger.error(e.getMessage());

diff --git a/src/test/java/io/fabric8/maven/docker/UrlBuilderTest.java b/src/test/java/io/fabric8/maven/docker/UrlBuilderTest.java
@@ -34,15 +34,12 @@ public class UrlBuilderTest {
     public void listContainers() throws MalformedURLException, UnsupportedEncodingException {
         UrlBuilder builder = new UrlBuilder("","1.0");
 
-        assertEquals("/1.0/containers/json?limit=100",builder.listContainers(100));
-        assertEquals("/1.0/containers/json",builder.listContainers(0));
+        assertEquals("/1.0/containers/json",builder.listContainers());
         assertEquals("/1.0/containers/json?filters=" + URLEncoder.encode("{\"ancestor\":[\"nginx\"]}","UTF8"),
-                     builder.listContainers(0, "ancestor", "nginx"));
-        assertEquals("/1.0/containers/json?limit=10&filters=" + URLEncoder.encode("{\"ancestor\":[\"nginx\"]}","UTF8"),
-                     builder.listContainers(10, "ancestor", "nginx"));
+                     builder.listContainers("ancestor", "nginx"));
 
         try {
-            builder.listContainers(10,"ancestor");
+            builder.listContainers("ancestor");
             fail();
         } catch (IllegalArgumentException exp) {
             assertTrue(exp.getMessage().contains("pair"));