diff --git a/core/src/main/java/hudson/init/impl/GroovyInitScript.java b/core/src/main/java/hudson/init/impl/GroovyInitScript.java index 1d184cffc6494e79597f543e0d8c4e68229b01ba..20cd1a481c006f82f2f69557f5a7fc1bcf72ccb0 100644 --- a/core/src/main/java/hudson/init/impl/GroovyInitScript.java +++ b/core/src/main/java/hudson/init/impl/GroovyInitScript.java @@ -38,6 +38,6 @@ import static hudson.init.InitMilestone.*; public class GroovyInitScript { @Initializer(after=JOB_LOADED) public static void init(Jenkins j) { - new GroovyHookScript("init").run(); + new GroovyHookScript("init", j.servletContext, j.getRootDir(), j.getPluginManager().uberClassLoader).run(); } } diff --git a/core/src/main/java/hudson/model/Computer.java b/core/src/main/java/hudson/model/Computer.java index e30f5c390acadde493ae9b65a806eb16a06fdc4a..4ce4a3b7a39db9ac05bf9c7b02023a7c29662e5a 100644 --- a/core/src/main/java/hudson/model/Computer.java +++ b/core/src/main/java/hudson/model/Computer.java @@ -2,7 +2,8 @@ * The MIT License * * Copyright (c) 2004-2010, Sun Microsystems, Inc., Kohsuke Kawaguchi, - * Red Hat, Inc., Seiji Sogabe, Stephen Connolly, Thomas J. Black, Tom Huybrechts, CloudBees, Inc. + * Red Hat, Inc., Seiji Sogabe, Stephen Connolly, Thomas J. Black, Tom Huybrechts, + * CloudBees, Inc., Christopher Simons * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -1384,13 +1385,19 @@ public /*transient*/ abstract class Computer extends Actionable implements Acces public void doConfigSubmit( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException, FormException { checkPermission(CONFIGURE); - String name = Util.fixEmptyAndTrim(req.getSubmittedForm().getString("name")); - Jenkins.checkGoodName(name); + String proposedName = Util.fixEmptyAndTrim(req.getSubmittedForm().getString("name")); + Jenkins.checkGoodName(proposedName); Node node = getNode(); if (node == null) { throw new ServletException("No such node " + nodeName); } + + if ((!proposedName.equals(nodeName)) + && Jenkins.getActiveInstance().getNode(proposedName) != null) { + throw new FormException(Messages.ComputerSet_SlaveAlreadyExists(proposedName), "name"); + } + Node result = node.reconfigure(req, req.getSubmittedForm()); replaceBy(result); diff --git a/core/src/main/java/hudson/model/Executor.java b/core/src/main/java/hudson/model/Executor.java index 9310ca6df2f9153f1902ac8eacc03673a1150bd0..817166c60ef6e7e8db54252d58cb5532014d4315 100644 --- a/core/src/main/java/hudson/model/Executor.java +++ b/core/src/main/java/hudson/model/Executor.java @@ -466,7 +466,9 @@ public class Executor extends Thread implements ModelObject { } private void finish2() { - for (RuntimeException e1: owner.getTerminatedBy()) LOGGER.log(Level.WARNING, String.format("%s termination trace", getName()), e1); + for (RuntimeException e1 : owner.getTerminatedBy()) { + LOGGER.log(Level.FINE, String.format("%s termination trace", getName()), e1); + } if (causeOfDeath == null) {// let this thread die and be replaced by a fresh unstarted instance owner.removeExecutor(this); } diff --git a/core/src/main/java/hudson/model/UsageStatistics.java b/core/src/main/java/hudson/model/UsageStatistics.java index 4d2cde54116fe0038e9953dd2a5d50b150509131..b6d2af2496dcb659c99e0283118d1f3262950e50 100644 --- a/core/src/main/java/hudson/model/UsageStatistics.java +++ b/core/src/main/java/hudson/model/UsageStatistics.java @@ -31,6 +31,7 @@ import hudson.node_monitors.ArchitectureMonitor.DescriptorImpl; import hudson.util.IOUtils; import hudson.util.Secret; import static hudson.util.TimeUnit2.DAYS; +import static hudson.init.InitMilestone.COMPLETED; import jenkins.model.Jenkins; import net.sf.json.JSONObject; @@ -95,9 +96,12 @@ public class UsageStatistics extends PageDecorator { * Returns true if it's time for us to check for new version. */ public boolean isDue() { - // user opted out. no data collection. - if(!Jenkins.getInstance().isUsageStatisticsCollected() || DISABLED) return false; - + final Jenkins j = Jenkins.getInstance(); + // user opted out or Jenkins not fully initialized. no data collection. + if (j == null || j.isUsageStatisticsCollected() || DISABLED || COMPLETED.compareTo(j.getInitLevel()) > 0) { + return false; + } + long now = System.currentTimeMillis(); if(now - lastAttempt > DAY) { lastAttempt = now; diff --git a/core/src/main/java/hudson/slaves/NodeProvisioner.java b/core/src/main/java/hudson/slaves/NodeProvisioner.java index 216c8b3ef1cff0ce1fb57c25333d6bfa26b03457..3f5ea3642b8eb8c874d2b8e6fb43f5f9e2306875 100644 --- a/core/src/main/java/hudson/slaves/NodeProvisioner.java +++ b/core/src/main/java/hudson/slaves/NodeProvisioner.java @@ -690,11 +690,10 @@ public class NodeProvisioner { int workloadToProvision = (int) Math.round(Math.floor(excessWorkload + m)); - for (CloudProvisioningListener cl : CloudProvisioningListener.all()) - // consider displaying reasons in a future cloud ux - { + for (CloudProvisioningListener cl : CloudProvisioningListener.all()) { if (cl.canProvision(c, state.getLabel(), workloadToProvision) != null) { - break CLOUD; + // consider displaying reasons in a future cloud ux + continue CLOUD; } } diff --git a/core/src/main/java/hudson/util/BootFailure.java b/core/src/main/java/hudson/util/BootFailure.java index 8ad6bedc29ff2fa369dc7c1597fcab0dcf47f49e..eefb4da81978f361b000dfc72f848cab77d4a089 100644 --- a/core/src/main/java/hudson/util/BootFailure.java +++ b/core/src/main/java/hudson/util/BootFailure.java @@ -40,7 +40,10 @@ public abstract class BootFailure extends ErrorObject { LOGGER.log(Level.SEVERE, "Failed to initialize Jenkins",this); WebApp.get(context).setApp(this); - new GroovyHookScript("boot-failure") + if (home == null) { + return; + } + new GroovyHookScript("boot-failure", context, home, BootFailure.class.getClassLoader()) .bind("exception",this) .bind("home",home) .bind("servletContext", context) diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index 942088b1c23621e3f30a3d81c82fa1f356c64810..b7a8c1d9d520da6fbfb33f3171fe7b6b69464145 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -431,7 +431,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve private transient volatile boolean isQuietingDown; private transient volatile boolean terminating; - private List jdks = new ArrayList(); + private volatile List jdks = new ArrayList(); private transient volatile DependencyGraph dependencyGraph; private final transient AtomicBoolean dependencyGraphDirty = new AtomicBoolean(); @@ -886,6 +886,17 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve executeReactor(null, graphBuilder); } + /** + * Maintains backwards compatibility. Invoked by XStream when this object is de-serialized. + */ + @SuppressWarnings({"unused"}) + private Object readResolve() { + if (jdks == null) { + jdks = new ArrayList<>(); + } + return this; + } + /** * Executes a reactor. * @@ -1663,9 +1674,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve return Messages.Hudson_DisplayName(); } - public synchronized List getJDKs() { - if(jdks==null) - jdks = new ArrayList(); + public List getJDKs() { return jdks; } @@ -1676,7 +1685,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve * set JDK installations from external code. */ @Restricted(NoExternalUse.class) - public synchronized void setJDKs(Collection jdks) { + public void setJDKs(Collection jdks) { this.jdks = new ArrayList(jdks); } diff --git a/core/src/main/java/jenkins/util/groovy/GroovyHookScript.java b/core/src/main/java/jenkins/util/groovy/GroovyHookScript.java index 8a946534fd0858e58bd637b8e89d3853287bb774..f59b00d75dd9fee3277fbce963d2a54f4b5db42c 100644 --- a/core/src/main/java/jenkins/util/groovy/GroovyHookScript.java +++ b/core/src/main/java/jenkins/util/groovy/GroovyHookScript.java @@ -3,8 +3,6 @@ package jenkins.util.groovy; import groovy.lang.Binding; import groovy.lang.GroovyCodeSource; import groovy.lang.GroovyShell; -import jenkins.model.Jenkins; - import java.io.File; import java.io.FileFilter; import java.io.IOException; @@ -12,9 +10,10 @@ import java.net.URL; import java.util.Arrays; import java.util.Set; import java.util.TreeSet; -import java.util.logging.Logger; - import static java.util.logging.Level.WARNING; +import java.util.logging.Logger; +import javax.annotation.Nonnull; +import javax.servlet.ServletContext; /** * A collection of Groovy scripts that are executed as various hooks. @@ -40,9 +39,15 @@ import static java.util.logging.Level.WARNING; public class GroovyHookScript { private final String hook; private final Binding bindings = new Binding(); + private final ServletContext servletContext; + private final File home; + private final ClassLoader loader; - public GroovyHookScript(String hook) { + public GroovyHookScript(String hook, @Nonnull ServletContext servletContext, @Nonnull File home, @Nonnull ClassLoader loader) { this.hook = hook; + this.servletContext = servletContext; + this.home = home; + this.loader = loader; } public GroovyHookScript bind(String name, Object o) { @@ -55,23 +60,22 @@ public class GroovyHookScript { } public void run() { - Jenkins j = Jenkins.getInstance(); final String hookGroovy = hook+".groovy"; final String hookGroovyD = hook+".groovy.d"; try { - URL bundled = j.servletContext.getResource("/WEB-INF/"+ hookGroovy); + URL bundled = servletContext.getResource("/WEB-INF/"+ hookGroovy); execute(bundled); } catch (IOException e) { LOGGER.log(WARNING, "Failed to execute /WEB-INF/"+hookGroovy,e); } - Set resources = j.servletContext.getResourcePaths("/WEB-INF/"+ hookGroovyD +"/"); + Set resources = servletContext.getResourcePaths("/WEB-INF/"+ hookGroovyD +"/"); if (resources!=null) { // sort to execute them in a deterministic order for (String res : new TreeSet(resources)) { try { - URL bundled = j.servletContext.getResource(res); + URL bundled = servletContext.getResource(res); execute(bundled); } catch (IOException e) { LOGGER.log(WARNING, "Failed to execute " + res, e); @@ -79,10 +83,10 @@ public class GroovyHookScript { } } - File script = new File(j.getRootDir(), hookGroovy); + File script = new File(home, hookGroovy); execute(script); - File scriptD = new File(j.getRootDir(), hookGroovyD); + File scriptD = new File(home, hookGroovyD); if (scriptD.isDirectory()) { File[] scripts = scriptD.listFiles(new FileFilter() { public boolean accept(File f) { @@ -129,7 +133,7 @@ public class GroovyHookScript { * Can be used to customize the environment in which the script runs. */ protected GroovyShell createShell() { - return new GroovyShell(Jenkins.getInstance().getPluginManager().uberClassLoader, bindings); + return new GroovyShell(loader, bindings); } private static final Logger LOGGER = Logger.getLogger(GroovyHookScript.class.getName()); diff --git a/test/src/test/groovy/hudson/util/BootFailureTest.groovy b/test/src/test/groovy/hudson/util/BootFailureTest.groovy deleted file mode 100644 index 389ad757e547218b20312f3c8e17c7273802bd28..0000000000000000000000000000000000000000 --- a/test/src/test/groovy/hudson/util/BootFailureTest.groovy +++ /dev/null @@ -1,115 +0,0 @@ -package hudson.util - -import hudson.WebAppMain -import hudson.model.Hudson -import hudson.model.listeners.ItemListener -import jenkins.model.Jenkins -import org.junit.After -import org.junit.Rule -import org.junit.Test -import org.junit.rules.TemporaryFolder -import org.jvnet.hudson.test.HudsonHomeLoader -import org.jvnet.hudson.test.JenkinsRule -import org.jvnet.hudson.test.TestEnvironment -import org.jvnet.hudson.test.TestExtension -import org.kohsuke.stapler.WebApp - -import javax.servlet.ServletContextEvent - -/** - * - * - * @author Kohsuke Kawaguchi - */ -class BootFailureTest { - @Rule - public TemporaryFolder tmpDir = new TemporaryFolder(); - - static boolean makeBootFail = true; - - @Rule - public JenkinsRule j = new JenkinsRule() { - @Override - void before() throws Throwable { - env = new TestEnvironment(testDescription); - env.pin(); - // don't let Jenkins start automatically - } - - @Override - public Hudson newHudson() throws Exception { - def ws = createWebServer() - def wa = new WebAppMain() { - @Override - WebAppMain.FileAndDescription getHomeDir(ServletContextEvent event) { - return new WebAppMain.FileAndDescription(homeLoader.allocate(),"test"); - } - } - wa.contextInitialized(new ServletContextEvent(ws)); - wa.joinInit(); - - def a = WebApp.get(ws).app; - if (a instanceof Jenkins) - return a; - return null; // didn't boot - } - } - - @After - public void tearDown() { - Jenkins.getInstance()?.cleanUp() - } - - public static class SeriousError extends Error {} - - @TestExtension() - public static class InduceBootFailure extends ItemListener { - @Override - void onLoaded() { - if (makeBootFail) - throw new SeriousError(); - } - } - - @Test - void runBootFailureScript() { - final def home = tmpDir.newFolder() - j.with({ -> home} as HudsonHomeLoader) - - // creates a script - new File(home,"boot-failure.groovy").text = "hudson.util.BootFailureTest.problem = exception"; - def d = new File(home, "boot-failure.groovy.d") - d.mkdirs(); - new File(d,"1.groovy").text = "hudson.util.BootFailureTest.runRecord << '1'"; - new File(d,"2.groovy").text = "hudson.util.BootFailureTest.runRecord << '2'"; - - // first failed boot - makeBootFail = true; - assert j.newHudson()==null; - assert bootFailures(home)==1; - - // second failed boot - problem = null; - runRecord = []; - assert j.newHudson()==null; - assert bootFailures(home)==2; - assert runRecord==["1","2"] - - // make sure the script has actually run - assert problem.cause instanceof SeriousError - - // if it boots well, the failure record should be gone - makeBootFail = false; - assert j.newHudson()!=null; - assert !BootFailure.getBootFailureFile(home).exists() - } - - private static int bootFailures(File home) { - return BootFailure.getBootFailureFile(home).readLines().size() - } - - // to be set by the script - public static Exception problem; - public static def runRecord = []; - -} diff --git a/test/src/test/java/hudson/model/ComputerTest.java b/test/src/test/java/hudson/model/ComputerTest.java index b58b00f45e5380a80177ef0eb47d8f782f84d4da..8b6efad4138f23a7b2d4721a5d38730ef84a17e9 100644 --- a/test/src/test/java/hudson/model/ComputerTest.java +++ b/test/src/test/java/hudson/model/ComputerTest.java @@ -1,7 +1,7 @@ /* * The MIT License * - * Copyright (c) 2015 Red Hat, Inc. + * Copyright (c) 2015 Red Hat, Inc.; Christopher Simons * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -23,16 +23,26 @@ */ package hudson.model; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.*; +import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; +import com.gargoylesoftware.htmlunit.html.HtmlForm; +import com.gargoylesoftware.htmlunit.html.HtmlPage; + import java.io.File; import jenkins.model.Jenkins; import hudson.slaves.DumbSlave; +import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.JenkinsRule.WebClient; public class ComputerTest { @@ -52,4 +62,29 @@ public class ComputerTest { assertTrue("Slave log should be kept", keep.toComputer().getLogFile().exists()); } + + /** + * Verify we can't rename a node over an existing node. + */ + @Issue("JENKINS-31321") + @Test + public void testProhibitRenameOverExistingNode() throws Exception { + final String NOTE = "Rename node to name of another node should fail."; + + Node nodeA = j.createSlave("nodeA", null, null); + Node nodeB = j.createSlave("nodeB", null, null); + + WebClient wc = j.createWebClient(); + HtmlForm form = wc.getPage(nodeB, "configure").getFormByName("config"); + form.getInputByName("_.name").setValueAttribute("nodeA"); + + try { + j.submit(form); + fail(NOTE); + } catch (FailingHttpStatusCodeException e) { + assertThat(NOTE, e.getStatusCode(), equalTo(400)); + assertThat(NOTE, e.getResponse().getContentAsString(), + containsString("Slave called ‘nodeA’ already exists")); + } + } } diff --git a/test/src/test/java/hudson/util/BootFailureTest.java b/test/src/test/java/hudson/util/BootFailureTest.java new file mode 100644 index 0000000000000000000000000000000000000000..5ab16ed0c99a066066c467b842aa5326a4a05994 --- /dev/null +++ b/test/src/test/java/hudson/util/BootFailureTest.java @@ -0,0 +1,164 @@ +package hudson.util; + +import hudson.WebAppMain; +import hudson.model.Hudson; +import hudson.model.listeners.ItemListener; +import static hudson.util.BootFailureTest.makeBootFail; +import java.io.File; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import javax.servlet.ServletContext; +import javax.servlet.ServletContextEvent; +import jenkins.model.Jenkins; +import org.apache.commons.io.FileUtils; +import org.junit.After; +import static org.junit.Assert.*; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.jvnet.hudson.test.HudsonHomeLoader; +import org.jvnet.hudson.test.Issue; +import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.TestEnvironment; +import org.jvnet.hudson.test.TestExtension; +import org.kohsuke.stapler.WebApp; + +/** + * + * + * @author Kohsuke Kawaguchi + */ +public class BootFailureTest { + @Rule + public TemporaryFolder tmpDir = new TemporaryFolder(); + + static boolean makeBootFail = true; + static WebAppMain wa; + + static class CustomRule extends JenkinsRule { + @Override + public void before() throws Throwable { + env = new TestEnvironment(testDescription); + env.pin(); + // don't let Jenkins start automatically + } + + @Override + public Hudson newHudson() throws Exception { + ServletContext ws = createWebServer(); + wa = new WebAppMain() { + @Override + public WebAppMain.FileAndDescription getHomeDir(ServletContextEvent event) { + try { + return new WebAppMain.FileAndDescription(homeLoader.allocate(), "test"); + } catch (Exception x) { + throw new AssertionError(x); + } + } + }; + wa.contextInitialized(new ServletContextEvent(ws)); + wa.joinInit(); + + Object a = WebApp.get(ws).getApp(); + if (a instanceof Hudson) { + return (Hudson) a; + } + return null; // didn't boot + } + } + @Rule + public CustomRule j = new CustomRule(); + + @After + public void tearDown() { + Jenkins j = Jenkins.getInstance(); + if (j != null) { + j.cleanUp(); + } + } + + public static class SeriousError extends Error {} + + @TestExtension("runBootFailureScript") + public static class InduceBootFailure extends ItemListener { + @Override + public void onLoaded() { + if (makeBootFail) + throw new SeriousError(); + } + } + + @Test + public void runBootFailureScript() throws Exception { + final File home = tmpDir.newFolder(); + j.with(new HudsonHomeLoader() { + @Override + public File allocate() throws Exception { + return home; + } + }); + + // creates a script + FileUtils.write(new File(home, "boot-failure.groovy"), "hudson.util.BootFailureTest.problem = exception"); + File d = new File(home, "boot-failure.groovy.d"); + d.mkdirs(); + FileUtils.write(new File(d, "1.groovy"), "hudson.util.BootFailureTest.runRecord << '1'"); + FileUtils.write(new File(d, "2.groovy"), "hudson.util.BootFailureTest.runRecord << '2'"); + + // first failed boot + makeBootFail = true; + assertNull(j.newHudson()); + assertEquals(1, bootFailures(home)); + + // second failed boot + problem = null; + runRecord = new ArrayList(); + assertNull(j.newHudson()); + assertEquals(2, bootFailures(home)); + assertEquals(Arrays.asList("1", "2"), runRecord); + + // make sure the script has actually run + assertEquals(SeriousError.class, problem.getCause().getClass()); + + // if it boots well, the failure record should be gone + makeBootFail = false; + assertNotNull(j.newHudson()); + assertFalse(BootFailure.getBootFailureFile(home).exists()); + } + + private static int bootFailures(File home) throws IOException { + return FileUtils.readLines(BootFailure.getBootFailureFile(home)).size(); + } + + @Issue("JENKINS-24696") + @Test + public void interruptedStartup() throws Exception { + final File home = tmpDir.newFolder(); + j.with(new HudsonHomeLoader() { + @Override + public File allocate() throws Exception { + return home; + } + }); + File d = new File(home, "boot-failure.groovy.d"); + d.mkdirs(); + FileUtils.write(new File(d, "1.groovy"), "hudson.util.BootFailureTest.runRecord << '1'"); + j.newHudson(); + assertEquals(Collections.singletonList("1"), runRecord); + } + @TestExtension("interruptedStartup") + public static class PauseBoot extends ItemListener { + @Override + public void onLoaded() { + wa.contextDestroyed(null); + } + } + + // to be set by the script + public static Exception problem; + public static List runRecord = new ArrayList(); + +}