From d34b17ee4b85787be56a2d6f32186e3839d5482d Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Sat, 12 Oct 2024 08:06:18 -0400 Subject: [PATCH 1/6] [JENKINS-73835] Do not allow builds to be deleted while they are still running and ensure build discarders run after builds are fully complete (#9810) * [JENKINS-73835] Do not allow builds to be deleted while they are still running * [JENKINS-73835] Avoid redundant calls to Job.logRotate when builds complete and always call Job.logRotate after build finalization * [JENKINS-73835] Add issue reference to RunTest.buildsMayNotBeDeletedWhileRunning * [JENKINS-73835] Adjust DeleteBuildsCommandTest.deleteBuildsShouldSuccessEvenTheBuildIsRunning to match new behavior * [JENKINS-73835] Run/delete.jelly should check Run.isLogUpdated, not Run.isBuilding --- core/src/main/java/hudson/model/Run.java | 9 +++----- .../main/java/hudson/tasks/LogRotator.java | 2 +- .../model/BackgroundGlobalBuildDiscarder.java | 13 +++++++++++- .../model/GlobalBuildDiscarderListener.java | 13 ++++++++++-- .../resources/hudson/model/Run/delete.jelly | 2 +- .../hudson/cli/DeleteBuildsCommandTest.java | 21 ++++++------------- test/src/test/java/hudson/model/RunTest.java | 14 +++++++++++++ .../java/hudson/tasks/LogRotatorTest.java | 16 ++++++++++++++ 8 files changed, 64 insertions(+), 26 deletions(-) diff --git a/core/src/main/java/hudson/model/Run.java b/core/src/main/java/hudson/model/Run.java index 450be07b7f1a..ec7eaabf32c6 100644 --- a/core/src/main/java/hudson/model/Run.java +++ b/core/src/main/java/hudson/model/Run.java @@ -1551,6 +1551,9 @@ public synchronized void deleteArtifacts() throws IOException { * if we fail to delete. */ public void delete() throws IOException { + if (isLogUpdated()) { + throw new IOException("Unable to delete " + this + " because it is still running"); + } synchronized (this) { // Avoid concurrent delete. See https://issues.jenkins.io/browse/JENKINS-61687 if (isPendingDelete) { @@ -1885,12 +1888,6 @@ protected final void execute(@NonNull RunExecution job) { LOGGER.log(Level.SEVERE, "Failed to save build record", e); } } - - try { - getParent().logRotate(); - } catch (Exception e) { - LOGGER.log(Level.SEVERE, "Failed to rotate log", e); - } } finally { onEndBuilding(); if (logger != null) { diff --git a/core/src/main/java/hudson/tasks/LogRotator.java b/core/src/main/java/hudson/tasks/LogRotator.java index ca95081b0385..0b47f035d809 100644 --- a/core/src/main/java/hudson/tasks/LogRotator.java +++ b/core/src/main/java/hudson/tasks/LogRotator.java @@ -250,7 +250,7 @@ private boolean shouldKeepRun(Run r, Run lsb, Run lstb) { LOGGER.log(FINER, "{0} is not to be removed or purged of artifacts because it’s the last stable build", r); return true; } - if (r.isBuilding()) { + if (r.isLogUpdated()) { LOGGER.log(FINER, "{0} is not to be removed or purged of artifacts because it’s still building", r); return true; } diff --git a/core/src/main/java/jenkins/model/BackgroundGlobalBuildDiscarder.java b/core/src/main/java/jenkins/model/BackgroundGlobalBuildDiscarder.java index 1a42c0577a47..ad33643879f5 100644 --- a/core/src/main/java/jenkins/model/BackgroundGlobalBuildDiscarder.java +++ b/core/src/main/java/jenkins/model/BackgroundGlobalBuildDiscarder.java @@ -31,6 +31,7 @@ import java.io.IOException; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.stream.Stream; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; @@ -56,8 +57,18 @@ protected void execute(TaskListener listener) throws IOException, InterruptedExc } } + /** + * Runs all globally configured build discarders against a job. + */ public static void processJob(TaskListener listener, Job job) { - GlobalBuildDiscarderConfiguration.get().getConfiguredBuildDiscarders().forEach(strategy -> { + processJob(listener, job, GlobalBuildDiscarderConfiguration.get().getConfiguredBuildDiscarders().stream()); + } + + /** + * Runs the specified build discarders against a job. + */ + public static void processJob(TaskListener listener, Job job, Stream strategies) { + strategies.forEach(strategy -> { String displayName = strategy.getDescriptor().getDisplayName(); if (strategy.isApplicable(job)) { try { diff --git a/core/src/main/java/jenkins/model/GlobalBuildDiscarderListener.java b/core/src/main/java/jenkins/model/GlobalBuildDiscarderListener.java index 7ddea84c4241..6d2c58b44774 100644 --- a/core/src/main/java/jenkins/model/GlobalBuildDiscarderListener.java +++ b/core/src/main/java/jenkins/model/GlobalBuildDiscarderListener.java @@ -35,7 +35,7 @@ import org.kohsuke.accmod.restrictions.NoExternalUse; /** - * Run background build discarders on an individual job once a build is finalized + * Run build discarders on an individual job once a build is finalized */ @Extension @Restricted(NoExternalUse.class) @@ -46,6 +46,15 @@ public class GlobalBuildDiscarderListener extends RunListener { @Override public void onFinalized(Run run) { Job job = run.getParent(); - BackgroundGlobalBuildDiscarder.processJob(new LogTaskListener(LOGGER, Level.FINE), job); + try { + // Job-level build discarder execution is unconditional. + job.logRotate(); + } catch (Exception e) { + LOGGER.log(Level.WARNING, e, () -> "Failed to rotate log for " + run); + } + // Avoid calling Job.logRotate twice in case JobGlobalBuildDiscarderStrategy is configured globally. + BackgroundGlobalBuildDiscarder.processJob(new LogTaskListener(LOGGER, Level.FINE), job, + GlobalBuildDiscarderConfiguration.get().getConfiguredBuildDiscarders().stream() + .filter(s -> !(s instanceof JobGlobalBuildDiscarderStrategy))); } } diff --git a/core/src/main/resources/hudson/model/Run/delete.jelly b/core/src/main/resources/hudson/model/Run/delete.jelly index 83e3cbd77e71..7443203f2650 100644 --- a/core/src/main/resources/hudson/model/Run/delete.jelly +++ b/core/src/main/resources/hudson/model/Run/delete.jelly @@ -27,7 +27,7 @@ THE SOFTWARE. --> - + diff --git a/test/src/test/java/hudson/cli/DeleteBuildsCommandTest.java b/test/src/test/java/hudson/cli/DeleteBuildsCommandTest.java index dfac64fc1b5a..5e2e917e9808 100644 --- a/test/src/test/java/hudson/cli/DeleteBuildsCommandTest.java +++ b/test/src/test/java/hudson/cli/DeleteBuildsCommandTest.java @@ -32,21 +32,18 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; import static org.junit.Assert.assertNotNull; -import static org.junit.Assume.assumeFalse; -import hudson.Functions; import hudson.model.ExecutorTest; import hudson.model.FreeStyleProject; import hudson.model.Item; import hudson.model.Run; import hudson.model.labels.LabelAtom; import hudson.tasks.Shell; -import java.io.IOException; import jenkins.model.Jenkins; -import org.junit.AssumptionViolatedException; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; /** @@ -139,8 +136,8 @@ public class DeleteBuildsCommandTest { assertThat(result.stdout(), containsString("Deleted 0 builds")); } - @Test public void deleteBuildsShouldSuccessEvenTheBuildIsRunning() throws Exception { - assumeFalse("You can't delete files that are in use on Windows", Functions.isWindows()); + @Issue("JENKINS-73835") + @Test public void deleteBuildsShouldFailIfTheBuildIsRunning() throws Exception { FreeStyleProject project = j.createFreeStyleProject("aProject"); ExecutorTest.startBlockingBuild(project); assertThat(((FreeStyleProject) j.jenkins.getItem("aProject")).getBuilds(), hasSize(1)); @@ -148,15 +145,9 @@ public class DeleteBuildsCommandTest { final CLICommandInvoker.Result result = command .authorizedTo(Jenkins.READ, Item.READ, Run.DELETE) .invokeWithArgs("aProject", "1"); - assertThat(result, succeeded()); - assertThat(result.stdout(), containsString("Deleted 1 builds")); - assertThat(((FreeStyleProject) j.jenkins.getItem("aProject")).getBuilds(), hasSize(0)); - assertThat(project.isBuilding(), equalTo(false)); - try { - project.delete(); - } catch (IOException | InterruptedException x) { - throw new AssumptionViolatedException("Could not delete test project (race condition?)", x); - } + assertThat(result, failedWith(1)); + assertThat(result, hasNoStandardOutput()); + assertThat(result.stderr(), containsString("Unable to delete aProject #1 because it is still running")); } @Test public void deleteBuildsShouldSuccessEvenTheBuildIsStuckInTheQueue() throws Exception { diff --git a/test/src/test/java/hudson/model/RunTest.java b/test/src/test/java/hudson/model/RunTest.java index 34be1f14c2ef..4df895309abb 100644 --- a/test/src/test/java/hudson/model/RunTest.java +++ b/test/src/test/java/hudson/model/RunTest.java @@ -30,6 +30,7 @@ import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import hudson.ExtensionList; @@ -62,6 +63,7 @@ import org.junit.experimental.categories.Category; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.SleepBuilder; import org.jvnet.hudson.test.SmokeTest; import org.jvnet.hudson.test.TestExtension; import org.kohsuke.stapler.DataBoundConstructor; @@ -128,6 +130,18 @@ public void onDeleted(Saveable o, XmlFile file) { } } + @Issue("JENKINS-73835") + @Test public void buildsMayNotBeDeletedWhileRunning() throws Exception { + var p = j.createFreeStyleProject(); + p.getBuildersList().add(new SleepBuilder(999999)); + var b = p.scheduleBuild2(0).waitForStart(); + var ex = assertThrows(IOException.class, () -> b.delete()); + assertThat(ex.getMessage(), containsString("Unable to delete " + b + " because it is still running")); + b.getExecutor().interrupt(); + j.waitForCompletion(b); + b.delete(); // Works fine. + } + @Issue("SECURITY-1902") @Test public void preventXssInBadgeTooltip() throws Exception { j.jenkins.setQuietPeriod(0); diff --git a/test/src/test/java/hudson/tasks/LogRotatorTest.java b/test/src/test/java/hudson/tasks/LogRotatorTest.java index b72e837dbd86..118a860147a1 100644 --- a/test/src/test/java/hudson/tasks/LogRotatorTest.java +++ b/test/src/test/java/hudson/tasks/LogRotatorTest.java @@ -50,8 +50,10 @@ import java.util.concurrent.TimeoutException; import java.util.logging.Level; import java.util.logging.Logger; +import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; +import org.jvnet.hudson.test.BuildWatcher; import org.jvnet.hudson.test.FailureBuilder; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; @@ -62,6 +64,9 @@ */ public class LogRotatorTest { + @ClassRule + public static BuildWatcher watcher = new BuildWatcher(); + @Rule public JenkinsRule j = new JenkinsRule(); @@ -96,6 +101,17 @@ public void successVsFailureWithRemoveLastBuild() throws Exception { assertEquals(2, numberOf(project.getLastFailedBuild())); } + @Test + public void ableToDeleteCurrentBuild() throws Exception { + var p = j.createFreeStyleProject(); + // Keep 0 builds, i.e. immediately delete builds as they complete. + LogRotator logRotator = new LogRotator(-1, 0, -1, -1); + logRotator.setRemoveLastBuild(true); + p.setBuildDiscarder(logRotator); + j.buildAndAssertStatus(Result.SUCCESS, p); + assertNull(p.getBuildByNumber(1)); + } + @Test @Issue("JENKINS-2417") public void stableVsUnstable() throws Exception { From 829be14884ec3af76ee9c28b5670856e2829476a Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 12 Oct 2024 06:06:31 -0600 Subject: [PATCH 2/6] Fill in since annotations (#9849) Co-authored-by: timja <21194782+timja@users.noreply.github.com> --- core/src/main/java/hudson/model/ComputerSet.java | 2 +- .../main/java/hudson/model/listeners/SaveableListener.java | 4 ++-- core/src/main/java/hudson/security/AuthorizationStrategy.java | 2 +- core/src/main/java/jenkins/model/IComputer.java | 2 +- core/src/main/java/jenkins/model/IDisplayExecutor.java | 2 +- core/src/main/java/jenkins/model/IExecutor.java | 2 +- .../main/java/jenkins/model/ModelObjectWithContextMenu.java | 4 ++-- core/src/main/java/jenkins/model/queue/ITask.java | 2 +- 8 files changed, 10 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/hudson/model/ComputerSet.java b/core/src/main/java/hudson/model/ComputerSet.java index f8e4905b0904..fcc0aa4e41e7 100644 --- a/core/src/main/java/hudson/model/ComputerSet.java +++ b/core/src/main/java/hudson/model/ComputerSet.java @@ -118,7 +118,7 @@ public static List get_monitors() { * @deprecated Use {@link #getComputers()} instead. * @return All {@link Computer} instances managed by this set. */ - @Deprecated(since = "TODO") + @Deprecated(since = "2.480") public Computer[] get_all() { return getComputers().stream().filter(Computer.class::isInstance).toArray(Computer[]::new); } diff --git a/core/src/main/java/hudson/model/listeners/SaveableListener.java b/core/src/main/java/hudson/model/listeners/SaveableListener.java index 02747877e76f..14877d080bf5 100644 --- a/core/src/main/java/hudson/model/listeners/SaveableListener.java +++ b/core/src/main/java/hudson/model/listeners/SaveableListener.java @@ -60,7 +60,7 @@ public void onChange(Saveable o, XmlFile file) {} * The saveable object. * @param file * The {@link XmlFile} for this saveable object. - * @since TODO + * @since 2.480 */ public void onDeleted(Saveable o, XmlFile file) {} @@ -92,7 +92,7 @@ public static void fireOnChange(Saveable o, XmlFile file) { /** * Fires the {@link #onDeleted} event. - * @since TODO + * @since 2.480 */ public static void fireOnDeleted(Saveable o, XmlFile file) { Listeners.notify(SaveableListener.class, false, l -> l.onDeleted(o, file)); diff --git a/core/src/main/java/hudson/security/AuthorizationStrategy.java b/core/src/main/java/hudson/security/AuthorizationStrategy.java index 08fa10fe9897..5ca218f6a8fd 100644 --- a/core/src/main/java/hudson/security/AuthorizationStrategy.java +++ b/core/src/main/java/hudson/security/AuthorizationStrategy.java @@ -162,7 +162,7 @@ public abstract class AuthorizationStrategy extends AbstractDescribableImpl Date: Sat, 12 Oct 2024 08:06:47 -0400 Subject: [PATCH 3/6] Correctly iterate `IComputer`s from `ComputerSet/index.jelly` (#9852) --- core/src/main/resources/hudson/model/ComputerSet/index.jelly | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/resources/hudson/model/ComputerSet/index.jelly b/core/src/main/resources/hudson/model/ComputerSet/index.jelly index 1b3c9aa1ea02..c96346ac4949 100644 --- a/core/src/main/resources/hudson/model/ComputerSet/index.jelly +++ b/core/src/main/resources/hudson/model/ComputerSet/index.jelly @@ -73,7 +73,7 @@ THE SOFTWARE. - +
From 4622a85d170b2a5a3b681ba0a6f7dcce6a9195b1 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Sat, 12 Oct 2024 06:07:46 -0600 Subject: [PATCH 4/6] Update dependency globals to v15.11.0 (#9856) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- package.json | 2 +- yarn.lock | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/package.json b/package.json index 6e01d39f94bf..bee799d33b55 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,7 @@ "eslint": "9.12.0", "eslint-config-prettier": "9.1.0", "eslint-formatter-checkstyle": "8.40.0", - "globals": "15.10.0", + "globals": "15.11.0", "handlebars-loader": "1.7.3", "mini-css-extract-plugin": "2.9.1", "postcss": "8.4.47", diff --git a/yarn.lock b/yarn.lock index 1c9ced4c8b74..c2f3678b1cc0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3999,10 +3999,10 @@ __metadata: languageName: node linkType: hard -"globals@npm:15.10.0": - version: 15.10.0 - resolution: "globals@npm:15.10.0" - checksum: 10c0/fef8f320e88f01f1492fef1b04b056908e1f6726eeaffe3bca03247237300c2d86e71585ee641b62ba71460a6eaff0d6ca7fca284e61bd1b3f833c7ad68b160a +"globals@npm:15.11.0": + version: 15.11.0 + resolution: "globals@npm:15.11.0" + checksum: 10c0/861e39bb6bd9bd1b9f355c25c962e5eb4b3f0e1567cf60fa6c06e8c502b0ec8706b1cce055d69d84d0b7b8e028bec5418cf629a54e7047e116538d1c1c1a375c languageName: node linkType: hard @@ -4418,7 +4418,7 @@ __metadata: eslint: "npm:9.12.0" eslint-config-prettier: "npm:9.1.0" eslint-formatter-checkstyle: "npm:8.40.0" - globals: "npm:15.10.0" + globals: "npm:15.11.0" handlebars: "npm:4.7.8" handlebars-loader: "npm:1.7.3" hotkeys-js: "npm:3.12.2" From d0d0cc88c765a25cee7e5214582b9db1b2253949 Mon Sep 17 00:00:00 2001 From: Basil Crow Date: Sat, 12 Oct 2024 08:59:27 -0700 Subject: [PATCH 5/6] Winstone 8.2: Upgrade Jetty from 12.0.13 to Jetty 12.0.14 (#9841) --- pom.xml | 2 +- war/pom.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 0ae24f547d80..1a472b3fd02a 100644 --- a/pom.xml +++ b/pom.xml @@ -97,7 +97,7 @@ THE SOFTWARE. 1.30 false - 8.1 + 8.2 20.18.0 diff --git a/war/pom.xml b/war/pom.xml index 6511cded90fc..770d20d1047c 100644 --- a/war/pom.xml +++ b/war/pom.xml @@ -645,7 +645,7 @@ THE SOFTWARE. org.eclipse.jetty.ee9 jetty-ee9-maven-plugin - 12.0.13 + 12.0.14