Skip to content

fix(deploy): give Compose its own wait deadline before the SIGKILL - #95

Merged
owine merged 1 commit into
mainfrom
fix/compose-wait-timeout
Sep 20, 2026
Merged

owine merged 1 commit into
mainfrom
fix/compose-wait-timeout

Conversation

@owine

@owine owine commented Sep 20, 2026 •

Copy link
Copy Markdown
Owner

Replaces #94, which GitHub auto-closed when its base branch (fix/retry-transient-registry-throttles, now merged as #93) was deleted. Same commit, rebased onto main; a closed PR's base cannot be retargeted, so it needed a new number. The review comments on #94 still apply.

The problem

Every up --wait in this workflow is bounded only by the external timeout command. That means the way a slow stack fails is a SIGKILL delivered wherever Compose happens to be — including between the rename and the removal of a recreate, which strands a <shortid>_<service> container still holding the service's fixed container_name.

That isn't hypothetical; the workflow already documents it, in the comment on the step that exists to mop it up:

A compose up --wait that gets SIGKILLed by the per-stack timeout mid-recreate can strand a renamed <shortid>_<service> container that still owns the service's fixed container_name. Left in place it blocks both the next recreate and the rollback job's own compose up with "container name already in use" — a hard failure, not a health one.

So today the cleanup step is load-bearing for damage caused by how the timeout fires.

The change

--wait-timeout at 90% of service-startup-timeout on all five up --wait sites — three in deploy, two in rollback. Compose hits its own deadline first and exits on its own terms, naming the services that never became healthy rather than dying silently mid-operation. The external timeout is demoted to a backstop for a genuinely wedged Compose.

90% rather than a fixed subtrahend so the margin scales with whatever callers configure (300s → 270s) without a second input to keep in sync. Computed in shell because GitHub expressions have no arithmetic operators.

What this does not fix

The cleanup step stays, and should. --wait-timeout bounds the final --wait phase, not the depends_on: service_healthy waits Compose performs mid-sequence. A stack that stalls on a dependency can still burn the whole budget and take the SIGKILL with later services half-recreated. This makes the stranding rare; it does not make it impossible. The code comment says so explicitly so nobody deletes the cleanup step on the strength of this PR.

Verification

  • yamllint --strict clean
  • actionlint clean apart from the pre-existing job.workflow_sha finding (present on main too)
  • shellcheck -x clean over all 16 scripts under scripts/
  • docker compose up --dry-run -d --quiet-pull --wait --wait-timeout 270 --remove-orphans confirmed rc=0 against Compose v5.5.1 on the piwine runner — non-mutating, and its plan happens to show the exact a774a6ece886_network-optimizer rename this guards against

Summary by Sourcery

Give every deployment and rollback Compose update its own scaled wait deadline before the external timeout terminates it.

Bug Fixes:

  • Prevent Compose deployments and rollbacks from being abruptly SIGKILLed during the normal service-wait phase by giving each operation its own deadline.

Enhancements:

  • Apply a Compose wait timeout set to 90% of the configured service startup timeout across all deployment and rollback stack updates, while retaining the external timeout as a backstop.
  • Document the timeout behavior and its remaining limitations in the workflow and project guidance.

Documentation:

  • Document the Compose wait-timeout policy for reusable workflows.

Every `up --wait` is bounded only by the external `timeout` command, so
the way a slow stack fails is a SIGKILL delivered wherever Compose
happens to be. Killed between the rename and the removal of a recreate,
that strands a `<shortid>_<service>` container still holding the
service's fixed `container_name` — which is why "Cleanup failed existing
stacks on failure" exists at all.

Add `--wait-timeout` at 90% of `service-startup-timeout` to all five
`up --wait` sites (three deploy, two rollback). Compose now hits its own
deadline first and exits on its own terms, naming the services that never
became healthy instead of dying silently, and the external `timeout`
becomes a backstop for a genuinely wedged Compose.

This is a mitigation, not a cure, and the cleanup step stays:
`--wait-timeout` bounds the final `--wait` phase, not the `depends_on:
service_healthy` waits Compose performs mid-sequence, so a stack that
stalls on a dependency can still burn the whole budget and take the
SIGKILL with later services half-recreated.

90% rather than a fixed subtrahend so the margin scales with whatever
callers configure (300s -> 270s) without a second input to keep in sync.
Computed in shell because GitHub expressions have no arithmetic
operators.

Verified: yamllint --strict clean; actionlint clean apart from the
pre-existing job.workflow_sha finding; `up --dry-run -d --quiet-pull
--wait --wait-timeout 270 --remove-orphans` confirmed rc=0 against
Compose v5.5.1 on the piwine runner, whose plan shows the very
`<shortid>_<service>` rename this guards.
@sourcery-ai

sourcery-ai Bot commented Sep 20, 2026 •

Copy link
Copy Markdown

Reviewer's Guide

The workflow now computes a 90%-of-budget Compose wait timeout and applies it to every deploy and rollback up --wait, allowing Compose to report unhealthy services and exit cleanly before the external SIGKILL; the external timeout and cleanup safeguards remain for genuinely wedged or mid-sequence operations.

Sequence diagram for Compose wait timeout handling

sequenceDiagram
    participant Workflow
    participant Compose
    participant Cleanup

    Workflow->>Workflow: Compute Compose wait budget
    Workflow->>Compose: docker compose up --wait --wait-timeout COMPOSE_WAIT_TIMEOUT
    alt Compose reaches wait timeout
        Compose-->>Workflow: Exit with unhealthy services
        Workflow->>Cleanup: Run failure cleanup
    else Compose is genuinely wedged
        Workflow->>Compose: External timeout sends SIGKILL
        Workflow->>Cleanup: Remove stranded recreate containers
    else Compose completes in time
        Compose-->>Workflow: Exit successfully
    end
Loading

File-Level Changes

Change Details Files
Add a Compose-level wait deadline before the external timeout can SIGKILL it.
  • Compute a per-job budget at 90% of the configured startup timeout and export it through GITHUB_ENV.
  • Pass --wait-timeout to all three deploy and two rollback docker compose up --wait invocations.
  • Retain the external timeout as a backstop for wedged Compose processes.
.github/workflows/deploy.yml
Document the timeout layering and its remaining cleanup limitations.
  • Explain how Compose's deadline reduces mid-recreate container-name conflicts.
  • Clarify that dependency health waits can still outlive the Compose wait phase and require the existing cleanup step.
  • Record the new --wait-timeout behavior in the workflow documentation.
.github/workflows/deploy.yml
CLAUDE.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've reviewed your changes and they look great!

Sourcery assessment

Needs a human reviewer. This changes production deployment timing and can cause Compose to stop during a recreate, leaving a partially updated stack or causing an outage before the external timeout and cleanup paths run. Reverting prevents future occurrences, but it cannot undo any partial deployment or service disruption that already happened.


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

@owine
owine merged commit 19f7c09 into main Sep 20, 2026
3 checks passed
@owine
owine deleted the fix/compose-wait-timeout branch September 20, 2026 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant