Add a cloud event helper #1090

afrittoli · 2019-07-16T18:01:48Z

Changes

Add a cloud event helper that is going to be used by the
CloudEventPipelineResource to send cloud events.
This includes an initial list of the type of events that can be
generated by Tekton.

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

See the contribution guide for more details.

Double check this list of stuff that's easy to miss:

If you are adding a new binary/image to the cmd dir, please update
the release Task and TaskRun to build and release this image

dibyom

Couple of minor nits but otherwise looks good to me!
/lgtm

pkg/reconciler/v1alpha1/taskrun/resources/cloudevent/cloudevent_test.go

pkg/reconciler/v1alpha1/taskrun/resources/cloudevent/cloudeventclient.go

Implements in the TaskRun controller the logic to provide the cloud event pipeline resource. This commits puts together the API, cloud event helper and resource definition from three pull requests: - tektoncd#1090 - tektoncd#1091 - tektoncd#1092 It adds unit tests for the new code and one E2E YAML test. The YAML test runs a simple http server that can receive the cloudevent for test purposes. The list of cloud events to be sent is added to the TaskRun status and processed by the TaskRun controller once the pod associated to the TaskRun completes its execution. The `isDone` definition of the TaskRun is not altered, the reconciler checks for events to be sent once the TaskRun.isDone is true. Retries are not implemented yet in the sense that every scheduled event will be attempted exactly once, but it may be that those attempts happen across different invocations of Reconcile. Signed-off-by: Andrea Frittoli <andrea.frittoli@uk.ibm.com>

Add a cloud event helper that is going to be used by the CloudEventPipelineResource to send cloud events. This includes an initial list of the type of events that can be generated by Tekton. Signed-off-by: Andrea Frittoli <andrea.frittoli@uk.ibm.com>

tekton-robot · 2019-07-16T19:11:50Z

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

File	Old Coverage	New Coverage
pkg/reconciler/v1alpha1/taskrun/resources/cloudevent/cloudevent.go	Do not exist	88.6%
pkg/reconciler/v1alpha1/taskrun/resources/cloudevent/cloudeventclient.go	Do not exist	11.1%
pkg/reconciler/v1alpha1/taskrun/resources/cloudevent/cloudeventsfakeclient.go	Do not exist	75.0%

afrittoli · 2019-07-16T20:20:25Z

/test pull-tekton-pipeline-build-tests

dlorenc · 2019-07-22T14:31:08Z

/lgtm

dlorenc · 2019-07-22T14:31:13Z

/approve

tekton-robot · 2019-07-22T14:31:19Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afrittoli, dibyom, dlorenc

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~~ [dlorenc]

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

afrittoli · 2019-07-22T16:45:19Z

@akihikokuroda I want to address your comments on #837, but I didn't get a chance yet since I was traveling.
The changes to be made are:

move TaskRun from the root of the event body to a TaskRun field, so that it's easier to add data in future if needed
add versioning to the cloudevent headers

Since this one has already been reviewed, I could address your comments on a PR on top. Until #837 merges the resource won't be actually usable.
@dlorenc thoughs?

afrittoli · 2019-07-23T02:51:35Z

/test pull-tekton-pipeline-integration-tests

bobcatfish · 2019-07-23T19:27:25Z

Weeeird seems like a pretty similar error both times:

I0723 03:06:20.887] --- FAIL: TestTaskRunPipelineRunCancel (2.12s)
I0723 03:06:20.888]     --- FAIL: TestTaskRunPipelineRunCancel/With_retries (3.82s)
I0723 03:06:20.888]         init_test.go:111: Create namespace arendelle-5kh2p to deploy to
I0723 03:06:20.888]         init_test.go:123: Verify SA "default" is created in namespace "arendelle-5kh2p"
I0723 03:06:20.889]         cancel_test.go:66: Creating Task in namespace arendelle-5kh2p
I0723 03:06:20.889]         cancel_test.go:74: Creating Pipeline in namespace arendelle-5kh2p
I0723 03:06:20.889]         cancel_test.go:84: Creating PipelineRun in namespace arendelle-5kh2p
I0723 03:06:20.889]         cancel_test.go:89: Waiting for Pipelinerun pear in namespace arendelle-5kh2p to be started
I0723 03:06:20.889]         cancel_test.go:110: Waiting for TaskRuns from PipelineRun pear in namespace arendelle-5kh2p to be running
I0723 03:06:20.890]         cancel_test.go:139: Failed to cancel PipelineRun `pear`: Operation cannot be fulfilled on pipelineruns.tekton.dev "pear": the object has been modified; please apply your changes to the latest version and try again

