Skip to content

Commit 17a9b4c

Browse files
ishtmeet.singhoz-agent
authored andcommitted
feat: cascade delete all children on pipeline delete
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>
1 parent 424965e commit 17a9b4c

3 files changed

Lines changed: 343 additions & 11 deletions

File tree

go/components/pipeline/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ go_library(
1313
"//go/api:go_default_library",
1414
"//go/api/handler:go_default_library",
1515
"//go/base/env:go_default_library",
16+
"//go/base/revision:go_default_library",
1617
"//go/components/pipelinerun:go_default_library",
1718
"//go/components/triggerrun:go_default_library",
1819
"//proto/api:go_default_library",
@@ -37,6 +38,7 @@ go_test(
3738
"//go/api:go_default_library",
3839
"//go/api/handler:go_default_library",
3940
"//go/base/env:go_default_library",
41+
"//go/base/revision:go_default_library",
4042
"//go/components/pipelinerun:go_default_library",
4143
"//go/components/triggerrun:go_default_library",
4244
"//proto/api:go_default_library",

go/components/pipeline/controller.go

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"github.com/michelangelo-ai/michelangelo/go/api"
2222
apiHandler "github.com/michelangelo-ai/michelangelo/go/api/handler"
2323
"github.com/michelangelo-ai/michelangelo/go/base/env"
24+
"github.com/michelangelo-ai/michelangelo/go/base/revision"
2425
"github.com/michelangelo-ai/michelangelo/go/components/pipelinerun"
2526
"github.com/michelangelo-ai/michelangelo/go/components/triggerrun"
2627
apipb "github.com/michelangelo-ai/michelangelo/proto-go/api"
@@ -48,6 +49,7 @@ type Reconciler struct {
4849
apiHandlerFactory apiHandler.Factory
4950
triggerRunManager triggerrun.Manager
5051
pipelineRunManager pipelinerun.Manager
52+
revisionManager revision.Manager
5153
}
5254

5355
// Reconcile is the main reconciliation loop entry point for Pipeline resources.
@@ -224,11 +226,54 @@ func (r *Reconciler) handleDeletion(ctx context.Context, pipeline *v2pb.Pipeline
224226
return ctrl.Result{RequeueAfter: reconcileInterval}, nil
225227
}
226228

227-
// Delete steps will be added in subsequent PRs
228-
logger.Info("Children found, requeueing for cascade delete",
229+
// All children are terminal — delete them (fatal: error causes requeue)
230+
logger.Info("All children terminal, deleting",
229231
zap.Int("triggerRuns", len(triggerRuns)),
230232
zap.Int("pipelineRuns", len(pipelineRuns)))
231-
return ctrl.Result{RequeueAfter: reconcileInterval}, nil
233+
234+
if err := r.triggerRunManager.DeleteAllTriggerRuns(ctx, pipeline.Namespace, pipeline.Name); err != nil {
235+
logger.Error("Failed to delete trigger runs during cascade delete",
236+
zap.Error(err),
237+
zap.String("operation", "delete_all_trigger_runs"),
238+
zap.String("namespace", pipeline.Namespace),
239+
zap.String("name", pipeline.Name))
240+
return ctrl.Result{}, fmt.Errorf("delete trigger runs for pipeline %s/%s: %w", pipeline.Namespace, pipeline.Name, err)
241+
}
242+
if err := r.pipelineRunManager.DeleteAllPipelineRuns(ctx, pipeline.Namespace, pipeline.Name); err != nil {
243+
logger.Error("Failed to delete pipeline runs during cascade delete",
244+
zap.Error(err),
245+
zap.String("operation", "delete_all_pipeline_runs"),
246+
zap.String("namespace", pipeline.Namespace),
247+
zap.String("name", pipeline.Name))
248+
return ctrl.Result{}, fmt.Errorf("delete pipeline runs for pipeline %s/%s: %w", pipeline.Namespace, pipeline.Name, err)
249+
}
250+
251+
// Best-effort revision cleanup. Revisions are a tracking concern; failing to
252+
// clean them up should not block finalizer removal. The deployment controller
253+
// follows the same pattern in go/components/deployment/controller.go.
254+
if err := r.revisionManager.DeleteAllRevisions(ctx, pipeline.Namespace, pipeline.Name, "Pipeline"); err != nil {
255+
// Best-effort: revision cleanup failure does not block finalizer removal.
256+
// Logged at Info per the Michelangelo logging convention (Warn is not used;
257+
// see docs/contributing/dev/go/code-style — Info covers expected transient
258+
// failures, Error is reserved for actionable issues that block reconcile).
259+
logger.Info("Failed to delete revisions during cascade delete (continuing)",
260+
zap.Error(err),
261+
zap.String("operation", "delete_all_revisions"),
262+
zap.String("namespace", pipeline.Namespace),
263+
zap.String("name", pipeline.Name))
264+
}
265+
266+
logger.Info("All children deleted, removing finalizer")
267+
controllerutil.RemoveFinalizer(pipeline, api.PipelineFinalizer)
268+
if err := r.Update(ctx, pipeline, &metav1.UpdateOptions{}); err != nil {
269+
logger.Error("Failed to remove finalizer after cascade delete",
270+
zap.Error(err),
271+
zap.String("operation", "remove_finalizer"),
272+
zap.String("namespace", pipeline.Namespace),
273+
zap.String("name", pipeline.Name))
274+
return ctrl.Result{}, fmt.Errorf("remove finalizer on pipeline %s/%s: %w", pipeline.Namespace, pipeline.Name, err)
275+
}
276+
return ctrl.Result{}, nil
232277
}
233278

234279
// formatRevisionName generates a standardized revision name for a pipeline.
@@ -270,6 +315,9 @@ func (r *Reconciler) Register(mgr ctrl.Manager) error {
270315
r.Handler = handler
271316
r.triggerRunManager = triggerrun.NewManager(handler, r.logger)
272317
r.pipelineRunManager = pipelinerun.NewManager(handler, r.logger)
318+
if r.revisionManager == nil {
319+
r.revisionManager = revision.NewNoOpManager()
320+
}
273321
return ctrl.NewControllerManagedBy(mgr).
274322
For(&v2pb.Pipeline{}).
275323
Complete(r)

0 commit comments

Comments
 (0)