Skip to content

fix(deployments): protect carried-forward containers on compose deploy failure - #517

Open
santhiprakash wants to merge 1 commit into
oblien:mainfrom
santhiprakash:fix/onfailure-carried-forward-containers
Open

fix(deployments): protect carried-forward containers on compose deploy failure#517
santhiprakash wants to merge 1 commit into
oblien:mainfrom
santhiprakash:fix/onfailure-carried-forward-containers

Conversation

@santhiprakash

Copy link
Copy Markdown
Contributor

Summary

Fixes #500onFailure no longer destroys containers carried verbatim from the still-live previous release when a multi-service compose deploy fails partway through.

Problem

On a compose project, a redeploy that carries unchanged services forward and then fails on a later step runs onFailure, which iterated every service row on the failed deployment and called runtime.destroy on each containerId. Carried-forward rows name the same container as the active release, so a failed redeploy took the live app down while activeDeploymentId still pointed at the previous release.

Triage / Root cause

onFailure in deployment-lifecycle.ts used a naive destroy loop over repos.service.listByDeployment(dep.id). Reject, delete, and build-cancel teardown paths already consult collectDeploymentManifest(..., { protectRetained: true }) to skip artifacts a live or retained release still references; the failure path was the lone outlier.

Fix

  • Replace the per-row runtime.destroy loop with collectDeploymentManifest + executeCleanup using protectRetained: true, matching cancel/reject/delete.
  • Add on-failure-keep-set.test.ts regression coverage modeled on reject-keep-set.test.ts.

Verification

cd apps/api && bun run test test/modules/deployments/on-failure-keep-set.test.ts test/modules/deployments/reject-keep-set.test.ts

Result: 9 tests passed.

Notes / Risks

  • Scope is limited to the failure teardown path; onCancelled still uses the old destroy loop (separate issue if needed).
  • provisioned.imageRef cleanup at the top of onFailure is unchanged; compose pipeline already cleans built service images before calling onFailure.

…y failure

- Problem: onFailure destroyed every service container on the failed
  deployment's rows, including containers carried verbatim from the still-live
  previous release, taking the app down while activeDeploymentId stayed put.
- Fix: tear down failed-deploy resources via collectDeploymentManifest with
  protectRetained: true, matching reject/delete/cancel teardown paths.
- Verification: bun run test test/modules/deployments/on-failure-keep-set.test.ts
  test/modules/deployments/reject-keep-set.test.ts (9 passed).

Fixes oblien#500
@santhiprakash
santhiprakash force-pushed the fix/onfailure-carried-forward-containers branch from ef438dd to dfa10ed Compare August 15, 2026 04:59
@santhiprakash

Copy link
Copy Markdown
Contributor Author

Rebased onto current main and re-ran the tests locally: on-failure-keep-set.test.ts (3 passed) and reject-keep-set.test.ts (6 passed).

The Typecheck/Test CI failures on this PR are pre-existing on main itself — apps/api/src/lib/auth.ts at lines 418–428 contains unresolved merge conflict markers (from the patch v0.6.6 commit, a52e2566), and the same checks fail on upstream main right now. This PR doesn't touch that file.

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.

onFailure destroys carried-forward (still-live) containers when a compose deploy fails — a failed redeploy can take down the running app

1 participant