But the logic this PR is adding isn't being invoked yet so it seems pretty unlikely these changes are the cause! 🤔

bobcatfish · 2019-07-23T19:27:34Z

/test pull-tekton-pipeline-integration-tests

afrittoli · 2019-07-23T22:59:55Z

Weeeird seems like a pretty similar error both times:

I0723 03:06:20.887] --- FAIL: TestTaskRunPipelineRunCancel (2.12s)
I0723 03:06:20.888]     --- FAIL: TestTaskRunPipelineRunCancel/With_retries (3.82s)
I0723 03:06:20.888]         init_test.go:111: Create namespace arendelle-5kh2p to deploy to
I0723 03:06:20.888]         init_test.go:123: Verify SA "default" is created in namespace "arendelle-5kh2p"
I0723 03:06:20.889]         cancel_test.go:66: Creating Task in namespace arendelle-5kh2p
I0723 03:06:20.889]         cancel_test.go:74: Creating Pipeline in namespace arendelle-5kh2p
I0723 03:06:20.889]         cancel_test.go:84: Creating PipelineRun in namespace arendelle-5kh2p
I0723 03:06:20.889]         cancel_test.go:89: Waiting for Pipelinerun pear in namespace arendelle-5kh2p to be started
I0723 03:06:20.889]         cancel_test.go:110: Waiting for TaskRuns from PipelineRun pear in namespace arendelle-5kh2p to be running
I0723 03:06:20.890]         cancel_test.go:139: Failed to cancel PipelineRun `pear`: Operation cannot be fulfilled on pipelineruns.tekton.dev "pear": the object has been modified; please apply your changes to the latest version and try again

But the logic this PR is adding isn't being invoked yet so it seems pretty unlikely these changes are the cause! 🤔

That was my though exactly. There seems to be some kind of race condition happening. Having test results data with test granularity (I think we only do it on test job granularity today) might be helpful to troubleshoot this; I look into it.

Implements in the TaskRun controller the logic to provide the cloud event pipeline resource. This commits puts together the API, cloud event helper and resource definition from four pull requests: - tektoncd#1090 - tektoncd#1091 - tektoncd#1092 - TBD TBD: address review comments TBD: test for interaction with the image digest export It adds unit tests for the new code and one E2E YAML test. The YAML test runs a simple http server that can receive the cloudevent for test purposes. The list of cloud events to be sent is added to the TaskRun status and processed by the TaskRun controller once the pod associated to the TaskRun completes its execution. The `isDone` definition of the TaskRun is not altered, the reconciler checks for events to be sent once the TaskRun.isDone is true. Retries are not implemented yet in the sense that every scheduled event will be attempted exactly once, but it may be that those attempts happen across different invocations of Reconcile. Signed-off-by: Andrea Frittoli <andrea.frittoli@uk.ibm.com>

Implements in the TaskRun controller the logic to provide the cloud event pipeline resource. This commits puts together the API, cloud event helper and resource definition from four pull requests: - tektoncd#1090 - tektoncd#1091 - tektoncd#1092 TBD: Test for interaction with the image digest export It adds unit tests for the new code and one E2E YAML test. The YAML test runs a simple http server that can receive the cloudevent for test purposes. The list of cloud events to be sent is added to the TaskRun status and processed by the TaskRun controller once the pod associated to the TaskRun completes its execution. The `isDone` definition of the TaskRun is not altered, the reconciler checks for events to be sent once the TaskRun.isDone is true. Retries are not implemented yet in the sense that every scheduled event will be attempted exactly once, but it may be that those attempts happen across different invocations of Reconcile. Signed-off-by: Andrea Frittoli <andrea.frittoli@uk.ibm.com>

