From 6bf700827a12bb1348c95944cfc52d47a74371d2 Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Tue, 12 Dec 2017 10:38:20 -0500 Subject: [PATCH 1/5] Do not use InstallState in PluginManager when loading detached plugins --- core/src/main/java/hudson/PluginManager.java | 5 ++--- core/src/main/java/jenkins/install/InstallUtil.java | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/hudson/PluginManager.java b/core/src/main/java/hudson/PluginManager.java index c816805ba6f6..7fb2ed87124a 100644 --- a/core/src/main/java/hudson/PluginManager.java +++ b/core/src/main/java/hudson/PluginManager.java @@ -679,9 +679,8 @@ protected static void addDependencies(URL hpiResUrl, String fromPath, Set d * */ protected void loadDetachedPlugins() { - InstallState installState = Jenkins.getActiveInstance().getInstallState(); - if (InstallState.UPGRADE.equals(installState)) { - VersionNumber lastExecVersion = new VersionNumber(InstallUtil.getLastExecVersion()); + VersionNumber lastExecVersion = new VersionNumber(InstallUtil.getLastExecVersion()); + if (lastExecVersion.isNewerThan(InstallUtil.NEW_INSTALL_VERSION) && lastExecVersion.isOlderThan(Jenkins.getVersion())) { LOGGER.log(INFO, "Upgrading Jenkins. The last running version was {0}. This Jenkins is version {1}.", new Object[] {lastExecVersion, Jenkins.VERSION}); diff --git a/core/src/main/java/jenkins/install/InstallUtil.java b/core/src/main/java/jenkins/install/InstallUtil.java index e4868e95c157..d89cac2df541 100644 --- a/core/src/main/java/jenkins/install/InstallUtil.java +++ b/core/src/main/java/jenkins/install/InstallUtil.java @@ -70,7 +70,7 @@ public class InstallUtil { private static final Logger LOGGER = Logger.getLogger(InstallUtil.class.getName()); // tests need this to be 1.0 - private static final VersionNumber NEW_INSTALL_VERSION = new VersionNumber("1.0"); + public static final VersionNumber NEW_INSTALL_VERSION = new VersionNumber("1.0"); private static final VersionNumber FORCE_NEW_INSTALL_VERSION = new VersionNumber("0.0"); /** From 0518a290596fdb9fca5b63e5782a6d38c6182d5d Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Tue, 12 Dec 2017 10:44:56 -0500 Subject: [PATCH 2/5] Add test for installing detached plugins after upgrading Jenkins --- .../test/java/hudson/PluginManagerUtil.java | 11 +++ .../install/LoadDetachedPluginsTest.java | 86 +++++++++++++++++++ 2 files changed, 97 insertions(+) create mode 100644 test/src/test/java/jenkins/install/LoadDetachedPluginsTest.java diff --git a/test/src/test/java/hudson/PluginManagerUtil.java b/test/src/test/java/hudson/PluginManagerUtil.java index 2263622b4c44..423213b7d6c4 100644 --- a/test/src/test/java/hudson/PluginManagerUtil.java +++ b/test/src/test/java/hudson/PluginManagerUtil.java @@ -26,6 +26,8 @@ import jenkins.RestartRequiredException; import jenkins.model.Jenkins; import org.apache.commons.io.FileUtils; +import org.junit.runner.Description; +import org.jvnet.hudson.test.RestartableJenkinsRule; import org.jvnet.hudson.test.JenkinsRule; import java.io.File; @@ -47,6 +49,15 @@ public void before() throws Throwable { }; } + public static RestartableJenkinsRule newRestartableJenkinsRule() { + return new RestartableJenkinsRule() { + @Override + public JenkinsRule createJenkinsRule(Description description) { + return newJenkinsRule(); + } + }; + } + public static void dynamicLoad(String plugin, Jenkins jenkins) throws IOException, InterruptedException, RestartRequiredException { dynamicLoad(plugin, jenkins, false); } diff --git a/test/src/test/java/jenkins/install/LoadDetachedPluginsTest.java b/test/src/test/java/jenkins/install/LoadDetachedPluginsTest.java new file mode 100644 index 000000000000..74caf55781de --- /dev/null +++ b/test/src/test/java/jenkins/install/LoadDetachedPluginsTest.java @@ -0,0 +1,86 @@ +/* + * The MIT License + * + * Copyright 2017 CloudBees, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package jenkins.install; + +import hudson.ClassicPluginStrategy; +import hudson.ClassicPluginStrategy.DetachedPlugin; +import hudson.PluginManager; +import hudson.PluginManagerUtil; +import hudson.PluginWrapper; +import hudson.util.VersionNumber; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +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.RestartableJenkinsRule; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThan; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; + +public class LoadDetachedPluginsTest { + + @Rule public RestartableJenkinsRule rr = PluginManagerUtil.newRestartableJenkinsRule(); + + @Issue("JENKINS-48365") + @Test + public void detachedPluginsInstalledAfterUpgrade() throws IOException { + VersionNumber since = new VersionNumber("1.1"); + rr.then(r -> { + List detachedPlugins = ClassicPluginStrategy.getDetachedPlugins(since); + assertFalse("Many plugins have been detached since the given version", detachedPlugins.isEmpty()); + assertTrue("Detached plugins should not be installed on a new instance", + getInstalledDetachedPlugins(r, detachedPlugins).isEmpty()); + // Trick PluginManager into thinking an upgrade happened when Jenkins restarts. + InstallUtil.saveLastExecVersion(since.toString()); + }); + rr.then(r -> { + List detachedPlugins = ClassicPluginStrategy.getDetachedPlugins(since); + assertThat("Plugins have been detached since the pre-upgrade version", + detachedPlugins.size(), greaterThan(15)); + assertThat("Plugins detached between the pre-upgrade version and the current version should be installed", + getInstalledDetachedPlugins(r, detachedPlugins).size(), equalTo(detachedPlugins.size())); + }); + } + + private List getInstalledDetachedPlugins(JenkinsRule r, List detachedPlugins) { + PluginManager pluginManager = r.jenkins.getPluginManager(); + List installedPlugins = new ArrayList<>(); + for (DetachedPlugin plugin : detachedPlugins) { + PluginWrapper wrapper = pluginManager.getPlugin(plugin.getShortName()); + if (wrapper != null) { + installedPlugins.add(wrapper); + assertTrue(wrapper.isActive()); + } + } + return installedPlugins; + } + +} From efd8b7153b8b7b29b8af6b73236e61d041b251cc Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Tue, 12 Dec 2017 17:16:30 -0500 Subject: [PATCH 3/5] Use LocalData to test upgrades from before and after 2.0 --- .../install/LoadDetachedPluginsTest.java | 43 +++++++++++++++---- .../upgradeFromJenkins1/config.xml | 34 +++++++++++++++ ...enkins.install.InstallUtil.lastExecVersion | 1 + 3 files changed, 69 insertions(+), 9 deletions(-) create mode 100644 test/src/test/resources/jenkins/install/LoadDetachedPluginsTest/upgradeFromJenkins1/config.xml create mode 100644 test/src/test/resources/jenkins/install/LoadDetachedPluginsTest/upgradeFromJenkins2/jenkins.install.InstallUtil.lastExecVersion diff --git a/test/src/test/java/jenkins/install/LoadDetachedPluginsTest.java b/test/src/test/java/jenkins/install/LoadDetachedPluginsTest.java index 74caf55781de..f70b4d3a9394 100644 --- a/test/src/test/java/jenkins/install/LoadDetachedPluginsTest.java +++ b/test/src/test/java/jenkins/install/LoadDetachedPluginsTest.java @@ -38,10 +38,12 @@ import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.RestartableJenkinsRule; +import org.jvnet.hudson.test.recipes.LocalData; +import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; -import static org.junit.Assert.assertFalse; +import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; @@ -51,25 +53,48 @@ public class LoadDetachedPluginsTest { @Issue("JENKINS-48365") @Test - public void detachedPluginsInstalledAfterUpgrade() throws IOException { - VersionNumber since = new VersionNumber("1.1"); + @LocalData + public void upgradeFromJenkins1() throws IOException { + VersionNumber since = new VersionNumber("1.550"); rr.then(r -> { List detachedPlugins = ClassicPluginStrategy.getDetachedPlugins(since); - assertFalse("Many plugins have been detached since the given version", detachedPlugins.isEmpty()); - assertTrue("Detached plugins should not be installed on a new instance", - getInstalledDetachedPlugins(r, detachedPlugins).isEmpty()); - // Trick PluginManager into thinking an upgrade happened when Jenkins restarts. - InstallUtil.saveLastExecVersion(since.toString()); + assertThat("Plugins have been detached since the pre-upgrade version", + detachedPlugins.size(), greaterThan(4)); + assertThat("Plugins detached between the pre-upgrade version and the current version should be installed", + getInstalledDetachedPlugins(r, detachedPlugins).size(), equalTo(detachedPlugins.size())); }); + } + + @Issue("JENKINS-48365") + @Test + @LocalData + public void upgradeFromJenkins2() { + VersionNumber since = new VersionNumber("2.0"); rr.then(r -> { List detachedPlugins = ClassicPluginStrategy.getDetachedPlugins(since); assertThat("Plugins have been detached since the pre-upgrade version", - detachedPlugins.size(), greaterThan(15)); + detachedPlugins.size(), greaterThan(1)); assertThat("Plugins detached between the pre-upgrade version and the current version should be installed", getInstalledDetachedPlugins(r, detachedPlugins).size(), equalTo(detachedPlugins.size())); }); } + @Test + public void newInstallation() { + rr.then(r -> { + List detachedPlugins = ClassicPluginStrategy.getDetachedPlugins(); + assertThat("Detached plugins should exist", detachedPlugins, not(empty())); + assertThat("Detached plugins should not be installed on a new instance", + getInstalledDetachedPlugins(r, detachedPlugins), empty()); + }); + rr.then(r -> { + List detachedPlugins = ClassicPluginStrategy.getDetachedPlugins(); + assertThat("Detached plugins should exist", detachedPlugins, not(empty())); + assertThat("Detached plugins should not be installed after restarting", + getInstalledDetachedPlugins(r, detachedPlugins), empty()); + }); + } + private List getInstalledDetachedPlugins(JenkinsRule r, List detachedPlugins) { PluginManager pluginManager = r.jenkins.getPluginManager(); List installedPlugins = new ArrayList<>(); diff --git a/test/src/test/resources/jenkins/install/LoadDetachedPluginsTest/upgradeFromJenkins1/config.xml b/test/src/test/resources/jenkins/install/LoadDetachedPluginsTest/upgradeFromJenkins1/config.xml new file mode 100644 index 000000000000..6fcea351c2fa --- /dev/null +++ b/test/src/test/resources/jenkins/install/LoadDetachedPluginsTest/upgradeFromJenkins1/config.xml @@ -0,0 +1,34 @@ + + + + 1.550 + 2 + NORMAL + true + + + false + + ${JENKINS_HOME}/workspace/${ITEM_FULLNAME} + ${ITEM_ROOTDIR}/builds + + + + + 5 + 0 + + + + All + false + false + + + + All + 0 + + + + \ No newline at end of file diff --git a/test/src/test/resources/jenkins/install/LoadDetachedPluginsTest/upgradeFromJenkins2/jenkins.install.InstallUtil.lastExecVersion b/test/src/test/resources/jenkins/install/LoadDetachedPluginsTest/upgradeFromJenkins2/jenkins.install.InstallUtil.lastExecVersion new file mode 100644 index 000000000000..415b19fc3623 --- /dev/null +++ b/test/src/test/resources/jenkins/install/LoadDetachedPluginsTest/upgradeFromJenkins2/jenkins.install.InstallUtil.lastExecVersion @@ -0,0 +1 @@ +2.0 \ No newline at end of file From cfbc34ed729e0564f75c1497f49cdffcefaeebe3 Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Tue, 12 Dec 2017 17:37:25 -0500 Subject: [PATCH 4/5] Assert that detached plugins and their dependencies load successfully --- .../jenkins/install/LoadDetachedPluginsTest.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/test/src/test/java/jenkins/install/LoadDetachedPluginsTest.java b/test/src/test/java/jenkins/install/LoadDetachedPluginsTest.java index f70b4d3a9394..eecc1e2b4909 100644 --- a/test/src/test/java/jenkins/install/LoadDetachedPluginsTest.java +++ b/test/src/test/java/jenkins/install/LoadDetachedPluginsTest.java @@ -62,6 +62,7 @@ public void upgradeFromJenkins1() throws IOException { detachedPlugins.size(), greaterThan(4)); assertThat("Plugins detached between the pre-upgrade version and the current version should be installed", getInstalledDetachedPlugins(r, detachedPlugins).size(), equalTo(detachedPlugins.size())); + assertNoFailedPlugins(r); }); } @@ -76,6 +77,7 @@ public void upgradeFromJenkins2() { detachedPlugins.size(), greaterThan(1)); assertThat("Plugins detached between the pre-upgrade version and the current version should be installed", getInstalledDetachedPlugins(r, detachedPlugins).size(), equalTo(detachedPlugins.size())); + assertNoFailedPlugins(r); }); } @@ -86,12 +88,14 @@ public void newInstallation() { assertThat("Detached plugins should exist", detachedPlugins, not(empty())); assertThat("Detached plugins should not be installed on a new instance", getInstalledDetachedPlugins(r, detachedPlugins), empty()); + assertNoFailedPlugins(r); }); rr.then(r -> { List detachedPlugins = ClassicPluginStrategy.getDetachedPlugins(); assertThat("Detached plugins should exist", detachedPlugins, not(empty())); assertThat("Detached plugins should not be installed after restarting", getInstalledDetachedPlugins(r, detachedPlugins), empty()); + assertNoFailedPlugins(r); }); } @@ -102,10 +106,16 @@ private List getInstalledDetachedPlugins(JenkinsRule r, List Date: Sat, 16 Dec 2017 13:31:28 +0100 Subject: [PATCH 5/5] [JENKINS-48365] Restrict InstallUtil#NEW_INSTALL_VERSION --- core/src/main/java/jenkins/install/InstallUtil.java | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/main/java/jenkins/install/InstallUtil.java b/core/src/main/java/jenkins/install/InstallUtil.java index d89cac2df541..5b3f95555b35 100644 --- a/core/src/main/java/jenkins/install/InstallUtil.java +++ b/core/src/main/java/jenkins/install/InstallUtil.java @@ -70,6 +70,7 @@ public class InstallUtil { private static final Logger LOGGER = Logger.getLogger(InstallUtil.class.getName()); // tests need this to be 1.0 + @Restricted(NoExternalUse.class) public static final VersionNumber NEW_INSTALL_VERSION = new VersionNumber("1.0"); private static final VersionNumber FORCE_NEW_INSTALL_VERSION = new VersionNumber("0.0");