Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adding nil Checks to TaskRun and PipelineRun Describe Commands #418

Merged
merged 1 commit into from
Oct 31, 2019
Merged

Adding nil Checks to TaskRun and PipelineRun Describe Commands #418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pkg/cmd/pipelinerun/describe.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ import (

const templ = `Name: {{ .PipelineRun.Name }}
Namespace: {{ .PipelineRun.Namespace }}
{{- if ne .PipelineRun.Spec.PipelineRef.Name "" }}
Pipeline Ref: {{ .PipelineRun.Spec.PipelineRef.Name }}
{{- end }}
{{- if ne .PipelineRun.Spec.DeprecatedServiceAccount "" }}
Service Account (deprecated): {{ .PipelineRun.Spec.DeprecatedServiceAccount }}
{{- end }}
Expand Down
131 changes: 131 additions & 0 deletions pkg/cmd/pipelinerun/describe_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -601,3 +601,134 @@ No taskruns

test.AssertOutput(t, expected, actual)
}

func TestPipelineRunDescribe_without_pipelineref(t *testing.T) {
clock := clockwork.NewFakeClock()

cs, _ := test.SeedTestData(t, pipelinetest.Data{
PipelineRuns: []*v1alpha1.PipelineRun{
tb.PipelineRun("pipeline-run", "ns",
cb.PipelineRunCreationTimestamp(clock.Now()),
tb.PipelineRunLabel("tekton.dev/pipeline", "pipeline"),
tb.PipelineRunStatus(),
),
},
Namespaces: []*corev1.Namespace{
{
ObjectMeta: metav1.ObjectMeta{
Name: "ns",
},
},
},
})

p := &test.Params{Tekton: cs.Pipeline, Clock: clock, Kube: cs.Kube}

pipelinerun := Command(p)
clock.Advance(10 * time.Minute)
actual, err := test.ExecuteCommand(pipelinerun, "desc", "pipeline-run", "-n", "ns")
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
expected := `Name: pipeline-run
Namespace: ns

Status
STARTED DURATION STATUS
--- --- ---

Resources
No resources

Params
No params

Taskruns
No taskruns
`

test.AssertOutput(t, expected, actual)
}

func TestPipelineRunDescribe_no_resourceref(t *testing.T) {
clock := clockwork.NewFakeClock()

trs := []*v1alpha1.TaskRun{
tb.TaskRun("tr-1", "ns",
tb.TaskRunStatus(
tb.TaskRunStartTime(clock.Now().Add(2*time.Minute)),
cb.TaskRunCompletionTime(clock.Now().Add(5*time.Minute)),
tb.StatusCondition(apis.Condition{
Type: apis.ConditionSucceeded,
Status: corev1.ConditionTrue,
}),
),
),
}

cs, _ := test.SeedTestData(t, pipelinetest.Data{
PipelineRuns: []*v1alpha1.PipelineRun{
tb.PipelineRun("pipeline-run", "ns",
cb.PipelineRunCreationTimestamp(clock.Now()),
tb.PipelineRunLabel("tekton.dev/pipeline", "pipeline"),
tb.PipelineRunSpec("pipeline",
tb.PipelineRunDeprecatedServiceAccountName("test-sa", "test-sa-deprecated"),
tb.PipelineRunParam("test-param", "param-value"),
tb.PipelineRunResourceBinding("test-resource"),
),
tb.PipelineRunStatus(
tb.PipelineRunTaskRunsStatus("tr-1", &v1alpha1.PipelineRunTaskRunStatus{
PipelineTaskName: "t-1",
Status: &trs[0].Status,
}),
tb.PipelineRunStatusCondition(apis.Condition{
Status: corev1.ConditionTrue,
Reason: resources.ReasonSucceeded,
}),
tb.PipelineRunStartTime(clock.Now()),
cb.PipelineRunCompletionTime(clock.Now().Add(5*time.Minute)),
),
),
},
Namespaces: []*corev1.Namespace{
{
ObjectMeta: metav1.ObjectMeta{
Name: "ns",
},
},
},
})

p := &test.Params{Tekton: cs.Pipeline, Clock: clock, Kube: cs.Kube}

pipelinerun := Command(p)
clock.Advance(10 * time.Minute)
actual, err := test.ExecuteCommand(pipelinerun, "desc", "pipeline-run", "-n", "ns")
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
expected := `Name: pipeline-run
Namespace: ns
Pipeline Ref: pipeline
Service Account (deprecated): test-sa-deprecated
Service Account: test-sa