Implements in the TaskRun controller the logic to provide the cloud event pipeline resource. This commits puts together the API, cloud event helper and resource definition from four pull requests: - tektoncd#1090 - tektoncd#1091 - tektoncd#1092 It adds unit tests for the new code and one E2E YAML test. The YAML test runs a simple http server that can receive the cloudevent for test purposes. The list of cloud events to be sent is added to the TaskRun status and processed by the TaskRun controller once the pod associated to the TaskRun completes its execution. The `isDone` definition of the TaskRun is not altered, the reconciler checks for events to be sent once the TaskRun.isDone is true. Retries are not implemented yet in the sense that every scheduled event will be attempted exactly once, but it may be that those attempts happen across different invocations of Reconcile. Signed-off-by: Andrea Frittoli <andrea.frittoli@uk.ibm.com>

Implements in the TaskRun controller the logic to provide the cloud event pipeline resource. This commits puts together the API, cloud event helper and resource definition from four pull requests: - #1090 - #1091 - #1092 It adds unit tests for the new code and one E2E YAML test. The YAML test runs a simple http server that can receive the cloudevent for test purposes. The list of cloud events to be sent is added to the TaskRun status and processed by the TaskRun controller once the pod associated to the TaskRun completes its execution. The `isDone` definition of the TaskRun is not altered, the reconciler checks for events to be sent once the TaskRun.isDone is true. Retries are not implemented yet in the sense that every scheduled event will be attempted exactly once, but it may be that those attempts happen across different invocations of Reconcile. Signed-off-by: Andrea Frittoli <andrea.frittoli@uk.ibm.com>

tekton-robot requested review from bobcatfish and dlorenc July 16, 2019 18:01

googlebot added the cla: yes Trying to make the CLA bot happy with ppl from different companies work on one commit label Jul 16, 2019

tekton-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jul 16, 2019

dibyom approved these changes Jul 16, 2019

View reviewed changes

pkg/reconciler/v1alpha1/taskrun/resources/cloudevent/cloudevent_test.go Show resolved Hide resolved

pkg/reconciler/v1alpha1/taskrun/resources/cloudevent/cloudeventclient.go Outdated Show resolved Hide resolved

tekton-robot assigned dibyom Jul 16, 2019

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

afrittoli mentioned this pull request Jul 16, 2019

Cloud Event output resource #837

Merged

3 tasks

afrittoli force-pushed the cloud_event_client branch from 3f5e879 to b5d8cc0 Compare July 16, 2019 18:39

tekton-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 16, 2019

Add a cloud event helper

50b709f

Add a cloud event helper that is going to be used by the CloudEventPipelineResource to send cloud events. This includes an initial list of the type of events that can be generated by Tekton. Signed-off-by: Andrea Frittoli <andrea.frittoli@uk.ibm.com>

afrittoli force-pushed the cloud_event_client branch from b5d8cc0 to 50b709f Compare July 16, 2019 19:07

tekton-robot assigned dlorenc Jul 22, 2019

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

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

tekton-robot merged commit 5be3663 into tektoncd:master Jul 23, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a cloud event helper #1090

Add a cloud event helper #1090

afrittoli commented Jul 16, 2019

dibyom left a comment

tekton-robot commented Jul 16, 2019

afrittoli commented Jul 16, 2019

dlorenc commented Jul 22, 2019

dlorenc commented Jul 22, 2019

tekton-robot commented Jul 22, 2019

afrittoli commented Jul 22, 2019

afrittoli commented Jul 23, 2019

bobcatfish commented Jul 23, 2019

bobcatfish commented Jul 23, 2019

afrittoli commented Jul 23, 2019

Add a cloud event helper #1090

Add a cloud event helper #1090

Conversation

afrittoli commented Jul 16, 2019

Changes

Submitter Checklist

dibyom left a comment

Choose a reason for hiding this comment

tekton-robot commented Jul 16, 2019

afrittoli commented Jul 16, 2019

dlorenc commented Jul 22, 2019

dlorenc commented Jul 22, 2019

tekton-robot commented Jul 22, 2019

afrittoli commented Jul 22, 2019

afrittoli commented Jul 23, 2019

bobcatfish commented Jul 23, 2019

bobcatfish commented Jul 23, 2019

afrittoli commented Jul 23, 2019