Add condition list command #319

16yuki0702 · 2019-09-18T03:55:29Z

Changes

Add condition subcommand and feature of listing its resources #286

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)
Includes docs (if user facing)
Commit messages follow commit message best practices

See the contribution guide
for more details.

Release Notes

you can list condition resources from tkn

tekton-robot · 2019-09-18T03:55:43Z

Hi @16yuki0702. 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.

hrishin · 2019-09-18T13:04:53Z

/ok-to-test

tekton-robot · 2019-09-18T13:07:06Z

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/cmd/condition/condition.go	Do not exist	100.0%
pkg/cmd/condition/list.go	Do not exist	65.0%

tekton-robot · 2019-09-19T05:08:04Z

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/cmd/condition/condition.go	Do not exist	100.0%
pkg/cmd/condition/list.go	Do not exist	80.0%

khrm · 2019-09-20T05:13:40Z

pkg/cmd/condition/list_test.go

+				"condition2   20 seconds ago",
+				"condition3   3 weeks ago",
+				"",
+			},


Can we have a test for https://github.com/tektoncd/cli/pull/319/files#diff-3b9bcd9007c070bb31b5ef44695b2016R81 also?
Use PrependReactor to simulate error.

@khrm Thank you for your review!!
I just added test case, please check this out.

@khrm I think test error occured from outputting error message to Err stream.
In my opinion To fix this, the error message should be returned to caller not immediately output Err stream like below.

return fmt.Errorf("failed to list conditions from %s namespace \n%s", p.Namespace(), err)

But this is different from other error messages.
ex. https://github.com/tektoncd/cli/blob/master/pkg/cmd/pipeline/list.go#L93

If adding this test case I think we should create other pull request to fix all other files, how do you think 🤔??

@khrm

@16yuki0702 Run hack/update-codegen.sh from root directory to fix the error.

Oh I totally misunderstood 😓
very sorry and thank you for your advise 😃

tekton-robot · 2019-09-20T06:15:00Z

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/cmd/condition/condition.go	Do not exist	100.0%
pkg/cmd/condition/list.go	Do not exist	87.5%

16yuki0702 · 2019-09-20T11:14:58Z

/retest

khrm · 2019-09-20T11:48:49Z

@16yuki0702 Run hack/update-codegen.sh from root directory to fix the error.

tekton-robot · 2019-09-20T11:59:34Z

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/cmd/condition/condition.go	Do not exist	100.0%
pkg/cmd/condition/list.go	Do not exist	87.5%

tekton-robot · 2019-09-21T15:29:09Z

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/cmd/condition/condition.go	Do not exist	100.0%
pkg/cmd/condition/list.go	Do not exist	87.5%

tekton-robot · 2019-09-26T01:11:40Z

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/cmd/condition/condition.go	Do not exist	100.0%
pkg/cmd/condition/list.go	Do not exist	87.5%

piyush-garg · 2019-10-01T05:43:16Z

pkg/cmd/condition/list.go

+
+	conditions, err := listAllConditions(cs.Tekton, p.Namespace())
+	if err != nil {
+		return err


Just minor comment can you add here as well fmt.Fprintf(s.Err, "Failed to list conditions from %s namespace \n", p.Namespace())

Also if you can add a test for this scenario will be good

@piyush-garg Thank you for your review!!
I just fixed it 👍

piyush-garg · 2019-10-01T05:47:19Z

Also, there is need to rebase and generate docs again.

tekton-robot · 2019-10-01T07:38:18Z

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/cmd/condition/condition.go	Do not exist	100.0%
pkg/cmd/condition/list.go	Do not exist	90.2%

piyush-garg

/lgtm

Thanks for the PR @16yuki0702

chmouel · 2019-10-01T07:58:04Z

/lgtm
/approve
/meow

(sorry taking time reviewing this, thanks @16yuki0702 )

tekton-robot · 2019-10-01T07:58:05Z

@chmouel:

In response to this:

/lgtm
/approve
/meow

(sorry taking time reviewing this, thanks @16yuki0702 )

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 · 2019-10-01T07:58:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 16yuki0702, chmouel, piyush-garg

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]

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

