diff --git a/docs/configuration.md b/docs/configuration.md index 38a66bd..7a2dfc0 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -77,7 +77,12 @@ artifacts: rootDir: "" # number of builds to keep per branch retentionPerBranch: 3 - # Keep each branch's latest green (successful) build even beyond retentionPerBranch. + # Additionally drop builds older than this age; suffixes s (seconds), m (minutes), h (hours), d (days). + # Empty means no age limit. + # Combines with retentionPerBranch: a build is kept only while it satisfies both limits. + # A branch's newest build is never age-pruned, so dormant branches keep their last status. + retentionMaxAge: "" + # Keep each branch's latest green (successful) build even beyond retentionPerBranch and retentionMaxAge. # This backs the permanent artifact URLs /branches//... — they always serve # the latest green build of a branch and stay valid while newer builds fail. # The kept build is still dropped once its branch is deleted from origin. diff --git a/docs/migration-from-legacy.md b/docs/migration-from-legacy.md index c44b76d..0caed4f 100644 --- a/docs/migration-from-legacy.md +++ b/docs/migration-from-legacy.md @@ -27,7 +27,7 @@ Branch-level keys below live under `branches.`; use `branches.default` for | `GITTALLY_ARTIFACT_SERVER_PORT` | `server.port` | | `GITTALLY_ARTIFACT_SERVER_BIND_ADDRESS` | `server.bindAddress` | | `GITTALLY_ARTIFACT_PUBLIC_BASE_URL` | `server.publicBaseUrl` | -| `GITTALLY_ARTIFACT_BUILD_RETENTION_PER_BRANCH` | `artifacts.retentionPerBranch` — build count only; the legacy age suffix (`h`/`d`) is not supported | +| `GITTALLY_ARTIFACT_BUILD_RETENTION_PER_BRANCH` | `artifacts.retentionPerBranch` for a count, `artifacts.retentionMaxAge` for a legacy age value (`h`/`d` suffix); unlike legacy, both limits can be combined | | `GITTALLY_IMPRESSUM_URL` | `server.impressumUrl` | | `GITTALLY_AUTO_BUILD_BRANCHES` | `branches..autoBuild.enabled: true` per branch instead of a branch list | | `GITTALLY_AUTO_BUILD_TIMES` | `branches..autoBuild.times` — YAML list, per branch | diff --git a/docs/prs/2026-07-08-PR#000-age-based-artifact-retention.md b/docs/prs/2026-07-08-PR#000-age-based-artifact-retention.md new file mode 100644 index 0000000..029b679 --- /dev/null +++ b/docs/prs/2026-07-08-PR#000-age-based-artifact-retention.md @@ -0,0 +1,105 @@ +> **WARNING:** This document describes only the change applied in this PR. +> It may already be outdated once the next PR is merged. +> Historic PR-documentation is not maintained along with new PRs — treat it as a snapshot, not as current documentation. + +## The Problem + +Retention is count-based only: `artifacts.retentionPerBranch` keeps the newest N builds per branch, no matter how old they are. +The legacy script also supported an age limit (`GITTALLY_ARTIFACT_BUILD_RETENTION_PER_BRANCH=3d`), which the rewrite dropped — the migration doc declared the age suffix unsupported. +Without an age limit there is no way to bound how long stale build logs and reports stay on disk, e.g. on branches that build rarely. + +## Non-Goals + +- No either/or single config value like legacy (`3` *or* `3d`); count and age are separate keys that can be combined. +- No age pruning of a branch's newest build — a dormant branch keeps its last status and artifacts. +- No change to the existing pruning of branches deleted from origin. +- No size-based (bytes) retention. + +## The Scenarios + +### Feature: age-based build retention combinable with count-based retention + +#### Background + +- `artifacts.retentionMaxAge` takes the shared duration format (`s`/`m`/`h`/`d` suffix, like `watcher.newBranchMaxAge`); empty (the default) means no age limit. +- Pruning runs in the watcher poll cycle; artifact directories follow the surviving build results. + +#### Scenario#000.01: Builds older than retentionMaxAge are pruned + +So that stale logs and reports do not stay on disk indefinitely. + +- **Given** `artifacts.retentionMaxAge: 30d` + - **and** a branch with builds within the retention count, some older than 30 days +- **When** the watcher prunes results and artifacts +- **Then** the builds older than 30 days are dropped together with their artifacts + - **and** the younger builds are kept. + +##### Verified by + +- [FileBuildResultRepositoryTest](../../src/test/kotlin/de/hoennig/gittally/build/FileBuildResultRepositoryTest.kt) +- [WatcherTest](../../src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt) + +#### Scenario#000.02: Count and age combine as independent caps + +So that one config can bound disk usage by count and staleness by age at the same time — which legacy could not. + +- **Given** `artifacts.retentionPerBranch: 2` and `artifacts.retentionMaxAge: 30d` +- **When** the watcher prunes +- **Then** a build is kept only while it is among the branch's newest 2 builds **and** younger than 30 days + - **and** a build violating either limit is dropped. + +##### Verified by + +- [FileBuildResultRepositoryTest](../../src/test/kotlin/de/hoennig/gittally/build/FileBuildResultRepositoryTest.kt) + +#### Scenario#000.03: A branch's newest build is never age-pruned + +So that a dormant branch keeps its last build status visible, matching the legacy age-retention behavior. + +- **Given** a branch whose newest build is older than `retentionMaxAge` +- **When** the watcher prunes +- **Then** that newest build is kept regardless of its age. + +##### Verified by + +- [FileBuildResultRepositoryTest](../../src/test/kotlin/de/hoennig/gittally/build/FileBuildResultRepositoryTest.kt) + +#### Scenario#000.04: keepLatestGreen shields the latest green build from the age limit + +So that the permanent `/branches//…` artifact links stay valid while newer builds fail, even under an age limit. + +- **Given** `artifacts.keepLatestGreen: true` (the default) + - **and** a branch whose latest green build is older than `retentionMaxAge`, followed by newer failed builds +- **When** the watcher prunes +- **Then** the latest green build and its artifacts are kept. + +##### Verified by + +- [FileBuildResultRepositoryTest](../../src/test/kotlin/de/hoennig/gittally/build/FileBuildResultRepositoryTest.kt) + +#### Scenario#000.05: The default keeps existing behavior unchanged + +So that existing installations are unaffected by the new key. + +- **Given** `artifacts.retentionMaxAge` is unset or empty +- **When** the watcher prunes +- **Then** only the count-based retention applies, exactly as before this PR. + +##### Verified by + +- [FileBuildResultRepositoryTest](../../src/test/kotlin/de/hoennig/gittally/build/FileBuildResultRepositoryTest.kt) (pre-existing count-only prune tests) + +## The Solution + +- New config key `artifacts.retentionMaxAge`, parsed with the existing `DurationParser`; all three config sync points are updated (`GitTallyConfig`, the `init` templates, `docs/configuration.md`). +- `BuildResultRepository.prune` takes an optional `retentionCutoff: Instant?`; entries started before the cutoff are dropped, except each branch's newest entry and — with `keepLatestGreen` — the newest green entry. +- The `Watcher` computes the cutoff from its injected `Clock` on every poll cycle, so the repository stays clock-free and deterministic in tests, and config changes apply without restart like the other retention keys. +- Artifact directories need no separate handling: the artifact store already prunes by the surviving build results. + +## Open Questions + +- An invalid `retentionMaxAge` value (e.g. `30x`) surfaces as a poll-cycle error in the log and the watcher state, not as a startup failure — consistent with how `watcher.newBranchMaxAge` is handled. + +## Additional Changes + +- `docs/migration-from-legacy.md` now maps a legacy age value of `GITTALLY_ARTIFACT_BUILD_RETENTION_PER_BRANCH` to `artifacts.retentionMaxAge` instead of declaring the age suffix unsupported. diff --git a/src/main/kotlin/de/hoennig/gittally/build/BuildResultRepository.kt b/src/main/kotlin/de/hoennig/gittally/build/BuildResultRepository.kt index 1ab4038..f184a1b 100644 --- a/src/main/kotlin/de/hoennig/gittally/build/BuildResultRepository.kt +++ b/src/main/kotlin/de/hoennig/gittally/build/BuildResultRepository.kt @@ -1,5 +1,7 @@ package de.hoennig.gittally.build +import java.time.Instant + interface BuildResultRepository { fun append(result: BuildResult) @@ -37,13 +39,16 @@ interface BuildResultRepository { /** * Keeps the newest [retentionPerBranch] entries per branch and drops entries of branches - * not contained in [originBranches]. With [keepLatestGreen], the newest SUCCESS entry of - * each surviving branch is kept even beyond the retention count, so the permanent + * not contained in [originBranches]. With [retentionCutoff], entries started before the + * cutoff are dropped even within the retention count — except each branch's newest entry, + * so dormant branches keep their last status. With [keepLatestGreen], the newest SUCCESS + * entry of each surviving branch is kept even beyond both limits, so the permanent * `/branches/…` artifact links stay valid while newer builds fail. Returns the removed entries. */ fun prune( originBranches: Collection, retentionPerBranch: Int, keepLatestGreen: Boolean = false, + retentionCutoff: Instant? = null, ): List } diff --git a/src/main/kotlin/de/hoennig/gittally/build/FileBuildResultRepository.kt b/src/main/kotlin/de/hoennig/gittally/build/FileBuildResultRepository.kt index 66657f7..0520f70 100644 --- a/src/main/kotlin/de/hoennig/gittally/build/FileBuildResultRepository.kt +++ b/src/main/kotlin/de/hoennig/gittally/build/FileBuildResultRepository.kt @@ -10,6 +10,7 @@ import org.slf4j.LoggerFactory import java.nio.file.Files import java.nio.file.Path import java.nio.file.StandardCopyOption +import java.time.Instant /** * Stores build results as a JSON file, e.g. `.git/gittally/build-results.json`. @@ -123,6 +124,7 @@ class FileBuildResultRepository( originBranches: Collection, retentionPerBranch: Int, keepLatestGreen: Boolean, + retentionCutoff: Instant?, ): List { synchronized(lock) { val results = load() @@ -137,6 +139,10 @@ class FileBuildResultRepository( entries .sortedByDescending { it.startedAt } .take(retentionPerBranch.coerceAtLeast(0)) + .filterIndexed { index, entry -> + // the branch's newest entry is never age-pruned + index == 0 || retentionCutoff == null || !entry.startedAt.isBefore(retentionCutoff) + } val latestGreen = entries .filter { keepLatestGreen && it.status == BuildStatus.SUCCESS } diff --git a/src/main/kotlin/de/hoennig/gittally/commands/InitCommand.kt b/src/main/kotlin/de/hoennig/gittally/commands/InitCommand.kt index 1e79452..94dacfb 100644 --- a/src/main/kotlin/de/hoennig/gittally/commands/InitCommand.kt +++ b/src/main/kotlin/de/hoennig/gittally/commands/InitCommand.kt @@ -156,7 +156,10 @@ class InitCommand( rootDir: "" # number of builds to keep per branch retentionPerBranch: 3 - # keep each branch's latest green build beyond the retention count, + # additionally drop builds older than this age (h/d suffix, e.g. 30d); empty = no age limit; + # a branch's newest build is never age-pruned + retentionMaxAge: "" + # keep each branch's latest green build beyond the retention limits, # so the permanent /branches//... artifact URLs stay valid while newer builds fail keepLatestGreen: true diff --git a/src/main/kotlin/de/hoennig/gittally/config/GitTallyConfig.kt b/src/main/kotlin/de/hoennig/gittally/config/GitTallyConfig.kt index 6298d3d..6b230b2 100644 --- a/src/main/kotlin/de/hoennig/gittally/config/GitTallyConfig.kt +++ b/src/main/kotlin/de/hoennig/gittally/config/GitTallyConfig.kt @@ -71,9 +71,17 @@ data class BuildsConfig( data class ArtifactsConfig( val retentionPerBranch: Int = 3, /** - * Keep each branch's latest green (SUCCESS) build beyond [retentionPerBranch], - * so the permanent `/branches//…` artifact URLs stay valid while newer - * builds fail; the build is still dropped once its branch is gone from origin. + * Additionally drop builds older than this age (e.g. `30d` or `12h`); empty means no + * age limit. Combines with [retentionPerBranch] — a build is kept only while it + * satisfies both limits. A branch's newest build is never age-pruned, so dormant + * branches keep their last status. + */ + val retentionMaxAge: String = "", + /** + * Keep each branch's latest green (SUCCESS) build beyond [retentionPerBranch] and + * [retentionMaxAge], so the permanent `/branches//…` artifact URLs stay + * valid while newer builds fail; the build is still dropped once its branch is gone + * from origin. */ val keepLatestGreen: Boolean = true, /** diff --git a/src/main/kotlin/de/hoennig/gittally/watcher/Watcher.kt b/src/main/kotlin/de/hoennig/gittally/watcher/Watcher.kt index bdb7997..c92d61a 100644 --- a/src/main/kotlin/de/hoennig/gittally/watcher/Watcher.kt +++ b/src/main/kotlin/de/hoennig/gittally/watcher/Watcher.kt @@ -250,7 +250,16 @@ class Watcher( originBranches: List, workingDir: Path, ) { - repository.prune(originBranches, config.artifacts.retentionPerBranch, config.artifacts.keepLatestGreen) + val retentionCutoff = + config.artifacts.retentionMaxAge + .takeIf { it.isNotBlank() } + ?.let { clock.instant().minus(DurationParser.parse(it)) } + repository.prune( + originBranches, + config.artifacts.retentionPerBranch, + config.artifacts.keepLatestGreen, + retentionCutoff, + ) artifactStore.prune(repository.history()) pruneWorktrees(originBranches, workingDir) } diff --git a/src/test/kotlin/de/hoennig/gittally/build/FileBuildResultRepositoryTest.kt b/src/test/kotlin/de/hoennig/gittally/build/FileBuildResultRepositoryTest.kt index 9c55bbf..2a0aad4 100644 --- a/src/test/kotlin/de/hoennig/gittally/build/FileBuildResultRepositoryTest.kt +++ b/src/test/kotlin/de/hoennig/gittally/build/FileBuildResultRepositoryTest.kt @@ -220,6 +220,90 @@ class FileBuildResultRepositoryTest : FunSpec() { ) } + test("prune drops entries older than the retention cutoff even within the retention count") { + val repository = FileBuildResultRepository(newFile()) + repository.append(result(branch = "main", status = BuildStatus.FAILED, startedOffsetSeconds = 0)) + repository.append(result(branch = "main", status = BuildStatus.FAILED, startedOffsetSeconds = 60)) + repository.append(result(branch = "main", status = BuildStatus.FAILED, startedOffsetSeconds = 120)) + + val removed = + repository.prune( + originBranches = listOf("main"), + retentionPerBranch = 3, + retentionCutoff = baseTime.plusSeconds(90), + ) + + removed shouldContainExactlyInAnyOrder + listOf( + result(branch = "main", status = BuildStatus.FAILED, startedOffsetSeconds = 0), + result(branch = "main", status = BuildStatus.FAILED, startedOffsetSeconds = 60), + ) + repository.history() shouldContainExactly + listOf(result(branch = "main", status = BuildStatus.FAILED, startedOffsetSeconds = 120)) + } + + test("prune never age-prunes a branch's newest entry") { + val repository = FileBuildResultRepository(newFile()) + repository.append(result(branch = "main", status = BuildStatus.FAILED, startedOffsetSeconds = 0)) + + val removed = + repository.prune( + originBranches = listOf("main"), + retentionPerBranch = 3, + retentionCutoff = baseTime.plusSeconds(300), + ) + + removed.shouldBeEmpty() + repository.history() shouldContainExactly + listOf(result(branch = "main", status = BuildStatus.FAILED, startedOffsetSeconds = 0)) + } + + test("prune applies the retention count and cutoff as independent limits") { + val repository = FileBuildResultRepository(newFile()) + repository.append(result(branch = "main", status = BuildStatus.FAILED, startedOffsetSeconds = 0)) + repository.append(result(branch = "main", status = BuildStatus.FAILED, startedOffsetSeconds = 60)) + repository.append(result(branch = "main", status = BuildStatus.FAILED, startedOffsetSeconds = 120)) + + val removed = + repository.prune( + originBranches = listOf("main"), + // the count drops the entry at 0, the cutoff drops the entry at 60 + retentionPerBranch = 2, + retentionCutoff = baseTime.plusSeconds(90), + ) + + removed shouldContainExactlyInAnyOrder + listOf( + result(branch = "main", status = BuildStatus.FAILED, startedOffsetSeconds = 0), + result(branch = "main", status = BuildStatus.FAILED, startedOffsetSeconds = 60), + ) + repository.history() shouldContainExactly + listOf(result(branch = "main", status = BuildStatus.FAILED, startedOffsetSeconds = 120)) + } + + test("prune with keepLatestGreen keeps the newest green build beyond the retention cutoff") { + val repository = FileBuildResultRepository(newFile()) + repository.append(result(branch = "main", status = BuildStatus.SUCCESS, startedOffsetSeconds = 0)) + repository.append(result(branch = "main", status = BuildStatus.FAILED, startedOffsetSeconds = 60)) + repository.append(result(branch = "main", status = BuildStatus.FAILED, startedOffsetSeconds = 120)) + + val removed = + repository.prune( + originBranches = listOf("main"), + retentionPerBranch = 3, + keepLatestGreen = true, + retentionCutoff = baseTime.plusSeconds(90), + ) + + removed shouldContainExactly + listOf(result(branch = "main", status = BuildStatus.FAILED, startedOffsetSeconds = 60)) + repository.history() shouldContainExactly + listOf( + result(branch = "main", status = BuildStatus.FAILED, startedOffsetSeconds = 120), + result(branch = "main", status = BuildStatus.SUCCESS, startedOffsetSeconds = 0), + ) + } + test("prune with keepLatestGreen keeps the newest green build beyond the retention count") { val repository = FileBuildResultRepository(newFile()) repository.append(result(branch = "main", status = BuildStatus.SUCCESS, startedOffsetSeconds = 0)) diff --git a/src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt b/src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt index d948263..e447be2 100644 --- a/src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt +++ b/src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt @@ -420,6 +420,18 @@ class WatcherTest : FunSpec() { dropping.repository.history().map { it.status } shouldContainExactly listOf(BuildStatus.FAILED) } + test("poll drops builds older than retentionMaxAge but keeps the branch's newest build") { + // seeds start one hour before the fixed clock, so a 30m age limit cuts them off + val harness = Harness(GitTallyConfig(artifacts = ArtifactsConfig(retentionMaxAge = "30m"))) + harness.seed("main", BuildStatus.FAILED, commit = "commit-1") + harness.seed("main", BuildStatus.FAILED, commit = "commit-2") + every { harness.gitService.originBranches(any()) } returns listOf("main") + + harness.watcher.poll(harness.workingDir) + + harness.repository.history().map { it.commit } shouldContainExactly listOf("commit-2") + } + test("worktrees of queued or running builds are never pruned") { val harness = Harness() harness.seed("busy", BuildStatus.RUNNING, commit = "commit-1")