Merge branch 'master' into JENKINS-16942

jenkinsci · Mar 26, 2013 · 00b28d4 · 00b28d4
2 parents 398fa74 + 57900a2
commit 00b28d4
Show file tree

Hide file tree

Showing 33 changed files with 460 additions and 200 deletions.
diff --git a/changelog.html b/changelog.html
@@ -54,6 +54,20 @@
 
 <!-- Record your changes in the trunk here. -->
 <div id="trunk" style="display:none"><!--=TRUNK-BEGIN=-->
+<ul class=image>
+  <li class=bug>
+    Flyweight tasks should execute on the master if there's no static
+    executors available.
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-7291">issue 7291</a>)
+  <li class=rfe>
+    Promote the use of 'H' in cron.
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-17311">issue 17311</a>)
+</ul>
+</div><!--=TRUNK-END=-->
+
+<!-- these changes are controlled by the release process. DO NOT MODIFY -->
+<div id="rc" style="display:none;"><!--=BEGIN=-->
+<h3><a name=v1.509>What's new in 1.509</a> <!--=DATE=--></h3>
 <ul class=image>
   <li class='major bug'>
     Heavy thread congestion saving fingerprints.
@@ -83,13 +97,13 @@
     View name should not allow "..".    
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-16608">issue 16608</a>)
 </ul>
-</div><!--=TRUNK-END=-->
-
-<!-- these changes are controlled by the release process. DO NOT MODIFY -->
-<div id="rc" style="display:none;"><!--=BEGIN=-->
-<h3><a name=v1.508>What's new in 1.508</a> <!--=DATE=--></h3>
-<!--=RC-CHANGES=-->
 </div><!--=END=-->
+<h3><a name=v1.508>What's new in 1.508</a> (2013/03/25)</h3>
+<ul class=image>
+  <li class='major bug'>
+    Fixing a regression in 1.507 that causes a failure to load matrix jobs.
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-17337">issue 17337</a>)
+</ul>
 <h3><a name=v1.507>What's new in 1.507</a> (2013/03/24)</h3>
 <ul class=image>
   <li class=rfe>

diff --git a/cli/pom.xml b/cli/pom.xml
@@ -5,7 +5,7 @@
   <parent>
     <artifactId>pom</artifactId>
     <groupId>org.jenkins-ci.main</groupId>
-    <version>1.508-SNAPSHOT</version>
+    <version>1.510-SNAPSHOT</version>
   </parent>
 
   <artifactId>cli</artifactId>

diff --git a/core/pom.xml b/core/pom.xml
@@ -29,7 +29,7 @@ THE SOFTWARE.
   <parent>
     <groupId>org.jenkins-ci.main</groupId>
     <artifactId>pom</artifactId>
-    <version>1.508-SNAPSHOT</version>
+    <version>1.510-SNAPSHOT</version>
     <relativePath>../pom.xml</relativePath>
   </parent>
 

