From 676bf4a1049d55e48e1bc239fc742b3d3780a284 Mon Sep 17 00:00:00 2001 From: tmp Date: Mon, 31 Aug 2026 03:53:32 +0000 Subject: [PATCH 1/2] fix(up): enforce healthcheck.timeout and treat start_period as a grace window --- .../Commands/ComposeUp.swift | 43 +++++++++--- .../HealthcheckTimeoutTests.swift | 69 +++++++++++++++++++ 2 files changed, 103 insertions(+), 9 deletions(-) create mode 100644 Tests/Container-Compose-StaticTests/HealthcheckTimeoutTests.swift diff --git a/Sources/Container-Compose/Commands/ComposeUp.swift b/Sources/Container-Compose/Commands/ComposeUp.swift index d42efd7..b158212 100644 --- a/Sources/Container-Compose/Commands/ComposeUp.swift +++ b/Sources/Container-Compose/Commands/ComposeUp.swift @@ -1253,24 +1253,38 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { let retries = max(healthcheck.retries ?? 3, 1) let interval = Healthcheck.parseDuration(healthcheck.interval, default: 30) let startPeriod = Healthcheck.parseDuration(healthcheck.start_period, default: 0) + let timeout = Healthcheck.parseDuration(healthcheck.timeout, default: 30) - if startPeriod > 0 { - try await Task.sleep(nanoseconds: UInt64(startPeriod * 1_000_000_000)) - } - - for attempt in 1...retries { - let exitCode = try await streamCommand( + @Sendable func probeSucceeded() async throws -> Bool { + try await streamCommand( "container", args: ["exec", containerName] + execArguments, + timeout: timeout, onStdout: { _ in }, onStderr: { _ in } - ) - if exitCode == 0 { + ) == 0 + } + + // Compose `start_period` is a grace window, not a delay: probes run + // during it and their failures do not consume the retry budget, and the + // first success ends the wait immediately. + if startPeriod > 0 { + let deadline = ContinuousClock.now.advanced(by: .seconds(startPeriod)) + while ContinuousClock.now < deadline { + if try await probeSucceeded() { + return + } + try await Task.sleep(for: .seconds(interval)) + } + } + + for attempt in 1...retries { + if try await probeSucceeded() { return } if attempt < retries { - try await Task.sleep(nanoseconds: UInt64(interval * 1_000_000_000)) + try await Task.sleep(for: .seconds(interval)) } } @@ -1481,6 +1495,7 @@ extension ComposeUp { func streamCommand( _ command: String, args: [String] = [], + timeout: TimeInterval? = nil, onStdout: @escaping (@Sendable (String) -> Void), onStderr: @escaping (@Sendable (String) -> Void) ) async throws -> Int32 { @@ -1526,6 +1541,16 @@ extension ComposeUp { do { try process.run() + if let timeout, timeout > 0 { + // Docker treats a check that overruns `timeout` as one failed + // attempt, not as a hang. Terminating the child makes the + // termination handler fire with a non-zero status, so the + // caller sees a normal failure. + DispatchQueue.global().asyncAfter(deadline: .now() + timeout) { [weak process] in + guard let process, process.isRunning else { return } + process.terminate() + } + } } catch { continuation.resume(throwing: error) } diff --git a/Tests/Container-Compose-StaticTests/HealthcheckTimeoutTests.swift b/Tests/Container-Compose-StaticTests/HealthcheckTimeoutTests.swift new file mode 100644 index 0000000..5b6c462 --- /dev/null +++ b/Tests/Container-Compose-StaticTests/HealthcheckTimeoutTests.swift @@ -0,0 +1,69 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2025 Morris Richman and the Container-Compose project authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +import Testing +import Foundation +@testable import ContainerComposeCore + +@Suite("Healthcheck Timeout Tests") +struct HealthcheckTimeoutTests { + + /// `healthcheck.timeout` is only useful if an overrunning probe is actually + /// killed. Without the timeout the child below runs for its full 30s and + /// exits 0, so this test fails on both counts. + @Test("streamCommand terminates a child that overruns its timeout") + func streamCommandHonoursTimeout() async throws { + let composeUp = try ComposeUp.parse(["-d", "--cwd", NSTemporaryDirectory()]) + + let start = ContinuousClock.now + let exitCode = try await composeUp.streamCommand( + "sleep", + args: ["30"], + timeout: 1, + onStdout: { _ in }, + onStderr: { _ in } + ) + let elapsed = ContinuousClock.now - start + + // Generous window: the point is "well under 30s", not millisecond accuracy. + #expect(elapsed < .seconds(15)) + #expect(exitCode != 0) + } + + /// A probe that finishes inside its timeout must be reported verbatim. + @Test("streamCommand leaves a fast child alone") + func streamCommandDoesNotKillFastChild() async throws { + let composeUp = try ComposeUp.parse(["-d", "--cwd", NSTemporaryDirectory()]) + + let exitCode = try await composeUp.streamCommand( + "true", + args: [], + timeout: 10, + onStdout: { _ in }, + onStderr: { _ in } + ) + + #expect(exitCode == 0) + } + + /// `timeout` has a Compose default of 30s; an omitted value must not be + /// read as "no timeout". + @Test("Omitted healthcheck timeout falls back to the Compose default") + func omittedTimeoutUsesDefault() throws { + let healthcheck = Healthcheck(test: ["CMD", "true"]) + #expect(Healthcheck.parseDuration(healthcheck.timeout, default: 30) == 30) + } +} From 94f7f69abc314f0c28a57da6360b877f76000d18 Mon Sep 17 00:00:00 2001 From: Mikimoto Date: Wed, 9 Sep 2026 03:29:13 +0000 Subject: [PATCH 2/2] fix(up): escalate a timed-out healthcheck probe to SIGKILL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The probe timeout terminated the `container exec` child with SIGTERM and assumed it would die. It does not: `container exec` ignores SIGTERM while the process it proxies is stuck. Measured against pgadmin's own healthcheck — the exec survived `timeout 25` untouched and only ended on SIGKILL (exit 137). The consequence was worse than a slow probe. `Process.terminationHandler` never fires, so the continuation never resumes, so `waitUntilServiceIsHealthy` never returns and the whole `up` stalls indefinitely, silently, with no error and no further output. Two runs died this way before the cause was found; both looked like a hung image pull. Escalate two seconds after the SIGTERM. A probe that overruns its timeout is then one failed attempt, which is what Docker does and what the retry loop below already assumes. Untested by the suite: this path needs a live daemon, as does the timeout it fixes. Verified by hand against the exec that reproduced it. --- Sources/Container-Compose/Commands/ComposeUp.swift | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Sources/Container-Compose/Commands/ComposeUp.swift b/Sources/Container-Compose/Commands/ComposeUp.swift index b158212..915ae9a 100644 --- a/Sources/Container-Compose/Commands/ComposeUp.swift +++ b/Sources/Container-Compose/Commands/ComposeUp.swift @@ -1549,6 +1549,16 @@ extension ComposeUp { DispatchQueue.global().asyncAfter(deadline: .now() + timeout) { [weak process] in guard let process, process.isRunning else { return } process.terminate() + // `container exec` ignores SIGTERM while the process it + // is proxying is stuck — measured: a probe that hangs + // survives `timeout 25` and only dies to SIGKILL (137). + // Without escalating, the termination handler never + // fires, the continuation never resumes, and `up` stalls + // forever instead of recording one failed attempt. + DispatchQueue.global().asyncAfter(deadline: .now() + 2) { [weak process] in + guard let process, process.isRunning else { return } + kill(process.processIdentifier, SIGKILL) + } } } } catch {