Skip to content
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.

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

[JENKINS-31801] Add Pipeline throttle(category) step #46

Merged
merged 20 commits into from May 19, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -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;
Expand All @@ -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 {
Copy link
Member

Choose a reason for hiding this comment

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

If you just have the one field, probably better to inline that into the execution state.

private String categories;
private List<String> categories;

@DataBoundConstructor
public ThrottleStep(@Nonnull String categories) {
public ThrottleStep(@Nonnull List<String> categories) {
this.categories = categories;
}

@Nonnull
public String getCategories() {
public List<String> getCategories() {
return categories;
}

@Nonnull
public List<String> getCategoriesList() {
List<String> 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);
Expand Down Expand Up @@ -80,6 +63,14 @@ public Set<? extends Class<?>> getRequiredContext() {
public FormValidation doCheckCategoryName(@QueryParameter String value) {
Copy link
Member

Choose a reason for hiding this comment

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

Restricted ? not sure if RequirePost is mandatory

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question. This doesn't actually expose any meaningful information - all you can do is tell whether there's a category with a given name, not anything about the category, etc. I think it's harmless, but defer to others.

return ThrottleJobProperty.fetchDescriptor().doCheckCategoryName(value);
}

public List<ThrottleJobProperty.ThrottleCategory> getCategories() {
return ThrottleJobProperty.fetchDescriptor().getCategories();
Copy link
Member

Choose a reason for hiding this comment

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

Wrap to the unmodifyableList?

}

public ListBoxModel doFillCategoryItems() {
return ThrottleJobProperty.fetchDescriptor().doFillCategoryItems();
}
}

}
Copy link
Member

Choose a reason for hiding this comment

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

No descriptor?

Copy link
Member Author

Choose a reason for hiding this comment

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

whoops.

Expand Up @@ -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;
Expand All @@ -24,7 +23,7 @@ public ThrottleStepExecution(@Nonnull ThrottleStep step, StepContext context) {

@Nonnull
public List<String> getCategories() {
return step.getCategoriesList();
return step.getCategories();
}

@Override
Expand Down
@@ -1,7 +1,18 @@
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define"
xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
<f:entry title="${%Categories}" field="categories">
<f:textbox/>
</f:entry>
<j:choose>
<j:when test="${!empty(descriptor.categories)}">
<f:entry title="${%Categories}">
<j:forEach var="cat" items="${descriptor.categories}">
<f:checkbox name="categories" json="${cat.categoryName}" checked="${instance.categories.contains(cat.categoryName)}" />
<label class="attach-previous">${cat.categoryName}</label>
<st:nbsp/>
</j:forEach>
</f:entry>
</j:when>
<j:otherwise>
No categories configured
</j:otherwise>
</j:choose>
</j:jelly>
@@ -0,0 +1,3 @@
<div>
<p>One or more throttle categories in a list.</p>
</div>

This file was deleted.

Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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<String> 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<String> categories, String label) {
List<String> 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" +
Expand All @@ -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}");
}
});
}
Expand Down