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

Add support for embedded trigger binding to EventListener describe command #1014

Merged
merged 1 commit into from
Jun 11, 2020

Conversation

savitaashture
Copy link
Contributor

@savitaashture savitaashture commented Jun 9, 2020

Changes

Added changes to support embedded trigger binding to EventListener describe command

Fixes #1004

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.

Output:

tkn el describe github-listener-interceptor -n clitest
Name:                      github-listener-interceptor
Namespace:                 clitest
Service Account:           tekton-triggers-github-sa
Service Type:              NodePort
URL:                       http://el-github-listener-interceptor.clitest.svc.cluster.local:8080
EventListnerServiceName:   el-github-listener-interceptor
 
EventListenerTriggers

 NAME
 ∙ github-listener
 
 BINDINGS

  REF                 KIND             APIVERSION
  ∙ github-binding    TriggerBinding   
  ∙ github-binding1   TriggerBinding   
  ∙ github-binding2   TriggerBinding   v1alpha1
 
  SPEC
  ∙ Params
    gitrevision        $(body.head_commit.id)
    gitrepositoryurl   $(body.repository.url)
    gitrevision        $(body.head_commit.id)
    gitrepositoryurl   $(body.repository.url)
    gitrevision        $(body.head_commit.id)
    gitrepositoryurl   $(body.repository.url)
  ∙ Params
    gitrevision        $(body.head_commit.id)
    gitrepositoryurl   $(body.repository.url)
 
 TEMPLATE NAME       APIVERSION
 ∙ github-template   
 
 INTERCEPTORS
- github:
    eventTypes:
    - pull_request
    secretRef:
      secretKey: secretToken
      secretName: github-secret

@tekton-robot tekton-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 9, 2020
@tekton-robot
Copy link
Contributor

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

File Old Coverage New Coverage Delta
pkg/cmd/eventlistener/describe.go 82.0% 83.7% 1.7

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.

Looks fine to me for the most part, but am curious why APIVERSION isn't being populated when I run this locally? I also noticed it's missing from your pr description out so thought I would ask.

tb1 TriggerBinding
REF KIND APIVERSION
tb2 ClusterTriggerBinding v1alpha1
SPEC
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer all binding ref to be together in single table

Copy link
Contributor

Choose a reason for hiding this comment

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

Something like

Name:        el1
Namespace:   ns
 
EventListenerTriggers

 BINDINGS

  REF   KIND                    APIVERSION
  tb1   TriggerBinding          
  tb2   ClusterTriggerBinding   v1alpha1
  tb3   TriggerBinding          v1alpha1

  SPEC
  Params
    key    value
    key1   value1
    key2   value2
 
 TEMPLATE NAME   APIVERSION
 tt1             v1alpha1

@tekton-robot
Copy link
Contributor

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

File Old Coverage New Coverage Delta
pkg/cmd/eventlistener/describe.go 82.0% 86.4% 4.4

@savitaashture
Copy link
Contributor Author

Looks fine to me for the most part, but am curious why APIVERSION isn't being populated when I run this locally? I also noticed it's missing from your pr description out so thought I would ask.

Its because while creating EventListener yaml i have not mentioned specific APIVersion so thats why its not showing in the Description.
But if we specify in the yaml then it will display
I have updated PR description please have a look
Thank you

@tekton-robot
Copy link
Contributor

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

File Old Coverage New Coverage Delta
pkg/cmd/eventlistener/describe.go 82.0% 86.4% 4.4

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.

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 11, 2020
@chmouel
Copy link
Member

chmouel commented Jun 11, 2020

/lgtm
/approve

👍

