From af877cd2ebd90c669a2bc1f87bcc0b3814a8a108 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Wed, 8 Mar 2017 14:30:20 -0800 Subject: [PATCH 01/20] [JENKINS-31801] Initial work on throttle(category) step - needs tests --- pom.xml | 26 +-- .../ThrottleJobProperty.java | 182 +++++++++++++++++- .../ThrottleQueueTaskDispatcher.java | 27 ++- .../pipeline/ThrottleStep.java | 28 +++ .../pipeline/ThrottleStepExecution.java | 69 +++++++ .../pipeline/ThrottledStepInfo.java | 52 +++++ .../throttleconcurrents/Messages.properties | 4 +- 7 files changed, 370 insertions(+), 18 deletions(-) create mode 100644 src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java create mode 100644 src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java create mode 100644 src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottledStepInfo.java diff --git a/pom.xml b/pom.xml index 7c1b984f..c617df5d 100644 --- a/pom.xml +++ b/pom.xml @@ -26,7 +26,7 @@ THE SOFTWARE. org.jenkins-ci.plugins plugin - 2.6 + 2.23 throttle-concurrents @@ -44,10 +44,10 @@ THE SOFTWARE. - 1.609.3 + 1.642.3 UTF-8 - 1.6 - 1.6 + 1.7 + 1.7 false @@ -107,7 +107,17 @@ THE SOFTWARE. matrix-project 1.4.1 - + + org.jenkins-ci.plugins.workflow + workflow-api + 2.3 + + + org.jenkins-ci.plugins.workflow + workflow-step-api + 2.3 + + org.jenkins-ci.plugins @@ -140,12 +150,6 @@ THE SOFTWARE. 0.7.2 test - - com.google.guava - guava - 18.0 - test - diff --git a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java index 1f26ada6..e8e6a060 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java @@ -3,6 +3,7 @@ import hudson.Extension; import hudson.matrix.MatrixConfiguration; import hudson.model.AbstractDescribableImpl; +import hudson.model.Computer; import hudson.model.Descriptor; import hudson.model.Item; import hudson.model.ItemGroup; @@ -10,6 +11,9 @@ import hudson.model.JobProperty; import hudson.model.JobPropertyDescriptor; import hudson.model.Queue; +import hudson.model.Run; +import hudson.model.TaskListener; +import hudson.plugins.throttleconcurrents.pipeline.ThrottledStepInfo; import hudson.util.FormValidation; import hudson.util.ListBoxModel; import hudson.Util; @@ -17,6 +21,7 @@ import hudson.matrix.MatrixProject; import hudson.matrix.MatrixRun; +import java.io.IOException; import java.util.Arrays; import java.util.ArrayList; import java.util.Collection; @@ -24,15 +29,24 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.TreeMap; import java.util.WeakHashMap; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.ExecutionException; +import java.util.logging.Level; +import java.util.logging.Logger; import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; + import jenkins.model.Jenkins; import net.sf.json.JSONObject; import org.apache.commons.lang.ArrayUtils; import org.apache.commons.lang.StringUtils; +import org.jenkinsci.plugins.workflow.flow.FlowExecution; +import org.jenkinsci.plugins.workflow.flow.FlowExecutionOwner; +import org.jenkinsci.plugins.workflow.steps.StepExecution; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.QueryParameter; import org.kohsuke.stapler.StaplerRequest; @@ -229,20 +243,20 @@ static List getCategoryTasks(String category) { assert category != null && !category.equals(""); List categoryTasks = new ArrayList(); Collection properties; - DescriptorImpl descriptor = Jenkins.getActiveInstance().getDescriptorByType(DescriptorImpl.class); + DescriptorImpl descriptor = Jenkins.getActiveInstance().getDescriptorByType(DescriptorImpl.class); synchronized (descriptor.propertiesByCategoryLock) { - Map _properties = descriptor.propertiesByCategory.get(category); + Map _properties = descriptor.propertiesByCategory.get(category); properties = _properties != null ? new ArrayList(_properties.keySet()) : Collections.emptySet(); } for (ThrottleJobProperty t : properties) { if (t.getThrottleEnabled()) { if (t.getCategories() != null && t.getCategories().contains(category)) { - Job p = t.owner; + Job p = t.owner; if (/*is a task*/ p instanceof Queue.Task && /* not deleted */getItem(p.getParent(), p.getName()) == p && /* has not since been reconfigured */ p.getProperty(ThrottleJobProperty.class) == t) { categoryTasks.add((Queue.Task) p); if (p instanceof MatrixProject && t.isThrottleMatrixConfigurations()) { - for (MatrixConfiguration mc : ((MatrixProject)p).getActiveConfigurations()) { + for (MatrixConfiguration mc : ((MatrixProject) p).getActiveConfigurations()) { categoryTasks.add(mc); } } @@ -250,8 +264,61 @@ static List getCategoryTasks(String category) { } } } + return categoryTasks; } + + static List getThrottledPipelinesForCategory(String category) { + List throttledPipelines = new ArrayList<>(); + + DescriptorImpl descriptor = Jenkins.getActiveInstance().getDescriptorByType(DescriptorImpl.class); + for (Map.Entry currentPipeline : descriptor.getThrottledPipelinesForCategory(category).entrySet()) { + Run flowNodeRun = Run.fromExternalizableId(currentPipeline.getKey()); + + if (flowNodeRun == null) { + // No run found, so remove the throttle. + descriptor.removeThrottledPipelineForCategory(currentPipeline.getKey(), category, null); + } else if (!(flowNodeRun instanceof FlowExecutionOwner.Executable)) { + // If for some reason we've somehow ended up with a non-pipeline job, remove the throttle. + descriptor.removeThrottledPipelineForCategory(currentPipeline.getKey(), category, null); + } else if (!flowNodeRun.isBuilding()) { + // The run is done building, so remove the throttle. + descriptor.removeThrottledPipelineForCategory(currentPipeline.getKey(), category, null); + } else { + FlowExecutionOwner owner = ((FlowExecutionOwner.Executable)flowNodeRun).asFlowExecutionOwner(); + FlowExecution execution = owner.getOrNull(); + if (execution == null) { + // For some reason, the flow execution is null, so again? Remove the throttle. + descriptor.removeThrottledPipelineForCategory(currentPipeline.getKey(), category, null); + } else { + try { + for (StepExecution stepExec : execution.getCurrentExecutions(false).get()) { + try { + Computer c = stepExec.getContext().get(Computer.class); + ThrottledStepInfo candidateInfo = stepExec.getContext().get(ThrottledStepInfo.class); + if (c != null && candidateInfo != null) { + ThrottledStepInfo info = candidateInfo.forCategory(category); + if (info != null) { + if (info.getNode() == null && c.getNode() != null) { + info.setNode(c.getNode().getNodeName()); + } + throttledPipelines.add(info); + } + } + } catch (IOException e) { + // TODO: What do we do here if anything? + } + } + } catch (InterruptedException | ExecutionException e) { + // TODO: What do we do here if anything? + } + } + } + } + + return throttledPipelines; + } + private static Item getItem(ItemGroup group, String name) { if (group instanceof Jenkins) { return ((Jenkins) group).getItemMap().get(name); @@ -262,7 +329,11 @@ private static Item getItem(ItemGroup group, String name) { @Extension public static final class DescriptorImpl extends JobPropertyDescriptor { + private static final Logger LOGGER = Logger.getLogger(DescriptorImpl.class.getName()); + private List categories; + + private Map> throttledPipelines; /** Map from category names, to properties including that category. */ private transient Map> propertiesByCategory @@ -369,7 +440,108 @@ public ListBoxModel doFillCategoryItems() { return m; } - + + @Override + public void load() { + super.load(); + if (throttledPipelines == null) { + throttledPipelines = new TreeMap<>(); + } + LOGGER.log(Level.FINE, "load: {0}", throttledPipelines); + } + + @Override + public void save() { + super.save(); + LOGGER.log(Level.FINE, "save: {0}", throttledPipelines); + } + + @Nonnull + public synchronized Map getThrottledPipelinesForCategory(@Nonnull String category) { + return internalGetThrottledPipelinesForCategory(category); + } + + @Nonnull + private Map internalGetThrottledPipelinesForCategory(@Nonnull String category) { + if (getCategoryByName(category) != null) { + if (throttledPipelines.containsKey(category)) { + return throttledPipelines.get(category); + } + } + return new TreeMap<>(); + } + + public synchronized void addThrottledPipelineForCategory(@Nonnull String runId, + @Nonnull String category, + TaskListener listener) { + if (getCategoryByName(category) == null) { + if (listener != null) { + listener.getLogger().println(Messages.ThrottleJobProperty_DescriptorImpl_NoSuchCategory(category)); + } + } else { + Map currentPipelines = internalGetThrottledPipelinesForCategory(category); + + if (!currentPipelines.containsKey(runId)) { + currentPipelines.put(runId, 1); + } else { + currentPipelines.put(runId, currentPipelines.get(runId) + 1); + } + + throttledPipelines.put(category, currentPipelines); + } + } + + public synchronized void removeThrottledPipelineForCategory(@Nonnull String runId, + @Nonnull String category, + TaskListener listener) { + if (getCategoryByName(category) == null) { + if (listener != null) { + listener.getLogger().println(Messages.ThrottleJobProperty_DescriptorImpl_NoSuchCategory(category)); + } + } else { + Map currentPipelines = internalGetThrottledPipelinesForCategory(category); + + if (!currentPipelines.isEmpty()) { + if (currentPipelines.containsKey(runId)) { + Integer currentCount = currentPipelines.get(runId); + if (currentCount > 1) { + currentPipelines.put(runId, currentCount - 1); + } else { + currentPipelines.remove(runId); + } + } + } + + if (currentPipelines.isEmpty()) { + throttledPipelines.remove(category); + } else { + throttledPipelines.put(category, currentPipelines); + } + } + } + + public synchronized void removeAllFromRunForCategory(@Nonnull String runId, + @Nonnull String category, + TaskListener listener) { + if (getCategoryByName(category) == null) { + if (listener != null) { + listener.getLogger().println(Messages.ThrottleJobProperty_DescriptorImpl_NoSuchCategory(category)); + } + } else { + Map currentPipelines = internalGetThrottledPipelinesForCategory(category); + + if (!currentPipelines.isEmpty()) { + if (currentPipelines.containsKey(runId)) { + currentPipelines.remove(runId); + } + } + if (currentPipelines.isEmpty()) { + throttledPipelines.remove(category); + } else { + throttledPipelines.put(category, currentPipelines); + } + } + } } public static final class ThrottleCategory extends AbstractDescribableImpl { diff --git a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java index 2980aa37..9b6a3b9d 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java @@ -16,6 +16,7 @@ import hudson.model.labels.LabelAtom; import hudson.model.queue.CauseOfBlockage; import hudson.model.queue.QueueTaskDispatcher; +import hudson.plugins.throttleconcurrents.pipeline.ThrottledStepInfo; import hudson.security.ACL; import hudson.security.NotSerilizableSecurityContext; import hudson.model.Action; @@ -97,17 +98,21 @@ else if (tjp.getThrottleOption().equals("category")) { // Double check category itself isn't null if (category != null) { + int runCount = 0; + // Max concurrent per node for category int maxConcurrentPerNode = getMaxConcurrentPerNodeBasedOnMatchingLabels( node, category, category.getMaxConcurrentPerNode().intValue()); if (maxConcurrentPerNode > 0) { - int runCount = 0; for (Task catTask : categoryTasks) { if (jenkins.getQueue().isPending(catTask)) { return CauseOfBlockage.fromMessage(Messages._ThrottleQueueTaskDispatcher_BuildPending()); } runCount += buildsOfProjectOnNode(node, catTask); } + List throttledPipelines = ThrottleJobProperty.getThrottledPipelinesForCategory(catNm); + runCount += pipelinesOnNode(node, throttledPipelines); + // This would mean that there are as many or more builds currently running than are allowed. if (runCount >= maxConcurrentPerNode) { return CauseOfBlockage.fromMessage(Messages._ThrottleQueueTaskDispatcher_MaxCapacityOnNode(runCount)); @@ -228,6 +233,8 @@ else if (tjp.getThrottleOption().equals("category")) { } totalRunCount += buildsOfProjectOnAllNodes(catTask); } + List throttledPipelines = ThrottleJobProperty.getThrottledPipelinesForCategory(catNm); + totalRunCount += pipelinesOnAllNodes(throttledPipelines); if (totalRunCount >= maxConcurrentTotal) { return CauseOfBlockage.fromMessage(Messages._ThrottleQueueTaskDispatcher_MaxCapacityTotal(totalRunCount)); @@ -363,6 +370,24 @@ private ThrottleJobProperty getThrottleJobProperty(Task task) { return null; } + private int pipelinesOnNode(@Nonnull Node node, @Nonnull List throttledPipelines) { + int runCount = 0; + + String nodeName = node.getNodeName(); + + for (ThrottledStepInfo info : throttledPipelines) { + if (nodeName.equals(info.getNode())) { + runCount++; + } + } + + return runCount; + } + + private int pipelinesOnAllNodes(@Nonnull List throttledPipelines) { + return throttledPipelines.size(); + } + private int buildsOfProjectOnNode(Node node, Task task) { if (!shouldBeThrottled(task, getThrottleJobProperty(task))) { return 0; diff --git a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java new file mode 100644 index 00000000..d1d322ac --- /dev/null +++ b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java @@ -0,0 +1,28 @@ +package hudson.plugins.throttleconcurrents.pipeline; + +import org.jenkinsci.plugins.workflow.steps.Step; +import org.jenkinsci.plugins.workflow.steps.StepContext; +import org.jenkinsci.plugins.workflow.steps.StepExecution; +import org.kohsuke.stapler.DataBoundConstructor; + +import javax.annotation.Nonnull; + +public class ThrottleStep extends Step { + private String category; + + @DataBoundConstructor + public ThrottleStep(@Nonnull String category) { + this.category = category; + } + + @Nonnull + public String getCategory() { + return category; + } + + @Override + public StepExecution start(StepContext context) throws Exception { + return new ThrottleStepExecution(this, context); + } + +} diff --git a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java new file mode 100644 index 00000000..fab42254 --- /dev/null +++ b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java @@ -0,0 +1,69 @@ +package hudson.plugins.throttleconcurrents.pipeline; + +import hudson.model.Computer; +import hudson.model.Run; +import hudson.model.TaskListener; +import hudson.plugins.throttleconcurrents.ThrottleJobProperty; +import jenkins.model.Jenkins; +import org.jenkinsci.plugins.workflow.steps.BodyExecution; +import org.jenkinsci.plugins.workflow.steps.BodyExecutionCallback; +import org.jenkinsci.plugins.workflow.steps.StepContext; +import org.jenkinsci.plugins.workflow.steps.StepExecution; + +import javax.annotation.Nonnull; + +public class ThrottleStepExecution extends StepExecution { + private final ThrottleStep step; + + private BodyExecution body; + + public ThrottleStepExecution(@Nonnull ThrottleStep step, StepContext context) { + super(context); + this.step = step; + } + + public String getCategory() { + return step.getCategory(); + } + + @Override + public boolean start() throws Exception { + ThrottledStepInfo info = new ThrottledStepInfo(getCategory()); + + ThrottledStepInfo parentInfo = getContext().get(ThrottledStepInfo.class); + if (parentInfo != null) { + // Make sure we record the node used for the parent throttle step if it exists. + Computer computer = getContext().get(Computer.class); + if (computer != null && computer.getNode() != null) { + parentInfo.setNode(computer.getNode().getNodeName()); + } + info.setParentInfo(parentInfo); + } + + Run r = getContext().get(Run.class); + TaskListener listener = getContext().get(TaskListener.class); + + ThrottleJobProperty.DescriptorImpl descriptor = Jenkins.getActiveInstance().getDescriptorByType(ThrottleJobProperty.DescriptorImpl.class); + + descriptor.addThrottledPipelineForCategory(r.getExternalizableId(), getCategory(), listener); + + body = getContext().newBodyInvoker() + .withContext(info) + .withCallback(BodyExecutionCallback.wrap(getContext())) + .start(); + return false; + } + + @Override + public void stop(Throwable cause) throws Exception { + if (body != null) + body.cancel(cause); + + Run r = getContext().get(Run.class); + TaskListener listener = getContext().get(TaskListener.class); + + ThrottleJobProperty.DescriptorImpl descriptor = Jenkins.getActiveInstance().getDescriptorByType(ThrottleJobProperty.DescriptorImpl.class); + + descriptor.removeThrottledPipelineForCategory(r.getExternalizableId(), getCategory(), listener); + } +} diff --git a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottledStepInfo.java b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottledStepInfo.java new file mode 100644 index 00000000..96a46413 --- /dev/null +++ b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottledStepInfo.java @@ -0,0 +1,52 @@ +package hudson.plugins.throttleconcurrents.pipeline; + +import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; +import java.io.Serializable; + +public class ThrottledStepInfo implements Serializable { + private String category; + + private String node; + + private ThrottledStepInfo parentInfo; + + public ThrottledStepInfo(@Nonnull String category) { + this.category = category; + } + + @Nonnull + public String getCategory() { + return category; + } + + public void setNode(@Nonnull String node) { + this.node = node; + } + + @CheckForNull + public String getNode() { + return node; + } + + public void setParentInfo(@Nonnull ThrottledStepInfo parentInfo) { + this.parentInfo = parentInfo; + } + + @CheckForNull + public ThrottledStepInfo getParentInfo() { + return parentInfo; + } + + public ThrottledStepInfo forCategory(@Nonnull String cat) { + if (cat.equals(category)) { + return this; + } else if (getParentInfo() != null) { + return getParentInfo().forCategory(cat); + } else { + return null; + } + } + + private static final long serialVersionUID = 1L; +} diff --git a/src/main/resources/hudson/plugins/throttleconcurrents/Messages.properties b/src/main/resources/hudson/plugins/throttleconcurrents/Messages.properties index 98caeb7d..f9feb4c5 100644 --- a/src/main/resources/hudson/plugins/throttleconcurrents/Messages.properties +++ b/src/main/resources/hudson/plugins/throttleconcurrents/Messages.properties @@ -3,4 +3,6 @@ ThrottleQueueTaskDispatcher.MaxCapacityTotal=Already running {0} builds across a ThrottleQueueTaskDispatcher.BuildPending=A build is pending launch ThrottleQueueTaskDispatcher.OnlyOneWithMatchingParameters=A build with matching parameters is already running -ThrottleMatrixProjectOptions.DisplayName=Additional options for Matrix projects \ No newline at end of file +ThrottleMatrixProjectOptions.DisplayName=Additional options for Matrix projects + +ThrottleJobProperty.DescriptorImpl.NoSuchCategory=Requested category "{0}" does not exist, so cannot throttle. \ No newline at end of file From c000c30c99d04b8d9093d61ce239e3d578014c2d Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Fri, 10 Mar 2017 14:18:19 -0800 Subject: [PATCH 02/20] Reworked to no longer rely on StepExecutions. --- pom.xml | 15 +- .../ThrottleJobProperty.java | 144 +++++++++--------- .../ThrottleQueueTaskDispatcher.java | 95 +++++++++--- .../pipeline/ThrottleStep.java | 32 ++++ .../pipeline/ThrottleStepExecution.java | 61 +++++--- .../pipeline/ThrottledStepInfo.java | 52 ------- .../ThrottleConcurrentTest.java | 2 + 7 files changed, 227 insertions(+), 174 deletions(-) delete mode 100644 src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottledStepInfo.java diff --git a/pom.xml b/pom.xml index c617df5d..9b7f6163 100644 --- a/pom.xml +++ b/pom.xml @@ -44,10 +44,10 @@ THE SOFTWARE. - 1.642.3 + 1.609.3 UTF-8 - 1.7 - 1.7 + 1.6 + 1.6 false @@ -110,12 +110,17 @@ THE SOFTWARE. org.jenkins-ci.plugins.workflow workflow-api - 2.3 + 2.12 org.jenkins-ci.plugins.workflow workflow-step-api - 2.3 + 2.7 + + + org.jenkins-ci.plugins.workflow + workflow-durable-task-step + 2.8 diff --git a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java index e8e6a060..2240fd01 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java @@ -3,7 +3,6 @@ import hudson.Extension; import hudson.matrix.MatrixConfiguration; import hudson.model.AbstractDescribableImpl; -import hudson.model.Computer; import hudson.model.Descriptor; import hudson.model.Item; import hudson.model.ItemGroup; @@ -13,11 +12,9 @@ import hudson.model.Queue; import hudson.model.Run; import hudson.model.TaskListener; -import hudson.plugins.throttleconcurrents.pipeline.ThrottledStepInfo; import hudson.util.FormValidation; import hudson.util.ListBoxModel; import hudson.Util; -import hudson.matrix.MatrixBuild; import hudson.matrix.MatrixProject; import hudson.matrix.MatrixRun; @@ -32,7 +29,6 @@ import java.util.TreeMap; import java.util.WeakHashMap; import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.ExecutionException; import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.CheckForNull; @@ -46,7 +42,7 @@ import org.apache.commons.lang.StringUtils; import org.jenkinsci.plugins.workflow.flow.FlowExecution; import org.jenkinsci.plugins.workflow.flow.FlowExecutionOwner; -import org.jenkinsci.plugins.workflow.steps.StepExecution; +import org.jenkinsci.plugins.workflow.graph.FlowNode; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.QueryParameter; import org.kohsuke.stapler.StaplerRequest; @@ -243,7 +239,7 @@ static List getCategoryTasks(String category) { assert category != null && !category.equals(""); List categoryTasks = new ArrayList(); Collection properties; - DescriptorImpl descriptor = Jenkins.getActiveInstance().getDescriptorByType(DescriptorImpl.class); + DescriptorImpl descriptor = fetchDescriptor(); synchronized (descriptor.propertiesByCategoryLock) { Map _properties = descriptor.propertiesByCategory.get(category); properties = _properties != null ? new ArrayList(_properties.keySet()) : Collections.emptySet(); @@ -268,51 +264,40 @@ static List getCategoryTasks(String category) { return categoryTasks; } - static List getThrottledPipelinesForCategory(String category) { - List throttledPipelines = new ArrayList<>(); - - DescriptorImpl descriptor = Jenkins.getActiveInstance().getDescriptorByType(DescriptorImpl.class); - for (Map.Entry currentPipeline : descriptor.getThrottledPipelinesForCategory(category).entrySet()) { - Run flowNodeRun = Run.fromExternalizableId(currentPipeline.getKey()); - - if (flowNodeRun == null) { - // No run found, so remove the throttle. - descriptor.removeThrottledPipelineForCategory(currentPipeline.getKey(), category, null); - } else if (!(flowNodeRun instanceof FlowExecutionOwner.Executable)) { - // If for some reason we've somehow ended up with a non-pipeline job, remove the throttle. - descriptor.removeThrottledPipelineForCategory(currentPipeline.getKey(), category, null); - } else if (!flowNodeRun.isBuilding()) { - // The run is done building, so remove the throttle. - descriptor.removeThrottledPipelineForCategory(currentPipeline.getKey(), category, null); + static Map,List> getThrottledPipelineRunsForCategory(String category) { + Map,List> throttledPipelines = new TreeMap<>(); + + final DescriptorImpl descriptor = fetchDescriptor(); + for (Map.Entry> currentPipeline : descriptor.getThrottledPipelinesForCategory(category).entrySet()) { + Run flowNodeRun = Run.fromExternalizableId(currentPipeline.getKey()); + + List flowNodes = new ArrayList<>(); + + if (flowNodeRun == null || + !(flowNodeRun instanceof FlowExecutionOwner.Executable) || + !flowNodeRun.isBuilding()) { + descriptor.removeAllFromPipelineRunForCategory(currentPipeline.getKey(), category, null); } else { - FlowExecutionOwner owner = ((FlowExecutionOwner.Executable)flowNodeRun).asFlowExecutionOwner(); - FlowExecution execution = owner.getOrNull(); + FlowExecution execution = ((FlowExecutionOwner.Executable) flowNodeRun).asFlowExecutionOwner().getOrNull(); if (execution == null) { - // For some reason, the flow execution is null, so again? Remove the throttle. - descriptor.removeThrottledPipelineForCategory(currentPipeline.getKey(), category, null); + descriptor.removeAllFromPipelineRunForCategory(currentPipeline.getKey(), category, null); } else { - try { - for (StepExecution stepExec : execution.getCurrentExecutions(false).get()) { - try { - Computer c = stepExec.getContext().get(Computer.class); - ThrottledStepInfo candidateInfo = stepExec.getContext().get(ThrottledStepInfo.class); - if (c != null && candidateInfo != null) { - ThrottledStepInfo info = candidateInfo.forCategory(category); - if (info != null) { - if (info.getNode() == null && c.getNode() != null) { - info.setNode(c.getNode().getNodeName()); - } - throttledPipelines.add(info); - } - } - } catch (IOException e) { - // TODO: What do we do here if anything? + for (String flowNodeId : currentPipeline.getValue()) { + try { + FlowNode node = execution.getNode(flowNodeId); + if (node != null) { + flowNodes.add(node); + } else { + descriptor.removeThrottledPipelineForCategory(currentPipeline.getKey(), flowNodeId, category, null); } + } catch (IOException e) { + // do nothing } - } catch (InterruptedException | ExecutionException e) { - // TODO: What do we do here if anything? } } + if (!flowNodes.isEmpty()) { + throttledPipelines.put(flowNodeRun, flowNodes); + } } } @@ -326,6 +311,10 @@ private static Item getItem(ItemGroup group, String name) { return group.getItem(name); } } + + public static DescriptorImpl fetchDescriptor() { + return Jenkins.getActiveInstance().getDescriptorByType(DescriptorImpl.class); + } @Extension public static final class DescriptorImpl extends JobPropertyDescriptor { @@ -333,7 +322,7 @@ public static final class DescriptorImpl extends JobPropertyDescriptor { private List categories; - private Map> throttledPipelines; + private Map>> throttledPipelinesByCategory; /** Map from category names, to properties including that category. */ private transient Map> propertiesByCategory @@ -444,34 +433,35 @@ public ListBoxModel doFillCategoryItems() { @Override public void load() { super.load(); - if (throttledPipelines == null) { - throttledPipelines = new TreeMap<>(); + if (throttledPipelinesByCategory == null) { + throttledPipelinesByCategory = new TreeMap<>(); } - LOGGER.log(Level.FINE, "load: {0}", throttledPipelines); + LOGGER.log(Level.FINE, "load: {0}", throttledPipelinesByCategory); } @Override public void save() { super.save(); - LOGGER.log(Level.FINE, "save: {0}", throttledPipelines); + LOGGER.log(Level.FINE, "save: {0}", throttledPipelinesByCategory); } @Nonnull - public synchronized Map getThrottledPipelinesForCategory(@Nonnull String category) { + public synchronized Map> getThrottledPipelinesForCategory(@Nonnull String category) { return internalGetThrottledPipelinesForCategory(category); } @Nonnull - private Map internalGetThrottledPipelinesForCategory(@Nonnull String category) { + private Map> internalGetThrottledPipelinesForCategory(@Nonnull String category) { if (getCategoryByName(category) != null) { - if (throttledPipelines.containsKey(category)) { - return throttledPipelines.get(category); + if (throttledPipelinesByCategory.containsKey(category)) { + return throttledPipelinesByCategory.get(category); } } return new TreeMap<>(); } public synchronized void addThrottledPipelineForCategory(@Nonnull String runId, + @Nonnull String flowNodeId, @Nonnull String category, TaskListener listener) { if (getCategoryByName(category) == null) { @@ -479,19 +469,20 @@ public synchronized void addThrottledPipelineForCategory(@Nonnull String runId, listener.getLogger().println(Messages.ThrottleJobProperty_DescriptorImpl_NoSuchCategory(category)); } } else { - Map currentPipelines = internalGetThrottledPipelinesForCategory(category); + Map> currentPipelines = internalGetThrottledPipelinesForCategory(category); - if (!currentPipelines.containsKey(runId)) { - currentPipelines.put(runId, 1); - } else { - currentPipelines.put(runId, currentPipelines.get(runId) + 1); + List flowNodes = currentPipelines.get(runId); + if (flowNodes == null) { + flowNodes = new ArrayList<>(); } - - throttledPipelines.put(category, currentPipelines); + flowNodes.add(flowNodeId); + currentPipelines.put(runId, flowNodes); + throttledPipelinesByCategory.put(category, currentPipelines); } } public synchronized void removeThrottledPipelineForCategory(@Nonnull String runId, + @Nonnull String flowNodeId, @Nonnull String category, TaskListener listener) { if (getCategoryByName(category) == null) { @@ -499,36 +490,37 @@ public synchronized void removeThrottledPipelineForCategory(@Nonnull String runI listener.getLogger().println(Messages.ThrottleJobProperty_DescriptorImpl_NoSuchCategory(category)); } } else { - Map currentPipelines = internalGetThrottledPipelinesForCategory(category); + Map> currentPipelines = internalGetThrottledPipelinesForCategory(category); if (!currentPipelines.isEmpty()) { - if (currentPipelines.containsKey(runId)) { - Integer currentCount = currentPipelines.get(runId); - if (currentCount > 1) { - currentPipelines.put(runId, currentCount - 1); - } else { - currentPipelines.remove(runId); - } + List flowNodes = currentPipelines.get(runId); + if (flowNodes != null && flowNodes.contains(flowNodeId)) { + flowNodes.remove(flowNodeId); + } + if (flowNodes != null && !flowNodes.isEmpty()) { + currentPipelines.put(runId, flowNodes); + } else { + currentPipelines.remove(runId); } } if (currentPipelines.isEmpty()) { - throttledPipelines.remove(category); + throttledPipelinesByCategory.remove(category); } else { - throttledPipelines.put(category, currentPipelines); + throttledPipelinesByCategory.put(category, currentPipelines); } } } - public synchronized void removeAllFromRunForCategory(@Nonnull String runId, - @Nonnull String category, - TaskListener listener) { + public synchronized void removeAllFromPipelineRunForCategory(@Nonnull String runId, + @Nonnull String category, + TaskListener listener) { if (getCategoryByName(category) == null) { if (listener != null) { listener.getLogger().println(Messages.ThrottleJobProperty_DescriptorImpl_NoSuchCategory(category)); } } else { - Map currentPipelines = internalGetThrottledPipelinesForCategory(category); + Map> currentPipelines = internalGetThrottledPipelinesForCategory(category); if (!currentPipelines.isEmpty()) { if (currentPipelines.containsKey(runId)) { @@ -536,9 +528,9 @@ public synchronized void removeAllFromRunForCategory(@Nonnull String runId, } } if (currentPipelines.isEmpty()) { - throttledPipelines.remove(category); + throttledPipelinesByCategory.remove(category); } else { - throttledPipelines.put(category, currentPipelines); + throttledPipelinesByCategory.put(category, currentPipelines); } } } diff --git a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java index 9b6a3b9d..c0e217e7 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java @@ -3,29 +3,29 @@ import hudson.Extension; import hudson.matrix.MatrixConfiguration; import hudson.matrix.MatrixProject; -import hudson.model.AbstractProject; import hudson.model.ParameterValue; import hudson.model.Computer; import hudson.model.Executor; -import hudson.model.Hudson; import hudson.model.Job; import hudson.model.Node; import hudson.model.Queue; import hudson.model.Queue.Task; +import hudson.model.Run; import hudson.model.queue.WorkUnit; import hudson.model.labels.LabelAtom; import hudson.model.queue.CauseOfBlockage; import hudson.model.queue.QueueTaskDispatcher; -import hudson.plugins.throttleconcurrents.pipeline.ThrottledStepInfo; +import hudson.plugins.throttleconcurrents.pipeline.ThrottleStep; import hudson.security.ACL; import hudson.security.NotSerilizableSecurityContext; import hudson.model.Action; import hudson.model.ParametersAction; import hudson.model.queue.SubTask; +import java.io.IOException; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; @@ -36,6 +36,12 @@ import org.acegisecurity.context.SecurityContextHolder; import jenkins.model.Jenkins; +import org.jenkinsci.plugins.workflow.flow.FlowExecutionOwner; +import org.jenkinsci.plugins.workflow.graph.BlockStartNode; +import org.jenkinsci.plugins.workflow.graph.FlowNode; +import org.jenkinsci.plugins.workflow.graph.StepNode; +import org.jenkinsci.plugins.workflow.graphanalysis.LinearBlockHoppingScanner; +import org.jenkinsci.plugins.workflow.support.steps.ExecutorStepExecution; @Extension public class ThrottleQueueTaskDispatcher extends QueueTaskDispatcher { @@ -110,9 +116,14 @@ else if (tjp.getThrottleOption().equals("category")) { } runCount += buildsOfProjectOnNode(node, catTask); } - List throttledPipelines = ThrottleJobProperty.getThrottledPipelinesForCategory(catNm); - runCount += pipelinesOnNode(node, throttledPipelines); - + Map,List> throttledPipelines = ThrottleJobProperty.getThrottledPipelineRunsForCategory(catNm); + for (Map.Entry,List> entry : throttledPipelines.entrySet()) { + Run r = entry.getKey(); + List flowNodes = entry.getValue(); + if (r.isBuilding()) { + runCount += pipelinesOnNode(node, r, flowNodes); + } + } // This would mean that there are as many or more builds currently running than are allowed. if (runCount >= maxConcurrentPerNode) { return CauseOfBlockage.fromMessage(Messages._ThrottleQueueTaskDispatcher_MaxCapacityOnNode(runCount)); @@ -233,8 +244,14 @@ else if (tjp.getThrottleOption().equals("category")) { } totalRunCount += buildsOfProjectOnAllNodes(catTask); } - List throttledPipelines = ThrottleJobProperty.getThrottledPipelinesForCategory(catNm); - totalRunCount += pipelinesOnAllNodes(throttledPipelines); + Map,List> throttledPipelines = ThrottleJobProperty.getThrottledPipelineRunsForCategory(catNm); + for (Map.Entry,List> entry : throttledPipelines.entrySet()) { + Run r = entry.getKey(); + List flowNodes = entry.getValue(); + if (r.isBuilding()) { + totalRunCount += pipelinesOnAllNodes(r, flowNodes); + } + } if (totalRunCount >= maxConcurrentTotal) { return CauseOfBlockage.fromMessage(Messages._ThrottleQueueTaskDispatcher_MaxCapacityTotal(totalRunCount)); @@ -370,22 +387,29 @@ private ThrottleJobProperty getThrottleJobProperty(Task task) { return null; } - private int pipelinesOnNode(@Nonnull Node node, @Nonnull List throttledPipelines) { + private int pipelinesOnNode(@Nonnull Node node, @Nonnull Run run, @Nonnull List flowNodes) { int runCount = 0; + LOGGER.log(Level.FINE, "Checking for pipelines of {0} on node {1}", new Object[] {run.getDisplayName(), node.getDisplayName()}); - String nodeName = node.getNodeName(); - - for (ThrottledStepInfo info : throttledPipelines) { - if (nodeName.equals(info.getNode())) { - runCount++; + Computer computer = node.toComputer(); + if (computer != null) { //Not all nodes are certain to become computers, like nodes with 0 executors. + // Don't count flyweight tasks that might not consume an actual executor, unlike with builds. + for (Executor e : computer.getExecutors()) { + runCount += pipelinesOnExecutor(run, e, flowNodes); } } return runCount; } - private int pipelinesOnAllNodes(@Nonnull List throttledPipelines) { - return throttledPipelines.size(); + private int pipelinesOnAllNodes(@Nonnull Run run, @Nonnull List flowNodes) { + final Jenkins jenkins = Jenkins.getActiveInstance(); + int totalRunCount = pipelinesOnNode(jenkins, run, flowNodes); + + for (Node node : jenkins.getNodes()) { + totalRunCount += pipelinesOnNode(node, run, flowNodes); + } + return totalRunCount; } private int buildsOfProjectOnNode(Node node, Task task) { @@ -433,6 +457,43 @@ private int buildsOnExecutor(Task task, Executor exec) { return runCount; } + private int pipelinesOnExecutor(@Nonnull Run run, @Nonnull Executor exec, @Nonnull List flowNodes) { + int runCount = 0; + final Queue.Executable currentExecutable = exec.getCurrentExecutable(); + if (currentExecutable != null) { + if (currentExecutable.getParent() instanceof ExecutorStepExecution.PlaceholderTask) { + ExecutorStepExecution.PlaceholderTask task = (ExecutorStepExecution.PlaceholderTask)currentExecutable.getParent(); + if (task.run() != null && task.run().equals(run)) { + try { + FlowNode firstThrottle = firstThrottleStartNode(task.getNode()); + if (firstThrottle != null && flowNodes.contains(firstThrottle)) { + runCount++; + } + } catch (IOException | InterruptedException e) { + // TODO: do something? + } + } + } + } + + return runCount; + } + + @CheckForNull + private FlowNode firstThrottleStartNode(@Nonnull FlowNode inner) { + LinearBlockHoppingScanner scanner = new LinearBlockHoppingScanner(); + scanner.setup(inner); + for (FlowNode enclosing : scanner) { + if (enclosing instanceof BlockStartNode && enclosing instanceof StepNode) { + // Check if this is a *different* throttling node. + if (((StepNode)enclosing).getDescriptor().getClass().equals(ThrottleStep.DescriptorImpl.class)) { + return enclosing; + } + } + } + return null; + } + /** * @param node to compare labels with. * @param category to compare labels with. diff --git a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java index d1d322ac..1e3d427c 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java @@ -1,11 +1,21 @@ package hudson.plugins.throttleconcurrents.pipeline; +import hudson.Extension; +import hudson.model.TaskListener; +import hudson.plugins.throttleconcurrents.ThrottleJobProperty; +import hudson.util.FormValidation; +import hudson.util.ListBoxModel; +import jenkins.model.Jenkins; import org.jenkinsci.plugins.workflow.steps.Step; import org.jenkinsci.plugins.workflow.steps.StepContext; +import org.jenkinsci.plugins.workflow.steps.StepDescriptor; import org.jenkinsci.plugins.workflow.steps.StepExecution; import org.kohsuke.stapler.DataBoundConstructor; +import org.kohsuke.stapler.QueryParameter; import javax.annotation.Nonnull; +import java.util.Collections; +import java.util.Set; public class ThrottleStep extends Step { private String category; @@ -25,4 +35,26 @@ public StepExecution start(StepContext context) throws Exception { return new ThrottleStepExecution(this, context); } + @Extension + public static final class DescriptorImpl extends StepDescriptor { + @Override + public String getFunctionName() { + return "throttle"; + } + + @Override + public String getDisplayName() { + return "Throttle execution of node blocks within this body"; + } + + @Override + public Set> getRequiredContext() { + return Collections.singleton(TaskListener.class); + } + + public FormValidation doCheckCategoryName(@QueryParameter String value) { + return ThrottleJobProperty.fetchDescriptor().doCheckCategoryName(value); + } + } + } diff --git a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java index fab42254..d8cc9980 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java @@ -1,15 +1,15 @@ package hudson.plugins.throttleconcurrents.pipeline; -import hudson.model.Computer; import hudson.model.Run; import hudson.model.TaskListener; import hudson.plugins.throttleconcurrents.ThrottleJobProperty; -import jenkins.model.Jenkins; +import org.jenkinsci.plugins.workflow.graph.FlowNode; import org.jenkinsci.plugins.workflow.steps.BodyExecution; import org.jenkinsci.plugins.workflow.steps.BodyExecutionCallback; import org.jenkinsci.plugins.workflow.steps.StepContext; import org.jenkinsci.plugins.workflow.steps.StepExecution; +import javax.annotation.CheckForNull; import javax.annotation.Nonnull; public class ThrottleStepExecution extends StepExecution { @@ -28,42 +28,55 @@ public String getCategory() { @Override public boolean start() throws Exception { - ThrottledStepInfo info = new ThrottledStepInfo(getCategory()); - - ThrottledStepInfo parentInfo = getContext().get(ThrottledStepInfo.class); - if (parentInfo != null) { - // Make sure we record the node used for the parent throttle step if it exists. - Computer computer = getContext().get(Computer.class); - if (computer != null && computer.getNode() != null) { - parentInfo.setNode(computer.getNode().getNodeName()); - } - info.setParentInfo(parentInfo); - } - Run r = getContext().get(Run.class); TaskListener listener = getContext().get(TaskListener.class); + FlowNode flowNode = getContext().get(FlowNode.class); - ThrottleJobProperty.DescriptorImpl descriptor = Jenkins.getActiveInstance().getDescriptorByType(ThrottleJobProperty.DescriptorImpl.class); + ThrottleJobProperty.DescriptorImpl descriptor = ThrottleJobProperty.fetchDescriptor(); - descriptor.addThrottledPipelineForCategory(r.getExternalizableId(), getCategory(), listener); + String runId = null; + String flowNodeId = null; + + if (r != null && flowNode != null) { + runId = r.getExternalizableId(); + flowNodeId = flowNode.getId(); + descriptor.addThrottledPipelineForCategory(runId, flowNodeId, getCategory(), listener); + } body = getContext().newBodyInvoker() - .withContext(info) - .withCallback(BodyExecutionCallback.wrap(getContext())) + .withCallback(new Callback(runId, flowNodeId, getCategory())) .start(); return false; } @Override public void stop(Throwable cause) throws Exception { - if (body != null) - body.cancel(cause); - Run r = getContext().get(Run.class); - TaskListener listener = getContext().get(TaskListener.class); + } + + private static final class Callback extends BodyExecutionCallback.TailCall { + @CheckForNull + private String runId; + @CheckForNull + private String flowNodeId; + private String category; - ThrottleJobProperty.DescriptorImpl descriptor = Jenkins.getActiveInstance().getDescriptorByType(ThrottleJobProperty.DescriptorImpl.class); - descriptor.removeThrottledPipelineForCategory(r.getExternalizableId(), getCategory(), listener); + private static final long serialVersionUID = 1; + + Callback(String runId, String flowNodeId, @Nonnull String category) { + this.runId = runId; + this.flowNodeId = flowNodeId; + this.category = category; + } + + @Override protected void finished(StepContext context) throws Exception { + if (runId != null && flowNodeId != null) { + ThrottleJobProperty.fetchDescriptor().removeThrottledPipelineForCategory(runId, + flowNodeId, + category, + context.get(TaskListener.class)); + } + } } } diff --git a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottledStepInfo.java b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottledStepInfo.java deleted file mode 100644 index 96a46413..00000000 --- a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottledStepInfo.java +++ /dev/null @@ -1,52 +0,0 @@ -package hudson.plugins.throttleconcurrents.pipeline; - -import javax.annotation.CheckForNull; -import javax.annotation.Nonnull; -import java.io.Serializable; - -public class ThrottledStepInfo implements Serializable { - private String category; - - private String node; - - private ThrottledStepInfo parentInfo; - - public ThrottledStepInfo(@Nonnull String category) { - this.category = category; - } - - @Nonnull - public String getCategory() { - return category; - } - - public void setNode(@Nonnull String node) { - this.node = node; - } - - @CheckForNull - public String getNode() { - return node; - } - - public void setParentInfo(@Nonnull ThrottledStepInfo parentInfo) { - this.parentInfo = parentInfo; - } - - @CheckForNull - public ThrottledStepInfo getParentInfo() { - return parentInfo; - } - - public ThrottledStepInfo forCategory(@Nonnull String cat) { - if (cat.equals(category)) { - return this; - } else if (getParentInfo() != null) { - return getParentInfo().forCategory(cat); - } else { - return null; - } - } - - private static final long serialVersionUID = 1L; -} diff --git a/src/test/java/hudson/plugins/throttleconcurrents/ThrottleConcurrentTest.java b/src/test/java/hudson/plugins/throttleconcurrents/ThrottleConcurrentTest.java index 655f68db..5b204391 100644 --- a/src/test/java/hudson/plugins/throttleconcurrents/ThrottleConcurrentTest.java +++ b/src/test/java/hudson/plugins/throttleconcurrents/ThrottleConcurrentTest.java @@ -15,6 +15,7 @@ import hudson.slaves.RetentionStrategy; import hudson.tasks.Builder; import jenkins.model.Jenkins; +import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.JenkinsRule; @@ -25,6 +26,7 @@ import static org.assertj.core.api.Assertions.assertThat; +@Ignore("Depends on a newer version of Guava than can be used with Pipeline") public class ThrottleConcurrentTest extends ScenarioTest { @Rule @ScenarioState From 91cdb4bbbb31007c2caf2e9f0e87bc4e481d2fad Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Fri, 10 Mar 2017 14:19:12 -0800 Subject: [PATCH 03/20] Add a trailing newline to messages. --- .../hudson/plugins/throttleconcurrents/Messages.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/resources/hudson/plugins/throttleconcurrents/Messages.properties b/src/main/resources/hudson/plugins/throttleconcurrents/Messages.properties index f9feb4c5..50ba33d2 100644 --- a/src/main/resources/hudson/plugins/throttleconcurrents/Messages.properties +++ b/src/main/resources/hudson/plugins/throttleconcurrents/Messages.properties @@ -5,4 +5,4 @@ ThrottleQueueTaskDispatcher.OnlyOneWithMatchingParameters=A build with matching ThrottleMatrixProjectOptions.DisplayName=Additional options for Matrix projects -ThrottleJobProperty.DescriptorImpl.NoSuchCategory=Requested category "{0}" does not exist, so cannot throttle. \ No newline at end of file +ThrottleJobProperty.DescriptorImpl.NoSuchCategory=Requested category "{0}" does not exist, so cannot throttle. From d011b4059666ee0217d97f4eed8e9d03dd56d12f Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Fri, 10 Mar 2017 15:02:40 -0800 Subject: [PATCH 04/20] Make findbugs happy. --- .../ThrottleJobProperty.java | 58 +++++++++-------- .../ThrottleQueueTaskDispatcher.java | 62 +++++++++---------- .../pipeline/ThrottleStep.java | 7 ++- 3 files changed, 67 insertions(+), 60 deletions(-) diff --git a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java index 2240fd01..24397cb6 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java @@ -79,8 +79,8 @@ public ThrottleJobProperty(Integer maxConcurrentPerNode, String paramsToUseForLimit, @CheckForNull ThrottleMatrixProjectOptions matrixOptions ) { - this.maxConcurrentPerNode = maxConcurrentPerNode == null ? 0 : maxConcurrentPerNode; - this.maxConcurrentTotal = maxConcurrentTotal == null ? 0 : maxConcurrentTotal; + this.maxConcurrentPerNode = maxConcurrentPerNode; + this.maxConcurrentTotal = maxConcurrentTotal; this.categories = categories == null ? new CopyOnWriteArrayList() : new CopyOnWriteArrayList(categories); @@ -278,25 +278,28 @@ static Map,List> getThrottledPipelineRunsForCategory(String c !flowNodeRun.isBuilding()) { descriptor.removeAllFromPipelineRunForCategory(currentPipeline.getKey(), category, null); } else { - FlowExecution execution = ((FlowExecutionOwner.Executable) flowNodeRun).asFlowExecutionOwner().getOrNull(); - if (execution == null) { - descriptor.removeAllFromPipelineRunForCategory(currentPipeline.getKey(), category, null); - } else { - for (String flowNodeId : currentPipeline.getValue()) { - try { - FlowNode node = execution.getNode(flowNodeId); - if (node != null) { - flowNodes.add(node); - } else { - descriptor.removeThrottledPipelineForCategory(currentPipeline.getKey(), flowNodeId, category, null); + FlowExecutionOwner executionOwner = ((FlowExecutionOwner.Executable) flowNodeRun).asFlowExecutionOwner(); + if (executionOwner != null) { + FlowExecution execution = executionOwner.getOrNull(); + if (execution == null) { + descriptor.removeAllFromPipelineRunForCategory(currentPipeline.getKey(), category, null); + } else { + for (String flowNodeId : currentPipeline.getValue()) { + try { + FlowNode node = execution.getNode(flowNodeId); + if (node != null) { + flowNodes.add(node); + } else { + descriptor.removeThrottledPipelineForCategory(currentPipeline.getKey(), flowNodeId, category, null); + } + } catch (IOException e) { + // do nothing } - } catch (IOException e) { - // do nothing } } - } - if (!flowNodes.isEmpty()) { - throttledPipelines.put(flowNodeRun, flowNodes); + if (!flowNodes.isEmpty()) { + throttledPipelines.put(flowNodeRun, flowNodes); + } } } } @@ -433,10 +436,14 @@ public ListBoxModel doFillCategoryItems() { @Override public void load() { super.load(); + initThrottledPipelines(); + LOGGER.log(Level.FINE, "load: {0}", throttledPipelinesByCategory); + } + + private synchronized void initThrottledPipelines() { if (throttledPipelinesByCategory == null) { throttledPipelinesByCategory = new TreeMap<>(); } - LOGGER.log(Level.FINE, "load: {0}", throttledPipelinesByCategory); } @Override @@ -547,8 +554,8 @@ public ThrottleCategory(String categoryName, Integer maxConcurrentPerNode, Integer maxConcurrentTotal, List nodeLabeledPairs) { - this.maxConcurrentPerNode = maxConcurrentPerNode == null ? 0 : maxConcurrentPerNode; - this.maxConcurrentTotal = maxConcurrentTotal == null ? 0 : maxConcurrentTotal; + this.maxConcurrentPerNode = maxConcurrentPerNode; + this.maxConcurrentTotal = maxConcurrentTotal; this.categoryName = categoryName; this.nodeLabeledPairs = nodeLabeledPairs == null ? new ArrayList() : nodeLabeledPairs; @@ -598,21 +605,20 @@ public static final class NodeLabeledPair extends AbstractDescribableImpl executingUnitParams = getParametersFromWorkUnit(exec.getCurrentWorkUnit()); - executingUnitParams = doFilterParams(paramsToCompare, executingUnitParams); - - if (executingUnitParams.containsAll(itemParams)) { - LOGGER.log(Level.FINE, "build (" + exec.getCurrentWorkUnit() + - ") with identical parameters (" + - executingUnitParams + ") is already running."); - return true; - } + // TODO: refactor into a nameEquals helper method + final Queue.Executable currentExecutable = exec.getCurrentExecutable(); + final SubTask parentTask = currentExecutable != null ? currentExecutable.getParent() : null; + if (currentExecutable != null && + parentTask.getOwnerTask().getName().equals(item.task.getName())) { + List executingUnitParams = getParametersFromWorkUnit(exec.getCurrentWorkUnit()); + executingUnitParams = doFilterParams(paramsToCompare, executingUnitParams); + + if (executingUnitParams.containsAll(itemParams)) { + LOGGER.log(Level.FINE, "build (" + exec.getCurrentWorkUnit() + + ") with identical parameters (" + + executingUnitParams + ") is already running."); + return true; } } } } + return false; } @@ -349,10 +348,7 @@ public List getParametersFromWorkUnit(WorkUnit unit) { List actions = unit.context.actions; for (Action action : actions) { if (action instanceof ParametersAction) { - ParametersAction params = (ParametersAction) action; - if (params != null) { - paramsList = params.getParameters(); - } + paramsList = ((ParametersAction)action).getParameters(); } } } @@ -461,9 +457,10 @@ private int pipelinesOnExecutor(@Nonnull Run run, @Nonnull Executor exec, @ int runCount = 0; final Queue.Executable currentExecutable = exec.getCurrentExecutable(); if (currentExecutable != null) { - if (currentExecutable.getParent() instanceof ExecutorStepExecution.PlaceholderTask) { - ExecutorStepExecution.PlaceholderTask task = (ExecutorStepExecution.PlaceholderTask)currentExecutable.getParent(); - if (task.run() != null && task.run().equals(run)) { + SubTask parent = currentExecutable.getParent(); + if (parent instanceof ExecutorStepExecution.PlaceholderTask) { + ExecutorStepExecution.PlaceholderTask task = (ExecutorStepExecution.PlaceholderTask)parent; + if (run.equals(task.run())) { try { FlowNode firstThrottle = firstThrottleStartNode(task.getNode()); if (firstThrottle != null && flowNodes.contains(firstThrottle)) { @@ -480,14 +477,17 @@ private int pipelinesOnExecutor(@Nonnull Run run, @Nonnull Executor exec, @ } @CheckForNull - private FlowNode firstThrottleStartNode(@Nonnull FlowNode inner) { - LinearBlockHoppingScanner scanner = new LinearBlockHoppingScanner(); - scanner.setup(inner); - for (FlowNode enclosing : scanner) { - if (enclosing instanceof BlockStartNode && enclosing instanceof StepNode) { - // Check if this is a *different* throttling node. - if (((StepNode)enclosing).getDescriptor().getClass().equals(ThrottleStep.DescriptorImpl.class)) { - return enclosing; + private FlowNode firstThrottleStartNode(@CheckForNull FlowNode inner) { + if (inner != null) { + LinearBlockHoppingScanner scanner = new LinearBlockHoppingScanner(); + scanner.setup(inner); + for (FlowNode enclosing : scanner) { + if (enclosing != null && enclosing instanceof BlockStartNode && enclosing instanceof StepNode) { + // Check if this is a *different* throttling node. + StepDescriptor desc = ((StepNode) enclosing).getDescriptor(); + if (desc != null && desc.getClass().equals(ThrottleStep.DescriptorImpl.class)) { + return enclosing; + } } } } diff --git a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java index 1e3d427c..a174ef84 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java @@ -4,8 +4,6 @@ import hudson.model.TaskListener; import hudson.plugins.throttleconcurrents.ThrottleJobProperty; import hudson.util.FormValidation; -import hudson.util.ListBoxModel; -import jenkins.model.Jenkins; import org.jenkinsci.plugins.workflow.steps.Step; import org.jenkinsci.plugins.workflow.steps.StepContext; import org.jenkinsci.plugins.workflow.steps.StepDescriptor; @@ -14,10 +12,11 @@ import org.kohsuke.stapler.QueryParameter; import javax.annotation.Nonnull; +import java.io.Serializable; import java.util.Collections; import java.util.Set; -public class ThrottleStep extends Step { +public class ThrottleStep extends Step implements Serializable { private String category; @DataBoundConstructor @@ -35,6 +34,8 @@ public StepExecution start(StepContext context) throws Exception { return new ThrottleStepExecution(this, context); } + private static final long serialVersionUID = 1L; + @Extension public static final class DescriptorImpl extends StepDescriptor { @Override From 666ff55964bac19b619d51a6ce28c1444d68d450 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Fri, 10 Mar 2017 16:14:12 -0800 Subject: [PATCH 05/20] Whoops, this needs to take a block --- .../plugins/throttleconcurrents/pipeline/ThrottleStep.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java index a174ef84..8b3d5a18 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java @@ -48,6 +48,11 @@ public String getDisplayName() { return "Throttle execution of node blocks within this body"; } + @Override + public boolean takesImplicitBlockArgument() { + return true; + } + @Override public Set> getRequiredContext() { return Collections.singleton(TaskListener.class); From 42f4da1c9dc9a2f8334ba7616f1866a20d15c6f9 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Mon, 13 Mar 2017 12:42:11 -0700 Subject: [PATCH 06/20] Initial test, actually working Needed to bump to newer dependency versions, most notably to get PlaceholderTask.getNode(). Still a work in progress, mind you. --- pom.xml | 54 +++- .../ThrottleJobProperty.java | 16 ++ .../ThrottleQueueTaskDispatcher.java | 238 ++++++++++-------- .../throttleconcurrents/ThrottleStepTest.java | 122 +++++++++ 4 files changed, 319 insertions(+), 111 deletions(-) create mode 100644 src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java diff --git a/pom.xml b/pom.xml index 9b7f6163..47c13107 100644 --- a/pom.xml +++ b/pom.xml @@ -26,7 +26,7 @@ THE SOFTWARE. org.jenkins-ci.plugins plugin - 2.23 + 2.22 throttle-concurrents @@ -44,10 +44,10 @@ THE SOFTWARE. - 1.609.3 + 1.642.3 UTF-8 - 1.6 - 1.6 + 1.7 + 1.7 false @@ -105,7 +105,7 @@ THE SOFTWARE. org.jenkins-ci.plugins matrix-project - 1.4.1 + 1.8 org.jenkins-ci.plugins.workflow @@ -127,22 +127,58 @@ THE SOFTWARE. org.jenkins-ci.plugins cloudbees-folder - 4.0 + 6.0.2 test org.jenkins-ci.plugins matrix-auth - 1.2 + 1.4 test org.jenkins-ci.plugins credentials - 1.9.4 + 2.1.13 test - + + org.jenkins-ci.plugins.workflow + workflow-cps + 2.28 + test + + + org.jenkins-ci.plugins.workflow + workflow-job + 2.9 + test + + + org.jenkins-ci.plugins.workflow + workflow-basic-steps + 2.3 + test + + + org.jenkins-ci.plugins + junit + 1.15 + test + + + org.jenkins-ci.plugins.workflow + workflow-support + 2.13 + test + + + org.jenkins-ci.plugins.workflow + workflow-support + 2.13 + tests + test + org.assertj assertj-core diff --git a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java index 24397cb6..640a5aaa 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java @@ -235,6 +235,22 @@ public List getParamsToCompare() { return paramsToCompare; } + static List getCategoriesForRunAndFlowNode(String runId, String flowNodeId) { + List categories = new ArrayList<>(); + + final DescriptorImpl descriptor = fetchDescriptor(); + + for (ThrottleCategory cat : descriptor.getCategories()) { + Map> runs = descriptor.getThrottledPipelinesForCategory(cat.getCategoryName()); + + if (!runs.isEmpty() && runs.containsKey(runId) && runs.get(runId).contains(flowNodeId)) { + categories.add(cat.getCategoryName()); + } + } + + return categories; + } + static List getCategoryTasks(String category) { assert category != null && !category.equals(""); List categoryTasks = new ArrayList(); diff --git a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java index 06987a1a..030af70e 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java @@ -36,7 +36,8 @@ import org.acegisecurity.context.SecurityContextHolder; import jenkins.model.Jenkins; -import org.jenkinsci.plugins.workflow.flow.FlowExecutionOwner; +import org.jenkinsci.plugins.workflow.actions.BodyInvocationAction; +import org.jenkinsci.plugins.workflow.actions.LogAction; import org.jenkinsci.plugins.workflow.graph.BlockStartNode; import org.jenkinsci.plugins.workflow.graph.FlowNode; import org.jenkinsci.plugins.workflow.graph.StepNode; @@ -69,85 +70,94 @@ public CauseOfBlockage canTake(Node node, Task task) { private CauseOfBlockage canTakeImpl(Node node, Task task) { final Jenkins jenkins = Jenkins.getActiveInstance(); ThrottleJobProperty tjp = getThrottleJobProperty(task); - + List pipelineCategories = categoriesForPipeline(task); + // Handle multi-configuration filters - if (!shouldBeThrottled(task, tjp)) { + if (!shouldBeThrottled(task, tjp) && pipelineCategories.isEmpty()) { return null; } - if (tjp!=null && tjp.getThrottleEnabled()) { - CauseOfBlockage cause = canRunImpl(task, tjp); + if (!pipelineCategories.isEmpty() || (tjp!=null && tjp.getThrottleEnabled())) { + CauseOfBlockage cause = canRunImpl(task, tjp, pipelineCategories); if (cause != null) { return cause; } - - if (tjp.getThrottleOption().equals("project")) { - if (tjp.getMaxConcurrentPerNode().intValue() > 0) { - int maxConcurrentPerNode = tjp.getMaxConcurrentPerNode().intValue(); - int runCount = buildsOfProjectOnNode(node, task); - - // This would mean that there are as many or more builds currently running than are allowed. - if (runCount >= maxConcurrentPerNode) { - return CauseOfBlockage.fromMessage(Messages._ThrottleQueueTaskDispatcher_MaxCapacityOnNode(runCount)); + if (tjp != null) { + if (tjp.getThrottleOption().equals("project")) { + if (tjp.getMaxConcurrentPerNode().intValue() > 0) { + int maxConcurrentPerNode = tjp.getMaxConcurrentPerNode().intValue(); + int runCount = buildsOfProjectOnNode(node, task); + + // This would mean that there are as many or more builds currently running than are allowed. + if (runCount >= maxConcurrentPerNode) { + return CauseOfBlockage.fromMessage(Messages._ThrottleQueueTaskDispatcher_MaxCapacityOnNode(runCount)); + } } + } else if (tjp.getThrottleOption().equals("category")) { + return throttleCheckForCategoriesOnNode(node, jenkins, tjp.getCategories()); } + } else if (!pipelineCategories.isEmpty()) { + return throttleCheckForCategoriesOnNode(node, jenkins, pipelineCategories); } - else if (tjp.getThrottleOption().equals("category")) { - // If the project is in one or more categories... - if (tjp.getCategories() != null && !tjp.getCategories().isEmpty()) { - for (String catNm : tjp.getCategories()) { - // Quick check that catNm itself is a real string. - if (catNm != null && !catNm.equals("")) { - List categoryTasks = ThrottleJobProperty.getCategoryTasks(catNm); - - ThrottleJobProperty.ThrottleCategory category = - ((ThrottleJobProperty.DescriptorImpl)tjp.getDescriptor()).getCategoryByName(catNm); - - // Double check category itself isn't null - if (category != null) { - int runCount = 0; - - // Max concurrent per node for category - int maxConcurrentPerNode = getMaxConcurrentPerNodeBasedOnMatchingLabels( - node, category, category.getMaxConcurrentPerNode().intValue()); - if (maxConcurrentPerNode > 0) { - for (Task catTask : categoryTasks) { - if (jenkins.getQueue().isPending(catTask)) { - return CauseOfBlockage.fromMessage(Messages._ThrottleQueueTaskDispatcher_BuildPending()); - } - runCount += buildsOfProjectOnNode(node, catTask); - } - Map,List> throttledPipelines = ThrottleJobProperty.getThrottledPipelineRunsForCategory(catNm); - for (Map.Entry,List> entry : throttledPipelines.entrySet()) { - Run r = entry.getKey(); - List flowNodes = entry.getValue(); - if (r.isBuilding()) { - runCount += pipelinesOnNode(node, r, flowNodes); - } - } - // This would mean that there are as many or more builds currently running than are allowed. - if (runCount >= maxConcurrentPerNode) { - return CauseOfBlockage.fromMessage(Messages._ThrottleQueueTaskDispatcher_MaxCapacityOnNode(runCount)); - } + } + + return null; + } + + private CauseOfBlockage throttleCheckForCategoriesOnNode(Node node, Jenkins jenkins, List categories) { + // If the project is in one or more categories... + if (!categories.isEmpty()) { + for (String catNm : categories) { + // Quick check that catNm itself is a real string. + if (catNm != null && !catNm.equals("")) { + List categoryTasks = ThrottleJobProperty.getCategoryTasks(catNm); + + ThrottleJobProperty.ThrottleCategory category = + ThrottleJobProperty.fetchDescriptor().getCategoryByName(catNm); + + // Double check category itself isn't null + if (category != null) { + int runCount = 0; + // Max concurrent per node for category + int maxConcurrentPerNode = getMaxConcurrentPerNodeBasedOnMatchingLabels( + node, category, category.getMaxConcurrentPerNode().intValue()); + if (maxConcurrentPerNode > 0) { + for (Task catTask : categoryTasks) { + if (jenkins.getQueue().isPending(catTask)) { + return CauseOfBlockage.fromMessage(Messages._ThrottleQueueTaskDispatcher_BuildPending()); } + runCount += buildsOfProjectOnNode(node, catTask); + } + Map,List> throttledPipelines = ThrottleJobProperty.getThrottledPipelineRunsForCategory(catNm); + for (Map.Entry,List> entry : throttledPipelines.entrySet()) { + Run r = entry.getKey(); + List flowNodes = entry.getValue(); + if (r.isBuilding()) { + runCount += pipelinesOnNode(node, r, flowNodes); + } + } + // This would mean that there are as many or more builds currently running than are allowed. + if (runCount >= maxConcurrentPerNode) { + return CauseOfBlockage.fromMessage(Messages._ThrottleQueueTaskDispatcher_MaxCapacityOnNode(runCount)); } } } } } } - return null; } // @Override on jenkins 4.127+ , but still compatible with 1.399 public CauseOfBlockage canRun(Queue.Item item) { ThrottleJobProperty tjp = getThrottleJobProperty(item.task); - if (tjp!=null && tjp.getThrottleEnabled()) { - if (tjp.isLimitOneJobWithMatchingParams() && isAnotherBuildWithSameParametersRunningOnAnyNode(item)) { + List pipelineCategories = categoriesForPipeline(item.task); + + if (!pipelineCategories.isEmpty() || (tjp!=null && tjp.getThrottleEnabled())) { + if (tjp != null && tjp.isLimitOneJobWithMatchingParams() && isAnotherBuildWithSameParametersRunningOnAnyNode(item)) { return CauseOfBlockage.fromMessage(Messages._ThrottleQueueTaskDispatcher_OnlyOneWithMatchingParameters()); } - return canRun(item.task, tjp); + return canRun(item.task, tjp, pipelineCategories); } return null; } @@ -155,6 +165,7 @@ public CauseOfBlockage canRun(Queue.Item item) { @Nonnull private ThrottleMatrixProjectOptions getMatrixOptions(Task task) { ThrottleJobProperty tjp = getThrottleJobProperty(task); + if (tjp == null){ return ThrottleMatrixProjectOptions.DEFAULT; } @@ -186,9 +197,9 @@ private boolean shouldBeThrottled(@Nonnull Task task, @CheckForNull ThrottleJobP return true; } - public CauseOfBlockage canRun(Task task, ThrottleJobProperty tjp) { + public CauseOfBlockage canRun(Task task, ThrottleJobProperty tjp, List pipelineCategories) { if (Jenkins.getAuthentication() == ACL.SYSTEM) { - return canRunImpl(task, tjp); + return canRunImpl(task, tjp, pipelineCategories); } // Throttle-concurrent-builds requires READ permissions for all projects. @@ -198,73 +209,78 @@ public CauseOfBlockage canRun(Task task, ThrottleJobProperty tjp) { SecurityContextHolder.setContext(auth); try { - return canRunImpl(task, tjp); + return canRunImpl(task, tjp, pipelineCategories); } finally { SecurityContextHolder.setContext(orig); } } - private CauseOfBlockage canRunImpl(Task task, ThrottleJobProperty tjp) { + private CauseOfBlockage canRunImpl(Task task, ThrottleJobProperty tjp, List pipelineCategories) { final Jenkins jenkins = Jenkins.getActiveInstance(); - if (!shouldBeThrottled(task, tjp)) { + if (!shouldBeThrottled(task, tjp) && pipelineCategories.isEmpty()) { return null; } if (jenkins.getQueue().isPending(task)) { return CauseOfBlockage.fromMessage(Messages._ThrottleQueueTaskDispatcher_BuildPending()); } - if (tjp.getThrottleOption().equals("project")) { - if (tjp.getMaxConcurrentTotal().intValue() > 0) { - int maxConcurrentTotal = tjp.getMaxConcurrentTotal().intValue(); - int totalRunCount = buildsOfProjectOnAllNodes(task); + if (tjp != null) { + if (tjp.getThrottleOption().equals("project")) { + if (tjp.getMaxConcurrentTotal().intValue() > 0) { + int maxConcurrentTotal = tjp.getMaxConcurrentTotal().intValue(); + int totalRunCount = buildsOfProjectOnAllNodes(task); - if (totalRunCount >= maxConcurrentTotal) { - return CauseOfBlockage.fromMessage(Messages._ThrottleQueueTaskDispatcher_MaxCapacityTotal(totalRunCount)); + if (totalRunCount >= maxConcurrentTotal) { + return CauseOfBlockage.fromMessage(Messages._ThrottleQueueTaskDispatcher_MaxCapacityTotal(totalRunCount)); + } } + } else if (tjp.getThrottleOption().equals("category")) { + return throttleCheckForCategoriesAllNodes(jenkins, tjp.getCategories()); } + } else if (!pipelineCategories.isEmpty()) { + return throttleCheckForCategoriesAllNodes(jenkins, pipelineCategories); } - // If the project is in one or more categories... - else if (tjp.getThrottleOption().equals("category")) { - if (tjp.getCategories() != null && !tjp.getCategories().isEmpty()) { - for (String catNm : tjp.getCategories()) { - // Quick check that catNm itself is a real string. - if (catNm != null && !catNm.equals("")) { - List categoryTasks = ThrottleJobProperty.getCategoryTasks(catNm); - - ThrottleJobProperty.ThrottleCategory category = - ((ThrottleJobProperty.DescriptorImpl)tjp.getDescriptor()).getCategoryByName(catNm); - - // Double check category itself isn't null - if (category != null) { - if (category.getMaxConcurrentTotal().intValue() > 0) { - int maxConcurrentTotal = category.getMaxConcurrentTotal().intValue(); - int totalRunCount = 0; - - for (Task catTask : categoryTasks) { - if (jenkins.getQueue().isPending(catTask)) { - return CauseOfBlockage.fromMessage(Messages._ThrottleQueueTaskDispatcher_BuildPending()); - } - totalRunCount += buildsOfProjectOnAllNodes(catTask); - } - Map,List> throttledPipelines = ThrottleJobProperty.getThrottledPipelineRunsForCategory(catNm); - for (Map.Entry,List> entry : throttledPipelines.entrySet()) { - Run r = entry.getKey(); - List flowNodes = entry.getValue(); - if (r.isBuilding()) { - totalRunCount += pipelinesOnAllNodes(r, flowNodes); - } - } - if (totalRunCount >= maxConcurrentTotal) { - return CauseOfBlockage.fromMessage(Messages._ThrottleQueueTaskDispatcher_MaxCapacityTotal(totalRunCount)); - } + return null; + } + + private CauseOfBlockage throttleCheckForCategoriesAllNodes(Jenkins jenkins, @Nonnull List categories) { + for (String catNm : categories) { + // Quick check that catNm itself is a real string. + if (catNm != null && !catNm.equals("")) { + List categoryTasks = ThrottleJobProperty.getCategoryTasks(catNm); + + ThrottleJobProperty.ThrottleCategory category = + ThrottleJobProperty.fetchDescriptor().getCategoryByName(catNm); + + // Double check category itself isn't null + if (category != null) { + if (category.getMaxConcurrentTotal().intValue() > 0) { + int maxConcurrentTotal = category.getMaxConcurrentTotal().intValue(); + int totalRunCount = 0; + + for (Task catTask : categoryTasks) { + if (jenkins.getQueue().isPending(catTask)) { + return CauseOfBlockage.fromMessage(Messages._ThrottleQueueTaskDispatcher_BuildPending()); } + totalRunCount += buildsOfProjectOnAllNodes(catTask); + } + Map,List> throttledPipelines = ThrottleJobProperty.getThrottledPipelineRunsForCategory(catNm); + for (Map.Entry,List> entry : throttledPipelines.entrySet()) { + Run r = entry.getKey(); + List flowNodes = entry.getValue(); + if (r.isBuilding()) { + totalRunCount += pipelinesOnAllNodes(r, flowNodes); + } + } + if (totalRunCount >= maxConcurrentTotal) { + return CauseOfBlockage.fromMessage(Messages._ThrottleQueueTaskDispatcher_MaxCapacityTotal(totalRunCount)); } } + } } } - return null; } @@ -369,6 +385,21 @@ public List getParametersFromQueueItem(Queue.Item item) { return paramsList; } + private List categoriesForPipeline(Task task) { + if (task instanceof ExecutorStepExecution.PlaceholderTask) { + ExecutorStepExecution.PlaceholderTask placeholderTask = (ExecutorStepExecution.PlaceholderTask)task; + try { + FlowNode firstThrottle = firstThrottleStartNode(placeholderTask.getNode()); + if (firstThrottle != null) { + return ThrottleJobProperty.getCategoriesForRunAndFlowNode(placeholderTask.run().getExternalizableId(), + firstThrottle.getId()); + } + } catch (IOException | InterruptedException e) { + return new ArrayList<>(); + } + } + return new ArrayList<>(); + } @CheckForNull private ThrottleJobProperty getThrottleJobProperty(Task task) { @@ -482,7 +513,10 @@ private FlowNode firstThrottleStartNode(@CheckForNull FlowNode inner) { LinearBlockHoppingScanner scanner = new LinearBlockHoppingScanner(); scanner.setup(inner); for (FlowNode enclosing : scanner) { - if (enclosing != null && enclosing instanceof BlockStartNode && enclosing instanceof StepNode) { + if (enclosing != null && + enclosing instanceof BlockStartNode && + enclosing instanceof StepNode && + enclosing.getAction(BodyInvocationAction.class) == null) { // Check if this is a *different* throttling node. StepDescriptor desc = ((StepNode) enclosing).getDescriptor(); if (desc != null && desc.getClass().equals(ThrottleStep.DescriptorImpl.class)) { diff --git a/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java b/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java new file mode 100644 index 00000000..8b068351 --- /dev/null +++ b/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java @@ -0,0 +1,122 @@ +package hudson.plugins.throttleconcurrents; + +import hudson.model.Executor; +import hudson.model.Node; +import hudson.slaves.DumbSlave; +import hudson.slaves.NodeProperty; +import hudson.slaves.RetentionStrategy; +import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; +import org.jenkinsci.plugins.workflow.job.WorkflowJob; +import org.jenkinsci.plugins.workflow.job.WorkflowRun; +import org.jenkinsci.plugins.workflow.support.steps.ExecutorStepExecution; +import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep; +import org.junit.Before; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.junit.runners.model.Statement; +import org.jvnet.hudson.test.BuildWatcher; +import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.RestartableJenkinsRule; + +import java.util.Arrays; +import java.util.Collections; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +public class ThrottleStepTest { + private static final String ONE_PER_NODE = "one_per_node"; + private static final String TWO_TOTAL = "two_total"; + + @Rule + public RestartableJenkinsRule story = new RestartableJenkinsRule(); + + @ClassRule + public static BuildWatcher buildWatcher = new BuildWatcher(); + + @Rule + public TemporaryFolder firstAgentTmp = new TemporaryFolder(); + @Rule + public TemporaryFolder secondAgentTmp = new TemporaryFolder(); + + public void setupAgentsAndCategories() throws Exception { + DumbSlave firstAgent = new DumbSlave("first-agent", "dummy agent", firstAgentTmp.getRoot().getAbsolutePath(), + "4", Node.Mode.NORMAL, "on-agent", story.j.createComputerLauncher(null), + RetentionStrategy.NOOP, Collections.>emptyList()); + + DumbSlave secondAgent = new DumbSlave("second-agent", "dummy agent", secondAgentTmp.getRoot().getAbsolutePath(), + "4", Node.Mode.NORMAL, "on-agent", story.j.createComputerLauncher(null), + RetentionStrategy.NOOP, Collections.>emptyList()); + + story.j.jenkins.addNode(firstAgent); + story.j.jenkins.addNode(secondAgent); + + ThrottleJobProperty.ThrottleCategory firstCat = new ThrottleJobProperty.ThrottleCategory(ONE_PER_NODE, 1, 0, null); + ThrottleJobProperty.ThrottleCategory secondCat = new ThrottleJobProperty.ThrottleCategory(TWO_TOTAL, 0, 2, null); + + ThrottleJobProperty.DescriptorImpl descriptor = story.j.jenkins.getDescriptorByType(ThrottleJobProperty.DescriptorImpl.class); + assertNotNull(descriptor); + descriptor.setCategories(Arrays.asList(firstCat, secondCat)); + } + + @Test + public void onePerNode() throws Exception { + story.addStep(new Statement() { + @Override + public void evaluate() throws Throwable { + setupAgentsAndCategories(); + WorkflowJob firstJob = story.j.jenkins.createProject(WorkflowJob.class, "first-job"); + // This should be sandbox:true, but when I do that, I get org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException: Scripts not permitted to use method groovy.lang.GroovyObject invokeMethod java.lang.String java.lang.Object + // And I cannot figure out why. So for now... + firstJob.setDefinition(new CpsFlowDefinition("throttle('" + ONE_PER_NODE + "') {\n" + + " echo 'hi there'\n" + + " node('first-agent') {\n" + + " semaphore 'wait-first-job'\n" + + " }\n" + + "}\n", false)); + + WorkflowRun firstJobFirstRun = firstJob.scheduleBuild2(0).waitForStart(); + SemaphoreStep.waitForStart("wait-first-job/1", firstJobFirstRun); + + WorkflowJob secondJob = story.j.jenkins.createProject(WorkflowJob.class, "second-job"); + secondJob.setDefinition(new CpsFlowDefinition("throttle('" + ONE_PER_NODE + "') {\n" + + " node('first-agent') {\n" + + " semaphore 'wait-second-job'\n" + + " }\n" + + "}\n", false)); + + WorkflowRun secondJobFirstRun = secondJob.scheduleBuild2(0).waitForStart(); + story.j.waitForMessage("Still waiting to schedule task", secondJobFirstRun); + assertFalse(story.j.jenkins.getQueue().isEmpty()); + Node n = story.j.jenkins.getNode("first-agent"); + assertNotNull(n); + assertEquals(1, n.toComputer().countBusy()); + hasPlaceholderTaskForRun(n, firstJobFirstRun); + + SemaphoreStep.success("wait-first-job/1", null); + story.j.assertBuildStatusSuccess(story.j.waitForCompletion(firstJobFirstRun)); + SemaphoreStep.waitForStart("wait-second-job/1", secondJobFirstRun); + assertTrue(story.j.jenkins.getQueue().isEmpty()); + assertEquals(1, n.toComputer().countBusy()); + hasPlaceholderTaskForRun(n, secondJobFirstRun); + SemaphoreStep.success("wait-second-job/1", null); + story.j.assertBuildStatusSuccess(story.j.waitForCompletion(secondJobFirstRun)); + + } + }); + } + + private void hasPlaceholderTaskForRun(Node n, WorkflowRun r) throws Exception { + for (Executor exec : n.toComputer().getExecutors()) { + if (exec.getCurrentExecutable() != null) { + assertTrue(exec.getCurrentExecutable().getParent() instanceof ExecutorStepExecution.PlaceholderTask); + ExecutorStepExecution.PlaceholderTask task = (ExecutorStepExecution.PlaceholderTask)exec.getCurrentExecutable().getParent(); + assertEquals(r, task.run()); + } + } + } +} From 83787227220283dd8dcbcaabbbba521d2cac95ea Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Mon, 13 Mar 2017 13:15:15 -0700 Subject: [PATCH 07/20] Cleanup, commenting, javadoc --- pom.xml | 2 +- .../ThrottleJobProperty.java | 32 ++++++++++++++--- .../ThrottleQueueTaskDispatcher.java | 34 ++++++++++++++----- .../pipeline/ThrottleStepExecution.java | 6 ++-- 4 files changed, 57 insertions(+), 17 deletions(-) diff --git a/pom.xml b/pom.xml index 47c13107..9ce7abc1 100644 --- a/pom.xml +++ b/pom.xml @@ -32,7 +32,7 @@ THE SOFTWARE. throttle-concurrents hpi Jenkins Throttle Concurrent Builds Plug-in - 1.9.1-SNAPSHOT + 2.0-SNAPSHOT http://wiki.jenkins-ci.org/display/JENKINS/Throttle+Concurrent+Builds+Plugin Plugin to throttle the number of concurrent builds of a single job per node. diff --git a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java index 640a5aaa..4ec23f51 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java @@ -235,7 +235,16 @@ public List getParamsToCompare() { return paramsToCompare; } - static List getCategoriesForRunAndFlowNode(String runId, String flowNodeId) { + /** + * Get the list of categories for a given run ID (from {@link Run#getExternalizableId()}) and flow node ID (from + * {@link FlowNode#getId()}, if that run/flow node combination is recorded for one or more categories. + * + * @param runId The run ID + * @param flowNodeId The flow node ID + * @return A list of category names. May be empty. + */ + @Nonnull + static List getCategoriesForRunAndFlowNode(@Nonnull String runId, @Nonnull String flowNodeId) { List categories = new ArrayList<>(); final DescriptorImpl descriptor = fetchDescriptor(); @@ -251,8 +260,14 @@ static List getCategoriesForRunAndFlowNode(String runId, String flowNode return categories; } - static List getCategoryTasks(String category) { - assert category != null && !category.equals(""); + /** + * Get all {@link Queue.Task}s with {@link ThrottleJobProperty}s attached to them. + * + * @param category a non-null string, the category name. + * @return A list of {@link Queue.Task}s with {@link ThrottleJobProperty} attached. + */ + static List getCategoryTasks(@Nonnull String category) { + assert !StringUtils.isEmpty(category); List categoryTasks = new ArrayList(); Collection properties; DescriptorImpl descriptor = fetchDescriptor(); @@ -280,7 +295,16 @@ static List getCategoryTasks(String category) { return categoryTasks; } - static Map,List> getThrottledPipelineRunsForCategory(String category) { + /** + * Gets a map of {@link Run}s to a list of {@link FlowNode}s currently running for a given category. Removes any + * no longer valid run/flow node combinations from the internal tracking for that category, due to the run not being + * found, the run not being a {@link FlowExecutionOwner.Executable}, the run no longer building, etc + * + * @param category The category name to look for. + * @return a map of {@link Run}s to lists of {@link FlowNode}s for this category, if any. May be empty. + */ + @Nonnull + static Map,List> getThrottledPipelineRunsForCategory(@Nonnull String category) { Map,List> throttledPipelines = new TreeMap<>(); final DescriptorImpl descriptor = fetchDescriptor(); diff --git a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java index 030af70e..931d94c5 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java @@ -37,13 +37,13 @@ import jenkins.model.Jenkins; import org.jenkinsci.plugins.workflow.actions.BodyInvocationAction; -import org.jenkinsci.plugins.workflow.actions.LogAction; import org.jenkinsci.plugins.workflow.graph.BlockStartNode; import org.jenkinsci.plugins.workflow.graph.FlowNode; import org.jenkinsci.plugins.workflow.graph.StepNode; import org.jenkinsci.plugins.workflow.graphanalysis.LinearBlockHoppingScanner; import org.jenkinsci.plugins.workflow.steps.StepDescriptor; import org.jenkinsci.plugins.workflow.support.steps.ExecutorStepExecution; +import org.jenkinsci.plugins.workflow.support.steps.ExecutorStepExecution.PlaceholderTask; @Extension public class ThrottleQueueTaskDispatcher extends QueueTaskDispatcher { @@ -386,8 +386,8 @@ public List getParametersFromQueueItem(Queue.Item item) { } private List categoriesForPipeline(Task task) { - if (task instanceof ExecutorStepExecution.PlaceholderTask) { - ExecutorStepExecution.PlaceholderTask placeholderTask = (ExecutorStepExecution.PlaceholderTask)task; + if (task instanceof PlaceholderTask) { + PlaceholderTask placeholderTask = (PlaceholderTask)task; try { FlowNode firstThrottle = firstThrottleStartNode(placeholderTask.getNode()); if (firstThrottle != null) { @@ -484,18 +484,28 @@ private int buildsOnExecutor(Task task, Executor exec) { return runCount; } + /** + * Get the count of currently executing {@link PlaceholderTask}s on a given {@link Executor} for a given {@link Run} + * and list of {@link FlowNode}s in that run that have been throttled. + * + * @param run The {@link Run} we care about. + * @param exec The {@link Executor} we're checking on. + * @param flowNodes The list of {@link FlowNode}s associated with that run that have been throttled with a particular + * category. + * @return 1 if there's something currently executing on that executor and it's of that run and one of the provided + * flow nodes, 0 otherwise. + */ private int pipelinesOnExecutor(@Nonnull Run run, @Nonnull Executor exec, @Nonnull List flowNodes) { - int runCount = 0; final Queue.Executable currentExecutable = exec.getCurrentExecutable(); if (currentExecutable != null) { SubTask parent = currentExecutable.getParent(); - if (parent instanceof ExecutorStepExecution.PlaceholderTask) { - ExecutorStepExecution.PlaceholderTask task = (ExecutorStepExecution.PlaceholderTask)parent; + if (parent instanceof PlaceholderTask) { + PlaceholderTask task = (PlaceholderTask)parent; if (run.equals(task.run())) { try { FlowNode firstThrottle = firstThrottleStartNode(task.getNode()); if (firstThrottle != null && flowNodes.contains(firstThrottle)) { - runCount++; + return 1; } } catch (IOException | InterruptedException e) { // TODO: do something? @@ -504,9 +514,15 @@ private int pipelinesOnExecutor(@Nonnull Run run, @Nonnull Executor exec, @ } } - return runCount; + return 0; } + /** + * Given a {@link FlowNode}, find the {@link FlowNode} most directly enclosing this one that comes from a {@link ThrottleStep}. + * + * @param inner The inner {@link FlowNode} + * @return The most immediate enclosing {@link FlowNode} of the inner one that is associated with {@link ThrottleStep}. May be null. + */ @CheckForNull private FlowNode firstThrottleStartNode(@CheckForNull FlowNode inner) { if (inner != null) { @@ -516,6 +532,8 @@ private FlowNode firstThrottleStartNode(@CheckForNull FlowNode inner) { if (enclosing != null && enclosing instanceof BlockStartNode && enclosing instanceof StepNode && + // There are two BlockStartNodes (aka StepStartNodes) for ThrottleStep, so make sure we get the + // first one of those two, which will not have BodyInvocationAction.class on it. enclosing.getAction(BodyInvocationAction.class) == null) { // Check if this is a *different* throttling node. StepDescriptor desc = ((StepNode) enclosing).getDescriptor(); diff --git a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java index d8cc9980..1c48cb5f 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java @@ -15,8 +15,6 @@ public class ThrottleStepExecution extends StepExecution { private final ThrottleStep step; - private BodyExecution body; - public ThrottleStepExecution(@Nonnull ThrottleStep step, StepContext context) { super(context); this.step = step; @@ -43,7 +41,7 @@ public boolean start() throws Exception { descriptor.addThrottledPipelineForCategory(runId, flowNodeId, getCategory(), listener); } - body = getContext().newBodyInvoker() + getContext().newBodyInvoker() .withCallback(new Callback(runId, flowNodeId, getCategory())) .start(); return false; @@ -64,7 +62,7 @@ private static final class Callback extends BodyExecutionCallback.TailCall { private static final long serialVersionUID = 1; - Callback(String runId, String flowNodeId, @Nonnull String category) { + Callback(@CheckForNull String runId, @CheckForNull String flowNodeId, @Nonnull String category) { this.runId = runId; this.flowNodeId = flowNodeId; this.category = category; From 492c500ad95b593a7197703bf854814c61ad3bd1 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Mon, 13 Mar 2017 13:45:37 -0700 Subject: [PATCH 08/20] Test across all nodes Also discovered that Run is a very bad Map key. --- .../ThrottleJobProperty.java | 15 ++--- .../ThrottleQueueTaskDispatcher.java | 13 ++-- .../throttleconcurrents/ThrottleStepTest.java | 65 +++++++++++++++++++ 3 files changed, 79 insertions(+), 14 deletions(-) diff --git a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java index 4ec23f51..733d6eec 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java @@ -296,21 +296,20 @@ static List getCategoryTasks(@Nonnull String category) { } /** - * Gets a map of {@link Run}s to a list of {@link FlowNode}s currently running for a given category. Removes any + * Gets a map of IDs for {@link Run}s to a list of {@link FlowNode}s currently running for a given category. Removes any * no longer valid run/flow node combinations from the internal tracking for that category, due to the run not being * found, the run not being a {@link FlowExecutionOwner.Executable}, the run no longer building, etc * * @param category The category name to look for. - * @return a map of {@link Run}s to lists of {@link FlowNode}s for this category, if any. May be empty. + * @return a map of IDs for {@link Run}s to lists of {@link FlowNode}s for this category, if any. May be empty. */ @Nonnull - static Map,List> getThrottledPipelineRunsForCategory(@Nonnull String category) { - Map,List> throttledPipelines = new TreeMap<>(); + static Map> getThrottledPipelineRunsForCategory(@Nonnull String category) { + Map> throttledPipelines = new TreeMap<>(); final DescriptorImpl descriptor = fetchDescriptor(); for (Map.Entry> currentPipeline : descriptor.getThrottledPipelinesForCategory(category).entrySet()) { Run flowNodeRun = Run.fromExternalizableId(currentPipeline.getKey()); - List flowNodes = new ArrayList<>(); if (flowNodeRun == null || @@ -337,11 +336,11 @@ static Map,List> getThrottledPipelineRunsForCategory(@Nonnull } } } - if (!flowNodes.isEmpty()) { - throttledPipelines.put(flowNodeRun, flowNodes); - } } } + if (!flowNodes.isEmpty()) { + throttledPipelines.put(currentPipeline.getKey(), flowNodes); + } } return throttledPipelines; diff --git a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java index 931d94c5..6c207ccb 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java @@ -128,9 +128,9 @@ private CauseOfBlockage throttleCheckForCategoriesOnNode(Node node, Jenkins jenk } runCount += buildsOfProjectOnNode(node, catTask); } - Map,List> throttledPipelines = ThrottleJobProperty.getThrottledPipelineRunsForCategory(catNm); - for (Map.Entry,List> entry : throttledPipelines.entrySet()) { - Run r = entry.getKey(); + Map> throttledPipelines = ThrottleJobProperty.getThrottledPipelineRunsForCategory(catNm); + for (Map.Entry> entry : throttledPipelines.entrySet()) { + Run r = Run.fromExternalizableId(entry.getKey()); List flowNodes = entry.getValue(); if (r.isBuilding()) { runCount += pipelinesOnNode(node, r, flowNodes); @@ -264,9 +264,10 @@ private CauseOfBlockage throttleCheckForCategoriesAllNodes(Jenkins jenkins, @Non } totalRunCount += buildsOfProjectOnAllNodes(catTask); } - Map,List> throttledPipelines = ThrottleJobProperty.getThrottledPipelineRunsForCategory(catNm); - for (Map.Entry,List> entry : throttledPipelines.entrySet()) { - Run r = entry.getKey(); + Map> throttledPipelines = ThrottleJobProperty.getThrottledPipelineRunsForCategory(catNm); + for (Map.Entry> entry : throttledPipelines.entrySet()) { + Run r = Run.fromExternalizableId(entry.getKey()); + List flowNodes = entry.getValue(); if (r.isBuilding()) { totalRunCount += pipelinesOnAllNodes(r, flowNodes); diff --git a/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java b/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java index 8b068351..92fcf41f 100644 --- a/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java +++ b/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java @@ -110,6 +110,71 @@ public void evaluate() throws Throwable { }); } + @Test + public void twoTotal() throws Exception { + story.addStep(new Statement() { + @Override + public void evaluate() throws Throwable { + setupAgentsAndCategories(); + WorkflowJob firstJob = story.j.jenkins.createProject(WorkflowJob.class, "first-job"); + // This should be sandbox:true, but when I do that, I get org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException: Scripts not permitted to use method groovy.lang.GroovyObject invokeMethod java.lang.String java.lang.Object + // And I cannot figure out why. So for now... + firstJob.setDefinition(new CpsFlowDefinition("throttle('" + TWO_TOTAL + "') {\n" + + " echo 'hi there'\n" + + " node('first-agent') {\n" + + " semaphore 'wait-first-job'\n" + + " }\n" + + "}\n", false)); + + WorkflowRun firstJobFirstRun = firstJob.scheduleBuild2(0).waitForStart(); + SemaphoreStep.waitForStart("wait-first-job/1", firstJobFirstRun); + + WorkflowJob secondJob = story.j.jenkins.createProject(WorkflowJob.class, "second-job"); + secondJob.setDefinition(new CpsFlowDefinition("throttle('" + TWO_TOTAL + "') {\n" + + " node('second-agent') {\n" + + " semaphore 'wait-second-job'\n" + + " }\n" + + "}\n", false)); + + WorkflowRun secondJobFirstRun = secondJob.scheduleBuild2(0).waitForStart(); + SemaphoreStep.waitForStart("wait-second-job/1", secondJobFirstRun); + + WorkflowJob thirdJob = story.j.jenkins.createProject(WorkflowJob.class, "third-job"); + thirdJob.setDefinition(new CpsFlowDefinition("throttle('" + TWO_TOTAL + "') {\n" + + " node('on-agent') {\n" + + " semaphore 'wait-third-job'\n" + + " }\n" + + "}\n", false)); + + WorkflowRun thirdJobFirstRun = thirdJob.scheduleBuild2(0).waitForStart(); + story.j.waitForMessage("Still waiting to schedule task", thirdJobFirstRun); + assertFalse(story.j.jenkins.getQueue().isEmpty()); + Node n = story.j.jenkins.getNode("first-agent"); + assertNotNull(n); + assertEquals(1, n.toComputer().countBusy()); + hasPlaceholderTaskForRun(n, firstJobFirstRun); + + Node n2 = story.j.jenkins.getNode("second-agent"); + assertNotNull(n2); + assertEquals(1, n2.toComputer().countBusy()); + hasPlaceholderTaskForRun(n2, secondJobFirstRun); + + SemaphoreStep.success("wait-first-job/1", null); + story.j.assertBuildStatusSuccess(story.j.waitForCompletion(firstJobFirstRun)); + SemaphoreStep.waitForStart("wait-third-job/1", thirdJobFirstRun); + assertTrue(story.j.jenkins.getQueue().isEmpty()); + assertEquals(1, n.toComputer().countBusy()); + hasPlaceholderTaskForRun(n, thirdJobFirstRun); + + SemaphoreStep.success("wait-second-job/1", null); + story.j.assertBuildStatusSuccess(story.j.waitForCompletion(secondJobFirstRun)); + + SemaphoreStep.success("wait-third-job/1", null); + story.j.assertBuildStatusSuccess(story.j.waitForCompletion(thirdJobFirstRun)); + } + }); + } + private void hasPlaceholderTaskForRun(Node n, WorkflowRun r) throws Exception { for (Executor exec : n.toComputer().getExecutors()) { if (exec.getCurrentExecutable() != null) { From 89f7bcefaa0f2ca8e2f754a1bf98117db6871c58 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Mon, 13 Mar 2017 14:14:07 -0700 Subject: [PATCH 09/20] Add interop with freestyle test --- .../throttleconcurrents/ThrottleStepTest.java | 143 ++++++++++++++---- 1 file changed, 110 insertions(+), 33 deletions(-) diff --git a/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java b/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java index 92fcf41f..15d8d023 100644 --- a/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java +++ b/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java @@ -1,7 +1,15 @@ package hudson.plugins.throttleconcurrents; +import hudson.Launcher; +import hudson.model.AbstractBuild; +import hudson.model.BuildListener; import hudson.model.Executor; +import hudson.model.FreeStyleBuild; +import hudson.model.FreeStyleProject; +import hudson.model.Label; import hudson.model.Node; +import hudson.model.Queue; +import hudson.model.queue.QueueTaskFuture; import hudson.slaves.DumbSlave; import hudson.slaves.NodeProperty; import hudson.slaves.RetentionStrategy; @@ -10,18 +18,19 @@ import org.jenkinsci.plugins.workflow.job.WorkflowRun; import org.jenkinsci.plugins.workflow.support.steps.ExecutorStepExecution; import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep; -import org.junit.Before; import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; import org.junit.runners.model.Statement; import org.jvnet.hudson.test.BuildWatcher; -import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.RestartableJenkinsRule; +import org.jvnet.hudson.test.TestBuilder; +import java.io.IOException; import java.util.Arrays; import java.util.Collections; +import java.util.concurrent.Semaphore; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -70,24 +79,13 @@ public void onePerNode() throws Exception { public void evaluate() throws Throwable { setupAgentsAndCategories(); WorkflowJob firstJob = story.j.jenkins.createProject(WorkflowJob.class, "first-job"); - // This should be sandbox:true, but when I do that, I get org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException: Scripts not permitted to use method groovy.lang.GroovyObject invokeMethod java.lang.String java.lang.Object - // And I cannot figure out why. So for now... - firstJob.setDefinition(new CpsFlowDefinition("throttle('" + ONE_PER_NODE + "') {\n" + - " echo 'hi there'\n" + - " node('first-agent') {\n" + - " semaphore 'wait-first-job'\n" + - " }\n" + - "}\n", false)); + firstJob.setDefinition(getJobFlow("first", ONE_PER_NODE, "first-agent")); WorkflowRun firstJobFirstRun = firstJob.scheduleBuild2(0).waitForStart(); SemaphoreStep.waitForStart("wait-first-job/1", firstJobFirstRun); WorkflowJob secondJob = story.j.jenkins.createProject(WorkflowJob.class, "second-job"); - secondJob.setDefinition(new CpsFlowDefinition("throttle('" + ONE_PER_NODE + "') {\n" + - " node('first-agent') {\n" + - " semaphore 'wait-second-job'\n" + - " }\n" + - "}\n", false)); + secondJob.setDefinition(getJobFlow("second", ONE_PER_NODE, "first-agent")); WorkflowRun secondJobFirstRun = secondJob.scheduleBuild2(0).waitForStart(); story.j.waitForMessage("Still waiting to schedule task", secondJobFirstRun); @@ -117,34 +115,19 @@ public void twoTotal() throws Exception { public void evaluate() throws Throwable { setupAgentsAndCategories(); WorkflowJob firstJob = story.j.jenkins.createProject(WorkflowJob.class, "first-job"); - // This should be sandbox:true, but when I do that, I get org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException: Scripts not permitted to use method groovy.lang.GroovyObject invokeMethod java.lang.String java.lang.Object - // And I cannot figure out why. So for now... - firstJob.setDefinition(new CpsFlowDefinition("throttle('" + TWO_TOTAL + "') {\n" + - " echo 'hi there'\n" + - " node('first-agent') {\n" + - " semaphore 'wait-first-job'\n" + - " }\n" + - "}\n", false)); + firstJob.setDefinition(getJobFlow("first", TWO_TOTAL, "first-agent")); WorkflowRun firstJobFirstRun = firstJob.scheduleBuild2(0).waitForStart(); SemaphoreStep.waitForStart("wait-first-job/1", firstJobFirstRun); WorkflowJob secondJob = story.j.jenkins.createProject(WorkflowJob.class, "second-job"); - secondJob.setDefinition(new CpsFlowDefinition("throttle('" + TWO_TOTAL + "') {\n" + - " node('second-agent') {\n" + - " semaphore 'wait-second-job'\n" + - " }\n" + - "}\n", false)); + secondJob.setDefinition(getJobFlow("second", TWO_TOTAL, "second-agent")); WorkflowRun secondJobFirstRun = secondJob.scheduleBuild2(0).waitForStart(); SemaphoreStep.waitForStart("wait-second-job/1", secondJobFirstRun); WorkflowJob thirdJob = story.j.jenkins.createProject(WorkflowJob.class, "third-job"); - thirdJob.setDefinition(new CpsFlowDefinition("throttle('" + TWO_TOTAL + "') {\n" + - " node('on-agent') {\n" + - " semaphore 'wait-third-job'\n" + - " }\n" + - "}\n", false)); + thirdJob.setDefinition(getJobFlow("third", TWO_TOTAL, "on-agent")); WorkflowRun thirdJobFirstRun = thirdJob.scheduleBuild2(0).waitForStart(); story.j.waitForMessage("Still waiting to schedule task", thirdJobFirstRun); @@ -175,6 +158,100 @@ public void evaluate() throws Throwable { }); } + @Test + public void interopWithFreestyle() throws Exception { + final Semaphore semaphore = new Semaphore(1); + + story.addStep(new Statement() { + @Override + public void evaluate() throws Throwable { + setupAgentsAndCategories(); + WorkflowJob firstJob = story.j.jenkins.createProject(WorkflowJob.class, "first-job"); + firstJob.setDefinition(getJobFlow("first", ONE_PER_NODE, "first-agent")); + + WorkflowRun firstJobFirstRun = firstJob.scheduleBuild2(0).waitForStart(); + SemaphoreStep.waitForStart("wait-first-job/1", firstJobFirstRun); + + FreeStyleProject freeStyleProject = story.j.createFreeStyleProject("f"); + freeStyleProject.addProperty(new ThrottleJobProperty( + null, // maxConcurrentPerNode + null, // maxConcurrentTotal + Arrays.asList(ONE_PER_NODE), // categories + true, // throttleEnabled + "category", // throttleOption + false, + null, + ThrottleMatrixProjectOptions.DEFAULT + )); + freeStyleProject.setAssignedLabel(Label.get("first-agent")); + freeStyleProject.getBuildersList().add(new TestBuilder() { + @Override + public boolean perform(AbstractBuild build, Launcher launcher, BuildListener listener) throws InterruptedException, IOException { + semaphore.acquire(); + return true; + } + }); + + semaphore.acquire(); + + QueueTaskFuture futureBuild = freeStyleProject.scheduleBuild2(0); + assertFalse(story.j.jenkins.getQueue().isEmpty()); + assertEquals(1, story.j.jenkins.getQueue().getItems().length); + Queue.Item i = story.j.jenkins.getQueue().getItems()[0]; + assertTrue(i.task instanceof FreeStyleProject); + + Node n = story.j.jenkins.getNode("first-agent"); + assertNotNull(n); + assertEquals(1, n.toComputer().countBusy()); + hasPlaceholderTaskForRun(n, firstJobFirstRun); + SemaphoreStep.success("wait-first-job/1", null); + story.j.assertBuildStatusSuccess(story.j.waitForCompletion(firstJobFirstRun)); + + FreeStyleBuild freeStyleBuild = futureBuild.waitForStart(); + assertEquals(1, n.toComputer().countBusy()); + for (Executor e : n.toComputer().getExecutors()) { + if (e.isBusy()) { + assertEquals(freeStyleBuild, e.getCurrentExecutable()); + } + } + + WorkflowJob secondJob = story.j.jenkins.createProject(WorkflowJob.class, "second-job"); + secondJob.setDefinition(getJobFlow("second", ONE_PER_NODE, "first-agent")); + + WorkflowRun secondJobFirstRun = secondJob.scheduleBuild2(0).waitForStart(); + story.j.waitForMessage("Still waiting to schedule task", secondJobFirstRun); + assertFalse(story.j.jenkins.getQueue().isEmpty()); + + assertEquals(1, n.toComputer().countBusy()); + for (Executor e : n.toComputer().getExecutors()) { + if (e.isBusy()) { + assertEquals(freeStyleBuild, e.getCurrentExecutable()); + } + } + semaphore.release(); + + story.j.assertBuildStatusSuccess(story.j.waitForCompletion(freeStyleBuild)); + SemaphoreStep.waitForStart("wait-second-job/1", secondJobFirstRun); + assertTrue(story.j.jenkins.getQueue().isEmpty()); + assertEquals(1, n.toComputer().countBusy()); + hasPlaceholderTaskForRun(n, secondJobFirstRun); + SemaphoreStep.success("wait-second-job/1", null); + story.j.assertBuildStatusSuccess(story.j.waitForCompletion(secondJobFirstRun)); + } + }); + } + + private CpsFlowDefinition getJobFlow(String jobName, String category, String label) { + // This should be sandbox:true, but when I do that, I get org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException: Scripts not permitted to use method groovy.lang.GroovyObject invokeMethod java.lang.String java.lang.Object + // And I cannot figure out why. So for now... + return new CpsFlowDefinition("throttle('" + category + "') {\n" + + " echo 'hi there'\n" + + " node('" + label + "') {\n" + + " semaphore 'wait-" + jobName + "-job'\n" + + " }\n" + + "}\n", false); + } + private void hasPlaceholderTaskForRun(Node n, WorkflowRun r) throws Exception { for (Executor exec : n.toComputer().getExecutors()) { if (exec.getCurrentExecutable() != null) { From 33b8e0fd179e5d17767c71e6ce90e1b3039ba83b Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Mon, 13 Mar 2017 14:23:52 -0700 Subject: [PATCH 10/20] Add snippet generator support. --- .../plugins/throttleconcurrents/ThrottleStep/config.jelly | 7 +++++++ .../throttleconcurrents/ThrottleStep/help-category.html | 3 +++ 2 files changed, 10 insertions(+) create mode 100644 src/main/resources/hudson/plugins/throttleconcurrents/ThrottleStep/config.jelly create mode 100644 src/main/resources/hudson/plugins/throttleconcurrents/ThrottleStep/help-category.html diff --git a/src/main/resources/hudson/plugins/throttleconcurrents/ThrottleStep/config.jelly b/src/main/resources/hudson/plugins/throttleconcurrents/ThrottleStep/config.jelly new file mode 100644 index 00000000..6aa6a2a0 --- /dev/null +++ b/src/main/resources/hudson/plugins/throttleconcurrents/ThrottleStep/config.jelly @@ -0,0 +1,7 @@ + + + + + + diff --git a/src/main/resources/hudson/plugins/throttleconcurrents/ThrottleStep/help-category.html b/src/main/resources/hudson/plugins/throttleconcurrents/ThrottleStep/help-category.html new file mode 100644 index 00000000..a13becca --- /dev/null +++ b/src/main/resources/hudson/plugins/throttleconcurrents/ThrottleStep/help-category.html @@ -0,0 +1,3 @@ +
+

The name of a configured throttle category.

+
\ No newline at end of file From f09aea84f3bff33dc7cb42981a12dca8ccd89bcf Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Mon, 13 Mar 2017 14:30:34 -0700 Subject: [PATCH 11/20] Add snippetizer support and test --- pom.xml | 7 +++++++ .../throttleconcurrents/ThrottleStepTest.java | 14 ++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/pom.xml b/pom.xml index 9ce7abc1..7c7000f1 100644 --- a/pom.xml +++ b/pom.xml @@ -148,6 +148,13 @@ THE SOFTWARE. 2.28 test + + org.jenkins-ci.plugins.workflow + workflow-cps + 2.28 + tests + test + org.jenkins-ci.plugins.workflow workflow-job diff --git a/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java b/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java index 15d8d023..b0422124 100644 --- a/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java +++ b/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java @@ -10,10 +10,12 @@ import hudson.model.Node; import hudson.model.Queue; import hudson.model.queue.QueueTaskFuture; +import hudson.plugins.throttleconcurrents.pipeline.ThrottleStep; import hudson.slaves.DumbSlave; import hudson.slaves.NodeProperty; import hudson.slaves.RetentionStrategy; import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; +import org.jenkinsci.plugins.workflow.cps.SnippetizerTester; import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.jenkinsci.plugins.workflow.job.WorkflowRun; import org.jenkinsci.plugins.workflow.support.steps.ExecutorStepExecution; @@ -252,6 +254,18 @@ private CpsFlowDefinition getJobFlow(String jobName, String category, String lab "}\n", false); } + @Test + public void snippetizer() throws Exception { + story.addStep(new Statement() { + @Override + public void evaluate() throws Throwable { + setupAgentsAndCategories(); + SnippetizerTester st = new SnippetizerTester(story.j); + st.assertRoundTrip(new ThrottleStep(ONE_PER_NODE), "throttle('" + ONE_PER_NODE + "') {\n // some block\n}"); + } + }); + } + private void hasPlaceholderTaskForRun(Node n, WorkflowRun r) throws Exception { for (Executor exec : n.toComputer().getExecutors()) { if (exec.getCurrentExecutable() != null) { From aca9d87abd5f52a2f6d27188a857ce123d93dde6 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Mon, 20 Mar 2017 14:56:04 -0700 Subject: [PATCH 12/20] Review comments --- pom.xml | 5 ++--- .../throttleconcurrents/pipeline/ThrottleStepExecution.java | 2 ++ 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/pom.xml b/pom.xml index 7c7000f1..a0d59635 100644 --- a/pom.xml +++ b/pom.xml @@ -32,7 +32,7 @@ THE SOFTWARE. throttle-concurrents hpi Jenkins Throttle Concurrent Builds Plug-in - 2.0-SNAPSHOT + 2.0-beta-SNAPSHOT http://wiki.jenkins-ci.org/display/JENKINS/Throttle+Concurrent+Builds+Plugin Plugin to throttle the number of concurrent builds of a single job per node. @@ -46,10 +46,9 @@ THE SOFTWARE. 1.642.3 UTF-8 - 1.7 - 1.7 false + 7 diff --git a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java index 1c48cb5f..8596b046 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java @@ -20,6 +20,7 @@ public ThrottleStepExecution(@Nonnull ThrottleStep step, StepContext context) { this.step = step; } + @Nonnull public String getCategory() { return step.getCategory(); } @@ -38,6 +39,7 @@ public boolean start() throws Exception { if (r != null && flowNode != null) { runId = r.getExternalizableId(); flowNodeId = flowNode.getId(); + listener.getLogger().println("Throttling in run " + runId + " for category " + getCategory()); descriptor.addThrottledPipelineForCategory(runId, flowNodeId, getCategory(), listener); } From bb0fb3e79f2f579d112ece43bb00c43c0b04ec16 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Mon, 20 Mar 2017 17:31:42 -0700 Subject: [PATCH 13/20] Check for pending PlaceholderTasks as well. --- .../ThrottleQueueTaskDispatcher.java | 40 ++++++++--- .../throttleconcurrents/ThrottleStepTest.java | 68 ++++++++++++++++++- 2 files changed, 98 insertions(+), 10 deletions(-) diff --git a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java index 6c207ccb..4b81f581 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java @@ -42,7 +42,6 @@ import org.jenkinsci.plugins.workflow.graph.StepNode; import org.jenkinsci.plugins.workflow.graphanalysis.LinearBlockHoppingScanner; import org.jenkinsci.plugins.workflow.steps.StepDescriptor; -import org.jenkinsci.plugins.workflow.support.steps.ExecutorStepExecution; import org.jenkinsci.plugins.workflow.support.steps.ExecutorStepExecution.PlaceholderTask; @Extension @@ -130,6 +129,9 @@ private CauseOfBlockage throttleCheckForCategoriesOnNode(Node node, Jenkins jenk } Map> throttledPipelines = ThrottleJobProperty.getThrottledPipelineRunsForCategory(catNm); for (Map.Entry> entry : throttledPipelines.entrySet()) { + if (hasPendingPipelineForCategory(entry.getValue())) { + return CauseOfBlockage.fromMessage(Messages._ThrottleQueueTaskDispatcher_BuildPending()); + } Run r = Run.fromExternalizableId(entry.getKey()); List flowNodes = entry.getValue(); if (r.isBuilding()) { @@ -148,6 +150,16 @@ private CauseOfBlockage throttleCheckForCategoriesOnNode(Node node, Jenkins jenk return null; } + private boolean hasPendingPipelineForCategory(List flowNodes) { + for (Queue.BuildableItem pending : Jenkins.getActiveInstance().getQueue().getPendingItems()) { + if (isTaskThrottledPipeline(pending.task, flowNodes)) { + return true; + } + } + + return false; + } + // @Override on jenkins 4.127+ , but still compatible with 1.399 public CauseOfBlockage canRun(Queue.Item item) { ThrottleJobProperty tjp = getThrottleJobProperty(item.task); @@ -266,6 +278,9 @@ private CauseOfBlockage throttleCheckForCategoriesAllNodes(Jenkins jenkins, @Non } Map> throttledPipelines = ThrottleJobProperty.getThrottledPipelineRunsForCategory(catNm); for (Map.Entry> entry : throttledPipelines.entrySet()) { + if (hasPendingPipelineForCategory(entry.getValue())) { + return CauseOfBlockage.fromMessage(Messages._ThrottleQueueTaskDispatcher_BuildPending()); + } Run r = Run.fromExternalizableId(entry.getKey()); List flowNodes = entry.getValue(); @@ -503,13 +518,8 @@ private int pipelinesOnExecutor(@Nonnull Run run, @Nonnull Executor exec, @ if (parent instanceof PlaceholderTask) { PlaceholderTask task = (PlaceholderTask)parent; if (run.equals(task.run())) { - try { - FlowNode firstThrottle = firstThrottleStartNode(task.getNode()); - if (firstThrottle != null && flowNodes.contains(firstThrottle)) { - return 1; - } - } catch (IOException | InterruptedException e) { - // TODO: do something? + if (isTaskThrottledPipeline(task, flowNodes)) { + return 1; } } } @@ -518,6 +528,20 @@ private int pipelinesOnExecutor(@Nonnull Run run, @Nonnull Executor exec, @ return 0; } + private boolean isTaskThrottledPipeline(Task origTask, List flowNodes) { + if (origTask instanceof PlaceholderTask) { + PlaceholderTask task = (PlaceholderTask)origTask; + try { + FlowNode firstThrottle = firstThrottleStartNode(task.getNode()); + return firstThrottle != null && flowNodes.contains(firstThrottle); + } catch (IOException | InterruptedException e) { + // TODO: do something? + } + } + + return false; + } + /** * Given a {@link FlowNode}, find the {@link FlowNode} most directly enclosing this one that comes from a {@link ThrottleStep}. * diff --git a/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java b/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java index b0422124..19633d05 100644 --- a/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java +++ b/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java @@ -3,6 +3,7 @@ import hudson.Launcher; import hudson.model.AbstractBuild; import hudson.model.BuildListener; +import hudson.model.Computer; import hudson.model.Executor; import hudson.model.FreeStyleBuild; import hudson.model.FreeStyleProject; @@ -110,6 +111,65 @@ public void evaluate() throws Throwable { }); } + @Test + public void onePerNodeParallel() throws Exception { + story.addStep(new Statement() { + @Override + public void evaluate() throws Throwable { + setupAgentsAndCategories(); + WorkflowJob firstJob = story.j.jenkins.createProject(WorkflowJob.class, "first-job"); + firstJob.setDefinition(new CpsFlowDefinition("parallel(\n" + + " a: { " + getThrottleScript("first-branch-a", ONE_PER_NODE, "on-agent") + " },\n" + + " b: { " + getThrottleScript("first-branch-b", ONE_PER_NODE, "on-agent") + " },\n" + + " c: { " + getThrottleScript("first-branch-c", ONE_PER_NODE, "on-agent") + " }\n" + + ")\n", false)); + + WorkflowRun run1 = firstJob.scheduleBuild2(0).waitForStart(); + SemaphoreStep.waitForStart("wait-first-branch-a-job/1", run1); + SemaphoreStep.waitForStart("wait-first-branch-b-job/1", run1); + + WorkflowJob secondJob = story.j.jenkins.createProject(WorkflowJob.class, "second-job"); + secondJob.setDefinition(new CpsFlowDefinition("parallel(\n" + + " a: { " + getThrottleScript("second-branch-a", ONE_PER_NODE, "on-agent") + " },\n" + + " b: { " + getThrottleScript("second-branch-b", ONE_PER_NODE, "on-agent") + " },\n" + + " c: { " + getThrottleScript("second-branch-c", ONE_PER_NODE, "on-agent") + " }\n" + + ")\n", false)); + + WorkflowRun run2 = secondJob.scheduleBuild2(0).waitForStart(); + + Computer first = story.j.jenkins.getNode("first-agent").toComputer(); + Computer second = story.j.jenkins.getNode("second-agent").toComputer(); + assertEquals(1, first.countBusy()); + assertEquals(1, second.countBusy()); + + story.j.waitForMessage("Still waiting to schedule task", run1); + story.j.waitForMessage("Still waiting to schedule task", run2); + + SemaphoreStep.success("wait-first-branch-a-job/1", null); + SemaphoreStep.waitForStart("wait-first-branch-c-job/1", run1); + assertEquals(1, first.countBusy()); + assertEquals(1, second.countBusy()); + SemaphoreStep.success("wait-first-branch-b-job/1", null); + SemaphoreStep.waitForStart("wait-second-branch-a-job/1", run1); + assertEquals(1, first.countBusy()); + assertEquals(1, second.countBusy()); + SemaphoreStep.success("wait-first-branch-c-job/1", null); + SemaphoreStep.waitForStart("wait-second-branch-b-job/1", run1); + assertEquals(1, first.countBusy()); + assertEquals(1, second.countBusy()); + SemaphoreStep.success("wait-second-branch-a-job/1", null); + SemaphoreStep.waitForStart("wait-second-branch-c-job/1", run1); + assertEquals(1, first.countBusy()); + assertEquals(1, second.countBusy()); + SemaphoreStep.success("wait-second-branch-b-job/1", null); + SemaphoreStep.success("wait-second-branch-c-job/1", null); + + story.j.assertBuildStatusSuccess(story.j.waitForCompletion(run1)); + story.j.assertBuildStatusSuccess(story.j.waitForCompletion(run2)); + } + }); + } + @Test public void twoTotal() throws Exception { story.addStep(new Statement() { @@ -246,12 +306,16 @@ public boolean perform(AbstractBuild build, Launcher launcher, BuildListen private CpsFlowDefinition getJobFlow(String jobName, String category, String label) { // This should be sandbox:true, but when I do that, I get org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException: Scripts not permitted to use method groovy.lang.GroovyObject invokeMethod java.lang.String java.lang.Object // And I cannot figure out why. So for now... - return new CpsFlowDefinition("throttle('" + category + "') {\n" + + return new CpsFlowDefinition(getThrottleScript(jobName, category, label), false); + } + + private String getThrottleScript(String jobName, String category, String label) { + return "throttle('" + category + "') {\n" + " echo 'hi there'\n" + " node('" + label + "') {\n" + " semaphore 'wait-" + jobName + "-job'\n" + " }\n" + - "}\n", false); + "}\n"; } @Test From 13cafdf127c1b71d9d5c79b245a8dad8ae715d1a Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Fri, 7 Apr 2017 09:40:23 -0700 Subject: [PATCH 14/20] Allow multiple comma-separated categories --- .../ThrottleQueueTaskDispatcher.java | 22 ++++--- .../pipeline/ThrottleStep.java | 25 ++++++-- .../pipeline/ThrottleStepExecution.java | 29 +++++---- .../ThrottleStep/config.jelly | 2 +- .../ThrottleStep/help-category.html | 2 +- .../throttleconcurrents/ThrottleStepTest.java | 60 ++++++++++++++++++- 6 files changed, 110 insertions(+), 30 deletions(-) diff --git a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java index 4b81f581..61a54216 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java @@ -133,9 +133,11 @@ private CauseOfBlockage throttleCheckForCategoriesOnNode(Node node, Jenkins jenk return CauseOfBlockage.fromMessage(Messages._ThrottleQueueTaskDispatcher_BuildPending()); } Run r = Run.fromExternalizableId(entry.getKey()); - List flowNodes = entry.getValue(); - if (r.isBuilding()) { - runCount += pipelinesOnNode(node, r, flowNodes); + if (r != null) { + List flowNodes = entry.getValue(); + if (r.isBuilding()) { + runCount += pipelinesOnNode(node, r, flowNodes); + } } } // This would mean that there are as many or more builds currently running than are allowed. @@ -282,10 +284,11 @@ private CauseOfBlockage throttleCheckForCategoriesAllNodes(Jenkins jenkins, @Non return CauseOfBlockage.fromMessage(Messages._ThrottleQueueTaskDispatcher_BuildPending()); } Run r = Run.fromExternalizableId(entry.getKey()); - - List flowNodes = entry.getValue(); - if (r.isBuilding()) { - totalRunCount += pipelinesOnAllNodes(r, flowNodes); + if (r != null) { + List flowNodes = entry.getValue(); + if (r.isBuilding()) { + totalRunCount += pipelinesOnAllNodes(r, flowNodes); + } } } @@ -406,8 +409,9 @@ private List categoriesForPipeline(Task task) { PlaceholderTask placeholderTask = (PlaceholderTask)task; try { FlowNode firstThrottle = firstThrottleStartNode(placeholderTask.getNode()); - if (firstThrottle != null) { - return ThrottleJobProperty.getCategoriesForRunAndFlowNode(placeholderTask.run().getExternalizableId(), + Run r = placeholderTask.run(); + if (firstThrottle != null && r != null) { + return ThrottleJobProperty.getCategoriesForRunAndFlowNode(r.getExternalizableId(), firstThrottle.getId()); } } catch (IOException | InterruptedException e) { diff --git a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java index 8b3d5a18..3db4a4a8 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java @@ -13,20 +13,35 @@ import javax.annotation.Nonnull; import java.io.Serializable; +import java.util.ArrayList; import java.util.Collections; +import java.util.List; import java.util.Set; +import java.util.StringTokenizer; public class ThrottleStep extends Step implements Serializable { - private String category; + private String categories; @DataBoundConstructor - public ThrottleStep(@Nonnull String category) { - this.category = category; + public ThrottleStep(@Nonnull String categories) { + this.categories = categories; } @Nonnull - public String getCategory() { - return category; + public String getCategories() { + return categories; + } + + @Nonnull + public List getCategoriesList() { + List catList = new ArrayList<>(); + + StringTokenizer tokenizer = new StringTokenizer(categories, ","); + while (tokenizer.hasMoreTokens()) { + catList.add(tokenizer.nextToken().trim()); + } + + return catList; } @Override diff --git a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java index 8596b046..4ec28b4c 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java @@ -11,6 +11,8 @@ import javax.annotation.CheckForNull; import javax.annotation.Nonnull; +import java.util.ArrayList; +import java.util.List; public class ThrottleStepExecution extends StepExecution { private final ThrottleStep step; @@ -21,8 +23,8 @@ public ThrottleStepExecution(@Nonnull ThrottleStep step, StepContext context) { } @Nonnull - public String getCategory() { - return step.getCategory(); + public List getCategories() { + return step.getCategoriesList(); } @Override @@ -39,12 +41,13 @@ public boolean start() throws Exception { if (r != null && flowNode != null) { runId = r.getExternalizableId(); flowNodeId = flowNode.getId(); - listener.getLogger().println("Throttling in run " + runId + " for category " + getCategory()); - descriptor.addThrottledPipelineForCategory(runId, flowNodeId, getCategory(), listener); + for (String category : getCategories()) { + descriptor.addThrottledPipelineForCategory(runId, flowNodeId, category, listener); + } } getContext().newBodyInvoker() - .withCallback(new Callback(runId, flowNodeId, getCategory())) + .withCallback(new Callback(runId, flowNodeId, getCategories())) .start(); return false; } @@ -59,23 +62,25 @@ private static final class Callback extends BodyExecutionCallback.TailCall { private String runId; @CheckForNull private String flowNodeId; - private String category; + private List categories = new ArrayList<>(); private static final long serialVersionUID = 1; - Callback(@CheckForNull String runId, @CheckForNull String flowNodeId, @Nonnull String category) { + Callback(@CheckForNull String runId, @CheckForNull String flowNodeId, @Nonnull List categories) { this.runId = runId; this.flowNodeId = flowNodeId; - this.category = category; + this.categories.addAll(categories); } @Override protected void finished(StepContext context) throws Exception { if (runId != null && flowNodeId != null) { - ThrottleJobProperty.fetchDescriptor().removeThrottledPipelineForCategory(runId, - flowNodeId, - category, - context.get(TaskListener.class)); + for (String category : categories) { + ThrottleJobProperty.fetchDescriptor().removeThrottledPipelineForCategory(runId, + flowNodeId, + category, + context.get(TaskListener.class)); + } } } } diff --git a/src/main/resources/hudson/plugins/throttleconcurrents/ThrottleStep/config.jelly b/src/main/resources/hudson/plugins/throttleconcurrents/ThrottleStep/config.jelly index 6aa6a2a0..76c4f263 100644 --- a/src/main/resources/hudson/plugins/throttleconcurrents/ThrottleStep/config.jelly +++ b/src/main/resources/hudson/plugins/throttleconcurrents/ThrottleStep/config.jelly @@ -1,7 +1,7 @@ - + diff --git a/src/main/resources/hudson/plugins/throttleconcurrents/ThrottleStep/help-category.html b/src/main/resources/hudson/plugins/throttleconcurrents/ThrottleStep/help-category.html index a13becca..ee12118c 100644 --- a/src/main/resources/hudson/plugins/throttleconcurrents/ThrottleStep/help-category.html +++ b/src/main/resources/hudson/plugins/throttleconcurrents/ThrottleStep/help-category.html @@ -1,3 +1,3 @@
-

The name of a configured throttle category.

+

One or more comma-separated throttle category names.

\ No newline at end of file diff --git a/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java b/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java index 19633d05..1d582616 100644 --- a/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java +++ b/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java @@ -42,6 +42,7 @@ public class ThrottleStepTest { private static final String ONE_PER_NODE = "one_per_node"; + private static final String OTHER_ONE_PER_NODE = "other_one_per_node"; private static final String TWO_TOTAL = "two_total"; @Rule @@ -69,10 +70,11 @@ public void setupAgentsAndCategories() throws Exception { ThrottleJobProperty.ThrottleCategory firstCat = new ThrottleJobProperty.ThrottleCategory(ONE_PER_NODE, 1, 0, null); ThrottleJobProperty.ThrottleCategory secondCat = new ThrottleJobProperty.ThrottleCategory(TWO_TOTAL, 0, 2, null); + ThrottleJobProperty.ThrottleCategory thirdCat = new ThrottleJobProperty.ThrottleCategory(OTHER_ONE_PER_NODE, 1, 0, null); ThrottleJobProperty.DescriptorImpl descriptor = story.j.jenkins.getDescriptorByType(ThrottleJobProperty.DescriptorImpl.class); assertNotNull(descriptor); - descriptor.setCategories(Arrays.asList(firstCat, secondCat)); + descriptor.setCategories(Arrays.asList(firstCat, secondCat, thirdCat)); } @Test @@ -111,6 +113,59 @@ public void evaluate() throws Throwable { }); } + @Test + public void multipleCategories() throws Exception { + story.addStep(new Statement() { + @Override + public void evaluate() throws Throwable { + setupAgentsAndCategories(); + WorkflowJob firstJob = story.j.jenkins.createProject(WorkflowJob.class, "first-job"); + firstJob.setDefinition(getJobFlow("first", ONE_PER_NODE, "first-agent")); + + WorkflowRun firstJobFirstRun = firstJob.scheduleBuild2(0).waitForStart(); + SemaphoreStep.waitForStart("wait-first-job/1", firstJobFirstRun); + + WorkflowJob secondJob = story.j.jenkins.createProject(WorkflowJob.class, "second-job"); + secondJob.setDefinition(getJobFlow("second", OTHER_ONE_PER_NODE, "second-agent")); + + WorkflowRun secondJobFirstRun = secondJob.scheduleBuild2(0).waitForStart(); + SemaphoreStep.waitForStart("wait-second-job/1", secondJobFirstRun); + + WorkflowJob thirdJob = story.j.jenkins.createProject(WorkflowJob.class, "third-job"); + thirdJob.setDefinition(getJobFlow("third", + ONE_PER_NODE + ", " + OTHER_ONE_PER_NODE, + "on-agent")); + + WorkflowRun thirdJobFirstRun = thirdJob.scheduleBuild2(0).waitForStart(); + story.j.waitForMessage("Still waiting to schedule task", thirdJobFirstRun); + assertFalse(story.j.jenkins.getQueue().isEmpty()); + Node n = story.j.jenkins.getNode("first-agent"); + assertNotNull(n); + assertEquals(1, n.toComputer().countBusy()); + hasPlaceholderTaskForRun(n, firstJobFirstRun); + + Node n2 = story.j.jenkins.getNode("second-agent"); + assertNotNull(n2); + assertEquals(1, n2.toComputer().countBusy()); + hasPlaceholderTaskForRun(n2, secondJobFirstRun); + + SemaphoreStep.success("wait-first-job/1", null); + story.j.assertBuildStatusSuccess(story.j.waitForCompletion(firstJobFirstRun)); + + SemaphoreStep.waitForStart("wait-third-job/1", thirdJobFirstRun); + assertTrue(story.j.jenkins.getQueue().isEmpty()); + assertEquals(1, n.toComputer().countBusy()); + hasPlaceholderTaskForRun(n, thirdJobFirstRun); + + SemaphoreStep.success("wait-second-job/1", null); + story.j.assertBuildStatusSuccess(story.j.waitForCompletion(secondJobFirstRun)); + + SemaphoreStep.success("wait-third-job/1", null); + story.j.assertBuildStatusSuccess(story.j.waitForCompletion(thirdJobFirstRun)); + } + }); + } + @Test public void onePerNodeParallel() throws Exception { story.addStep(new Statement() { @@ -325,7 +380,8 @@ public void snippetizer() throws Exception { public void evaluate() throws Throwable { setupAgentsAndCategories(); SnippetizerTester st = new SnippetizerTester(story.j); - st.assertRoundTrip(new ThrottleStep(ONE_PER_NODE), "throttle('" + ONE_PER_NODE + "') {\n // some block\n}"); + st.assertRoundTrip(new ThrottleStep(ONE_PER_NODE), + "throttle('" + ONE_PER_NODE + "') {\n // some block\n}"); } }); } From fdd910275ebe066ceafe37f30293b633a8164da0 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Tue, 25 Apr 2017 12:14:30 -0400 Subject: [PATCH 15/20] Minor review responses, moving step UI to correct dir --- .../throttleconcurrents/ThrottleQueueTaskDispatcher.java | 2 ++ .../plugins/throttleconcurrents/pipeline/ThrottleStep.java | 2 +- .../plugins/throttleconcurrents/pipeline/Messages.properties | 1 + .../{ => pipeline}/ThrottleStep/config.jelly | 0 .../{ => pipeline}/ThrottleStep/help-category.html | 0 5 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 src/main/resources/hudson/plugins/throttleconcurrents/pipeline/Messages.properties rename src/main/resources/hudson/plugins/throttleconcurrents/{ => pipeline}/ThrottleStep/config.jelly (100%) rename src/main/resources/hudson/plugins/throttleconcurrents/{ => pipeline}/ThrottleStep/help-category.html (100%) diff --git a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java index 61a54216..9eb7671d 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java @@ -415,6 +415,8 @@ private List categoriesForPipeline(Task task) { firstThrottle.getId()); } } catch (IOException | InterruptedException e) { + LOGGER.log(Level.WARNING, "Error getting categories for pipeline {0}: {1}", + new Object[] {task.getDisplayName(), e}); return new ArrayList<>(); } } diff --git a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java index 3db4a4a8..a7b4c94d 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java @@ -60,7 +60,7 @@ public String getFunctionName() { @Override public String getDisplayName() { - return "Throttle execution of node blocks within this body"; + return Messages.ThrottleStep_DisplayName(); } @Override diff --git a/src/main/resources/hudson/plugins/throttleconcurrents/pipeline/Messages.properties b/src/main/resources/hudson/plugins/throttleconcurrents/pipeline/Messages.properties new file mode 100644 index 00000000..f3b21f5b --- /dev/null +++ b/src/main/resources/hudson/plugins/throttleconcurrents/pipeline/Messages.properties @@ -0,0 +1 @@ +ThrottleStep.DisplayName=Throttle execution of node blocks within this body \ No newline at end of file diff --git a/src/main/resources/hudson/plugins/throttleconcurrents/ThrottleStep/config.jelly b/src/main/resources/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep/config.jelly similarity index 100% rename from src/main/resources/hudson/plugins/throttleconcurrents/ThrottleStep/config.jelly rename to src/main/resources/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep/config.jelly diff --git a/src/main/resources/hudson/plugins/throttleconcurrents/ThrottleStep/help-category.html b/src/main/resources/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep/help-category.html similarity index 100% rename from src/main/resources/hudson/plugins/throttleconcurrents/ThrottleStep/help-category.html rename to src/main/resources/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep/help-category.html From 4ec20572ebb548f34f3092f11606ac48b7148513 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Tue, 25 Apr 2017 12:17:06 -0400 Subject: [PATCH 16/20] Go away, empty category names! --- .../plugins/throttleconcurrents/pipeline/ThrottleStep.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java index a7b4c94d..95e96fe2 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java @@ -4,6 +4,7 @@ import hudson.model.TaskListener; import hudson.plugins.throttleconcurrents.ThrottleJobProperty; import hudson.util.FormValidation; +import org.apache.commons.lang.StringUtils; import org.jenkinsci.plugins.workflow.steps.Step; import org.jenkinsci.plugins.workflow.steps.StepContext; import org.jenkinsci.plugins.workflow.steps.StepDescriptor; @@ -38,7 +39,10 @@ public List getCategoriesList() { StringTokenizer tokenizer = new StringTokenizer(categories, ","); while (tokenizer.hasMoreTokens()) { - catList.add(tokenizer.nextToken().trim()); + String nextToken = tokenizer.nextToken().trim(); + if (StringUtils.isNotEmpty(nextToken)) { + catList.add(nextToken); + } } return catList; From 968d77c798cf1a89c065b7e53c4dbcfd7f7a1bee Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Thu, 18 May 2017 10:24:24 -0400 Subject: [PATCH 17/20] Fixed up snippetizer, switched to a list of strings for the throttle step --- .../pipeline/ThrottleStep.java | 33 +++++++------------ .../pipeline/ThrottleStepExecution.java | 3 +- .../pipeline/ThrottleStep/config.jelly | 17 ++++++++-- .../ThrottleStep/help-categories.html | 3 ++ .../pipeline/ThrottleStep/help-category.html | 3 -- .../throttleconcurrents/ThrottleStepTest.java | 24 +++++++++++--- 6 files changed, 50 insertions(+), 33 deletions(-) create mode 100644 src/main/resources/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep/help-categories.html delete mode 100644 src/main/resources/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep/help-category.html diff --git a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java index 95e96fe2..a7b817bb 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep.java @@ -4,7 +4,7 @@ import hudson.model.TaskListener; import hudson.plugins.throttleconcurrents.ThrottleJobProperty; import hudson.util.FormValidation; -import org.apache.commons.lang.StringUtils; +import hudson.util.ListBoxModel; import org.jenkinsci.plugins.workflow.steps.Step; import org.jenkinsci.plugins.workflow.steps.StepContext; import org.jenkinsci.plugins.workflow.steps.StepDescriptor; @@ -14,40 +14,23 @@ import javax.annotation.Nonnull; import java.io.Serializable; -import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Set; -import java.util.StringTokenizer; public class ThrottleStep extends Step implements Serializable { - private String categories; + private List categories; @DataBoundConstructor - public ThrottleStep(@Nonnull String categories) { + public ThrottleStep(@Nonnull List categories) { this.categories = categories; } @Nonnull - public String getCategories() { + public List getCategories() { return categories; } - @Nonnull - public List getCategoriesList() { - List catList = new ArrayList<>(); - - StringTokenizer tokenizer = new StringTokenizer(categories, ","); - while (tokenizer.hasMoreTokens()) { - String nextToken = tokenizer.nextToken().trim(); - if (StringUtils.isNotEmpty(nextToken)) { - catList.add(nextToken); - } - } - - return catList; - } - @Override public StepExecution start(StepContext context) throws Exception { return new ThrottleStepExecution(this, context); @@ -80,6 +63,14 @@ public Set> getRequiredContext() { public FormValidation doCheckCategoryName(@QueryParameter String value) { return ThrottleJobProperty.fetchDescriptor().doCheckCategoryName(value); } + + public List getCategories() { + return ThrottleJobProperty.fetchDescriptor().getCategories(); + } + + public ListBoxModel doFillCategoryItems() { + return ThrottleJobProperty.fetchDescriptor().doFillCategoryItems(); + } } } diff --git a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java index 4ec28b4c..f8c0830f 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java @@ -4,7 +4,6 @@ import hudson.model.TaskListener; import hudson.plugins.throttleconcurrents.ThrottleJobProperty; import org.jenkinsci.plugins.workflow.graph.FlowNode; -import org.jenkinsci.plugins.workflow.steps.BodyExecution; import org.jenkinsci.plugins.workflow.steps.BodyExecutionCallback; import org.jenkinsci.plugins.workflow.steps.StepContext; import org.jenkinsci.plugins.workflow.steps.StepExecution; @@ -24,7 +23,7 @@ public ThrottleStepExecution(@Nonnull ThrottleStep step, StepContext context) { @Nonnull public List getCategories() { - return step.getCategoriesList(); + return step.getCategories(); } @Override diff --git a/src/main/resources/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep/config.jelly b/src/main/resources/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep/config.jelly index 76c4f263..c452ea95 100644 --- a/src/main/resources/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep/config.jelly +++ b/src/main/resources/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep/config.jelly @@ -1,7 +1,18 @@ - - - + + + + + + + + + + + + No categories configured + + diff --git a/src/main/resources/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep/help-categories.html b/src/main/resources/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep/help-categories.html new file mode 100644 index 00000000..c43611be --- /dev/null +++ b/src/main/resources/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep/help-categories.html @@ -0,0 +1,3 @@ +
+

One or more throttle categories in a list.

+
\ No newline at end of file diff --git a/src/main/resources/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep/help-category.html b/src/main/resources/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep/help-category.html deleted file mode 100644 index ee12118c..00000000 --- a/src/main/resources/hudson/plugins/throttleconcurrents/pipeline/ThrottleStep/help-category.html +++ /dev/null @@ -1,3 +0,0 @@ -
-

One or more comma-separated throttle category names.

-
\ No newline at end of file diff --git a/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java b/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java index 1d582616..6f795a49 100644 --- a/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java +++ b/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java @@ -15,6 +15,7 @@ import hudson.slaves.DumbSlave; import hudson.slaves.NodeProperty; import hudson.slaves.RetentionStrategy; +import org.apache.commons.lang.StringUtils; import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; import org.jenkinsci.plugins.workflow.cps.SnippetizerTester; import org.jenkinsci.plugins.workflow.job.WorkflowJob; @@ -31,8 +32,10 @@ import org.jvnet.hudson.test.TestBuilder; import java.io.IOException; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.concurrent.Semaphore; import static org.junit.Assert.assertEquals; @@ -359,13 +362,26 @@ public boolean perform(AbstractBuild build, Launcher launcher, BuildListen } private CpsFlowDefinition getJobFlow(String jobName, String category, String label) { + return getJobFlow(jobName, Collections.singletonList(category), label); + } + + private CpsFlowDefinition getJobFlow(String jobName, List categories, String label) { // This should be sandbox:true, but when I do that, I get org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException: Scripts not permitted to use method groovy.lang.GroovyObject invokeMethod java.lang.String java.lang.Object // And I cannot figure out why. So for now... - return new CpsFlowDefinition(getThrottleScript(jobName, category, label), false); + return new CpsFlowDefinition(getThrottleScript(jobName, categories, label), false); } private String getThrottleScript(String jobName, String category, String label) { - return "throttle('" + category + "') {\n" + + return getThrottleScript(jobName, Collections.singletonList(category), label); + } + + private String getThrottleScript(String jobName, List categories, String label) { + List quoted = new ArrayList<>(); + for (String c : categories) { + quoted.add("'" + c + "'"); + } + + return "throttle([" + StringUtils.join(quoted, ", ") + "]) {\n" + " echo 'hi there'\n" + " node('" + label + "') {\n" + " semaphore 'wait-" + jobName + "-job'\n" + @@ -380,8 +396,8 @@ public void snippetizer() throws Exception { public void evaluate() throws Throwable { setupAgentsAndCategories(); SnippetizerTester st = new SnippetizerTester(story.j); - st.assertRoundTrip(new ThrottleStep(ONE_PER_NODE), - "throttle('" + ONE_PER_NODE + "') {\n // some block\n}"); + st.assertRoundTrip(new ThrottleStep(Collections.singletonList(ONE_PER_NODE)), + "throttle(['" + ONE_PER_NODE + "']) {\n // some block\n}"); } }); } From 0b222d3369759ae9e7cf00527160d199575b5914 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Thu, 18 May 2017 10:49:10 -0400 Subject: [PATCH 18/20] Check for and respond to duplicate or non-existent category names --- .../pipeline/ThrottleStepExecution.java | 37 ++++++++++++++++- .../throttleconcurrents/ThrottleStepTest.java | 40 ++++++++++++++++++- 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java index f8c0830f..658f759c 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java @@ -3,6 +3,7 @@ import hudson.model.Run; import hudson.model.TaskListener; import hudson.plugins.throttleconcurrents.ThrottleJobProperty; +import org.apache.commons.lang.StringUtils; import org.jenkinsci.plugins.workflow.graph.FlowNode; import org.jenkinsci.plugins.workflow.steps.BodyExecutionCallback; import org.jenkinsci.plugins.workflow.steps.StepContext; @@ -11,7 +12,9 @@ import javax.annotation.CheckForNull; import javax.annotation.Nonnull; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Set; public class ThrottleStepExecution extends StepExecution { private final ThrottleStep step; @@ -26,6 +29,38 @@ public List getCategories() { return step.getCategories(); } + private List validateCategories(ThrottleJobProperty.DescriptorImpl descriptor, TaskListener listener) { + List undefinedCategories = new ArrayList<>(); + Set duplicates = new HashSet<>(); + List unique = new ArrayList<>(); + + if (descriptor.getCategories().isEmpty()) { + undefinedCategories.addAll(getCategories()); + } else { + for (String c : getCategories()) { + if (!unique.contains(c)) { + unique.add(c); + } else { + duplicates.add(c); + } + if (descriptor.getCategoryByName(c) == null) { + undefinedCategories.add(c); + } + } + } + + if (!duplicates.isEmpty()) { + listener.getLogger().println("One or more duplicate categories (" + StringUtils.join(duplicates, ", ") + + ") specified. Duplicates will be ignored."); + } + + if (!undefinedCategories.isEmpty()) { + throw new IllegalArgumentException("One or more specified categories do not exist: " + StringUtils.join(undefinedCategories, ", ")); + } + + return unique; + } + @Override public boolean start() throws Exception { Run r = getContext().get(Run.class); @@ -40,7 +75,7 @@ public boolean start() throws Exception { if (r != null && flowNode != null) { runId = r.getExternalizableId(); flowNodeId = flowNode.getId(); - for (String category : getCategories()) { + for (String category : validateCategories(descriptor, listener)) { descriptor.addThrottledPipelineForCategory(runId, flowNodeId, category, listener); } } diff --git a/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java b/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java index 6f795a49..63147d5d 100644 --- a/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java +++ b/src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java @@ -10,6 +10,7 @@ import hudson.model.Label; import hudson.model.Node; import hudson.model.Queue; +import hudson.model.Result; import hudson.model.queue.QueueTaskFuture; import hudson.plugins.throttleconcurrents.pipeline.ThrottleStep; import hudson.slaves.DumbSlave; @@ -116,6 +117,43 @@ public void evaluate() throws Throwable { }); } + @Test + public void duplicateCategories() throws Exception { + story.addStep(new Statement() { + @Override + public void evaluate() throws Throwable { + setupAgentsAndCategories(); + + WorkflowJob j = story.j.jenkins.createProject(WorkflowJob.class, "first-job"); + j.setDefinition(new CpsFlowDefinition("throttle(['" + ONE_PER_NODE + "', '" + ONE_PER_NODE +"']) { echo 'Hello' }", false)); + + WorkflowRun b = j.scheduleBuild2(0).waitForStart(); + + story.j.assertBuildStatusSuccess(story.j.waitForCompletion(b)); + + story.j.assertLogContains("One or more duplicate categories (" + ONE_PER_NODE + ") specified. Duplicates will be ignored.", b); + story.j.assertLogContains("Hello", b); + } + }); + } + + @Test + public void undefinedCategories() throws Exception { + story.addStep(new Statement() { + @Override + public void evaluate() throws Throwable { + WorkflowJob j = story.j.jenkins.createProject(WorkflowJob.class, "first-job"); + j.setDefinition(new CpsFlowDefinition("throttle(['undefined', 'also-undefined']) { echo 'Hello' }", false)); + + WorkflowRun b = j.scheduleBuild2(0).waitForStart(); + + story.j.assertBuildStatus(Result.FAILURE, story.j.waitForCompletion(b)); + story.j.assertLogContains("One or more specified categories do not exist: undefined, also-undefined", b); + story.j.assertLogNotContains("Hello", b); + } + }); + } + @Test public void multipleCategories() throws Exception { story.addStep(new Statement() { @@ -136,7 +174,7 @@ public void evaluate() throws Throwable { WorkflowJob thirdJob = story.j.jenkins.createProject(WorkflowJob.class, "third-job"); thirdJob.setDefinition(getJobFlow("third", - ONE_PER_NODE + ", " + OTHER_ONE_PER_NODE, + Arrays.asList(ONE_PER_NODE, OTHER_ONE_PER_NODE), "on-agent")); WorkflowRun thirdJobFirstRun = thirdJob.scheduleBuild2(0).waitForStart(); From 7d83b6081ef1fddbfa94a0919889e34f2ebad2ba Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Thu, 18 May 2017 13:22:38 -0400 Subject: [PATCH 19/20] Adding help for ThrottleJobProperty pointing out it doesn't work for Pipeline --- .../plugins/throttleconcurrents/ThrottleJobProperty/help.html | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 src/main/resources/hudson/plugins/throttleconcurrents/ThrottleJobProperty/help.html diff --git a/src/main/resources/hudson/plugins/throttleconcurrents/ThrottleJobProperty/help.html b/src/main/resources/hudson/plugins/throttleconcurrents/ThrottleJobProperty/help.html new file mode 100644 index 00000000..25dc588d --- /dev/null +++ b/src/main/resources/hudson/plugins/throttleconcurrents/ThrottleJobProperty/help.html @@ -0,0 +1,4 @@ +
+

Note that the Throttle Concurrent Builds configuration here does not work for Pipeline jobs.

+

For that, use the throttle step.

+
\ No newline at end of file From ee1e95946748600aea15ba71ad6eabc7e4070f34 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Thu, 18 May 2017 13:27:35 -0400 Subject: [PATCH 20/20] unmodifiableList --- .../throttleconcurrents/pipeline/ThrottleStepExecution.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java index 658f759c..45037c3c 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/pipeline/ThrottleStepExecution.java @@ -12,6 +12,7 @@ import javax.annotation.CheckForNull; import javax.annotation.Nonnull; import java.util.ArrayList; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -26,7 +27,7 @@ public ThrottleStepExecution(@Nonnull ThrottleStep step, StepContext context) { @Nonnull public List getCategories() { - return step.getCategories(); + return Collections.unmodifiableList(step.getCategories()); } private List validateCategories(ThrottleJobProperty.DescriptorImpl descriptor, TaskListener listener) {