Skip to content
Permalink
Browse files
Merge pull request #722 from ndeloof/master
[FIXED JENKINS-13502] Fix dependency graph computation when upstream build trigger is involved
  • Loading branch information
jglick committed Mar 1, 2013
2 parents fa50b3d + 7ae055c commit ef9c30c665c4a5c59e1a1af54072b95831eed831
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 55 deletions.
@@ -73,6 +73,9 @@
<li class=bug>
Revert ampersand encoding which can cause backwad incompatibility issue
(<a href="https://github.com/jenkinsci/jenkins/pull/683">pull 683</a>)
<li class=bug>
Fix dependency graph computation when upstream build trigger is involved
(<a href="https://issues.jenkins-ci.org/browse/JENKINS-13502">issue 13502</a>)
</ul>
</div><!--=TRUNK-END=-->

@@ -63,6 +63,7 @@
import hudson.scm.SCMRevisionState;
import hudson.scm.SCMS;
import hudson.search.SearchIndexBuilder;
import hudson.security.ACL;
import hudson.security.Permission;
import hudson.slaves.WorkspaceList;
import hudson.tasks.BuildStep;
@@ -88,6 +89,8 @@
import jenkins.scm.SCMCheckoutStrategyDescriptor;
import jenkins.util.TimeDuration;
import net.sf.json.JSONObject;
import org.acegisecurity.context.SecurityContext;
import org.acegisecurity.context.SecurityContextHolder;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.args4j.Argument;
@@ -747,60 +750,8 @@ public void doConfigSubmit( StaplerRequest req, StaplerResponse rsp ) throws IOE

// dependency setting might have been changed by the user, so rebuild.
Jenkins.getInstance().rebuildDependencyGraph();
convertUpstreamBuildTrigger(upstream);

// reflect the submission of the pseudo 'upstream build trriger'.
// this needs to be done after we release the lock on 'this',
// or otherwise we could dead-lock

for (AbstractProject<?,?> p : Jenkins.getInstance().getAllItems(AbstractProject.class)) {
// Don't consider child projects such as MatrixConfiguration:
if (!p.isConfigurable()) continue;
boolean isUpstream = upstream.contains(p);
synchronized(p) {
// does 'p' include us in its BuildTrigger?
DescribableList<Publisher,Descriptor<Publisher>> pl = p.getPublishersList();
BuildTrigger trigger = pl.get(BuildTrigger.class);
List<AbstractProject> newChildProjects = trigger == null ? new ArrayList<AbstractProject>():trigger.getChildProjects(p);
if(isUpstream) {
if(!newChildProjects.contains(this))
newChildProjects.add(this);
} else {
newChildProjects.remove(this);
}

if(newChildProjects.isEmpty()) {
pl.remove(BuildTrigger.class);
} else {
// here, we just need to replace the old one with the new one,
// but there was a regression (we don't know when it started) that put multiple BuildTriggers
// into the list.
// for us not to lose the data, we need to merge them all.
List<BuildTrigger> existingList = pl.getAll(BuildTrigger.class);
BuildTrigger existing;
switch (existingList.size()) {
case 0:
existing = null;
break;
case 1:
existing = existingList.get(0);
break;
default:
pl.removeAll(BuildTrigger.class);
Set<AbstractProject> combinedChildren = new HashSet<AbstractProject>();
for (BuildTrigger bt : existingList)
combinedChildren.addAll(bt.getChildProjects(p));
existing = new BuildTrigger(new ArrayList<AbstractProject>(combinedChildren),existingList.get(0).getThreshold());
pl.add(existing);
break;
}

if(existing!=null && existing.hasSame(p,newChildProjects))
continue; // no need to touch
pl.replace(new BuildTrigger(newChildProjects,
existing==null?Result.SUCCESS:existing.getThreshold()));
}
}
}

// notify the queue as the project might be now tied to different node
Jenkins.getInstance().getQueue().scheduleMaintenance();
@@ -809,7 +760,67 @@ public void doConfigSubmit( StaplerRequest req, StaplerResponse rsp ) throws IOE
Jenkins.getInstance().rebuildDependencyGraph();
}