@tekton-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chmouel

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

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 11, 2020
@tekton-robot tekton-robot merged commit a17c1a1 into tektoncd:master Jun 11, 2020
pradeepitm12 added a commit that referenced this pull request Jul 24, 2020
#1020 | [Chmouel Boudjnah] Update to 0.10.0 | 2020/06/10-16:40
#1022 | [Chmouel Boudjnah] Pass k8 context when printing the log command for clustertask start | 2020/06/11-15:31
#1022 | [Chmouel Boudjnah] Pass k8 context when printing the log command for pipeline start | 2020/06/11-15:31
#1022 | [Chmouel Boudjnah] Pass kubernetes context when printing the log command for start task | 2020/06/11-15:31
#1022 | [Chmouel Boudjnah] Add flags.GetTektonOptions(cmd *cobra.Command) | 2020/06/11-15:31
#1014 | [savitaashture] Add support for embedded trigger binding to EventListener describe command | 2020/06/11-15:56
#1023 | [Daniel Helfand] add clustertriggerbinding to useful commands docs | 2020/06/11-16:27
#1028 | [Daniel Helfand] fix go lint errors from 1.27 upgrade | 2020/06/19-09:59
#1027 | [Daniel Helfand] add bug, feature, and free form issue templates | 2020/06/22-10:17
#1024 | [Daniel Helfand] common way of referring to tekton resources for user facing messages: TaskRun | 2020/06/23-10:21
null | [Daniel Helfand] common way of referring to tekton resources for user facing messages: Task | 2020/06/24-09:31
null | [Divyansh42] Refactor v1beta1 unit tests | 2020/06/25-07:37
null | [vinamra28] Command Suggestion for Incorrect Subcommands | 2020/06/29-08:39
null | [praveen4g0] Refactor code | 2020/06/29-08:50
null | [Daniel Helfand] common way of referring to tekton resources for user facing messages: clustertaskh | 2020/06/29-14:42
null | [Daniel Helfand] common way of referring to tekton resources for user facing messages: Pipeline | 2020/06/30-09:25
null | [Daniel Helfand] common way of referring to tekton resources for user facing messages: PipelineRun | 2020/07/01-13:04
null | [Daniel Helfand] change kn to tkn in feature issue template | 2020/07/02-06:58
null | [Daniel Helfand] remove namespace validation | 2020/07/03-16:20
null | [Vincent Demeester] Update the pull_request_template for release-note 📖 | 2020/07/06-09:51
null | [Vincent Demeester] Add ppc64le to cross targets | 2020/07/06-18:00
null | [Vincent Demeester] Update Linux IBM note on 0.11.0 release | 2020/07/06-18:00
null | [Piyush Garg] Bump to use tektoncd/pipeline v0.14.0 | 2020/07/07-14:16
null | [Piyush Garg] Bump to use tektoncd/triggers v0.6.0 | 2020/07/08-05:30
null | [PuneetPunamiya] Enables auto selection of task for `describe` command if only one task is present | 2020/07/09-06:23
null | [Piyush Garg] Bump to tektoncd/triggers v0.6.1 | 2020/07/09-09:36
null | [Piyush Garg] Fix pipelinerun logs error | 2020/07/10-17:27
null | [Vincent Demeester] dep: update golang.org/x/text to v0.3.3 | 2020/07/10-18:31
null | [Divyansh42] Add --use-param-defaults option for tkn task start | 2020/07/16-09:36
null | [Piyush Garg] Bump to tektoncd/pipeline v0.14.2 | 2020/07/17-14:18
null | [Anshul Verma] Fixes 1068 tkn TR/PR describe --last command resulted in panic when there are no TR/PR present in the namespace. Fixed it to give an error message when no TR/PR is present in the namespace. | 2020/07/17-17:05
null | [Dan Lorenc] Add the misspell linter, and fix some misspellings. | 2020/07/20-16:16
null | [Daniel Helfand] add deadcode to golangci-lint | 2020/07/20-16:30
null | [Daniel Helfand] support VolumeClaimTemplates with --workspace | 2020/07/21-17:02
null | [vinamra28] Add Workspaces in Release pipeline instead os PipelineResources | 2020/07/22-08:21
null | [vinamra28] Use VolumeClaimTemplate instead of PVC | 2020/07/22-08:21
null | [Pradeep Kumar] adds finally logs in pipelinerun | 2020/07/22-10:32
null | [Pradeep Kumar] Update Version Param for golang-build Task | 2020/07/23-08:23
null | [Pradeep Kumar] correct catalog task golangci lint path | 2020/07/23-15:18

Signed-off-by: Pradeep Kumar <pradkuma@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. lgtm Indicates that a PR is ready to be merged. 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.

Add support for Embedded Trigger Binding in Eventlistener
5 participants