Record builds interrupted by a server shutdown as INTERRUPTED, not FAILED
A systemd stop killed the running build process and BuildExecutor classified the death as FAILED, posting a red Gitea status; FAILED is not restartable, so the startup recovery never re-enqueued the build. A ContextClosedEvent listener now sets a shuttingDown flag, terminates the process trees of executing builds, and drains until their INTERRUPTED results are persisted. Queued builds stay PENDING without starting a process; recovery re-enqueues both after the restart. INTERRUPTED publishes as Gitea state "pending" instead of "failure", since the build is going to be re-run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
fbdc6f54b7
commit
40f54a1298
@@ -5,6 +5,8 @@ import de.hoennig.gittally.config.ConfigLoader
|
||||
import de.hoennig.gittally.gitea.GiteaClient
|
||||
import org.slf4j.LoggerFactory
|
||||
import org.springframework.context.ApplicationEventPublisher
|
||||
import org.springframework.context.event.ContextClosedEvent
|
||||
import org.springframework.context.event.EventListener
|
||||
import org.springframework.stereotype.Service
|
||||
import java.io.IOException
|
||||
import java.io.InputStream
|
||||
@@ -51,6 +53,9 @@ class BuildExecutor(
|
||||
@Volatile
|
||||
private var slots: Semaphore? = null
|
||||
|
||||
/** Set on context close; in-flight builds then finish as INTERRUPTED instead of FAILED. */
|
||||
private val shuttingDown = AtomicBoolean(false)
|
||||
|
||||
/** The builds currently executing, newest last (queued builds are PENDING in the repository). */
|
||||
fun currentBuilds(): List<RunningBuild> = builds.values.filter { it.running }.map { it.runningBuild }
|
||||
|
||||
@@ -108,9 +113,37 @@ class BuildExecutor(
|
||||
return true
|
||||
}
|
||||
|
||||
/**
|
||||
* Runs when the application context closes (e.g. systemd SIGTERM): terminates the
|
||||
* process trees of all executing builds and waits (bounded) until their INTERRUPTED
|
||||
* results are persisted, so a shutdown is never recorded as a build failure.
|
||||
* Builds still queued stay PENDING; the watcher's startup recovery re-enqueues
|
||||
* both PENDING and INTERRUPTED builds after the restart.
|
||||
*/
|
||||
@EventListener(ContextClosedEvent::class)
|
||||
fun shutdown() {
|
||||
if (!shuttingDown.compareAndSet(false, true)) {
|
||||
return
|
||||
}
|
||||
val executing = builds.values.filter { it.running }
|
||||
if (executing.isEmpty()) {
|
||||
return
|
||||
}
|
||||
log.info("shutdown requested; interrupting {} executing build(s)", executing.size)
|
||||
executing.forEach { build -> build.process?.let { destroyProcessTree(it) } }
|
||||
val deadline = System.nanoTime() + Duration.ofMillis(SHUTDOWN_DRAIN_TIMEOUT_MILLIS).toNanos()
|
||||
while (executing.any { builds.containsKey(it.runningBuild.artifactKey) } && System.nanoTime() < deadline) {
|
||||
Thread.sleep(50)
|
||||
}
|
||||
executing
|
||||
.filter { builds.containsKey(it.runningBuild.artifactKey) }
|
||||
.forEach { log.warn("build of branch {} was not recorded as interrupted in time", it.runningBuild.branch) }
|
||||
}
|
||||
|
||||
private fun execute(build: ActiveBuild) {
|
||||
var slot: Semaphore? = null
|
||||
var finalStatus = BuildStatus.FAILED
|
||||
// null keeps the persisted status untouched (a queued build stays PENDING over a shutdown)
|
||||
var finalStatus: BuildStatus? = BuildStatus.FAILED
|
||||
var workspace: Path? = null
|
||||
try {
|
||||
slot = slotsFor(build.workingDir)
|
||||
@@ -119,6 +152,10 @@ class BuildExecutor(
|
||||
finalStatus = BuildStatus.CANCELLED
|
||||
return
|
||||
}
|
||||
if (shuttingDown.get()) {
|
||||
finalStatus = null
|
||||
return
|
||||
}
|
||||
build.running = true
|
||||
build.runningBuild.runningSince = Instant.now()
|
||||
transition(build, BuildStatus.RUNNING, duration = null)
|
||||
@@ -134,20 +171,34 @@ class BuildExecutor(
|
||||
when {
|
||||
build.cancelled.get() -> BuildStatus.CANCELLED
|
||||
exitCode == 0 -> BuildStatus.SUCCESS
|
||||
shuttingDown.get() -> BuildStatus.INTERRUPTED
|
||||
else -> BuildStatus.FAILED
|
||||
}
|
||||
} catch (e: Exception) {
|
||||
finalStatus = if (build.cancelled.get()) BuildStatus.CANCELLED else BuildStatus.FAILED
|
||||
log.error("build of branch {} crashed", build.runningBuild.branch, e)
|
||||
appendToLiveLog(build, "\nbuild crashed: ${e.message}\n")
|
||||
finalStatus =
|
||||
when {
|
||||
build.cancelled.get() -> BuildStatus.CANCELLED
|
||||
shuttingDown.get() -> BuildStatus.INTERRUPTED
|
||||
else -> BuildStatus.FAILED
|
||||
}
|
||||
if (finalStatus == BuildStatus.FAILED) {
|
||||
log.error("build of branch {} crashed", build.runningBuild.branch, e)
|
||||
appendToLiveLog(build, "\nbuild crashed: ${e.message}\n")
|
||||
}
|
||||
} finally {
|
||||
// pure build time, without the queue wait; null when the build never started executing
|
||||
val duration = build.runningBuild.runningSince?.let { Duration.between(it, Instant.now()) }
|
||||
val result = transition(build, finalStatus, duration)
|
||||
try {
|
||||
artifactStore.persist(result, build.runningBuild.stagingDir, workspace)
|
||||
} catch (e: Exception) {
|
||||
log.warn("could not persist artifacts of {}: {}", result.artifactKey, e.message)
|
||||
if (finalStatus == BuildStatus.INTERRUPTED) {
|
||||
log.info("build of branch {} interrupted by shutdown", build.runningBuild.branch)
|
||||
appendToLiveLog(build, "\nbuild interrupted by shutdown\n")
|
||||
}
|
||||
if (finalStatus != null) {
|
||||
// pure build time, without the queue wait; null when the build never started executing
|
||||
val duration = build.runningBuild.runningSince?.let { Duration.between(it, Instant.now()) }
|
||||
val result = transition(build, finalStatus, duration)
|
||||
try {
|
||||
artifactStore.persist(result, build.runningBuild.stagingDir, workspace)
|
||||
} catch (e: Exception) {
|
||||
log.warn("could not persist artifacts of {}: {}", result.artifactKey, e.message)
|
||||
}
|
||||
}
|
||||
builds.remove(build.runningBuild.artifactKey)
|
||||
slot?.release()
|
||||
@@ -193,7 +244,7 @@ class BuildExecutor(
|
||||
return cleanExitCode
|
||||
}
|
||||
}
|
||||
if (build.cancelled.get()) {
|
||||
if (build.cancelled.get() || shuttingDown.get()) {
|
||||
return CANCELLED_EXIT_CODE
|
||||
}
|
||||
return runCommand(build, branchConfig, branchConfig.buildCommand, workspace, stdoutLog, stderrLog, liveLog)
|
||||
@@ -220,15 +271,15 @@ class BuildExecutor(
|
||||
branchConfig = branchConfig,
|
||||
onAuxProcess = { aux ->
|
||||
// preparation phases (e.g. a Docker image build) must die on cancellation
|
||||
// too, otherwise a cancelled build blocks its slot until they finish
|
||||
// and shutdown too, otherwise they block their slot until they finish
|
||||
build.process = aux
|
||||
if (build.cancelled.get()) {
|
||||
if (build.cancelled.get() || shuttingDown.get()) {
|
||||
destroyProcessTree(aux)
|
||||
}
|
||||
},
|
||||
)
|
||||
build.process = process
|
||||
if (build.cancelled.get()) {
|
||||
if (build.cancelled.get() || shuttingDown.get()) {
|
||||
destroyProcessTree(process)
|
||||
}
|
||||
val stdoutPump = pump(process.inputStream, stdoutLog, liveLog)
|
||||
@@ -409,5 +460,6 @@ class BuildExecutor(
|
||||
const val LIVE_LOG_FILE = "build.log"
|
||||
private const val CANCELLED_EXIT_CODE = 130
|
||||
private const val PUMP_DRAIN_TIMEOUT_MILLIS = 10_000L
|
||||
private const val SHUTDOWN_DRAIN_TIMEOUT_MILLIS = 20_000L
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,12 +2,17 @@ package de.hoennig.gittally.gitea
|
||||
|
||||
import de.hoennig.gittally.build.BuildStatus
|
||||
|
||||
/** Gitea commit-status state published for this build status. */
|
||||
/**
|
||||
* Gitea commit-status state published for this build status.
|
||||
* INTERRUPTED publishes as `pending`, not `failure`: an interrupted build (e.g. a
|
||||
* server shutdown) is re-enqueued by the startup recovery, so the commit must not
|
||||
* turn red in between.
|
||||
*/
|
||||
fun BuildStatus.toGiteaState(): String =
|
||||
when (this) {
|
||||
BuildStatus.SUCCESS -> "success"
|
||||
BuildStatus.FAILED, BuildStatus.INTERRUPTED, BuildStatus.CANCELLED -> "failure"
|
||||
BuildStatus.PENDING, BuildStatus.RUNNING -> "pending"
|
||||
BuildStatus.FAILED, BuildStatus.CANCELLED -> "failure"
|
||||
BuildStatus.PENDING, BuildStatus.RUNNING, BuildStatus.INTERRUPTED -> "pending"
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -292,6 +292,74 @@ class BuildExecutorTest : FunSpec() {
|
||||
}
|
||||
}
|
||||
|
||||
test("shutdown kills an executing build and records INTERRUPTED, not FAILED") {
|
||||
val h = harness("echo \$\$ > pid-file; sleep 30")
|
||||
|
||||
val build = h.executor.startBuild("main", "abc123", h.workingDir)
|
||||
eventually(10.seconds) {
|
||||
Files.exists(h.workingDir.resolve("pid-file")).shouldBeTrue()
|
||||
}
|
||||
val pid =
|
||||
Files
|
||||
.readString(h.workingDir.resolve("pid-file"))
|
||||
.trim()
|
||||
.toLong()
|
||||
|
||||
h.executor.shutdown()
|
||||
|
||||
// shutdown() drains synchronously, so the result is already persisted
|
||||
h.repository.latestFor("main")?.status shouldBe BuildStatus.INTERRUPTED
|
||||
h.executor.currentBuilds().shouldBeEmpty()
|
||||
ProcessHandle.of(pid).map { it.isAlive }.orElse(false) shouldBe false
|
||||
h.events.map { it.result.status } shouldContainExactly
|
||||
listOf(BuildStatus.PENDING, BuildStatus.RUNNING, BuildStatus.INTERRUPTED)
|
||||
Files.readString(build.liveLogFile) shouldContain "build interrupted by shutdown"
|
||||
verify { h.giteaClient.publishStatus("abc123", BuildStatus.INTERRUPTED, any(), null, h.workingDir) }
|
||||
verify { h.artifactStore.persist(match { it.status == BuildStatus.INTERRUPTED }, build.stagingDir, any()) }
|
||||
}
|
||||
|
||||
test("a build still queued at shutdown stays PENDING for the startup recovery") {
|
||||
val h = harness("sleep 30")
|
||||
|
||||
val first = h.executor.startBuild("main", "sha-1", h.workingDir)
|
||||
val second = h.executor.startBuild("main", "sha-2", h.workingDir)
|
||||
eventually(10.seconds) {
|
||||
h.repository
|
||||
.history()
|
||||
.first { it.artifactKey == first.artifactKey }
|
||||
.status shouldBe BuildStatus.RUNNING
|
||||
}
|
||||
|
||||
h.executor.shutdown()
|
||||
|
||||
h.repository
|
||||
.history()
|
||||
.first { it.artifactKey == first.artifactKey }
|
||||
.status shouldBe BuildStatus.INTERRUPTED
|
||||
// the branch worker drops the queued build without a transition or a process
|
||||
awaitIdle(h)
|
||||
h.repository
|
||||
.history()
|
||||
.first { it.artifactKey == second.artifactKey }
|
||||
.status shouldBe BuildStatus.PENDING
|
||||
h.events
|
||||
.filter { it.result.artifactKey == second.artifactKey }
|
||||
.map { it.result.status } shouldContainExactly listOf(BuildStatus.PENDING)
|
||||
verify(exactly = 0) { h.giteaClient.publishStatus("sha-2", BuildStatus.RUNNING, any(), any(), any()) }
|
||||
}
|
||||
|
||||
test("shutdown without any build in flight is a no-op") {
|
||||
val h = harness("echo ok")
|
||||
|
||||
h.executor.startBuild("main", "abc123", h.workingDir)
|
||||
awaitStatus(h, "main", BuildStatus.SUCCESS)
|
||||
awaitIdle(h)
|
||||
|
||||
h.executor.shutdown()
|
||||
|
||||
h.repository.latestFor("main")?.status shouldBe BuildStatus.SUCCESS
|
||||
}
|
||||
|
||||
test("cancel with an unknown artifact key returns false") {
|
||||
val h = harness("echo ok")
|
||||
|
||||
|
||||
@@ -121,10 +121,11 @@ class GiteaClientTest :
|
||||
mapOf(
|
||||
BuildStatus.SUCCESS to "success",
|
||||
BuildStatus.FAILED to "failure",
|
||||
BuildStatus.INTERRUPTED to "failure",
|
||||
BuildStatus.CANCELLED to "failure",
|
||||
BuildStatus.PENDING to "pending",
|
||||
BuildStatus.RUNNING to "pending",
|
||||
// interrupted builds are re-enqueued on startup, so the commit must not turn red
|
||||
BuildStatus.INTERRUPTED to "pending",
|
||||
)
|
||||
|
||||
expectedStates.forEach { (status, state) ->
|
||||
|
||||
@@ -11,10 +11,11 @@ class GiteaStateMappingTest :
|
||||
test("maps each build status to its Gitea state") {
|
||||
BuildStatus.SUCCESS.toGiteaState() shouldBe "success"
|
||||
BuildStatus.FAILED.toGiteaState() shouldBe "failure"
|
||||
BuildStatus.INTERRUPTED.toGiteaState() shouldBe "failure"
|
||||
BuildStatus.CANCELLED.toGiteaState() shouldBe "failure"
|
||||
BuildStatus.PENDING.toGiteaState() shouldBe "pending"
|
||||
BuildStatus.RUNNING.toGiteaState() shouldBe "pending"
|
||||
// interrupted builds are re-enqueued on startup, so the commit must not turn red
|
||||
BuildStatus.INTERRUPTED.toGiteaState() shouldBe "pending"
|
||||
}
|
||||
|
||||
test("maps Gitea states back to build statuses") {
|
||||
|
||||
Reference in New Issue
Block a user