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.

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

feat(pkg/helper) Show timeout on TaskRun and PipelineRun describe #750

Conversation

waveywaves
Copy link
Member

@waveywaves waveywaves commented Feb 23, 2020

Changes

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

  • Includes tests (if functionality changed/added)
  • Run the code checkers with make check
  • Regenerate the manpages, docs and go formatting with make generated
  • Commit messages follow commit message best practices

See the contribution guide
for more details.

Release Notes

fixes #717

release-note

Adds Timeout information from TaskRuns and PipelineRuns to each of their `tkn describe` alternatives. 

@tekton-robot
Copy link
Contributor

Hi @waveywaves. Thanks for your PR.

I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@tekton-robot tekton-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 23, 2020
@waveywaves
Copy link
Member Author

Need some help with fixing the tests as I am getting Failed to execute templateError: template: Describe Pipelinerun:49:27: executing "Describe Pipelinerun" at <.PipelineRun.Spec.Timeout.Duration.String>: nil pointer evaluating *v1.Duration.Duration on some of the tests.

@eddycharly
Copy link
Member

eddycharly commented Feb 23, 2020

Shouldn’t the template be something like this ?

{{decorate "timeout" ""}} {{decorate "underline bold" "Timeout\n"}} {{if .PipelineRun.Spec.Timeout}}
{{.PipelineRun.Spec.Timeout.Duration.String}}
{{- else }}No timeout
{{- end }}

@waveywaves waveywaves force-pushed the 717-show-timeout-on-taskrun-pipelinerun-describe branch 2 times, most recently from 3344cba to d6c433d Compare February 24, 2020 18:05
@danielhelfand
Copy link
Member

Could you also please add a unit test for specifying a Timeout when it's not the default value (i.e., 1h0m0s) and when it's not present?

@danielhelfand
Copy link
Member

/ok-to-test

@tekton-robot tekton-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 25, 2020
@waveywaves waveywaves force-pushed the 717-show-timeout-on-taskrun-pipelinerun-describe branch from d6c433d to 8b37c4b Compare February 25, 2020 22:21
@tekton-robot tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 25, 2020
@waveywaves waveywaves force-pushed the 717-show-timeout-on-taskrun-pipelinerun-describe branch from 8b37c4b to 7b810c4 Compare February 25, 2020 22:28
@tekton-robot tekton-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 25, 2020
@waveywaves waveywaves force-pushed the 717-show-timeout-on-taskrun-pipelinerun-describe branch from 7b810c4 to d49a789 Compare February 26, 2020 01:01
@tekton-robot
Copy link
Contributor

The following is the coverage report on pkg/.
Say /test pull-tekton-cli-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/helper/taskrun/description/taskrun_description.go 53.5% 50.0% -3.5

@waveywaves waveywaves force-pushed the 717-show-timeout-on-taskrun-pipelinerun-describe branch from d49a789 to 3e9e198 Compare February 27, 2020 04:14
@tekton-robot
Copy link
Contributor

The following is the coverage report on pkg/.
Say /test pull-tekton-cli-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/helper/taskrun/description/taskrun_description.go 53.5% 50.0% -3.5

@danielhelfand
Copy link
Member

@waveywaves Thank you for making these changes! I think this all looks great.

If you could please add a unit test for both TaskRuns/PipelineRuns where the Timeout is not the default of 1h0m0s and squash your commits into a single commit, I think this will be ready to go.

Unit tests can be added for TaskRuns here and PipelineRuns here. You may also add Timeout to an already existing unit test such as this TaskRun test and PipelineRun test.

@waveywaves waveywaves force-pushed the 717-show-timeout-on-taskrun-pipelinerun-describe branch from 3e9e198 to b41c363 Compare February 28, 2020 12:53
@tekton-robot tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 28, 2020
@tekton-robot
Copy link
Contributor

The following is the coverage report on pkg/.
Say /test pull-tekton-cli-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/helper/taskrun/description/taskrun_description.go 53.5% 50.0% -3.5

@waveywaves
Copy link
Member Author

waveywaves commented Feb 28, 2020

