Skip to content

feat: cascade delete all children on pipeline delete#1112

Open
ishtoo1 wants to merge 1 commit into
feat/cascade-delete-kill-prsfrom
feat/cascade-delete-children
Open

feat: cascade delete all children on pipeline delete#1112
ishtoo1 wants to merge 1 commit into
feat/cascade-delete-kill-prsfrom
feat/cascade-delete-children

Conversation

@ishtoo1
Copy link
Copy Markdown
Contributor

@ishtoo1 ishtoo1 commented Apr 22, 2026

What type of PR is this? (check all applicable)

  • Feature

What changed?

  • After all TRs/PRs reach a terminal state, invoke DeleteAllTriggerRuns + DeleteAllPipelineRuns (fatal errors requeue).
  • Wire revision.Manager into the Pipeline Reconciler (defaults to revision.NewNoOpManager()) and call DeleteAllRevisions(ns, name, \"Pipeline\") after children are removed. Mirrors the deployment controller's pattern.
  • Best-effort revision cleanup — log but don't block finalizer removal.
  • Remove the finalizer only after all child CRs are deleted.
  • Add TestCascadeDelete_AllTerminal, TestCascadeDelete_RevisionsCleaned, and TestCascadeDelete_SkippedPRsTerminal.

Why?
Addressing #1091.
Users running ma pipeline delete previously had to manually kill active TriggerRuns/PipelineRuns, wait for terminal states, and delete each child CR one by one. This PR completes the controller-side cascade so the delete is a single atomic operation from the user's perspective.

How did you test it?

  • bazel test //go/components/pipeline/... — all new regression tests pass.

  • Manually against a pipeline with terminal children: ma pipeline delete removes every child CR before the pipeline CR disappears.

  • End-to-end behavior of the full cascade-delete stack is verified on a sandbox cluster; results are attached on docs: document pipeline cascade delete behavior #1114.

Potential risks

  • ma pipeline delete <pipeline> now cascades to all child TriggerRuns, PipelineRuns, and Revisions belonging to the pipeline. Every child CR is deleted. No undo.
  • Deletion is asynchronous. Tooling that polled for the pipeline CR to disappear after delete will continue to work, but anything that assumed the pipeline was gone as soon as ma pipeline delete returned will observe Terminating for as long as the cascade runs.
  • External tooling that previously issued its own cleanup for TRs/PRs/Revisions can be simplified. Double-delete is safe (IsNotFound is tolerated).

Release notes
⚠ Breaking: ma pipeline delete now cascades to all TriggerRuns, PipelineRuns, and Revisions belonging to the pipeline. Deletion is asynchronous — the pipeline CR remains in Terminating until the controller finishes the cascade.

Documentation Changes
User-facing documentation lands in feat/cascade-delete-docs.


Stacked on top of #1111.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 22, 2026

CLA assistant check
All committers have signed the CLA.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 23, 2026

Go Coverage Report (Bazel)

Total Coverage: 63.4%

Coverage Policy:

  • Baseline (existing code): ≥60% (current coverage)
  • New/changed code: ≥90% ✅ STRICTLY ENFORCED
  • Long-term goal: Improve baseline to 90%

View detailed HTML report in artifacts

@ishtoo1 ishtoo1 force-pushed the feat/cascade-delete-kill-prs branch from cece21f to c42691b Compare April 23, 2026 20:55
@ishtoo1 ishtoo1 force-pushed the feat/cascade-delete-children branch from c2f628c to a97b06a Compare April 23, 2026 20:55
@ishtoo1 ishtoo1 force-pushed the feat/cascade-delete-kill-prs branch from c42691b to 424965e Compare April 23, 2026 22:01
@ishtoo1 ishtoo1 force-pushed the feat/cascade-delete-children branch from a97b06a to 17a9b4c Compare April 23, 2026 22:01
@ishtoo1 ishtoo1 force-pushed the feat/cascade-delete-kill-prs branch from 424965e to 6c465c0 Compare April 23, 2026 22:54
@ishtoo1 ishtoo1 force-pushed the feat/cascade-delete-children branch from 17a9b4c to 1a4d392 Compare April 23, 2026 22:54
@ishtoo1 ishtoo1 force-pushed the feat/cascade-delete-kill-prs branch from 6c465c0 to 1404e77 Compare April 24, 2026 22:09
@ishtoo1 ishtoo1 force-pushed the feat/cascade-delete-children branch from 1a4d392 to a7ac07c Compare April 24, 2026 22:10
@ishtoo1 ishtoo1 force-pushed the feat/cascade-delete-kill-prs branch from 1404e77 to 8276079 Compare April 25, 2026 03:50
@ishtoo1 ishtoo1 force-pushed the feat/cascade-delete-children branch from a7ac07c to 1abceee Compare April 25, 2026 03:50
What:
- Replace placeholder with DeleteAllTriggerRuns + DeleteAllPipelineRuns calls
  (fatal errors cause requeue, best-effort revision cleanup follows).
- Wire revision.Manager into the Pipeline Reconciler (defaults to NoOpManager)
  and invoke DeleteAllRevisions after children are deleted, mirroring the
  deployment controller's pattern.
- Remove finalizer only after all children successfully deleted.
- Add TestCascadeDelete_AllTerminal, TestCascadeDelete_RevisionsCleaned, and
  TestCascadeDelete_SkippedPRsTerminal tests.

Why:
- Users running `ma pipeline delete` previously had to manually kill active
  TriggerRuns/PipelineRuns, wait for terminal states, and delete each child
  CR one by one. Now the controller does this transparently when a Pipeline
  CR is marked for deletion.

How to test:
- bazel test //go/components/pipeline/... (all new regression tests pass).
- Manually: `ma pipeline delete` against a pipeline with terminal children and
  observe that child CRs are removed before the pipeline CR disappears.

Breaking changes:
- `ma pipeline delete <pipeline>` now cascades to all child TriggerRuns,
  PipelineRuns, and Revisions belonging to the pipeline. Every child CR is
  deleted along with the pipeline. There is no undo.
- Deletion is asynchronous (finalizer removal happens after cascade completes).
  If you relied on `ma pipeline delete` returning once the Pipeline CR is
  actually gone from etcd, you now need to poll `ma pipeline get` or similar
  until NotFound.
- Any external tooling that previously issued its own cleanup for TRs/PRs/
  Revisions should be simplified: those children are now cleaned up by the
  controller. Double-delete is safe (NotFound is tolerated) but unnecessary.

Co-Authored-By: Oz <oz-agent@warp.dev>
@ishtoo1 ishtoo1 force-pushed the feat/cascade-delete-kill-prs branch from 8276079 to ca5b4c3 Compare April 25, 2026 05:57
@ishtoo1 ishtoo1 force-pushed the feat/cascade-delete-children branch from 1abceee to c248111 Compare April 25, 2026 05:57
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.

2 participants