Status
STARTED DURATION STATUS
10 minutes ago 5 minutes Succeeded

Resources
NAME RESOURCE REF
test-resource

Params
NAME VALUE
test-param param-value

Taskruns
NAME TASK NAME STARTED DURATION STATUS
tr-1 t-1 8 minutes ago 3 minutes Succeeded
`

test.AssertOutput(t, expected, actual)
}
5 changes: 4 additions & 1 deletion pkg/cmd/taskrun/describe.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ import (

const templ = `Name: {{ .TaskRun.Name }}
Namespace: {{ .TaskRun.Namespace }}
Task Ref: {{ .TaskRun.Spec.TaskRef.Name }}
{{- $tRefName := taskRefExists .TaskRun.Spec }}{{- if ne $tRefName "" }}
Task Ref: {{ $tRefName }}
{{- end }}
{{- if ne .TaskRun.Spec.DeprecatedServiceAccount "" }}
Service Account (deprecated): {{ .TaskRun.Spec.DeprecatedServiceAccount }}
{{- end }}
Expand Down Expand Up @@ -158,6 +160,7 @@ func printTaskRunDescription(s *cli.Stream, trName string, p cli.Params) error {
"formatDuration": formatted.Duration,
"formatCondition": formatted.Condition,
"hasFailed": hasFailed,
"taskRefExists": validate.TaskRefExists,
}

w := tabwriter.NewWriter(s.Out, 0, 5, 3, ' ', tabwriter.TabIndent)
Expand Down
145 changes: 145 additions & 0 deletions pkg/cmd/taskrun/describe_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,3 +279,148 @@ No steps

test.AssertOutput(t, expected, actual)
}

func TestTaskRunDescribe_no_taskref(t *testing.T) {
chmouel marked this conversation as resolved.
Show resolved Hide resolved
clock := clockwork.NewFakeClock()

trs := []*v1alpha1.TaskRun{
tb.TaskRun("tr-1", "ns",
tb.TaskRunStatus(
tb.TaskRunStartTime(clock.Now().Add(2*time.Minute)),
cb.TaskRunCompletionTime(clock.Now().Add(5*time.Minute)),
tb.StatusCondition(apis.Condition{
Status: corev1.ConditionFalse,
Reason: resources.ReasonFailed,
Message: "Testing tr failed",
}),
),
),
}

cs, _ := test.SeedTestData(t, pipelinetest.Data{
TaskRuns: trs,
Namespaces: []*corev1.Namespace{
{
ObjectMeta: metav1.ObjectMeta{
Name: "ns",
},
},
},
})

p := &test.Params{Tekton: cs.Pipeline, Clock: clock, Kube: cs.Kube}

taskrun := Command(p)
clock.Advance(10 * time.Minute)
actual, err := test.ExecuteCommand(taskrun, "desc", "tr-1", "-n", "ns")
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
expected := `Name: tr-1
Namespace: ns

Status
STARTED DURATION STATUS
8 minutes ago 3 minutes Failed

Message
Testing tr failed

Input Resources
No resources

Output Resources
No resources

Params
No params

Steps
No steps
`

test.AssertOutput(t, expected, actual)
}

