diff --git a/core/src/main/java/hudson/model/Run.java b/core/src/main/java/hudson/model/Run.java index e8c7965f30e4..cd574004154d 100644 --- a/core/src/main/java/hudson/model/Run.java +++ b/core/src/main/java/hudson/model/Run.java @@ -1541,6 +1541,20 @@ public ACL getACL() { return getParent().getACL(); } + /** + * Deletes this build's entire log, but keeps build + * + * @throws IOException + * if we fail to delete. + */ + public void deleteLog() throws IOException { + File rootDir = getRootDir(); + LOGGER.log(FINE, "{0}: {1} logs successfully deleted", new Object[] { this, rootDir }); + synchronized (this) { + Util.deleteFile(getLogFile()); + } + } + /** * Deletes this build's artifacts. * diff --git a/core/src/main/java/hudson/tasks/LogRotator.java b/core/src/main/java/hudson/tasks/LogRotator.java index c1efa625cf6e..f894ffe20fa9 100644 --- a/core/src/main/java/hudson/tasks/LogRotator.java +++ b/core/src/main/java/hudson/tasks/LogRotator.java @@ -39,7 +39,8 @@ import java.util.List; import java.util.logging.Logger; -import static java.util.logging.Level.*; +import static java.util.logging.Level.FINE; +import static java.util.logging.Level.FINER; /** * Default implementation of {@link BuildDiscarder}. @@ -76,10 +77,25 @@ public class LogRotator extends BuildDiscarder { */ private final Integer artifactNumToKeep; + /** + * If not -1 nor null, build logs are only kept up to this days. + * Null handling is necessary to remain data compatible with old versions. + * @since FIXME + */ + private final Integer logDaysToKeep; + + /** + * If not -1, only this number of build logs are kept. + * Null handling is necessary to remain data compatible with old versions. + * @since FIXME + */ + private final Integer logNumToKeep; + @DataBoundConstructor - public LogRotator (String daysToKeepStr, String numToKeepStr, String artifactDaysToKeepStr, String artifactNumToKeepStr) { + public LogRotator (String daysToKeepStr, String numToKeepStr, String artifactDaysToKeepStr, String artifactNumToKeepStr, String logDaysToKeepStr, String logNumToKeepStr) { this (parse(daysToKeepStr),parse(numToKeepStr), - parse(artifactDaysToKeepStr),parse(artifactNumToKeepStr)); + parse(artifactDaysToKeepStr),parse(artifactNumToKeepStr), + parse(logDaysToKeepStr),parse(logNumToKeepStr)); } public static int parse(String p) { @@ -93,30 +109,61 @@ public static int parse(String p) { /** * @deprecated since 1.350. - * Use {@link #LogRotator(int, int, int, int)} + * Use {@link #LogRotator(int, int, int, int, int, int)} */ @Deprecated public LogRotator(int daysToKeep, int numToKeep) { - this(daysToKeep, numToKeep, -1, -1); + this(daysToKeep, numToKeep, -1, -1, -1, -1); } - + /** + * @deprecated since FIXME + * Use {@link #LogRotator(int, int, int, int, int, int)} + */ + @Deprecated public LogRotator(int daysToKeep, int numToKeep, int artifactDaysToKeep, int artifactNumToKeep) { + this(daysToKeep, numToKeep, artifactDaysToKeep, artifactNumToKeep, -1, -1); + } + + public LogRotator(int daysToKeep, int numToKeep, int artifactDaysToKeep, int artifactNumToKeep, int logDaysToKeep, int logNumToKeep) { this.daysToKeep = daysToKeep; this.numToKeep = numToKeep; this.artifactDaysToKeep = artifactDaysToKeep; this.artifactNumToKeep = artifactNumToKeep; - + this.logDaysToKeep = logDaysToKeep; + this.logNumToKeep = logNumToKeep; + } @SuppressWarnings("rawtypes") public void perform(Job job) throws IOException, InterruptedException { - LOGGER.log(FINE, "Running the log rotation for {0} with numToKeep={1} daysToKeep={2} artifactNumToKeep={3} artifactDaysToKeep={4}", new Object[] {job, numToKeep, daysToKeep, artifactNumToKeep, artifactDaysToKeep}); - + LOGGER.log(FINE, "Running the log rotation for {0} with numToKeep={1} daysToKeep={2} artifactNumToKeep={3} " + + "artifactDaysToKeep={4} logNumToKeep={5} logDaysToKeep={6}", new Object[] {job, numToKeep, daysToKeep, + artifactNumToKeep, artifactDaysToKeep, logNumToKeep, logDaysToKeep}); + + performDeletionOf("build", job, numToKeep, daysToKeep, Run::delete); + performDeletionOf("artifacts", job, artifactNumToKeep, artifactDaysToKeep, Run::deleteArtifacts); + performDeletionOf("log", job, logNumToKeep, logDaysToKeep, Run::deleteLog); + } + + /** + * Deletes build metadata based on the parameters passed. + * + * Each build or build metadata have different delete functions as specified in {@link Run} + * @param buildMetaData String indicating whether builds, artifacts, or logs are being deleted (used for logging) + * @param job the Jenkins job whose builds and/or metadata are to be deleted + * @param numToKeep the number of the most recent builds to keep + * @param daysToKeep the number of days to keep the most recent builds up until + * @param runAction the deletion function passed + * @throws IOException if fail to delete + * @since FIXME + */ + private void performDeletionOf(String buildMetaData, Job job, Integer numToKeep, Integer daysToKeep, + RunAction runAction) throws IOException{ // always keep the last successful and the last stable builds Run lsb = job.getLastSuccessfulBuild(); Run lstb = job.getLastStableBuild(); - if(numToKeep!=-1) { + if(numToKeep!=null && numToKeep!=-1) { // Note that RunList.size is deprecated, and indeed here we are loading all the builds of the job. // However we would need to load the first numToKeep anyway, just to skip over them; // and we would need to load the rest anyway, to delete them. @@ -127,12 +174,12 @@ public void perform(Job job) throws IOException, InterruptedException { if (shouldKeepRun(r, lsb, lstb)) { continue; } - LOGGER.log(FINE, "{0} is to be removed", r); - r.delete(); + LOGGER.log(FINE, "{0}'s {1} to be removed", new Object[] {r, buildMetaData}); + runAction.call(r); } } - if(daysToKeep!=-1) { + if(daysToKeep!= null && daysToKeep!=-1) { Calendar cal = new GregorianCalendar(); cal.add(Calendar.DAY_OF_YEAR,-daysToKeep); Run r = job.getFirstBuild(); @@ -141,35 +188,8 @@ public void perform(Job job) throws IOException, InterruptedException { break; } if (!shouldKeepRun(r, lsb, lstb)) { - LOGGER.log(FINE, "{0} is to be removed", r); - r.delete(); - } - r = r.getNextBuild(); - } - } - - if(artifactNumToKeep!=null && artifactNumToKeep!=-1) { - List> builds = job.getBuilds(); - for (Run r : copy(builds.subList(Math.min(builds.size(), artifactNumToKeep), builds.size()))) { - if (shouldKeepRun(r, lsb, lstb)) { - continue; - } - LOGGER.log(FINE, "{0} is to be purged of artifacts", r); - r.deleteArtifacts(); - } - } - - if(artifactDaysToKeep!=null && artifactDaysToKeep!=-1) { - Calendar cal = new GregorianCalendar(); - cal.add(Calendar.DAY_OF_YEAR,-artifactDaysToKeep); - Run r = job.getFirstBuild(); - while (r != null) { - if (tooNew(r, cal)) { - break; - } - if (!shouldKeepRun(r, lsb, lstb)) { - LOGGER.log(FINE, "{0} is to be purged of artifacts", r); - r.deleteArtifacts(); + LOGGER.log(FINE, "{0}'s {1} to be removed", new Object[] {r, buildMetaData}); + runAction.call(r); } r = r.getNextBuild(); } @@ -228,6 +248,14 @@ public int getArtifactNumToKeep() { return unbox(artifactNumToKeep); } + public int getLogDaysToKeep() { + return unbox(logDaysToKeep); + } + + public int getLogNumToKeep() { + return unbox(logNumToKeep); + } + public String getDaysToKeepStr() { return toString(daysToKeep); } @@ -244,6 +272,14 @@ public String getArtifactNumToKeepStr() { return toString(artifactNumToKeep); } + public String getLogDaysToKeepStr() { + return toString(logDaysToKeep); + } + + public String getLogNumToKeepStr() { + return toString(logNumToKeep); + } + private int unbox(Integer i) { return i==null ? -1: i; } @@ -253,6 +289,15 @@ private String toString(Integer i) { return String.valueOf(i); } + /** + * Interface to allow for lambda functions to run a call on a given run. + * @since FIXME + */ + @FunctionalInterface + interface RunAction { + void call(Run r) throws IOException; + } + @Extension @Symbol("logRotator") public static final class LRDescriptor extends BuildDiscarderDescriptor { public String getDisplayName() { diff --git a/test/src/test/java/hudson/tasks/LogRotatorTest.java b/test/src/test/java/hudson/tasks/LogRotatorTest.java index 1990b3bc826d..4ce03ba3b893 100644 --- a/test/src/test/java/hudson/tasks/LogRotatorTest.java +++ b/test/src/test/java/hudson/tasks/LogRotatorTest.java @@ -55,6 +55,7 @@ import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.TestBuilder; +import org.jvnet.hudson.test.UnstableBuilder; /** * Verifies that the last successful and stable builds of a job will be kept if requested. @@ -67,7 +68,7 @@ public class LogRotatorTest { @Test public void successVsFailure() throws Exception { FreeStyleProject project = j.createFreeStyleProject(); - project.setLogRotator(new LogRotator(-1, 2, -1, -1)); + project.setBuildDiscarder(new LogRotator(-1, 2, -1, -1, -1, -1)); assertEquals(Result.SUCCESS, build(project)); // #1 project.getBuildersList().replaceBy(Collections.singleton(new FailureBuilder())); assertEquals(Result.FAILURE, build(project)); // #2 @@ -85,7 +86,7 @@ public void successVsFailure() throws Exception { @Issue("JENKINS-2417") public void stableVsUnstable() throws Exception { FreeStyleProject project = j.createFreeStyleProject(); - project.setLogRotator(new LogRotator(-1, 2, -1, -1)); + project.setBuildDiscarder(new LogRotator(-1, 2, -1, -1, -1, -1)); assertEquals(Result.SUCCESS, build(project)); // #1 project.getPublishersList().replaceBy(Collections.singleton(new TestsFail())); assertEquals(Result.UNSTABLE, build(project)); // #2 @@ -101,7 +102,7 @@ public void stableVsUnstable() throws Exception { @Issue("JENKINS-834") public void artifactDelete() throws Exception { FreeStyleProject project = j.createFreeStyleProject(); - project.setLogRotator(new LogRotator(-1, 6, -1, 2)); + project.setBuildDiscarder(new LogRotator(-1, 6, -1, 2, -1, -1)); project.getPublishersList().replaceBy(Collections.singleton(new ArtifactArchiver("f", "", true, false))); assertEquals("(no artifacts)", Result.FAILURE, build(project)); // #1 assertFalse(project.getBuildByNumber(1).getHasArtifacts()); @@ -153,7 +154,7 @@ public void artifactDelete() throws Exception { public void artifactsRetainedWhileBuilding() throws Exception { j.getInstance().setNumExecutors(3); FreeStyleProject p = j.createFreeStyleProject(); - p.setBuildDiscarder(new LogRotator(-1, 3, -1, 1)); + p.setBuildDiscarder(new LogRotator(-1, 3, -1, 1, -1, -1)); StallBuilder sync = new StallBuilder(); p.getBuildersList().replaceBy(Arrays.asList(new CreateArtifact(), sync)); p.setConcurrentBuild(true); @@ -201,6 +202,61 @@ public void artifactsRetainedWhileBuilding() throws Exception { assertThat("we have artifacts in run3", run3.getHasArtifacts(), is(true)); } + @Test + @Issue("JENKINS-51229") + public void logDelete() throws Exception + { + FreeStyleProject project = j.createFreeStyleProject(); + project.setBuildDiscarder(new LogRotator(-1, -1, -1, -1, -1, 2)); + assertEquals(Result.SUCCESS, build(project)); // #1 + project.getBuildersList().replaceBy(Collections.singleton(new FailureBuilder())); + assertEquals(Result.FAILURE, build(project)); // #2 + assertEquals(Result.FAILURE, build(project)); // #3 + assertEquals(1, numberOf(project.getLastSuccessfulBuild())); + project.getBuildersList().replaceBy(Collections. emptySet()); + assertEquals(Result.SUCCESS, build(project)); // #4 + assertFalse(project.getBuildByNumber(1).getLogFile().isFile()); + assertFalse(project.getBuildByNumber(2).getLogFile().isFile()); + assertTrue(project.getBuildByNumber(3).getLogFile().isFile()); + assertTrue(project.getBuildByNumber(4).getLogFile().isFile()); + } + + @Test + public void keepLogForMarkKeeped() throws Exception + { + FreeStyleProject project = j.createFreeStyleProject(); + project.setBuildDiscarder(new LogRotator(-1, -1, -1, -1, -1, 2)); + build(project); // #1 + project.getBuildByNumber(1).keepLog(); + build(project); // #2 + assertTrue(project.getBuildByNumber(1).getLogFile().isFile()); + build(project); // #3 + build(project); // #4 + assertTrue(project.getBuildByNumber(1).getLogFile().isFile()); + assertFalse(project.getBuildByNumber(2).getLogFile().isFile()); + assertTrue(project.getBuildByNumber(3).getLogFile().isFile()); + assertTrue(project.getBuildByNumber(4).getLogFile().isFile()); + } + + @Test + public void keepLogForLastStableBuild() throws Exception + { + FreeStyleProject project = j.createFreeStyleProject(); + project.setBuildDiscarder(new LogRotator(-1, -1, -1, -1, -1, 2)); + build(project); // #1 + project.getBuildersList().replaceBy(Collections.singleton(new UnstableBuilder())); + build(project); // #2 + build(project); // #3 + assertTrue(project.getBuildByNumber(1).getLogFile().isFile()); + assertTrue(project.getBuildByNumber(2).getLogFile().isFile()); + assertTrue(project.getBuildByNumber(3).getLogFile().isFile()); + build(project); // #4 + assertTrue(project.getBuildByNumber(1).getLogFile().isFile()); + assertFalse(project.getBuildByNumber(2).getLogFile().isFile()); + assertTrue(project.getBuildByNumber(3).getLogFile().isFile()); + assertTrue(project.getBuildByNumber(4).getLogFile().isFile()); + } + static Result build(FreeStyleProject project) throws Exception { return project.scheduleBuild2(0).get().getResult(); } diff --git a/test/src/test/java/jenkins/model/BuildDiscarderPropertyTest.java b/test/src/test/java/jenkins/model/BuildDiscarderPropertyTest.java index c137c8bc200c..15ed91a3c4a3 100644 --- a/test/src/test/java/jenkins/model/BuildDiscarderPropertyTest.java +++ b/test/src/test/java/jenkins/model/BuildDiscarderPropertyTest.java @@ -91,5 +91,7 @@ private static void verifyLogRotatorSanity(AbstractProject p) { assertEquals(3, d.getNumToKeep()); assertEquals(2, d.getArtifactDaysToKeep()); assertEquals(1, d.getArtifactNumToKeep()); + assertEquals(6, d.getLogDaysToKeep()); + assertEquals(5, d.getLogNumToKeep()); } } diff --git a/test/src/test/resources/jenkins/model/BuildDiscarderPropertyTest/buildDiscarderField.zip b/test/src/test/resources/jenkins/model/BuildDiscarderPropertyTest/buildDiscarderField.zip index fd32a7328236..1f234093d024 100644 Binary files a/test/src/test/resources/jenkins/model/BuildDiscarderPropertyTest/buildDiscarderField.zip and b/test/src/test/resources/jenkins/model/BuildDiscarderPropertyTest/buildDiscarderField.zip differ diff --git a/test/src/test/resources/jenkins/model/BuildDiscarderPropertyTest/logRotatorField/jobs/foo/config.xml b/test/src/test/resources/jenkins/model/BuildDiscarderPropertyTest/logRotatorField/jobs/foo/config.xml index a8609da6e20a..236e0be1d1cd 100644 --- a/test/src/test/resources/jenkins/model/BuildDiscarderPropertyTest/logRotatorField/jobs/foo/config.xml +++ b/test/src/test/resources/jenkins/model/BuildDiscarderPropertyTest/logRotatorField/jobs/foo/config.xml @@ -10,6 +10,8 @@ 3 2 1 + 6 + 5 false