Skip to content

Add PauseAction when throttling pipelines #134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nickb-carallon wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add PauseAction when throttling pipelines #134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,12 @@ THE SOFTWARE.
<artifactId>matrix-auth</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.pipeline-stage-view</groupId>
<artifactId>pipeline-stage-view</artifactId>
<version>2.13</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-basic-steps</artifactId>
Expand Down
57 changes: 45 additions & 12 deletions src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
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.actions.PauseAction;
import org.jenkinsci.plugins.workflow.support.concurrent.Timeout;
import org.jenkinsci.plugins.workflow.support.steps.ExecutorStepExecution.PlaceholderTask;

Expand All @@ -54,14 +55,17 @@ public class ThrottleQueueTaskDispatcher extends QueueTaskDispatcher {
@Deprecated
@Override
public @CheckForNull CauseOfBlockage canTake(Node node, Task task) {
CauseOfBlockage cause = null;
if (Jenkins.getAuthentication().equals(ACL.SYSTEM)) {
return canTakeImpl(node, task);
}

// Throttle-concurrent-builds requires READ permissions for all projects.
try (ACLContext ctx = ACL.as(ACL.SYSTEM)) {
return canTakeImpl(node, task);
cause = canTakeImpl(node, task);
} else {
// Throttle-concurrent-builds requires READ permissions for all projects.
try (ACLContext ctx = ACL.as(ACL.SYSTEM)) {
cause = canTakeImpl(node, task);
}
}
updatePauseAction(task, cause);
return cause;
}

private CauseOfBlockage canTakeImpl(Node node, Task task) {
Expand Down Expand Up @@ -218,14 +222,17 @@ private boolean shouldBeThrottled(@NonNull Task task, @CheckForNull ThrottleJobP
}

private CauseOfBlockage canRun(Task task, ThrottleJobProperty tjp, List<String> pipelineCategories) {
CauseOfBlockage cause = null;
if (Jenkins.getAuthentication().equals(ACL.SYSTEM)) {
return canRunImpl(task, tjp, pipelineCategories);
}

// Throttle-concurrent-builds requires READ permissions for all projects.
try (ACLContext ctx = ACL.as(ACL.SYSTEM)) {
return canRunImpl(task, tjp, pipelineCategories);
cause = canRunImpl(task, tjp, pipelineCategories);
} else {
// Throttle-concurrent-builds requires READ permissions for all projects.
try (ACLContext ctx = ACL.as(ACL.SYSTEM)) {
cause = canRunImpl(task, tjp, pipelineCategories);
}
}
updatePauseAction(task, cause);
return cause;
}

private CauseOfBlockage canRunImpl(Task task, ThrottleJobProperty tjp, List<String> pipelineCategories) {
Expand Down Expand Up @@ -695,5 +702,31 @@ private int getMaxConcurrentPerNodeBasedOnMatchingLabels(
return maxConcurrentPerNodeLabeledIfMatch;
}

private void updatePauseAction(Task task, CauseOfBlockage cause) {
Copy link
Member

@basil basil Jul 13, 2021

Choose a reason for hiding this comment

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

How does this relate to throttling of Pipeline jobs with a throttle job property rather than a throttle step? Every other usage of new PauseAction that I can find in the jenkinsci organization (e.g. in Lockable Resource, SonarQube, and Pipeline: Input Step) is from a StepExecution. Should this be specific to a throttle step (see ThrottleStep and ThrottleStepExecution)?

Copy link
Contributor Author

@nickb-carallon nickb-carallon Aug 1, 2022

Choose a reason for hiding this comment

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

The pause action only applies to stages within a pipeline, so doesn't apply for pipelines using the throttle job property.

if (task instanceof PlaceholderTask) {
Copy link
Member

@basil basil Jul 13, 2021

Choose a reason for hiding this comment

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

task can also be of type WorkflowJob. Do we need to handle this as well?

Copy link
Contributor Author

@nickb-carallon nickb-carallon Aug 1, 2022

Choose a reason for hiding this comment

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

No, see above.

PlaceholderTask placeholderTask = (PlaceholderTask) task;
try {
FlowNode flowNode = placeholderTask.getNode();
if (flowNode == null) {
return;
}

if (cause != null) {
if (PauseAction.getCurrentPause(flowNode) == null) {
flowNode.addAction(new PauseAction(cause.getShortDescription()));
}
} else {
if (PauseAction.getCurrentPause(flowNode) != null) {
PauseAction.endCurrentPause(flowNode);
}
}
} catch (IOException | InterruptedException e) {
LOGGER.log(Level.WARNING, "Error setting pause action on pipeline {0}: {1}", new Object[] {
task.getDisplayName(), e
});
}
}
}

private static final Logger LOGGER = Logger.getLogger(ThrottleQueueTaskDispatcher.class.getName());
}
8 changes: 8 additions & 0 deletions src/test/java/hudson/plugins/throttleconcurrents/TestUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import hudson.model.Computer;
import hudson.model.Executor;
import hudson.model.Node;
import hudson.model.Queue;
import hudson.model.queue.CauseOfBlockage;
import hudson.slaves.DumbSlave;
import hudson.slaves.RetentionStrategy;
Expand All @@ -17,6 +18,7 @@
import java.util.Set;
import jenkins.model.queue.CompositeCauseOfBlockage;
import org.jenkinsci.plugins.workflow.job.WorkflowRun;
import org.jenkinsci.plugins.workflow.support.actions.PauseAction;
import org.jenkinsci.plugins.workflow.support.steps.ExecutorStepExecution;
import org.junit.rules.TemporaryFolder;
import org.jvnet.hudson.test.JenkinsRule;
Expand Down Expand Up @@ -96,6 +98,12 @@ static Set<String> getBlockageReasons(CauseOfBlockage cob) {
}
}

static void hasPauseActionForItem(Queue.Item item) throws Exception {
assertTrue(item.task instanceof ExecutorStepExecution.PlaceholderTask);
ExecutorStepExecution.PlaceholderTask task = (ExecutorStepExecution.PlaceholderTask) item.task;
assertNotNull(task.getNode().getAction(PauseAction.class));
}

static void hasPlaceholderTaskForRun(Node n, WorkflowRun r) throws Exception {
for (Executor exec : n.toComputer().getExecutors()) {
if (exec.getCurrentExecutable() != null) {
Expand Down
12 changes: 12 additions & 0 deletions src/test/java/hudson/plugins/throttleconcurrents/ThrottleStepTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ public void onePerNode() throws Exception {
blockageReasons,
hasItem(Messages._ThrottleQueueTaskDispatcher_MaxCapacityOnNode(1)
.toString()));
TestUtil.hasPauseActionForItem(queuedItem);
Copy link
Member

@basil basil Jul 13, 2021

Choose a reason for hiding this comment

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

This assertion needs to be added to other tests as well, at least in ThrottleStepTest (if not also the other Pipeline tests).

Copy link
Contributor Author

@nickb-carallon nickb-carallon Aug 1, 2022

Choose a reason for hiding this comment

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

Done

assertEquals(1, agent.toComputer().countBusy());
TestUtil.hasPlaceholderTaskForRun(agent, firstJobFirstRun);

Expand Down Expand Up @@ -176,6 +177,8 @@ public void multipleCategories() throws Exception {
WorkflowRun thirdJobFirstRun = thirdJob.scheduleBuild2(0).waitForStart();
j.waitForMessage("Still waiting to schedule task", thirdJobFirstRun);
assertFalse(j.jenkins.getQueue().isEmpty());
Queue.Item queuedItem = j.jenkins.getQueue().getItems()[0];
TestUtil.hasPauseActionForItem(queuedItem);
assertEquals(1, firstAgent.toComputer().countBusy());
TestUtil.hasPlaceholderTaskForRun(firstAgent, firstJobFirstRun);

Expand Down Expand Up @@ -245,6 +248,9 @@ public void onePerNodeParallel() throws Exception {
j.waitForMessage("Still waiting to schedule task", run1);
j.waitForMessage("Still waiting to schedule task", run2);

Queue.Item queuedItem = j.jenkins.getQueue().getItems()[0];
TestUtil.hasPauseActionForItem(queuedItem);

SemaphoreStep.success("wait-first-branch-a-job/1", null);
SemaphoreStep.waitForStart("wait-first-branch-c-job/1", run1);
assertEquals(1, firstAgent.toComputer().countBusy());
Expand Down Expand Up @@ -293,6 +299,8 @@ public void twoTotal() throws Exception {
j.waitForMessage("Still waiting to schedule task", thirdJobFirstRun);
j.jenkins.getQueue().maintain();
assertFalse(j.jenkins.getQueue().isEmpty());
Queue.Item queuedItem = j.jenkins.getQueue().getItems()[0];
TestUtil.hasPauseActionForItem(queuedItem);
assertEquals(1, firstAgent.toComputer().countBusy());
TestUtil.hasPlaceholderTaskForRun(firstAgent, firstJobFirstRun);

Expand Down Expand Up @@ -377,6 +385,8 @@ public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListen
WorkflowRun secondJobFirstRun = secondJob.scheduleBuild2(0).waitForStart();
j.waitForMessage("Still waiting to schedule task", secondJobFirstRun);
assertFalse(j.jenkins.getQueue().isEmpty());
Queue.Item queuedItem = j.jenkins.getQueue().getItems()[0];
TestUtil.hasPauseActionForItem(queuedItem);

assertEquals(1, agent.toComputer().countBusy());
for (Executor e : agent.toComputer().getExecutors()) {
Expand Down Expand Up @@ -460,6 +470,7 @@ private String getThrottleScript(String jobName, List<String> categories, String
return "throttle(["
+ StringUtils.join(quoted, ", ")
+ "]) {\n"
+ "stage('wait') {\n"
+ " echo 'hi there'\n"
+ " node('"
+ label
Expand All @@ -468,6 +479,7 @@ private String getThrottleScript(String jobName, List<String> categories, String
+ jobName
+ "-job'\n"
+ " }\n"
+ "}\n"
+ "}\n";
}

Expand Down