Skip to content

Commit 424965e

Browse files
ishtmeet.singhishtoo1
authored andcommitted
feat: cascade kill active PipelineRuns on pipeline delete
Summary: Intent: - Kill active PipelineRuns before deleting them during pipeline cascade delete Changes: - Add kill-active-PipelineRuns step in handleDeletion: lists active PRs, sets Kill=true on each (best-effort), requeues - Add TestCascadeDelete_ActivePipelineRuns verifying Kill flag set on running PR and requeue Test Plan: - go test ./components/pipeline/... -v -count=1 (all tests pass) Revert Plan: Revert this PR via git revert. Jira Issues:
1 parent 97085c9 commit 424965e

2 files changed

Lines changed: 172 additions & 1 deletion

File tree

go/components/pipeline/controller.go

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,30 @@ func (r *Reconciler) handleDeletion(ctx context.Context, pipeline *v2pb.Pipeline
201201
return ctrl.Result{RequeueAfter: reconcileInterval}, nil
202202
}
203203

204-
// Kill and delete steps for PipelineRuns will be added in subsequent PRs
204+
// Kill active PipelineRuns (best-effort)
205+
activePRs, err := r.pipelineRunManager.ListActivePipelineRunsForPipeline(ctx, pipeline.Namespace, pipeline.Name)
206+
if err != nil {
207+
logger.Error("Failed to list active pipeline runs for cascade delete",
208+
zap.Error(err),
209+
zap.String("operation", "list_active_pipeline_runs"),
210+
zap.String("namespace", pipeline.Namespace),
211+
zap.String("name", pipeline.Name))
212+
return ctrl.Result{}, fmt.Errorf("list active pipeline runs for pipeline %s/%s: %w", pipeline.Namespace, pipeline.Name, err)
213+
}
214+
if len(activePRs) > 0 {
215+
for _, pr := range activePRs {
216+
if err := r.pipelineRunManager.KillPipelineRun(ctx, pr); err != nil {
217+
logger.Error("Failed to kill pipeline run during cascade delete",
218+
zap.Error(err),
219+
zap.String("operation", "kill_pipeline_run"),
220+
zap.String("namespace", pr.Namespace),
221+
zap.String("name", pr.Name))
222+
}
223+
}
224+
return ctrl.Result{RequeueAfter: reconcileInterval}, nil
225+
}
226+
227+
// Delete steps will be added in subsequent PRs
205228
logger.Info("Children found, requeueing for cascade delete",
206229
zap.Int("triggerRuns", len(triggerRuns)),
207230
zap.Int("pipelineRuns", len(pipelineRuns)))

go/components/pipeline/controller_test.go

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -638,6 +638,154 @@ func TestCascadeDelete_KillTriggerRunError_LogsAndContinues(t *testing.T) {
638638
require.ElementsMatch(t, []string{"tr-bad", "tr-good"}, trStub.killedNames)
639639
}
640640