16yuki0702 · 2019-10-01T08:50:24Z

@khrm @piyush-garg @chmouel Thank for reviews !!

#336 | [Chmouel Boudjnah] Update templating to use `$(…)` instead of `${…}` | 2019/09/26-04:37 #338 | [Chmouel Boudjnah] Update README to 0.4.0 | 2019/09/26-06:33 #338 | [Chmouel Boudjnah] Add Centos8 | 2019/09/26-06:33 #337 | [Chmouel Boudjnah] Fix spec file path | 2019/09/26-06:49 #323 | [Piyush Garg] Add task describe subcommand | 2019/09/30-07:10 #340 | [Chmouel Boudjnah] Cleanup -l flag across commands | 2019/09/30-10:50 #319 | [16yuki0702] Add condition list command | 2019/10/01-02:58 #343 | [Chmouel Boudjnah] Add information about docs and checks for github pr template | 2019/10/02-06:50 #344 | [Chmouel Boudjnah] Add documentation check | 2019/10/02-18:27 #342 | [Chmouel Boudjnah] Allow passing labels to start | 2019/10/03-02:44 null | [Chmouel Boudjnah] Update the new brew location | 2019/10/03-04:16 null | [danielhelfand] check if namespace exists for task list command | 2019/10/08-10:06 null | [Piyush Garg] Set namespace in e2e tests | 2019/10/09-03:16 null | [Pradeep Kumar] Adds command to create new resource interactively | 2019/10/09-06:00 null | [Pradeep Kumar] adds doc for resource create command | 2019/10/09-06:00 null | [Pradeep Kumar] review comments addressed | 2019/10/09-06:00 null | [Pradeep Kumar] removes os.Exit(1) for interrupts | 2019/10/09-06:00 null | [Pradeep Kumar] rebase and auto generate docs | 2019/10/09-06:00 null | [Vincent Demeester] Bump tektoncd/plumbing dependency ⛽️ | 2019/10/09-20:46 null | [danielhelfand] add namespace check to task delete command | 2019/10/09-21:07 null | [Piyush Garg] Refactoring resource list | 2019/10/10-07:50 null | [Piyush Garg] Fix unit tests | 2019/10/10-10:48 null | [Chmouel Boudjnah] Show logs right after starting the pipeline | 2019/10/10-11:08 null | [danielhelfand] add namespace check to task describe command | 2019/10/13-10:40 null | [Pradeep Kumar] Fixes test increase timeout for expect | 2019/10/15-02:13 null | [danielhelfand] add namespace check to taskrun desc command | 2019/10/16-03:28 null | [Chmouel Boudjnah] Don't show just Error if container came back only with "Error" | 2019/10/16-03:47 null | [danielhelfand] add namespace check to taskrun list command | 2019/10/16-03:47 null | [danielhelfand] add tkn condition to README | 2019/10/16-04:54 null | [Pradeep Kumar] Use *** instead of plain text for password | 2019/10/17-08:16 null | [Pradeep Kumar] Change way of question for insecure parameter | 2019/10/18-03:07 null | [danielhelfand] refactor NamespaceExists | 2019/10/18-09:47 null | [Piyush Garg] Add feature to start task | 2019/10/18-12:04 null | [Pradeep Kumar] removes complexity of read timeout from test | 2019/10/21-06:03 null | [Pradeep Kumar] remove test for interrupt change pull url | 2019/10/21-06:03 null | [Chmouel Boudjnah] Syncronize cobra fork with latest | 2019/10/21-23:17 null | [Chmouel Boudjnah] Update the whole go mod dependencies shbang | 2019/10/21-23:17 null | [danielhelfand] add namespace check to taskrun delete command | 2019/10/22-02:04 null | [danielhelfand] add namespace check to taskrun logs command | 2019/10/22-02:25 null | [danielhelfand] add namespace check to taskrun cancel command | 2019/10/22-02:36 null | [Piyush Garg] Fix panic on pipeline logs | 2019/10/22-09:26 null | [hriships] Change for release and tagging | 2019/10/23-04:02 null | [danielhelfand] add namespace check to pipeline list command | 2019/10/23-07:42 null | [danielhelfand] add namespace check to condition list command | 2019/10/24-01:38 null | [danielhelfand] add namespace check to pipeline logs command | 2019/10/24-01:50 null | [danielhelfand] add namespace check to clustertask commands | 2019/10/24-01:50 null | [danielhelfand] add namespace check to pipeline delete command | 2019/10/24-02:03 null | [danielhelfand] add namespace check to pipeline describe command | 2019/10/24-02:03 null | [16yuki0702] Add delete condition command | 2019/10/24-02:15 null | [16yuki0702] Extract checking option | 2019/10/24-02:15 null | [Chmouel Boudjnah] Bump tektoncd/pipeline to 0.8.0 | 2019/10/24-09:14 null | [Chmouel Boudjnah] Bump some dependency related to opencensus.io to fix go mod tidy | 2019/10/24-23:42 null | [Chmouel Boudjnah] Fix staticcheck error on version/version.go | 2019/10/24-23:42 null | [danielhelfand] add namespace check to resource list command | 2019/10/24-23:55 null | [Andrea Frittoli] Use targetURI for cloudEvent resource details | 2019/10/25-02:04 null | [danielhelfand] add namespace check to resource create command | 2019/10/25-02:26 null | [danielhelfand] add namespace check to condition delete command | 2019/10/25-02:26 null | [Vincent Demeester] Bump plumbing to latest version | 2019/10/25-03:23 null | [Chmouel Boudjnah] Add banners when building docs | 2019/10/25-03:50 null | [Chmouel Boudjnah] Increase GolangCI timeout | 2019/10/25-03:50 null | [Chmouel Boudjnah] Generate windows binary release files as zip file | 2019/10/25-04:30 null | [danielhelfand] add namespace check to resource delete command | 2019/10/25-04:47 null | [Chmouel Boudjnah] Remove 386 and arm archs | 2019/10/25-08:33 null | [danielhelfand] add namespace check to pr describe command | 2019/10/25-12:44 null | [danielhelfand] add namespace check to resource describe command | 2019/10/25-21:32 null | [danielhelfand] add namespace check to pr delete command | 2019/10/27-09:36 null | [danielhelfand] add namespace check to pr logs command | 2019/10/27-10:07 null | [danielhelfand] add namespace check to pr cancel command | 2019/10/27-12:48 null | [danielhelfand] add namespace check to pr list command | 2019/10/28-03:27 null | [danielhelfand] add namespace check to task start command | 2019/10/28-08:54 null | [danielhelfand] add namespace check to pipeline start command | 2019/10/28-09:51 null | [danielhelfand] clean up pipeline start test and correct task start namespace check | 2019/10/28-10:34 null | [Chmouel Boudjnah] Allow specify a specific release.yaml when setting up pipelines | 2019/10/29-10:48 null | [16yuki0702] Modify delete command to using delete options | 2019/10/29-21:01 null | [16yuki0702] Remove unnecessary struct | 2019/10/29-21:01 null | [Chmouel Boudjnah] Add installation instructions for Win to README kind/feature | 2019/10/30-05:03 null | [Piyush Garg] Fix make lint warning | 2019/10/30-09:12 null | [Chmouel Boudjnah] Make sure we are in the right directory when launching the tests | 2019/10/30-10:15 null | [Piyush Garg] Enable golint | 2019/10/31-04:22 null | [danielhelfand] check if taskref and pipelineref exist for describe commands | 2019/10/31-10:39 null | [Piyush Garg] Remove date from man pages | 2019/11/04-04:15 null | [Piyush Garg] Show logs after task start | 2019/11/04-21:49 null | [Piyush Garg] Fix taskrun log throwing error | 2019/11/04-21:49 null | [danielhelfand] remove space from resource and param errors | 2019/11/05-04:30 null | [Eric Carboni] Update README.md | 2019/11/05-04:30 null | [16yuki0702] Add options test cases | 2019/11/05-04:49 null | [Daniel Helfand] adding path information to Windows install instructions | 2019/11/06-02:46 null | [Pradeep Kumar] starts pipeline with new resource | 2019/11/06-05:09 null | [Piyush Garg] Show params in pipeline describe command | 2019/11/07-09:58 null | [Vincent Demeester] Add danielhelfand as OWNER 🐱 | 2019/11/07-10:19 Signed-off-by: Chmouel Boudjnah <[email protected]>