/**
/**
* Reflect the submission of the pseudo 'upstream build trigger'.
*/
/* package */ void convertUpstreamBuildTrigger(Set<AbstractProject> upstream) throws IOException {

SecurityContext saveCtx = ACL.impersonate(ACL.SYSTEM);
try {
for (AbstractProject<?,?> p : Jenkins.getInstance().getAllItems(AbstractProject.class)) {
// Don't consider child projects such as MatrixConfiguration:
if (!p.isConfigurable()) continue;
boolean isUpstream = upstream.contains(p);
synchronized(p) {
// does 'p' include us in its BuildTrigger?
DescribableList<Publisher,Descriptor<Publisher>> pl = p.getPublishersList();
BuildTrigger trigger = pl.get(BuildTrigger.class);
List<AbstractProject> newChildProjects = trigger == null ? new ArrayList<AbstractProject>():trigger.getChildProjects(p);
if(isUpstream) {
if(!newChildProjects.contains(this))
newChildProjects.add(this);
} else {
newChildProjects.remove(this);
}

if(newChildProjects.isEmpty()) {
pl.remove(BuildTrigger.class);
} else {
// here, we just need to replace the old one with the new one,
// but there was a regression (we don't know when it started) that put multiple BuildTriggers
// into the list. For us not to lose the data, we need to merge them all.
List<BuildTrigger> existingList = pl.getAll(BuildTrigger.class);
BuildTrigger existing;
switch (existingList.size()) {
case 0:
existing = null;
break;
case 1:
existing = existingList.get(0);
break;
default:
pl.removeAll(BuildTrigger.class);
Set<AbstractProject> combinedChildren = new HashSet<AbstractProject>();
for (BuildTrigger bt : existingList)
combinedChildren.addAll(bt.getChildProjects(p));
existing = new BuildTrigger(new ArrayList<AbstractProject>(combinedChildren),existingList.get(0).getThreshold());
pl.add(existing);
break;
}

if(existing!=null && existing.hasSame(p,newChildProjects))
continue; // no need to touch
pl.replace(new BuildTrigger(newChildProjects,
existing==null? Result.SUCCESS:existing.getThreshold()));
}
}
}
} finally {
SecurityContextHolder.setContext(saveCtx);
}
}

/**
* @deprecated
* Use {@link #scheduleBuild(Cause)}. Since 1.283
*/
@@ -27,7 +27,8 @@
import com.gargoylesoftware.htmlunit.html.HtmlForm;
import com.gargoylesoftware.htmlunit.html.HtmlInput;
import com.gargoylesoftware.htmlunit.html.HtmlPage;
import hudson.security.GlobalMatrixAuthorizationStrategy;
import hudson.security.*;
import hudson.tasks.BuildTrigger;
import hudson.tasks.Shell;
import hudson.scm.NullSCM;
import hudson.Launcher;
@@ -38,13 +39,21 @@
import hudson.util.StreamTaskListener;
import hudson.util.OneShotEvent;
import java.io.IOException;

import jenkins.model.Jenkins;
import org.acegisecurity.context.SecurityContext;
import org.acegisecurity.context.SecurityContextHolder;
import org.jvnet.hudson.test.HudsonTestCase;
import org.jvnet.hudson.test.Bug;
import org.jvnet.hudson.test.MemoryAssert;
import org.jvnet.hudson.test.recipes.PresetData;
import org.jvnet.hudson.test.recipes.PresetData.DataSet;

import java.io.File;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.Future;
import org.apache.commons.io.FileUtils;
import java.lang.ref.WeakReference;
@@ -285,4 +294,39 @@ public void testGetBuildAfterGC() throws Exception {
MemoryAssert.assertGC(new WeakReference(job.getLastBuild()));
assertTrue(job.getLastBuild() != null);
}

@Bug(13502)
public void testHandleBuildTrigger() throws Exception {
Project u = createFreeStyleProject("u"),
d = createFreeStyleProject("d"),
e = createFreeStyleProject("e");

u.addPublisher(new BuildTrigger("d", Result.SUCCESS));

jenkins.setSecurityRealm(createDummySecurityRealm());
ProjectMatrixAuthorizationStrategy authorizations = new ProjectMatrixAuthorizationStrategy();
jenkins.setAuthorizationStrategy(authorizations);

authorizations.add(Jenkins.ADMINISTER, "admin");
authorizations.add(Jenkins.READ, "user");

// user can READ u and CONFIGURE e
Map<Permission, Set<String>> permissions = new HashMap<Permission, Set<String>>();
permissions.put(Job.READ, Collections.singleton("user"));
u.addProperty(new AuthorizationMatrixProperty(permissions));

permissions = new HashMap<Permission, Set<String>>();
permissions.put(Job.CONFIGURE, Collections.singleton("user"));
e.addProperty(new AuthorizationMatrixProperty(permissions));

User user = User.get("user");
SecurityContext sc = ACL.impersonate(user.impersonate());
try {
e.convertUpstreamBuildTrigger(Collections.<AbstractProject>emptySet());
} finally {
SecurityContextHolder.setContext(sc);
}

assertEquals(1, u.getPublishersList().size());
}
}

0 comments on commit ef9c30c

Please sign in to comment.