func TestTaskRunDescribe_no_resourceref(t *testing.T) {
clock := clockwork.NewFakeClock()

trs := []*v1alpha1.TaskRun{
tb.TaskRun("tr-1", "ns",
tb.TaskRunStatus(
tb.TaskRunStartTime(clockwork.NewFakeClock().Now().Add(20*time.Second)),
tb.StatusCondition(apis.Condition{
Type: apis.ConditionSucceeded,
Status: corev1.ConditionTrue,
}),
tb.StepState(
cb.StepName("step1"),
tb.StateTerminated(0),
),
tb.StepState(
cb.StepName("step2"),
tb.StateTerminated(0),
),
),
tb.TaskRunSpec(
tb.TaskRunTaskRef("t1"),
tb.TaskRunInputs(tb.TaskRunInputsParam("input", "param")),
tb.TaskRunInputs(tb.TaskRunInputsParam("input2", "param2")),
tb.TaskRunInputs(tb.TaskRunInputsResource("git")),
tb.TaskRunInputs(tb.TaskRunInputsResource("image-input", tb.TaskResourceBindingRef("image"))),
tb.TaskRunOutputs(tb.TaskRunOutputsResource("image-output")),
tb.TaskRunOutputs(tb.TaskRunOutputsResource("image-output2")),
),
),
}

cs, _ := test.SeedTestData(t, pipelinetest.Data{
TaskRuns: trs,
Namespaces: []*corev1.Namespace{
{
ObjectMeta: metav1.ObjectMeta{
Name: "ns",
},
},
},
})

p := &test.Params{Tekton: cs.Pipeline, Clock: clock, Kube: cs.Kube}

taskrun := Command(p)
clock.Advance(10 * time.Minute)
actual, err := test.ExecuteCommand(taskrun, "desc", "tr-1", "-n", "ns")
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
expected := `Name: tr-1
Namespace: ns
Task Ref: t1

Status
STARTED DURATION STATUS
9 minutes ago --- Succeeded

Input Resources
NAME RESOURCE REF
git
image-input image

Output Resources
NAME RESOURCE REF
image-output
image-output2

Params
NAME VALUE
input param
input2 param2

Steps
NAME
step1
step2
`

test.AssertOutput(t, expected, actual)
}
15 changes: 15 additions & 0 deletions pkg/helper/validate/validate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,15 @@ package validate
import (
"fmt"

"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
k8s "k8s.io/client-go/kubernetes"
)

const (
fieldNotPresent = ""
)

type params interface {
KubeClient() (k8s.Interface, error)
Namespace() string
Expand All @@ -41,3 +46,13 @@ func NamespaceExists(p params) error {

return nil
}

// Check if TaskRef exists on a TaskRunSpec. Returns empty string if not present.
func TaskRefExists(spec v1alpha1.TaskRunSpec) string {

if spec.TaskRef == nil {
return fieldNotPresent
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can just return "" here

Copy link
Member Author

@danielhelfand danielhelfand Oct 31, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would personally prefer to keep it to communicate what an empty string represents. It can be used for PipelineRef, ResourceRef, TaskResourceBinding, and PipelineResourceBinding as well when nil checks can be added for those with next pipelines version update.

}

return spec.TaskRef.Name
}
21 changes: 21 additions & 0 deletions pkg/helper/validate/validate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"testing"

"github.com/tektoncd/cli/pkg/test"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
pipelinetest "github.com/tektoncd/pipeline/test"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -53,3 +54,23 @@ func TestNamespaceExists_Valid_Namespace(t *testing.T) {
err := NamespaceExists(p)
test.AssertOutput(t, nil, err)
}

func TestTaskRefExists_Present(t *testing.T) {
spec := v1alpha1.TaskRunSpec{
TaskRef: &v1alpha1.TaskRef{
Name: "Task",
},
}

output := TaskRefExists(spec)
test.AssertOutput(t, "Task", output)
}

func TestTaskRefExists_Not_Present(t *testing.T) {
spec := v1alpha1.TaskRunSpec{
TaskRef: nil,
}

output := TaskRefExists(spec)
test.AssertOutput(t, "", output)
}