googlebot added the cla: yes label Sep 18, 2019

tekton-robot requested review from hrishin and vdemeester September 18, 2019 03:55

tekton-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 18, 2019

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 Sep 18, 2019

tekton-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 19, 2019

khrm reviewed Sep 20, 2019

View reviewed changes

16yuki0702 force-pushed the add_list_command_to_condtion_resource branch from 58384bb to 9c861b6 Compare September 26, 2019 01:08

piyush-garg reviewed Oct 1, 2019

View reviewed changes

Add condition list command

531673e

16yuki0702 force-pushed the add_list_command_to_condtion_resource branch from 9c861b6 to 531673e Compare October 1, 2019 07:36

piyush-garg approved these changes Oct 1, 2019

View reviewed changes

tekton-robot assigned piyush-garg Oct 1, 2019

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 1, 2019

tekton-robot assigned chmouel Oct 1, 2019

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 1, 2019

tekton-robot merged commit d251560 into tektoncd:master Oct 1, 2019

piyush-garg mentioned this pull request Oct 1, 2019

Feature Request: tkn condition list Command #286

Closed

chmouel added this to the 0.5.0 🐱 milestone Oct 1, 2019

16yuki0702 deleted the add_list_command_to_condtion_resource branch October 2, 2019 09:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add condition list command #319