641+
func TestCascadeDelete_ActivePipelineRuns(t *testing.T) {
642+
now := metav1.Now()
643+
pipeline := &v2pb.Pipeline{
644+
ObjectMeta: metav1.ObjectMeta{
645+
Name: "test-pipeline",
646+
Namespace: "test-namespace",
647+
Finalizers: []string{api.PipelineFinalizer},
648+
DeletionTimestamp: &now,
649+
},
650+
Spec: v2pb.PipelineSpec{
651+
Commit: &v2pb.CommitInfo{GitRef: "abc123", Branch: "main"},
652+
},
653+
}
654+
runningPR := &v2pb.PipelineRun{
655+
ObjectMeta: metav1.ObjectMeta{Name: "pr-running", Namespace: "test-namespace"},
656+
Spec: v2pb.PipelineRunSpec{
657+
Pipeline: &apipb.ResourceIdentifier{Name: "test-pipeline", Namespace: "test-namespace"},
658+
},
659+
Status: v2pb.PipelineRunStatus{State: v2pb.PIPELINE_RUN_STATE_RUNNING},
660+
}
661+
662+
reconciler := setUpReconciler(t, []client.Object{pipeline, runningPR}, env.Context{})
663+
result, err := reconciler.Reconcile(context.Background(), ctrl.Request{
664+
NamespacedName: types.NamespacedName{Name: "test-pipeline", Namespace: "test-namespace"},
665+
})
666+
require.NoError(t, err)
667+
require.Equal(t, ctrl.Result{RequeueAfter: reconcileInterval}, result)
668+
669+
updatedPR := &v2pb.PipelineRun{}
670+
require.NoError(t, reconciler.Get(context.Background(), "test-namespace", "pr-running", &metav1.GetOptions{}, updatedPR))
671+
require.True(t, updatedPR.Spec.Kill)
672+
}
673+
674+
// stubPipelineRunManager implements pipelinerun.Manager with configurable
675+
// behavior to cover error branches in handleDeletion's PipelineRun path.
676+
type stubPipelineRunManager struct {
677+
listAll []*v2pb.PipelineRun
678+
listAllErr error
679+
listActive []*v2pb.PipelineRun
680+
listActiveErr error
681+
killErrByName map[string]error
682+
killedNames []string
683+
}
684+
685+
func (s *stubPipelineRunManager) ListPipelineRunsForPipeline(ctx context.Context, namespace, pipelineName string) ([]*v2pb.PipelineRun, error) {
686+
return s.listAll, s.listAllErr
687+
}
688+
689+
func (s *stubPipelineRunManager) ListActivePipelineRunsForPipeline(ctx context.Context, namespace, pipelineName string) ([]*v2pb.PipelineRun, error) {
690+
return s.listActive, s.listActiveErr
691+
}
692+
693+
func (s *stubPipelineRunManager) KillPipelineRun(ctx context.Context, pr *v2pb.PipelineRun) error {
694+
s.killedNames = append(s.killedNames, pr.Name)
695+
if err, ok := s.killErrByName[pr.Name]; ok {
696+
return err
697+
}
698+
return nil
699+
}
700+
701+
func (s *stubPipelineRunManager) DeleteAllPipelineRuns(ctx context.Context, namespace, pipelineName string) error {
702+
return nil
703+
}
704+
705+
func setUpReconcilerWithStubManagers(t *testing.T, initialObjects []client.Object, trMgr triggerrun.Manager, prMgr pipelinerun.Manager) *Reconciler {
706+
scheme := runtime.NewScheme()
707+
require.NoError(t, v2pb.AddToScheme(scheme))
708+
k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(initialObjects...).WithStatusSubresource(initialObjects...).Build()
709+
logger := zaptest.NewLogger(t)
710+
handler := apiHandler.NewFakeAPIHandler(k8sClient)
711+
if trMgr == nil {
712+
trMgr = triggerrun.NewManager(handler, logger)
713+
}
714+
if prMgr == nil {
715+
prMgr = pipelinerun.NewManager(handler, logger)
716+
}
717+
return &Reconciler{
718+
Handler: handler,
719+
logger: logger,
720+
triggerRunManager: trMgr,
721+
pipelineRunManager: prMgr,
722+
}
723+
}
724+
725+
func TestCascadeDelete_ListActivePipelineRunsError(t *testing.T) {
726+
now := metav1.Now()
727+
pipeline := &v2pb.Pipeline{
728+
ObjectMeta: metav1.ObjectMeta{
729+
Name: "test-pipeline",
730+
Namespace: "test-namespace",
731+
Finalizers: []string{api.PipelineFinalizer},
732+
DeletionTimestamp: &now,
733+
},
734+
Spec: v2pb.PipelineSpec{
735+
Commit: &v2pb.CommitInfo{GitRef: "abc123", Branch: "main"},
736+
},
737+
}
738+
activeErr := errors.New("list active pr boom")
739+
prStub := &stubPipelineRunManager{
740+
// First List (children check) returns one PR so we proceed past the empty-children branch.
741+
listAll: []*v2pb.PipelineRun{
742+
{ObjectMeta: metav1.ObjectMeta{Name: "pr-1", Namespace: "test-namespace"}},
743+
},
744+
listActiveErr: activeErr,
745+
}
746+
reconciler := setUpReconcilerWithStubManagers(t, []client.Object{pipeline}, nil, prStub)
747+
748+
_, err := reconciler.Reconcile(context.Background(), ctrl.Request{
749+
NamespacedName: types.NamespacedName{Name: "test-pipeline", Namespace: "test-namespace"},
750+
})
751+
require.Error(t, err)
752+
require.ErrorIs(t, err, activeErr)
753+
require.Contains(t, err.Error(), "list active pipeline runs for pipeline test-namespace/test-pipeline")
754+
}
755+
756+
func TestCascadeDelete_KillPipelineRunError_LogsAndContinues(t *testing.T) {
757+
now := metav1.Now()
758+
pipeline := &v2pb.Pipeline{
759+
ObjectMeta: metav1.ObjectMeta{
760+
Name: "test-pipeline",
761+
Namespace: "test-namespace",
762+
Finalizers: []string{api.PipelineFinalizer},
763+
DeletionTimestamp: &now,
764+
},
765+
Spec: v2pb.PipelineSpec{
766+
Commit: &v2pb.CommitInfo{GitRef: "abc123", Branch: "main"},
767+
},
768+
}
769+
active := []*v2pb.PipelineRun{
770+
{ObjectMeta: metav1.ObjectMeta{Name: "pr-bad", Namespace: "test-namespace"}},
771+
{ObjectMeta: metav1.ObjectMeta{Name: "pr-good", Namespace: "test-namespace"}},
772+
}
773+
prStub := &stubPipelineRunManager{
774+
listAll: active,
775+
listActive: active,
776+
killErrByName: map[string]error{"pr-bad": errors.New("kill boom")},
777+
}
778+
reconciler := setUpReconcilerWithStubManagers(t, []client.Object{pipeline}, nil, prStub)
779+
780+
result, err := reconciler.Reconcile(context.Background(), ctrl.Request{
781+
NamespacedName: types.NamespacedName{Name: "test-pipeline", Namespace: "test-namespace"},
782+
})
783+
require.NoError(t, err)
784+
require.Equal(t, ctrl.Result{RequeueAfter: reconcileInterval}, result)
785+
786+
require.ElementsMatch(t, []string{"pr-bad", "pr-good"}, prStub.killedNames)
787+
}
788+
641789
func setUpReconciler(t *testing.T, initialObjects []client.Object, env env.Context) *Reconciler {
642790
scheme := runtime.NewScheme()
643791
err := v2pb.AddToScheme(scheme)

0 commit comments

Comments
 (0)