From d5c3255b71e82f357b530a30f5b60df62c646b6a Mon Sep 17 00:00:00 2001 From: vinamra28 Date: Mon, 22 Mar 2021 06:50:52 +0530 Subject: [PATCH] Fix clustertask start command with --last flag `tkn clustertask start` command with `--last` flag was checking for the label `tekton.dev/task=name` in the created taskruns since any taskrun created from clustertask had the labels `tekton.dev/clusterTask=name` and `tekton.dev/task=name`. Since pipelines 0.22 release the label `tekton.dev/task=name` has been removed from the taskruns which were created from clustertasks. See: https://github.com/tektoncd/pipeline/pull/3764 Fixing this in pkg/task/tasklastrun.go and adding corresponding tests. Signed-off-by: vinamra28 --- pkg/cmd/clustertask/start_test.go | 6 +- pkg/task/tasklastrun.go | 22 ++-- pkg/task/tasklastrun_test.go | 187 ++++++++++++++++++++++++++++++ 3 files changed, 203 insertions(+), 12 deletions(-) diff --git a/pkg/cmd/clustertask/start_test.go b/pkg/cmd/clustertask/start_test.go index b2391d1a8e..06ee96f89d 100644 --- a/pkg/cmd/clustertask/start_test.go +++ b/pkg/cmd/clustertask/start_test.go @@ -322,7 +322,7 @@ func Test_ClusterTask_Start(t *testing.T) { ObjectMeta: v1.ObjectMeta{ Name: "taskrun-123", Namespace: "ns", - Labels: map[string]string{"tekton.dev/task": "clustertask-1"}, + Labels: map[string]string{"tekton.dev/clusterTask": "clustertask-1"}, }, Spec: v1alpha1.TaskRunSpec{ Params: []v1alpha1.Param{ @@ -1095,7 +1095,7 @@ func Test_ClusterTask_Start_v1beta1(t *testing.T) { ObjectMeta: v1.ObjectMeta{ Name: "taskrun-123", Namespace: "ns", - Labels: map[string]string{"tekton.dev/task": "clustertask-1"}, + Labels: map[string]string{"tekton.dev/clusterTask": "clustertask-1"}, }, Spec: v1beta1.TaskRunSpec{ Params: []v1beta1.Param{ @@ -1966,7 +1966,7 @@ func Test_start_clustertask_last_override_timeout(t *testing.T) { ObjectMeta: v1.ObjectMeta{ Name: trName, - Labels: map[string]string{"tekton.dev/clustertask": "clustertask"}, + Labels: map[string]string{"tekton.dev/clusterTask": "clustertask"}, Namespace: "ns", }, Spec: v1beta1.TaskRunSpec{ diff --git a/pkg/task/tasklastrun.go b/pkg/task/tasklastrun.go index a30f75d0f2..bd8a1ca36b 100644 --- a/pkg/task/tasklastrun.go +++ b/pkg/task/tasklastrun.go @@ -23,12 +23,19 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// LastRun returns the last taskrun for a given task -func LastRun(cs *cli.Clients, task string, ns, kind string) (*v1beta1.TaskRun, error) { +// LastRun returns the last taskrun for a given task/clustertask +func LastRun(cs *cli.Clients, resourceName, ns, kind string) (*v1beta1.TaskRun, error) { options := metav1.ListOptions{} - if task != "" { + + // change the label value to clusterTask if the resource is ClusterTask + label := "task" + if kind == "ClusterTask" { + label = "clusterTask" + } + + if resourceName != "" { options = metav1.ListOptions{ - LabelSelector: fmt.Sprintf("tekton.dev/task=%s", task), + LabelSelector: fmt.Sprintf("tekton.dev/%s=%s", label, resourceName), } } @@ -37,11 +44,8 @@ func LastRun(cs *cli.Clients, task string, ns, kind string) (*v1beta1.TaskRun, e return nil, err } - // this is required as the same label is getting added for both task and ClusterTask - runs.Items = FilterByRef(runs.Items, kind) - if len(runs.Items) == 0 { - return nil, fmt.Errorf("no TaskRuns related to %s %s found in namespace %s", kind, task, ns) + return nil, fmt.Errorf("no TaskRuns related to %s %s found in namespace %s", kind, resourceName, ns) } latest := runs.Items[0] @@ -54,7 +58,7 @@ func LastRun(cs *cli.Clients, task string, ns, kind string) (*v1beta1.TaskRun, e return &latest, nil } -// this will filter the taskrun which have reference to Task +// this will filter the taskrun which have reference to Task or ClusterTask func FilterByRef(taskruns []v1beta1.TaskRun, kind string) []v1beta1.TaskRun { var filtered []v1beta1.TaskRun for _, taskrun := range taskruns { diff --git a/pkg/task/tasklastrun_test.go b/pkg/task/tasklastrun_test.go index b63d0a5c84..ff20c72852 100644 --- a/pkg/task/tasklastrun_test.go +++ b/pkg/task/tasklastrun_test.go @@ -27,7 +27,9 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" pipelinetest "github.com/tektoncd/pipeline/test/v1alpha1" corev1 "k8s.io/api/core/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/pkg/apis" + duckv1beta1 "knative.dev/pkg/apis/duck/v1beta1" ) const ( @@ -135,3 +137,188 @@ func TestTaskrunLatest_no_run(t *testing.T) { expected := "no TaskRuns related to Task task found in namespace ns" test.AssertOutput(t, expected, err.Error()) } + +func TestTaskrunLatestForClusterTask_two_run(t *testing.T) { + clock := clockwork.NewFakeClock() + + var ( + taskCreated = clock.Now().Add(5 * time.Minute) + + firstRunCreated = clock.Now().Add(10 * time.Minute) + firstRunStarted = firstRunCreated.Add(2 * time.Second) + firstRunCompleted = firstRunStarted.Add(10 * time.Minute) + + secondRunCreated = firstRunCreated.Add(1 * time.Minute) + secondRunStarted = secondRunCreated.Add(2 * time.Second) + secondRunCompleted = secondRunStarted.Add(5 * time.Minute) + ) + clustertasks := []*v1alpha1.ClusterTask{ + { + ObjectMeta: v1.ObjectMeta{ + Name: "task", + CreationTimestamp: v1.Time{Time: taskCreated}, + }, + }, + } + taskruns := []*v1alpha1.TaskRun{ + { + ObjectMeta: v1.ObjectMeta{ + Name: "tr-1", + Namespace: "ns", + CreationTimestamp: v1.Time{Time: firstRunCreated}, + Labels: map[string]string{"tekton.dev/clusterTask": "task"}, + }, + Spec: v1alpha1.TaskRunSpec{ + TaskRef: &v1alpha1.TaskRef{ + Name: "task", + Kind: v1alpha1.ClusterTaskKind, + }, + }, + Status: v1beta1.TaskRunStatus{ + Status: duckv1beta1.Status{ + Conditions: duckv1beta1.Conditions{ + { + Status: corev1.ConditionTrue, + Reason: v1beta1.TaskRunReasonSuccessful.String(), + }, + }, + }, + TaskRunStatusFields: v1beta1.TaskRunStatusFields{ + StartTime: &v1.Time{Time: firstRunStarted}, + CompletionTime: &v1.Time{Time: firstRunCompleted}, + }, + }, + }, + { + ObjectMeta: v1.ObjectMeta{ + Name: "tr-2", + Namespace: "ns", + CreationTimestamp: v1.Time{Time: secondRunCompleted}, + Labels: map[string]string{"tekton.dev/clusterTask": "task"}, + }, + Spec: v1alpha1.TaskRunSpec{ + TaskRef: &v1alpha1.TaskRef{ + Name: "task", + Kind: v1alpha1.ClusterTaskKind, + }, + }, + Status: v1beta1.TaskRunStatus{ + Status: duckv1beta1.Status{ + Conditions: duckv1beta1.Conditions{ + { + Status: corev1.ConditionTrue, + Reason: v1beta1.TaskRunReasonSuccessful.String(), + }, + }, + }, + TaskRunStatusFields: v1beta1.TaskRunStatusFields{ + StartTime: &v1.Time{Time: secondRunStarted}, + CompletionTime: &v1.Time{Time: secondRunCompleted}, + }, + }, + }, + } + cs, _ := test.SeedTestData(t, pipelinetest.Data{ + ClusterTasks: clustertasks, + TaskRuns: taskruns, + }) + cs.Pipeline.Resources = cb.APIResourceList(versionA1, []string{"clustertask", "taskrun"}) + tdc := testDynamic.Options{} + dc, _ := tdc.Client( + cb.UnstructuredCT(clustertasks[0], versionA1), + cb.UnstructuredTR(taskruns[0], versionA1), + cb.UnstructuredTR(taskruns[1], versionA1), + ) + p := &test.Params{Tekton: cs.Pipeline, Clock: clock, Dynamic: dc} + client, err := p.Clients() + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + lastRun, err := LastRun(client, "task", "ns", "ClusterTask") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + test.AssertOutput(t, "tr-2", lastRun.Name) +} + +func TestFilterByRef(t *testing.T) { + clock := clockwork.NewFakeClock() + + var ( + firstRunCreated = clock.Now().Add(10 * time.Minute) + firstRunStarted = firstRunCreated.Add(2 * time.Second) + firstRunCompleted = firstRunStarted.Add(10 * time.Minute) + + secondRunCreated = firstRunCreated.Add(1 * time.Minute) + secondRunStarted = secondRunCreated.Add(2 * time.Second) + secondRunCompleted = secondRunStarted.Add(5 * time.Minute) + ) + taskruns := []v1beta1.TaskRun{ + { + ObjectMeta: v1.ObjectMeta{ + Name: "tr-1", + Namespace: "ns", + CreationTimestamp: v1.Time{Time: firstRunCreated}, + Labels: map[string]string{"tekton.dev/task": "task"}, + }, + Spec: v1beta1.TaskRunSpec{ + TaskRef: &v1alpha1.TaskRef{ + Name: "task", + Kind: v1alpha1.NamespacedTaskKind, + }, + }, + Status: v1beta1.TaskRunStatus{ + Status: duckv1beta1.Status{ + Conditions: duckv1beta1.Conditions{ + { + Status: corev1.ConditionTrue, + Reason: v1beta1.TaskRunReasonSuccessful.String(), + }, + }, + }, + TaskRunStatusFields: v1beta1.TaskRunStatusFields{ + StartTime: &v1.Time{Time: firstRunStarted}, + CompletionTime: &v1.Time{Time: firstRunCompleted}, + }, + }, + }, + { + ObjectMeta: v1.ObjectMeta{ + Name: "tr-2", + Namespace: "ns", + CreationTimestamp: v1.Time{Time: secondRunCompleted}, + Labels: map[string]string{"tekton.dev/clusterTask": "task"}, + }, + Spec: v1beta1.TaskRunSpec{ + TaskRef: &v1alpha1.TaskRef{ + Name: "task", + Kind: v1alpha1.ClusterTaskKind, + }, + }, + Status: v1beta1.TaskRunStatus{ + Status: duckv1beta1.Status{ + Conditions: duckv1beta1.Conditions{ + { + Status: corev1.ConditionTrue, + Reason: v1beta1.TaskRunReasonSuccessful.String(), + }, + }, + }, + TaskRunStatusFields: v1beta1.TaskRunStatusFields{ + StartTime: &v1.Time{Time: secondRunStarted}, + CompletionTime: &v1.Time{Time: secondRunCompleted}, + }, + }, + }, + } + + filteredClusterTask := FilterByRef(taskruns, "ClusterTask") + + test.AssertOutput(t, "tr-2", filteredClusterTask[0].Name) + + filteredTask := FilterByRef(taskruns, "Task") + + test.AssertOutput(t, "tr-1", filteredTask[0].Name) +}