Add condition list command #319

16yuki0702 commented Sep 18, 2019

tekton-robot commented Sep 18, 2019

hrishin commented Sep 18, 2019

tekton-robot commented Sep 18, 2019

tekton-robot commented Sep 19, 2019

khrm Sep 20, 2019

16yuki0702 Sep 20, 2019

16yuki0702 Sep 20, 2019

16yuki0702 Sep 20, 2019

tekton-robot commented Sep 20, 2019

16yuki0702 commented Sep 20, 2019

khrm commented Sep 20, 2019

tekton-robot commented Sep 20, 2019

tekton-robot commented Sep 21, 2019

tekton-robot commented Sep 26, 2019

piyush-garg Oct 1, 2019

piyush-garg Oct 1, 2019

16yuki0702 Oct 1, 2019

piyush-garg Oct 1, 2019

piyush-garg commented Oct 1, 2019

tekton-robot commented Oct 1, 2019

piyush-garg left a comment

chmouel commented Oct 1, 2019

tekton-robot commented Oct 1, 2019

tekton-robot commented Oct 1, 2019

16yuki0702 commented Oct 1, 2019

Add condition list command #319

Add condition list command #319

Conversation

16yuki0702 commented Sep 18, 2019

Changes

Submitter Checklist

Release Notes

tekton-robot commented Sep 18, 2019

hrishin commented Sep 18, 2019

tekton-robot commented Sep 18, 2019

tekton-robot commented Sep 19, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tekton-robot commented Sep 20, 2019

16yuki0702 commented Sep 20, 2019

khrm commented Sep 20, 2019

tekton-robot commented Sep 20, 2019

tekton-robot commented Sep 21, 2019

tekton-robot commented Sep 26, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

piyush-garg commented Oct 1, 2019

tekton-robot commented Oct 1, 2019

piyush-garg left a comment

Choose a reason for hiding this comment

chmouel commented Oct 1, 2019

tekton-robot commented Oct 1, 2019

tekton-robot commented Oct 1, 2019

16yuki0702 commented Oct 1, 2019