Skip to content

Commit

Permalink
Fix --ignore-running unexpectedly deleting incomplete TaskRuns
Browse files Browse the repository at this point in the history
  • Loading branch information
@colinodell
colinodell committed Sep 9, 2022
1 parent 8aa0ead commit 6c2ae7a
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 44 deletions.
19 changes: 7 additions & 12 deletions pkg/cmd/taskrun/delete.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ func deleteTaskRuns(s *cli.Stream, p cli.Params, trNames []string, opts *options
case opts.KeepSince > 0 && !opts.IgnoreRunning:
fmt.Fprintf(s.Out, "All but %d expired TaskRuns associated with %q %q deleted in namespace %q\n", numberOfDeletedTr, opts.ParentResource, opts.ParentResourceName, p.Namespace())
case opts.ParentResourceName != "" && !opts.IgnoreRunning:
fmt.Fprintf(s.Out, "All TaskRuns associated with %s %q deleted in namespace %q\n", opts.ParentResource, opts.ParentResourceName, p.Namespace())
fmt.Fprintf(s.Out, "All %d TaskRuns associated with %s %q deleted in namespace %q\n", numberOfDeletedTr, opts.ParentResource, opts.ParentResourceName, p.Namespace())
case opts.Keep > 0 && opts.KeepSince > 0:
fmt.Fprintf(s.Out, "%d TaskRuns(Completed) associated with %s %q has been deleted in namespace %q\n", numberOfDeletedTr, opts.ParentResource, opts.ParentResourceName, p.Namespace())
case opts.Keep > 0:
Expand All @@ -230,7 +230,7 @@ func deleteTaskRuns(s *cli.Stream, p cli.Params, trNames []string, opts *options
case opts.KeepSince > 0:
fmt.Fprintf(s.Out, "All but %d expired TaskRuns associated with %q %q deleted in namespace %q\n", numberOfDeletedTr, opts.ParentResource, opts.ParentResourceName, p.Namespace())
case opts.ParentResourceName != "":
fmt.Fprintf(s.Out, "All TaskRuns(Completed) associated with %s %q deleted in namespace %q\n", opts.ParentResource, opts.ParentResourceName, p.Namespace())
fmt.Fprintf(s.Out, "All %d TaskRuns(Completed) associated with %s %q deleted in namespace %q\n", numberOfDeletedTr, opts.ParentResource, opts.ParentResourceName, p.Namespace())
default:
d.PrintSuccesses(s)
}
Expand All @@ -251,9 +251,9 @@ func deleteTaskRuns(s *cli.Stream, p cli.Params, trNames []string, opts *options
case opts.KeepSince > 0:
fmt.Fprintf(s.Out, "%d expired Taskruns(Completed) has been deleted in namespace %q, kept %d\n", numberOfDeletedTr, p.Namespace(), numberOfKeptTr)
case !opts.IgnoreRunning:
fmt.Fprintf(s.Out, "All TaskRuns deleted in namespace %q\n", p.Namespace())
fmt.Fprintf(s.Out, "All %d TaskRuns deleted in namespace %q\n", numberOfDeletedTr, p.Namespace())
default:
fmt.Fprintf(s.Out, "All TaskRuns(Completed) deleted in namespace %q\n", p.Namespace())
fmt.Fprintf(s.Out, "All %d TaskRuns(Completed) deleted in namespace %q\n", numberOfDeletedTr, p.Namespace())
}
}
}
Expand Down Expand Up @@ -293,16 +293,11 @@ func allTaskRunNames(cs *cli.Clients, keep, since int, ignoreRunning bool, label
if ignoreRunning {
var taskRunTmp = []v1beta1.TaskRun{}
for _, v := range taskRuns.Items {
if v.Status.Conditions == nil {
if v.Status.CompletionTime == nil {
// Skip TaskRuns without CompletionTimes as they have not finished running yet
continue
}
for _, v2 := range v.Status.Conditions {
if v2.Reason == "Running" || v2.Reason == "Pending" || v2.Reason == "Started" {
continue
}
taskRunTmp = append(taskRunTmp, v)
break
}
taskRunTmp = append(taskRunTmp, v)
}
taskRuns.Items = taskRunTmp
}
Expand Down
101 changes: 69 additions & 32 deletions pkg/cmd/taskrun/delete_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -330,14 +330,12 @@ func TestTaskRunDelete(t *testing.T) {
},
Status: v1beta1.TaskRunStatus{
TaskRunStatusFields: v1beta1.TaskRunStatusFields{
CompletionTime: &metav1.Time{
Time: clock.Now().Add(10 * time.Minute),
},
CompletionTime: nil,
},
Status: duckv1beta1.Status{
Conditions: duckv1beta1.Conditions{
{
Status: corev1.ConditionTrue,
Status: corev1.ConditionUnknown,
Reason: v1beta1.TaskRunReasonStarted.String(),
},
},
Expand All @@ -358,14 +356,12 @@ func TestTaskRunDelete(t *testing.T) {
},
Status: v1beta1.TaskRunStatus{
TaskRunStatusFields: v1beta1.TaskRunStatusFields{
CompletionTime: &metav1.Time{
Time: clock.Now().Add(10 * time.Minute),
},
CompletionTime: nil,
},
Status: duckv1beta1.Status{
Conditions: duckv1beta1.Conditions{
{
Status: corev1.ConditionTrue,
Status: corev1.ConditionUnknown,
Reason: v1beta1.TaskRunReasonRunning.String(),
},
},
Expand All @@ -386,15 +382,12 @@ func TestTaskRunDelete(t *testing.T) {
},
Status: v1beta1.TaskRunStatus{
TaskRunStatusFields: v1beta1.TaskRunStatusFields{
CompletionTime: &metav1.Time{
// Use real time for testing keep-since and keep together
Time: time.Now(),
},
CompletionTime: nil,
},
Status: duckv1beta1.Status{
Conditions: duckv1beta1.Conditions{
{
Status: corev1.ConditionTrue,
Status: corev1.ConditionUnknown,
Reason: v1beta1.TaskRunReasonStarted.String(),
},
},
Expand All @@ -415,21 +408,44 @@ func TestTaskRunDelete(t *testing.T) {
},
Status: v1beta1.TaskRunStatus{
TaskRunStatusFields: v1beta1.TaskRunStatusFields{
CompletionTime: &metav1.Time{
// Use real time for testing keep-since and keep together
Time: time.Now(),
},
CompletionTime: nil,
},
Status: duckv1beta1.Status{
Conditions: duckv1beta1.Conditions{
{
Status: corev1.ConditionTrue,
Status: corev1.ConditionUnknown,
Reason: v1beta1.TaskRunReasonRunning.String(),
},
},
},
},
},
{
ObjectMeta: metav1.ObjectMeta{
Namespace: "ns",
Name: "tr0-14",
Labels: map[string]string{"tekton.dev/clusterTask": "random"},
},
Spec: v1beta1.TaskRunSpec{
TaskRef: &v1beta1.TaskRef{
Name: "random",
Kind: v1beta1.ClusterTaskKind,
},
},
Status: v1beta1.TaskRunStatus{
TaskRunStatusFields: v1beta1.TaskRunStatusFields{
CompletionTime: nil,
},
Status: duckv1beta1.Status{
Conditions: duckv1beta1.Conditions{
{
Status: corev1.ConditionUnknown,
Reason: "ExceededResourceQuota",
},
},
},
},
},
}

type clients struct {
Expand Down Expand Up @@ -459,6 +475,7 @@ func TestTaskRunDelete(t *testing.T) {
cb.UnstructuredV1beta1TR(trdata[10], version),
cb.UnstructuredV1beta1TR(trdata[11], version),
cb.UnstructuredV1beta1TR(trdata[12], version),
cb.UnstructuredV1beta1TR(trdata[13], version),
)
if err != nil {
t.Errorf("unable to create dynamic client: %v", err)
Expand Down Expand Up @@ -554,7 +571,7 @@ func TestTaskRunDelete(t *testing.T) {
input: seeds[0].pipelineClient,
inputStream: strings.NewReader("y"),
wantError: false,
want: "Are you sure you want to delete all TaskRuns related to Task \"random\" (y/n): All TaskRuns(Completed) associated with Task \"random\" deleted in namespace \"ns\"\n",
want: "Are you sure you want to delete all TaskRuns related to Task \"random\" (y/n): All 3 TaskRuns(Completed) associated with Task \"random\" deleted in namespace \"ns\"\n",
},
{
name: "Delete all with prompt",
Expand All @@ -563,7 +580,7 @@ func TestTaskRunDelete(t *testing.T) {
input: seeds[3].pipelineClient,
inputStream: strings.NewReader("y"),
wantError: false,
want: "Are you sure you want to delete all TaskRuns in namespace \"ns\" (y/n): All TaskRuns(Completed) deleted in namespace \"ns\"\n",
want: "Are you sure you want to delete all TaskRuns in namespace \"ns\" (y/n): All 9 TaskRuns(Completed) deleted in namespace \"ns\"\n",
},
{
name: "Delete all with -f",
Expand All @@ -572,7 +589,7 @@ func TestTaskRunDelete(t *testing.T) {
input: seeds[4].pipelineClient,
inputStream: nil,
wantError: false,
want: "All TaskRuns(Completed) deleted in namespace \"ns\"\n",
want: "All 9 TaskRuns(Completed) deleted in namespace \"ns\"\n",
},
{
name: "Delete all keeping 2",
Expand Down Expand Up @@ -653,7 +670,7 @@ func TestTaskRunDelete(t *testing.T) {
input: seeds[0].pipelineClient,
inputStream: strings.NewReader("y"),
wantError: false,
want: "Are you sure you want to delete all TaskRuns related to ClusterTask \"random\" (y/n): All TaskRuns(Completed) associated with ClusterTask \"random\" deleted in namespace \"ns\"\n",
want: "Are you sure you want to delete all TaskRuns related to ClusterTask \"random\" (y/n): All 5 TaskRuns(Completed) associated with ClusterTask \"random\" deleted in namespace \"ns\"\n",
},
{
name: "Error from deleting TaskRun with non-existing ClusterTask",
Expand Down Expand Up @@ -752,7 +769,7 @@ func TestTaskRunDelete(t *testing.T) {
input: seeds[4].pipelineClient,
inputStream: nil,
wantError: false,
want: "All TaskRuns(Completed) deleted in namespace \"ns\"\n",
want: "All 0 TaskRuns(Completed) deleted in namespace \"ns\"\n",
},
{
name: "Delete all with explicit --ignore-running true",
Expand All @@ -761,16 +778,16 @@ func TestTaskRunDelete(t *testing.T) {
input: seeds[4].pipelineClient,
inputStream: nil,
wantError: false,
want: "All TaskRuns(Completed) deleted in namespace \"ns\"\n",
want: "All 0 TaskRuns(Completed) deleted in namespace \"ns\"\n",
},
{
name: "Delete all with --ignore-running false",
name: "Delete all with --ignore-running false",
command: []string{"delete", "--all", "-f", "-n", "ns", "--ignore-running=false"},
dynamic: seeds[4].dynamicClient,
input: seeds[4].pipelineClient,
inputStream: nil,
wantError: false,
want: "All TaskRuns deleted in namespace \"ns\"\n",
want: "All 5 TaskRuns deleted in namespace \"ns\"\n",
},
{
name: "Delete the Task present and give error for non-existent Task",
Expand All @@ -797,7 +814,7 @@ func TestTaskRunDelete(t *testing.T) {
input: seeds[11].pipelineClient,
inputStream: nil,
wantError: false,
want: "All TaskRuns(Completed) associated with Task \"random\" deleted in namespace \"ns\"\n",
want: "All 4 TaskRuns(Completed) associated with Task \"random\" deleted in namespace \"ns\"\n",
},
{
name: "Delete all of task with explicit --ignore-running true",
Expand All @@ -806,7 +823,7 @@ func TestTaskRunDelete(t *testing.T) {
input: seeds[12].pipelineClient,
inputStream: nil,
wantError: false,
want: "All TaskRuns(Completed) associated with Task \"random\" deleted in namespace \"ns\"\n",
want: "All 4 TaskRuns(Completed) associated with Task \"random\" deleted in namespace \"ns\"\n",
},
{
name: "Delete all of task with --ignore-running false",
Expand All @@ -815,7 +832,7 @@ func TestTaskRunDelete(t *testing.T) {
input: seeds[13].pipelineClient,
inputStream: nil,
wantError: false,
want: "All TaskRuns associated with Task \"random\" deleted in namespace \"ns\"\n",
want: "All 6 TaskRuns associated with Task \"random\" deleted in namespace \"ns\"\n",
},
{
name: "Delete all of clustertask with default --ignore-running",
Expand All @@ -824,7 +841,7 @@ func TestTaskRunDelete(t *testing.T) {
input: seeds[14].pipelineClient,
inputStream: nil,
wantError: false,
want: "All TaskRuns(Completed) associated with ClusterTask \"random\" deleted in namespace \"ns\"\n",
want: "All 5 TaskRuns(Completed) associated with ClusterTask \"random\" deleted in namespace \"ns\"\n",
},
{
name: "Delete all of clustertask with explicit --ignore-running true",
Expand All @@ -833,7 +850,7 @@ func TestTaskRunDelete(t *testing.T) {
input: seeds[15].pipelineClient,
inputStream: nil,
wantError: false,
want: "All TaskRuns(Completed) associated with ClusterTask \"random\" deleted in namespace \"ns\"\n",
want: "All 5 TaskRuns(Completed) associated with ClusterTask \"random\" deleted in namespace \"ns\"\n",
},
{
name: "Delete all of clustertask with --ignore-running false",
Expand All @@ -842,7 +859,7 @@ func TestTaskRunDelete(t *testing.T) {
input: seeds[16].pipelineClient,
inputStream: nil,
wantError: false,
want: "All TaskRuns associated with ClusterTask \"random\" deleted in namespace \"ns\"\n",
want: "All 8 TaskRuns associated with ClusterTask \"random\" deleted in namespace \"ns\"\n",
},
}

Expand Down Expand Up @@ -912,6 +929,11 @@ func Test_ClusterTask_TaskRuns_Not_Deleted_With_Task_Option(t *testing.T) {
},
},
Status: v1beta1.TaskRunStatus{
TaskRunStatusFields: v1beta1.TaskRunStatusFields{
CompletionTime: &metav1.Time{
Time: time.Now(),
},
},
Status: duckv1beta1.Status{
Conditions: duckv1beta1.Conditions{
{
Expand Down Expand Up @@ -941,6 +963,11 @@ func Test_ClusterTask_TaskRuns_Not_Deleted_With_Task_Option(t *testing.T) {
},
},
Status: v1beta1.TaskRunStatus{
TaskRunStatusFields: v1beta1.TaskRunStatusFields{
CompletionTime: &metav1.Time{
Time: time.Now(),
},
},
Status: duckv1beta1.Status{
Conditions: duckv1beta1.Conditions{
{
Expand Down Expand Up @@ -1027,6 +1054,11 @@ func Test_Task_TaskRuns_Not_Deleted_With_ClusterTask_Option(t *testing.T) {
},
},
Status: v1beta1.TaskRunStatus{
TaskRunStatusFields: v1beta1.TaskRunStatusFields{
CompletionTime: &metav1.Time{
Time: time.Now(),
},
},
Status: duckv1beta1.Status{
Conditions: duckv1beta1.Conditions{
{
Expand Down Expand Up @@ -1056,6 +1088,11 @@ func Test_Task_TaskRuns_Not_Deleted_With_ClusterTask_Option(t *testing.T) {
},
},
Status: v1beta1.TaskRunStatus{
TaskRunStatusFields: v1beta1.TaskRunStatusFields{
CompletionTime: &metav1.Time{
Time: time.Now(),
},
},
Status: duckv1beta1.Status{
Conditions: duckv1beta1.Conditions{
{
Expand Down

0 comments on commit 6c2ae7a

Please sign in to comment.