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

Allow passing labels to start #342

Merged
merged 1 commit into from
Oct 3, 2019

Conversation

chmouel
Copy link
Member

@chmouel chmouel commented Oct 2, 2019

Changes

Add -l flag for start to pass labels to pipelineruns (and then to pods)

Closes #339

Submitter Checklist

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

Release Notes

Allow passing labels when starting a pipeline with the `-l` option

@tekton-robot tekton-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 2, 2019
@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/cmd/pipeline/start.go 91.7% 91.9% 0.2

return nil
}

pr.ObjectMeta.Labels = labels
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a thought, this will remove the old labels. I think we should keep the old labels and add new ones. In the case of repeat, we keep the new one.

Copy link
Member Author

@chmouel chmouel Oct 2, 2019

Choose a reason for hiding this comment

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

you are right, and i was just being lazy! thanks mister senior principal to keep me straight! 😀 👨‍🏫

Copy link
Contributor

Choose a reason for hiding this comment

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

You troll nicely 😄

Add -l flag for start to pass labels to pipelineruns (and then to pods)

Closes tektoncd#339

Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
@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/cmd/pipeline/start.go 91.7% 92.0% 0.3

Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

/lgtm
/hold
/cc @sthaha @hrishin

@@ -132,6 +134,7 @@ like cat,foo.bar
c.Flags().StringSliceVar(&opt.ServiceAccounts, "task-serviceaccount", []string{}, "pass the service account corresponding to the task")
flags.AddShellCompletion(c.Flags().Lookup("task-serviceaccount"), "__kubectl_get_serviceaccount")
c.Flags().BoolVarP(&opt.Last, "last", "L", false, "re-run the pipeline using last pipelinerun values")
Copy link
Member

Choose a reason for hiding this comment

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

Didn't we want to remove the shorthand flag for --last ?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah we moved it to -L #340 and unified labels to -l as kubectl and other kube tools,

Copy link
Member

Choose a reason for hiding this comment

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

fair enough 😛

@tekton-robot tekton-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 2, 2019
@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 2, 2019
@tekton-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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:

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

@vdemeester
Copy link
Member

/hold cancel

@tekton-robot tekton-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 3, 2019
@tekton-robot tekton-robot merged commit 1fe080a into tektoncd:master Oct 3, 2019
@chmouel chmouel added this to the 0.5.0 🐱 milestone Oct 3, 2019
chmouel added a commit that referenced this pull request Nov 8, 2019
#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 <chmouel@redhat.com>
@chmouel chmouel deleted the issue-339-pass-labels branch June 27, 2021 05:20
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. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a flag to pass labels when starting a Pipeline
5 participants