@danielhelfand Thanks for the help, I was a little confused wrt the tests for sure. I have written them for default and custom timeout with the generated testdata. Let me know if you have any suggestions.

- Update tests using .golden for PipelineRun/TaskRun desc Timeouts
@waveywaves waveywaves force-pushed the 717-show-timeout-on-taskrun-pipelinerun-describe branch from b41c363 to 8d5eda4 Compare February 28, 2020 18:33
@tekton-robot
Copy link
Contributor

The following is the coverage report on pkg/.
Say /test pull-tekton-cli-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/helper/taskrun/description/taskrun_description.go 53.5% 50.0% -3.5

Copy link
Member

@danielhelfand danielhelfand left a comment

Choose a reason for hiding this comment

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

Thanks, @waveywaves!

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 28, 2020
@chmouel
Copy link
Member

chmouel commented Mar 2, 2020

/lgtm
/approve

Thanks 👍🏼🔥

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 2, 2020
@tekton-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chmouel, danielhelfand

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [chmouel,danielhelfand]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tekton-robot tekton-robot merged commit e41df95 into tektoncd:master Mar 2, 2020
piyush-garg added a commit that referenced this pull request Apr 29, 2020
#754 | [16yuki0702] A little Refactoring log reader. changes are following. | 2020/02/28-04:53
#761 | [Daniel Helfand] improve keep description and unit tests for prompts | 2020/02/28-08:29
#759 | [Vincent Demeester] Add support for yubikey in debbuild 🦌 | 2020/02/28-08:41
#762 | [Daniel Helfand] version v0.8.0 README update | 2020/03/02-02:51
#750 | [Vibhav Bobade] - Add Timeout Description to TaskRun and PipelineRun | 2020/03/02-03:36
#751 | [Vibhav Bobade] Remove -t timeout shorthand from tkn task start | 2020/03/02-03:36
#760 | [Vincent Demeester] Use `#!/usr/bin/env bash` instead of `bin/bash` 👼 | 2020/03/02-03:50
#760 | [Vincent Demeester] Use scripts for goreleaser task 👼 | 2020/03/02-03:50
#756 | [Vincent Demeester] Move version out of the helper package 🌿 | 2020/03/02-07:56
null | [Vincent Demeester] Move log out of the helper package 🌿 | 2020/03/02-07:56
null | [Vincent Demeester] Move params out of the helper package 🌿 | 2020/03/02-07:56
null | [Vincent Demeester] Move options out of the helper package 🌿 | 2020/03/02-07:56
null | [Vincent Demeester] Move labels out of the helper package 🌿 | 2020/03/02-07:56
null | [Vincent Demeester] Move names out of the helper package 🌿 | 2020/03/02-07:56
null | [Vincent Demeester] Move file out of the helper package 🌿 | 2020/03/02-07:56
null | [Vincent Demeester] Move validate out of the helper package 🌿 | 2020/03/02-07:56
null | [Vincent Demeester] Move pods out of the helper package 🌿 | 2020/03/02-07:56
null | [Vincent Demeester] Move deleter out of the helper package 🌿 | 2020/03/02-07:56
null | [Vincent Demeester] Move task out of the helper package 🌿 | 2020/03/02-07:56
null | [Vincent Demeester] Move taskrun out of the helper package 🌿 | 2020/03/02-07:56
null | [Vincent Demeester] Move pipelinerun out of the helper package 🌿 | 2020/03/02-07:56
null | [Vincent Demeester] Move pipeline out of the helper package 🌿 | 2020/03/02-07:56
null | [Vincent Demeester] Fix logs_test.go import 😇 | 2020/03/02-07:56
null | [Vincent Demeester] Run goimports 😅 | 2020/03/02-07:56
null | [Vincent Demeester] Re-generate the docs 📖 | 2020/03/02-07:56
null | [Piyush Garg] Remove the name param in cluster resource creation | 2020/03/03-07:50
null | [Pavol Pitonak] Fix several typos | 2020/03/04-08:39
null | [Pradeep Kumar] Change timeout to string for tkn task start | 2020/03/05-09:04
null | [Daniel Helfand] use text/template instead of html/template for pipelinerun describe | 2020/03/06-03:17
null | [Vincent Demeester] Bump to v0.11.0-rc1 🚌 | 2020/03/06-11:30
null | [16yuki0702] Fix SIGSEGV on pipelinerun logs. Related issue is #775 | 2020/03/08-21:24
null | [Vincent Demeester] Add tkn official image on contrib 🖌 | 2020/03/09-06:59
null | [Daniel Helfand] add sidecar status to tkn tr describe | 2020/03/10-08:18
null | [Daniel Helfand] convert --all to delete all pipelines and tasks in namespace | 2020/03/10-08:41
null | [Vibhav Bobade] Add flag to reverse sort taskruns/pipelineruns | 2020/03/10-08:41
null | [pthangad] Added Pipeline E2E tests | 2020/03/10-09:40
null | [Dibyo Mukherjee] Update Triggers dependency for CLI | 2020/03/11-03:15
null | [Piyush Garg] Bump to tektoncd/pipeline 0.11.0-rc2 | 2020/03/11-11:31
null | [Daniel Helfand] change pipelineruns to taskruns for tkn tr delete --keep desc | 2020/03/13-01:19
null | [Daniel Helfand] change status check for pr and tr cancel for color | 2020/03/13-01:37
null | [Piyush Garg] Add description to task, clustertask, condition list | 2020/03/13-03:32
null | [pthangad] Build tkn binary before running cli tests | 2020/03/13-04:24
null | [Piyush Garg] Add description to describe command | 2020/03/13-11:29
null | [Daniel Helfand] change tr and pr sorting from sort.Slice to sort.Sort | 2020/03/16-03:35
null | [Daniel Helfand] remove duplicate error message when deleting nonexistent resource | 2020/03/17-10:58
null | [pthangad] Add license to e2e tests | 2020/03/18-04:53
null | [Pradeep Kumar] Refactor task list cmd with v1beta1 support | 2020/03/18-10:18
null | [Daniel Helfand] add warning message in v0.9.0 about clustertask timeout | 2020/03/20-02:59
null | [Vibhav Bobade] Add flag to list TaskRuns from all namespaces | 2020/03/20-09:37
null | [Vibhav Bobade] Add flag to list PipelineRuns from all namespaces | 2020/03/20-09:37
null | [Vibhav Bobade] Update e2e to support TaskRun and PipelineRun List to use templates | 2020/03/20-09:37
null | [Daniel Helfand] start pipelinerun from local or remote file | 2020/03/20-10:37
null | [Pradeep Kumar] Add v1beta1 support for clustertask list | 2020/03/23-10:56
null | [Pradeep Kumar] Adds test for list v1beta1 | 2020/03/23-11:42
null | [Pradeep Kumar] v1beta1 support for taskrun list | 2020/03/24-11:26
null | [Daniel Helfand] fix test failure from go v1.14.0 update | 2020/03/26-06:43
null | [Pradeep Kumar] v1beta1 support for pipeline list cmd | 2020/03/26-12:18
null | [Vincent Demeester] Add RELEASE_VERSION build-arg to the image 📟 | 2020/03/26-15:50
null | [Pradeep Kumar] v1beta1 support for taskrun delete | 2020/03/27-03:22
null | [Pradeep Kumar] v1beta1 support for pipelinerun list & delete cmd | 2020/03/27-08:44
null | [Vincent Demeester] Sort resources in start subcommands 🌮 | 2020/03/30-07:14
null | [Vincent Demeester] Add ROADMAP.md 🤖 | 2020/03/30-07:38
null | [Khurram Baig] Add workspace option to pipeline start | 2020/03/30-07:55
null | [Vibhav Bobade] Add --noheaders flag to taskrun list subcommand | 2020/03/31-01:35
null | [Vibhav Bobade] Add --noheaders flag to pipelinerun list subcommand | 2020/03/31-01:35
null | [savitaashture] Fix tkn pipeline version issue | 2020/03/31-02:09
null | [Pradeep Kumar] remove redundant code | 2020/03/31-02:43
null | [Pradeep Kumar] v1beta1 support for task delete command | 2020/03/31-03:05
null | [Pradeep Kumar] v1beta1 support for pipeline delete command | 2020/03/31-04:43
null | [Khurram Baig] Add workspace to task start | 2020/03/31-07:19
null | [Daniel Helfand] clustertriggerbinding list command | 2020/03/31-07:47
null | [Daniel Helfand] add tkn ctb delete command | 2020/04/01-02:08
null | [Yulia Gaponenko] Add tekton cli build for s390x architecture This will add s390x target to Makefile and to .goreleaser.yaml | 2020/04/01-02:39
null | [Pradeep Kumar] v1beta1 support for clustertask delete | 2020/04/01-04:28
null | [Pradeep Kumar] fix typo and change function name | 2020/04/01-05:41
null | [Vincent Demeester] Import docs/README.md from website | 2020/04/01-07:55
null | [savitaashture] Add changes to display TaskRun and PipelineRun labels | 2020/04/02-02:27
null | [savitaashture] fix review comments | 2020/04/02-02:27
null | [Piyush Garg] Bump to tektoncd/pipeline v0.11.0 | 2020/04/02-07:25
null | [Daniel Helfand] warning messages about removing create -f | 2020/04/02-23:31
null | [Piyush Garg] Bump tektoncd/triggers to v0.4.0-rc1 | 2020/04/03-12:11
null | [Pradeep Kumar] v1beta1 support for task and taskrun logs command | 2020/04/03-18:03
null | [Pradeep Kumar] adds test for v1beta1 address review comments | 2020/04/03-18:03
null | [Daniel Helfand] add pradeepitm12 to OWNERS | 2020/04/04-09:02
null | [Pradeep Kumar] v1beta1 support for pipeline and run logs command | 2020/04/06-08:50
null | [Daniel Helfand] use timeout of --last or --use-pipelinerun/--use-taskrun | 2020/04/06-09:18
null | [Pradeep Kumar] v1beta1 support for taskrun cancel | 2020/04/06-16:37
null | [Piyush Garg] Bump task describe command to v1beta1 | 2020/04/07-09:56
null | [Khurram Baig] Add Workspace to Interactive Start Pipeline | 2020/04/07-10:23
null | [Piyush Garg] Bump tektoncd/triggers to v0.4.0 | 2020/04/07-17:51
null | [Pradeep Kumar] fix tasrun cancel command | 2020/04/08-04:55
null | [Pradeep Kumar] v1beta1 support for pipelinerun cancel | 2020/04/08-11:42
null | [Daniel Helfand] properly capture APIVersion with --dry-run | 2020/04/08-14:35
null | [Pradeep Kumar] use List from task pkg | 2020/04/09-20:46
null | [Piyush Garg] Refactoring in make file | 2020/04/10-07:52
null | [Piyush Garg] Bump clustertask describe to v1beta1 | 2020/04/12-15:01
null | [Piyush Garg] Fix Makefile | 2020/04/12-15:01
null | [Daniel Helfand] sort taskruns by start time for task describe | 2020/04/12-15:50
null | [Daniel Helfand] sort taskruns by start time for clustertask describe | 2020/04/12-16:05
null | [Piyush Garg] Bump pipeline desc to v1beta1 | 2020/04/13-03:01
null | [Daniel Helfand] sort pipelineruns by start time for pipeline describe | 2020/04/13-14:32
null | [Piyush Garg] Bump pipelinerun describe to v1beta1 | 2020/04/14-07:13
null | [Daniel Helfand] update release process docs on updating plumbing tkn images | 2020/04/14-07:40
null | [Piyush Garg] Fix e2e on pipeline 0.10 | 2020/04/15-11:39
null | [Piyush Garg] Add test for task package | 2020/04/15-12:17
null | [Anshul Verma] Flag to start a Pipeline without prompting for default param value. Even when a param is given through -p, other params to have their default values. | 2020/04/15-14:25
null | [Piyush Garg] Fix lint error and handle properly | 2020/04/15-17:07
null | [Piyush Garg] Fix listing of taskrun | 2020/04/16-07:01
null | [Daniel Helfand] remove default timeout of 1h for pipeline and task start | 2020/04/16-07:48
null | [Chmouel Boudjnah] Use real temporary directory when building debian package | 2020/04/16-10:15
null | [Chmouel Boudjnah] Set properly the release version in Debian package | 2020/04/16-10:15
null | [Nidhi Kakkar] Modified param flag  description | 2020/04/16-15:48
null | [Carlos Iriarte] chore: fix typo inavlid => invalid | 2020/04/17-07:34
null | [Piyush Garg] Bump tektoncd/pipeline to v0.11.2 | 2020/04/17-08:10
null | [Piyush Garg] Bump taskrun describe to v1beta1 | 2020/04/17-09:01
null | [Vibhav Bobade] Sort TaskRuns based on StartTime before deletion with keep | 2020/04/17-11:49
null | [Vibhav Bobade] Add --no-headers and --all-namespaces flag for Task `list` subcommand | 2020/04/17-20:00
null | [Vibhav Bobade] Add --no-headers and --all-namespaces flag for Pipeline `list` | 2020/04/17-20:00
null | [Vibhav Bobade] Docs --no-headers --all-namespaces flags for task/pipeline list | 2020/04/17-20:00
null | [Pradeep Kumar] v1beta1 support for task start command | 2020/04/20-12:49
null | [Pradeep Kumar] adds test for v1beta1 | 2020/04/20-12:49
null | [Daniel Helfand] check status of taskrun for tkn pr describe | 2020/04/20-15:05
null | [Anshul Verma] Corrects `logs` command to validate `--limit` option's value if it is <=0 Resolves #909 | 2020/04/20-15:29
null | [Chmouel Boudjnah] Disable colouring on windows | 2020/04/21-10:59
null | [savitaashture] Add tkn triggerbinding describe command | 2020/04/21-14:53
null | [Daniel Helfand] fix keep description and remove need to use with --all flag | 2020/04/21-16:17
null | [savitaashture] fix aliases for clustertriggerbindings | 2020/04/21-20:07
null | [savitaashture] Add tkn clustertriggerbinding describe command | 2020/04/22-08:31
null | [Piyush Garg] Bump pipeline start to v1beta1 | 2020/04/22-08:51
null | [Pradeep Kumar] little refactor for consistency | 2020/04/22-12:53
null | [savitaashture] Add tkn triggertemplate describe command | 2020/04/22-15:55
null | [Daniel Helfand] fix alignment of output resource TYPE column for task desc | 2020/04/24-08:58
null | [Piyush Garg] Implement convertDown function for Taskrun | 2020/04/24-11:50
null | [savitaashture] Add params description to describe commands | 2020/04/24-12:52
null | [Pradeep Kumar] v1beta1 support for task start command | 2020/04/24-16:28
null | [Daniel Helfand] fix --dry-run with --filename tests for all start commands | 2020/04/27-07:24
null | [Piyush Garg] Bump tekton/pipeline to v0.11.3 | 2020/04/27-08:56
null | [Piyush Garg] Fix pipeline version not working | 2020/04/27-10:24
null | [Vincent Demeester] Build and publish tkn image on release 📌 | 2020/04/27-20:16
null | [Piyush Garg] Add description command for condition | 2020/04/28-03:37
null | [savitaashture] Add eventlistener describe command | 2020/04/28-06:10
null | [savitaashture] Update template for EventListener Describe command | 2020/04/28-10:46
null | [Anshul Verma] Enable auto selection of the task for `logs` command if only one task is present. Fixes #916 | 2020/04/28-12:27
null | [Daniel Helfand] fix description typo and add test with output option for el desc | 2020/04/28-13:48
null | [Piyush Garg] Remove namespace flag from clusterwide cmd's | 2020/04/28-15:02
null | [Piyush Garg] Add flag to skip check flag in version cmd | 2020/04/29-06:56

Signed-off-by: Piyush Garg <pgarg@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cla: yes lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show Timeout as Part of TaskRun/PipelineRun Describe Commands
6 participants