diff --git a/core/src/main/grammar/crontab.g b/core/src/main/grammar/crontab.g
@@ -93,7 +93,7 @@ term [int field]
 returns [long bits=0]
 throws ANTLRException
 {
-  int d=1,s,e,t;
+  int d=NO_STEP,s,e,t;
 }
   : (token "-")=> s=token "-" e=token ( "/" d=token )?
   {
@@ -108,13 +108,13 @@ throws ANTLRException
   {
     bits = doRange(d,field);
   }
-  | ("H" "(")=> "H" "(" s=token "-" e=token ")"
+  | ("H" "(")=> "H" "(" s=token "-" e=token ")" ( "/" d=token )?
   {
-    bits = doHash(s,e);
+    bits = doHash(s,e,d);
   }
-  | "H"
+  | "H" ( "/" d=token )?
   {
-    bits = doHash(field);
+    bits = doHash(d,field);
   }
   ;
 

diff --git a/core/src/main/java/hudson/model/AbstractCIBase.java b/core/src/main/java/hudson/model/AbstractCIBase.java
@@ -30,6 +30,7 @@
 import hudson.security.AccessControlled;
 import hudson.slaves.ComputerListener;
 import hudson.slaves.RetentionStrategy;
+import jenkins.model.Jenkins;
 import org.kohsuke.stapler.StaplerFallback;
 import org.kohsuke.stapler.StaplerProxy;
 
@@ -116,7 +117,8 @@ private void updateComputer(Node n, Map<String,Computer> byNameMap, Set<Computer
         if (c!=null) {
             c.setNode(n); // reuse
         } else {
-            if(n.getNumExecutors()>0) {
+            // we always need Computer for the master as a fallback in case there's no other Computer.
+            if(n.getNumExecutors()>0 || n==Jenkins.getInstance()) {
                 computers.put(n, c = n.createComputer());
                 if (!n.isHoldOffLaunchUntilSave() && automaticSlaveLaunch) {
                     RetentionStrategy retentionStrategy = c.getRetentionStrategy();

diff --git a/core/src/main/java/hudson/model/Computer.java b/core/src/main/java/hudson/model/Computer.java
@@ -32,6 +32,7 @@
 import hudson.console.AnnotatedLargeText;
 import hudson.init.Initializer;
 import hudson.model.Descriptor.FormException;
+import hudson.model.Queue.FlyweightTask;
 import hudson.model.labels.LabelAtom;
 import hudson.model.queue.WorkUnit;
 import hudson.node_monitors.NodeMonitor;
@@ -111,7 +112,9 @@
  * This object is related to {@link Node} but they have some significant difference.
  * {@link Computer} primarily works as a holder of {@link Executor}s, so
  * if a {@link Node} is configured (probably temporarily) with 0 executors,
- * you won't have a {@link Computer} object for it.
+ * you won't have a {@link Computer} object for it (except for the master node,
+ * which always get its {@link Computer} in case we have no static executors and
+ * we need to run a {@link FlyweightTask} - see JENKINS-7291 for more discussion.)
  *
  * Also, even if you remove a {@link Node}, it takes time for the corresponding
  * {@link Computer} to be removed, if some builds are already in progress on that
@@ -164,7 +167,6 @@
     protected final Object statusChangeLock = new Object();
 
     public Computer(Node node) {
-        assert node.getNumExecutors()!=0 : "Computer created with 0 executors";
         setNode(node);
     }
 
@@ -817,8 +819,10 @@ public final long getDemandStartMilliseconds() {
      *
      * Note that if an executor dies, we'll leave it in {@link #executors} until
      * the administrator yanks it out, so that we can see why it died.
+     *
+     * @since 1.509
      */
-    private boolean isAlive() {
+    protected boolean isAlive() {
         for (Executor e : executors)
             if (e.isAlive())
                 return true;

diff --git a/core/src/main/java/hudson/model/Queue.java b/core/src/main/java/hudson/model/Queue.java
@@ -1058,9 +1058,10 @@ public String hash(Node node) {
                 }
             });
             Jenkins h = Jenkins.getInstance();
-            hash.add(h, h.getNumExecutors()*100);
+            // Even if master is configured with zero executors, we may need to run a flyweight task like MatrixProject on it.
+            hash.add(h, Math.max(h.getNumExecutors()*100, 1));
             for (Node n : h.getNodes())
-                hash.add(n,n.getNumExecutors()*100);
+                hash.add(n, n.getNumExecutors()*100);
 
             Label lbl = p.getAssignedLabel();
             for (Node n : hash.list(p.task.getFullDisplayName())) {
@@ -1457,7 +1458,7 @@ private Object readResolve() {
 
         @Override
         public String toString() {
-            return getClass().getName()+':'+task.toString();
+            return getClass().getName() + ':' + task + ':' + getWhy();
         }
     }
 

diff --git a/core/src/main/java/hudson/scheduler/BaseParser.java b/core/src/main/java/hudson/scheduler/BaseParser.java
@@ -87,17 +87,37 @@ protected long doRange( int step, int field ) throws ANTLRException {
 
     /**
      * Uses {@link Hash} to choose a random (but stable) value from within this field.
+     *
+     * @param step
+     *      Increments. For example, 15 if "H/15". Or {@link #NO_STEP} to indicate
+     *      the special constant for "H" without the step value.
      */
-    protected long doHash( int field ) {
+    protected long doHash(int step, int field) throws ANTLRException {
         int u = UPPER_BOUNDS[field];
         if (field==2) u = 28;   // day of month can vary depending on month, so to make life simpler, just use [1,28] that's always safe
         if (field==4) u = 6;   // Both 0 and 7 of day of week are Sunday. For better distribution, limit upper bound to 6
-        int h = hash.next(u+1 - LOWER_BOUNDS[field]); // upper bound is inclusive
-        return 1L << (h+LOWER_BOUNDS[field]);
+        return doHash(LOWER_BOUNDS[field], u, step);
     }
 
-    protected long doHash( int s, int e ) {
-        return 1L << (s+hash.next(e+1-s));
+    protected long doHash(int s, int e, int step) throws ANTLRException {
+        if (step > e - s + 1) {
+            error(Messages.BaseParser_OutOfRange(step, 1, e - s + 1));
+            throw new AssertionError();
+        } else if (step > 1) {
+            long bits = 0;
+            for (int i = hash.next(step) + s; i <= e; i += step) {
+                bits |= 1L << i;
+            }
+            assert bits != 0;
+            return bits;
+        } else if (step <=0) {
+            error(Messages.BaseParser_MustBePositive(step));
+            throw new AssertionError();
+        } else {
+            assert step==NO_STEP;
+            // step=1 (i.e. omitted) in the case of hash is actually special; means pick one value, not step by 1
+            return 1L << (s+hash.next(e+1-s));
+        }
     }
 
     protected void rangeCheck(int value, int field) throws ANTLRException {
@@ -122,7 +142,11 @@ protected Hash getHashForTokens() {
 
     /**
      * This property hashes tokens in the cron tab tokens like @daily so that they spread evenly.
-     * This is more aggressive optimization that changes the semantics, so not on by default.
      */
-    public static boolean HASH_TOKENS = Boolean.getBoolean(BaseParser.class.getName()+".hash");
+    public static boolean HASH_TOKENS = !"false".equals(System.getProperty(BaseParser.class.getName()+".hash"));
+
+    /**
+     * Constant that indicates no step value.
+     */
+    public static final int NO_STEP = 1;
 }
diff --git a/core/src/main/java/hudson/scheduler/CronTab.java b/core/src/main/java/hudson/scheduler/CronTab.java
@@ -29,8 +29,11 @@
 import java.util.Calendar;
 import java.util.GregorianCalendar;
 import java.util.Locale;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import static java.util.Calendar.*;
+import javax.annotation.CheckForNull;
 
 /**
  * Table for driving scheduled tasks.
@@ -412,21 +415,58 @@ private String toString(String key, long bit) {
      * but semantically suspicious combinations, like
      * "* 0 * * *"
      */
-    public String checkSanity() {
-        for( int i=0; i<5; i++ ) {
+    public @CheckForNull String checkSanity() {
+        OUTER: for (int i = 0; i < 5; i++) {
             long bitMask = (i<4)?bits[i]:(long)dayOfWeek;
             for( int j=BaseParser.LOWER_BOUNDS[i]; j<=BaseParser.UPPER_BOUNDS[i]; j++ ) {
                 if(!checkBits(bitMask,j)) {
                     // this rank has a sparse entry.
                     // if we have a sparse rank, one of them better be the left-most.
                     if(i>0)
-                        return Messages.CronTab_do_you_really_mean_every_minute_when_you(spec, "0 " + spec.substring(spec.indexOf(' ')+1));
+                        return Messages.CronTab_do_you_really_mean_every_minute_when_you(spec, "H " + spec.substring(spec.indexOf(' ') + 1));
                     // once we find a sparse rank, upper ranks don't matter
-                    return null;
+                    break OUTER;
                 }
             }
         }
 
+        String hashified = hashify(spec);
+        if (hashified != null) {
+            return Messages.CronTab_spread_load_evenly_by_using_rather_than_(hashified, spec);
+        }
+
         return null;
     }
+
+    /**
+     * Checks a prospective crontab specification to see if it could benefit from balanced hashes.
+     * @param spec a (legal) spec
+     * @return a similar spec that uses a hash, if such a transformation is necessary; null if it is OK as is
+     * @since 1.509
+     */
+    public static @CheckForNull String hashify(String spec) {
+        if (spec.contains("H")) {
+            // if someone is already using H, presumably he knows what it is, so a warning is likely false positive
+            return null;
+        } else if (spec.startsWith("*/")) {// "*/15 ...." (every N minutes) to hash
+            return "H" + spec.substring(1);
+        } else if (spec.matches("\\d+ .+")) {// "0 ..." (certain minute) to hash
+            return "H " + spec.substring(spec.indexOf(' ') + 1);
+        } else {
+            Matcher m = Pattern.compile("0(,(\\d+)(,\\d+)*)( .+)").matcher(spec);
+            if (m.matches()) { // 0,15,30,45 to H/15
+                int period = Integer.parseInt(m.group(2));
+                if (period > 0) {
+                    StringBuilder b = new StringBuilder();
+                    for (int i = period; i < 60; i += period) {
+                        b.append(',').append(i);
+                    }
+                    if (b.toString().equals(m.group(1))) {
+                        return "H/" + period + m.group(4);
+                    }
+                }
+            }
+            return null;
+        }
+    }
 }
diff --git a/core/src/main/java/hudson/security/ACL.java b/core/src/main/java/hudson/security/ACL.java
@@ -23,6 +23,7 @@
  */
 package hudson.security;
 
+import jenkins.security.NonSerializableSecurityContext;
 import jenkins.model.Jenkins;
 import org.acegisecurity.AccessDeniedException;
 import org.acegisecurity.Authentication;
@@ -31,7 +32,6 @@
 import org.acegisecurity.providers.UsernamePasswordAuthenticationToken;
 import org.acegisecurity.acls.sid.PrincipalSid;
 import org.acegisecurity.acls.sid.Sid;
-import hudson.model.Executor;
 
 /**
  * Gate-keeper that controls access to Hudson's model objects.
@@ -116,7 +116,8 @@ public String toString() {
      * 
      * <p>
      * When the impersonation is over, be sure to restore the previous authentication
-     * via {@code SecurityContextHolder.setContext(returnValueFromThisMethod)}.
+     * via {@code SecurityContextHolder.setContext(returnValueFromThisMethod)};
+     * or just use {@link #impersonate(Authentication,Runnable)}.
      * 
      * <p>
      * We need to create a new {@link SecurityContext} instead of {@link SecurityContext#setAuthentication(Authentication)}
@@ -125,7 +126,23 @@ public String toString() {
      */
     public static SecurityContext impersonate(Authentication auth) {
         SecurityContext old = SecurityContextHolder.getContext();
-        SecurityContextHolder.setContext(new NotSerilizableSecurityContext(auth));
+        SecurityContextHolder.setContext(new NonSerializableSecurityContext(auth));
         return old;
     }
+
+    /**
+     * Safer variant of {@link #impersonate(Authentication)} that does not require a finally-block.
+     * @param auth authentication, such as {@link #SYSTEM}
+     * @param body an action to run with this alternate authentication in effect
+     * @since 1.509
+     */
+    public static void impersonate(Authentication auth, Runnable body) {
+        SecurityContext old = impersonate(auth);
+        try {
+            body.run();
+        } finally {
+            SecurityContextHolder.setContext(old);
+        }
+    }
+
 }
diff --git a/core/src/main/java/hudson/security/HttpSessionContextIntegrationFilter2.java b/core/src/main/java/hudson/security/HttpSessionContextIntegrationFilter2.java
@@ -23,6 +23,7 @@
  */
 package hudson.security;
 
+import jenkins.security.NonSerializableSecurityContext;
 import org.acegisecurity.context.HttpSessionContextIntegrationFilter;
 import org.acegisecurity.context.SecurityContext;
 import org.acegisecurity.Authentication;
@@ -43,7 +44,7 @@
  */
 public class HttpSessionContextIntegrationFilter2 extends HttpSessionContextIntegrationFilter {
     public HttpSessionContextIntegrationFilter2() throws ServletException {
-        setContext(NotSerilizableSecurityContext.class);
+        setContext(NonSerializableSecurityContext.class);
     }
 
     public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain) throws IOException, ServletException {

diff --git a/core/src/main/java/hudson/security/InvalidatableUserDetails.java b/core/src/main/java/hudson/security/InvalidatableUserDetails.java
@@ -28,6 +28,7 @@
 import org.acegisecurity.userdetails.UserDetails;
 
 import javax.servlet.http.HttpSession;
+import jenkins.security.NonSerializableSecurityContext;
 
 /**
  * {@link UserDetails} that can mark {@link Authentication} invalid.
@@ -51,7 +52,7 @@
  * @author Kohsuke Kawaguchi
  * @deprecated
  *      Starting 1.285, Hudson stops persisting {@link Authentication} altogether
- *      (see {@link NotSerilizableSecurityContext}), so there's no need to use this mechanism.
+ *      (see {@link NonSerializableSecurityContext}), so there's no need to use this mechanism.
  */
 public interface InvalidatableUserDetails extends UserDetails {
     boolean isInvalid();