Honor template name when limiting agents by template (jenkinsci#816)

GEBIT · May 31, 2023 · a55ba1f · a55ba1f
1 parent 5511f55
commit a55ba1f
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 9 deletions.
diff --git a/src/main/java/com/nirima/jenkins/plugins/docker/DockerCloud.java b/src/main/java/com/nirima/jenkins/plugins/docker/DockerCloud.java
@@ -16,6 +16,7 @@
 import com.github.dockerjava.api.command.PushImageCmd;
 import com.github.dockerjava.api.command.StartContainerCmd;
 import com.github.dockerjava.api.model.AuthConfig;
+import com.github.dockerjava.api.model.Container;
 import com.github.dockerjava.api.model.Version;
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.NonNull;
@@ -601,32 +602,48 @@ public synchronized void removeTemplate(DockerTemplate t) {
             templates.remove(t);
         }
     }
+
+    /**
+     * @deprecated use countContainersInDocker(String, String) instead
+     * @see DockerCloud#countContainersInDocker(String, String)
+     */
+    @Deprecated
+    public int countContainersInDocker(final String imageName) throws Exception {
+    	return countContainersInDocker(imageName, null);
+    }
 
     /**
      * Counts the number of instances currently running in Docker that are using
-     * the specified image.
+     * the specified image and template.<br>
+     * Returns all containers matching the given filter parameters.
+     * 
      * <p>
      * <b>WARNING:</b> This method can be slow so it should be called sparingly.
      *
      * @param imageName
-     *            If null, then all instances belonging to this Jenkins instance
-     *            are counted. Otherwise, only those started with the specified
-     *            image are counted.
+     * 		        filter for imageName. If null, no filter is active for imageName.
+     * @param templateName
+     *              filter for templateName. If null, no filter is active for templateName.
+     *
      * @return The number of containers.
      * @throws Exception if anything went wrong.
      */
-    public int countContainersInDocker(final String imageName) throws Exception {
+    public int countContainersInDocker(final String imageName, final String templateName) throws Exception {
         final Map<String, String> labelFilter = new HashMap<>();
         labelFilter.put(
                 DockerContainerLabelKeys.JENKINS_INSTANCE_ID,
                 DockerTemplateBase.getJenkinsInstanceIdForContainerLabel());
         if (imageName != null) {
             labelFilter.put(DockerContainerLabelKeys.CONTAINER_IMAGE, imageName);
         }
-        final List<?> containers;
+        if (templateName != null) {
+        	labelFilter.put(DockerContainerLabelKeys.TEMPLATE_NAME, templateName);
+        }
+        final List<Container> containers;
         try (final DockerClient client = dockerApi.getClient()) {
             containers = client.listContainersCmd().withLabelFilter(labelFilter).exec();
         }
+
         final int count = containers.size();
         return count;
     }
@@ -636,14 +653,15 @@ public int countContainersInDocker(final String imageName) throws Exception {
      */
     private boolean canAddProvisionedAgent(DockerTemplate t) throws Exception {
         final String templateImage = t.getImage();
+        final String templateName = t.getName();
         final int templateContainerCap = t.instanceCap;
         final int cloudContainerCap = getContainerCap();
 
         final boolean haveCloudContainerCap = cloudContainerCap > 0 && cloudContainerCap != Integer.MAX_VALUE;
         final boolean haveTemplateContainerCap = templateContainerCap > 0 && templateContainerCap != Integer.MAX_VALUE;
         final int estimatedTotalAgents;
         if (haveCloudContainerCap) {
-            final int totalContainersInCloud = countContainersInDocker(null);
+            final int totalContainersInCloud = countContainersInDocker(null, null);
             final int containersInProgress = countContainersInProgress();
             estimatedTotalAgents = totalContainersInCloud + containersInProgress;
             if (estimatedTotalAgents >= cloudContainerCap) {
@@ -659,7 +677,7 @@ private boolean canAddProvisionedAgent(DockerTemplate t) throws Exception {
         }
         final int estimatedTemplateAgents;
         if (haveTemplateContainerCap) {
-            final int totalContainersOfThisTemplateInCloud = countContainersInDocker(templateImage);
+			final int totalContainersOfThisTemplateInCloud = countContainersInDocker(templateImage, templateName);
             final int containersInProgress = countContainersInProgress(t);
             estimatedTemplateAgents = totalContainersOfThisTemplateInCloud + containersInProgress;
             if (estimatedTemplateAgents >= templateContainerCap) {

diff --git a/src/main/resources/com/nirima/jenkins/plugins/docker/DockerTemplate/help-name.html b/src/main/resources/com/nirima/jenkins/plugins/docker/DockerTemplate/help-name.html
@@ -4,4 +4,8 @@
     Jenkins will append a unique ID to this name in order to create individual node names.
     <p>
     If blank or just whitespace, a default of "docker" will be used.
+    <p>
+    When using the same docker image in multiple docker agent templates, make sure to set an individual name for each template.<br/>
+	This is needed for capping by number of container instance by template to work correctly.
+	</p>
 </div>
diff --git a/src/test/java/io/jenkins/docker/connector/DockerComputerConnectorTest.java b/src/test/java/io/jenkins/docker/connector/DockerComputerConnectorTest.java
@@ -148,7 +148,7 @@ private boolean dockerIsStillBusy() throws Exception {
                         return true;
                     }
                 }
-                final int containersInDocker = cloud.countContainersInDocker(null);
+                final int containersInDocker = cloud.countContainersInDocker(null, null);
                 if (containersInDocker > 0) {
                     return true;
                 }