Skip to content

Commit

Permalink
Fix deletion when TaskRun is part of PipelineRun
Browse files Browse the repository at this point in the history
In case a TaskRun is initiated by a PipelineRun, the ownerRef is internally used by the pipeline controller to create dependency. In these cases, we don't want the TaskRuns to be deleted separately.
  • Loading branch information
sayan-biswas authored and tekton-robot committed Jan 8, 2024
1 parent ca1ca87 commit bb31dbd
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 5 deletions.
18 changes: 15 additions & 3 deletions pkg/watcher/reconciler/dynamic/dynamic.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,21 @@ func (r *Reconciler) deleteUponCompletion(ctx context.Context, o results.Object)
return nil
}

if ownerReferences := o.GetOwnerReferences(); r.cfg.CheckOwner && len(ownerReferences) > 0 {
logger.Debugw("Resource is owned by another object, deferring deletion to parent resource(s)", zap.Any("results.tekton.dev/ownerReferences", ownerReferences))
return nil
if ownerReferences := o.GetOwnerReferences(); len(ownerReferences) > 0 {
// do not delete if the object is owned by a PipelineRun object
// This can be removed once the PipelineRun controller is patched to stop updating the PipelineRun object
// when child TaskRuns are deleted
for _, or := range ownerReferences {
if or.Kind == "PipelineRun" {
logger.Debugw("Resource is owned by a PipelineRun, deferring deletion to parent PipelineRun", zap.Any("tekton.dev/PipelineRun", or.Name))
return nil
}
}
// do not delete if CheckOwner flag is enabled and the object has some owner references
if r.cfg.CheckOwner {
logger.Debugw("Resource is owned by another object, deferring deletion to parent resource(s)", zap.Any("results.tekton.dev/ownerReferences", ownerReferences))
return nil
}
}

completionTime, err := getCompletionTime(o)
Expand Down
38 changes: 36 additions & 2 deletions pkg/watcher/reconciler/dynamic/dynamic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -335,9 +335,12 @@ func TestReconcile_TaskRun(t *testing.T) {
taskrun.OwnerReferences = []metav1.OwnerReference{{
APIVersion: "v1",
Kind: "test",
Name: "test-parent",
Name: "test-owner",
}}

// delete taskrun before creating
trclient.Delete(ctx, taskrun.GetName(), metav1.DeleteOptions{})

// Recreate the object to retest the deletion
if _, err := trclient.Create(ctx, taskrun, metav1.CreateOptions{}); err != nil {
t.Fatal(err)
Expand All @@ -353,16 +356,47 @@ func TestReconcile_TaskRun(t *testing.T) {
}
})

t.Run("do not delete object with owner references has PipelineRun object and owner check is disabled", func(t *testing.T) {
r.cfg.CheckOwner = false

taskrun.OwnerReferences = []metav1.OwnerReference{{
APIVersion: "tekton.dev/v1",
Kind: "PipelineRun",
Name: "test-pipelinerun",
UID: types.UID(uuid.NewString()),
}}

// Delete taskrun before creating
trclient.Delete(ctx, taskrun.GetName(), metav1.DeleteOptions{})

// Recreate TaskRun object to retest the deletion
if _, err := trclient.Create(ctx, taskrun, metav1.CreateOptions{}); err != nil {
t.Fatal(err)
}

if err := r.Reconcile(ctx, taskrun); err != nil {
t.Fatal(err)
}

// Make sure that the TaskRun is not deleted
if _, err := trclient.Get(ctx, taskrun.GetName(), metav1.GetOptions{}); apierrors.IsNotFound(err) {
t.Fatalf("Want Found, but got %v", err)
}
})

t.Run("do not delete object with owner references when owner check is enabled", func(t *testing.T) {
r.cfg.CheckOwner = true

taskrun.OwnerReferences = []metav1.OwnerReference{{
APIVersion: "v1",
Kind: "test",
Name: "test-parent",
Name: "test-owner",
UID: types.UID(uuid.NewString()),
}}

// delete taskrun before creating
trclient.Delete(ctx, taskrun.GetName(), metav1.DeleteOptions{})

// Recreate the object to retest the deletion
if _, err := trclient.Create(ctx, taskrun, metav1.CreateOptions{}); err != nil {
t.Fatal(err)
Expand Down

0 comments on commit bb31dbd

Please sign in to comment.