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

Cloud Events for Runs #3862

Closed
afrittoli opened this issue Mar 31, 2021 · 15 comments · Fixed by #4663
Closed

Cloud Events for Runs #3862

afrittoli opened this issue Mar 31, 2021 · 15 comments · Fixed by #4663
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@afrittoli
Copy link
Member

Feature request

Cloud events should be sent Runs.

It needs to be identified whether we want this to be a responsibility of the custom task controllers, the pipeline controller or a dedicated one.

Use case

We support cloud events for taskruns and pipelineruns, we should support them for runs too.

@afrittoli afrittoli added kind/feature Categorizes issue or PR as related to a new feature. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. labels Mar 31, 2021
@imjasonh
Copy link
Member

I like the idea of basic cloudevents for Run start/success/failure that everybody gets out of the box, and documentation/samples for Custom Task authors to add their own if they want.

The pipelines controller doesn't currently do anything with Runs, I'm not even sure it has a no-op reconciler for them, so if we decide we want to support this we'd add one and do it there.

@tekton-robot
Copy link
Collaborator

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale with a justification.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 29, 2021
@tekton-robot
Copy link
Collaborator

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 15, 2021
@pritidesai
Copy link
Member

@jerop to add this as part of the promotion of custom tasks to beta, issue #4313

@pritidesai
Copy link
Member

/remove lifecycle/rotten

@pritidesai
Copy link
Member

remove-lifecycle rotten

@jerop
Copy link
Member

jerop commented Oct 19, 2021

/lifecycle frozen

@tekton-robot tekton-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. labels Oct 19, 2021
@afrittoli afrittoli added this to the Pipelines v0.30 milestone Oct 19, 2021
@afrittoli
Copy link
Member Author

Custom tasks can be part of a pipeline now - in that scenario we could send cloud events on behalf of the Run controller.
We cannot control cloud events for Runs outside of pipelines today, as that would be up to the custom run controller, but we could add support for Runs in the experimental cloud-events controller instead, which would then take care of all kind of Runs.

@jerop
Copy link
Member

jerop commented Nov 15, 2021

moved this to milestone for v0.31

@pritidesai
Copy link
Member

/assign @afrittoli

@afrittoli
Copy link
Member Author

afrittoli commented Mar 1, 2022

The PipelineRun controller creates the Run so it could send that event.
The PipelineRun controller also watches for changes to Runs so it would need to check if there was a change between the local copy of the status and the one reported by the informer. This could be done in updateRunsStatusDirectly - however since we plan on removing the local copy of the status, I think it would not be wise to implement a new feature that relies on that.

My proposal here would be to implement this in the external controller instead. This would then be implemented once the external controller replaced the inbuilt one for sending events.

@pritidesai @vdemeester @jerop thoughts?

@vdemeester
Copy link
Member

I tend to agree on the external controller 👼🏼

@pritidesai
Copy link
Member

thanks @afrittoli!

Yup implementing as part of an external controller sounds reasonable. This can give freedom to the external controller in designing the events based on their requirements.

@afrittoli afrittoli removed the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Mar 10, 2022
afrittoli added a commit to afrittoli/pipeline that referenced this issue Mar 10, 2022
Add the definitions for new CloudEvents for Runs.
The events are not sent yet. The event types are documented and
the cloudevent package supports creating events for Runs.

Partially addresses tektoncd#3862

Signed-off-by: Andrea Frittoli <[email protected]>
afrittoli added a commit to afrittoli/pipeline that referenced this issue Mar 10, 2022
Add the definitions for new CloudEvents for Runs.
The events are not sent yet. The event types are documented and
the cloudevent package supports creating events for Runs.

Partially addresses tektoncd#3862

Signed-off-by: Andrea Frittoli <[email protected]>
afrittoli added a commit to afrittoli/pipeline that referenced this issue Mar 11, 2022
Add the definitions for new CloudEvents for Runs.
The events are not sent yet. The event types are documented and
the cloudevent package supports creating events for Runs.

Partially addresses tektoncd#3862

Signed-off-by: Andrea Frittoli <[email protected]>
afrittoli added a commit to afrittoli/pipeline that referenced this issue Mar 11, 2022
Emit CloudEvents for Runs. This is achieved by:
- add a new read-only controller for Runs
- emit CloudEvents only (no k8s events) on every reconcile of a Run
- use an ephemeral cache to store sent events across reconcile runs.
  This is required because since the Runs controller only observes
  Runs, it does not have the context to know what was changed in the
  Run and though if a new event is required.

The ephemeral cache logic is largely taken from the same
functionality implemented in tektoncd/experimental/cloudevents

Fixes tektoncd#3862

Signed-off-by: Andrea Frittoli <[email protected]>
afrittoli added a commit to afrittoli/pipeline that referenced this issue Mar 11, 2022
Emit CloudEvents for Runs. This is achieved by:
- add a new read-only controller for Runs
- emit CloudEvents only (no k8s events) on every reconcile of a Run
- use an ephemeral cache to store sent events across reconcile runs.
  This is required because since the Runs controller only observes
  Runs, it does not have the context to know what was changed in the
  Run and though if a new event is required.

The ephemeral cache logic is largely taken from the same
functionality implemented in tektoncd/experimental/cloudevents

Fixes tektoncd#3862

Signed-off-by: Andrea Frittoli <[email protected]>
@afrittoli
Copy link
Member Author

@afrittoli
Copy link
Member Author

/cc @lbernick

afrittoli added a commit to afrittoli/pipeline that referenced this issue Mar 11, 2022
Emit CloudEvents for Runs. This is achieved by:
- add a new read-only controller for Runs
- emit CloudEvents only (no k8s events) on every reconcile of a Run
- use an ephemeral cache to store sent events across reconcile runs.
  This is required because since the Runs controller only observes
  Runs, it does not have the context to know what was changed in the
  Run and though if a new event is required.

The ephemeral cache logic is largely taken from the same
functionality implemented in tektoncd/experimental/cloudevents

Fixes tektoncd#3862

Signed-off-by: Andrea Frittoli <[email protected]>
afrittoli added a commit to afrittoli/pipeline that referenced this issue Mar 11, 2022
Emit CloudEvents for Runs. This is achieved by:
- add a new read-only controller for Runs
- emit CloudEvents only (no k8s events) on every reconcile of a Run
- use an ephemeral cache to store sent events across reconcile runs.
  This is required because since the Runs controller only observes
  Runs, it does not have the context to know what was changed in the
  Run and though if a new event is required.

The ephemeral cache logic is largely taken from the same
functionality implemented in tektoncd/experimental/cloudevents

Fixes tektoncd#3862

Signed-off-by: Andrea Frittoli <[email protected]>
afrittoli added a commit to afrittoli/pipeline that referenced this issue Mar 11, 2022
Emit CloudEvents for Runs. This is achieved by:
- add a new read-only controller for Runs
- emit CloudEvents only (no k8s events) on every reconcile of a Run
- use an ephemeral cache to store sent events across reconcile runs.
  This is required because since the Runs controller only observes
  Runs, it does not have the context to know what was changed in the
  Run and though if a new event is required.

The ephemeral cache logic is largely taken from the same
functionality implemented in tektoncd/experimental/cloudevents

Fixes tektoncd#3862

Signed-off-by: Andrea Frittoli <[email protected]>
afrittoli added a commit to afrittoli/pipeline that referenced this issue Mar 11, 2022
Emit CloudEvents for Runs. This is achieved by:
- add a new read-only controller for Runs
- emit CloudEvents only (no k8s events) on every reconcile of a Run
- use an ephemeral cache to store sent events across reconcile runs.
  This is required because since the Runs controller only observes
  Runs, it does not have the context to know what was changed in the
  Run and though if a new event is required.

The ephemeral cache logic is largely taken from the same
functionality implemented in tektoncd/experimental/cloudevents

Fixes tektoncd#3862

Signed-off-by: Andrea Frittoli <[email protected]>
afrittoli added a commit to afrittoli/pipeline that referenced this issue Mar 14, 2022
Add the definitions for new CloudEvents for Runs.
The events are not sent yet. The event types are documented and
the cloudevent package supports creating events for Runs.

Partially addresses tektoncd#3862

Signed-off-by: Andrea Frittoli <[email protected]>
afrittoli added a commit to afrittoli/pipeline that referenced this issue Mar 14, 2022
Add the definitions for new CloudEvents for Runs.
The events are not sent yet. The event types are documented and
the cloudevent package supports creating events for Runs.

Partially addresses tektoncd#3862

Signed-off-by: Andrea Frittoli <[email protected]>
afrittoli added a commit to afrittoli/pipeline that referenced this issue Mar 14, 2022
Emit CloudEvents for Runs. This is achieved by:
- add a new read-only controller for Runs
- emit CloudEvents only (no k8s events) on every reconcile of a Run
- use an ephemeral cache to store sent events across reconcile runs.
  This is required because since the Runs controller only observes
  Runs, it does not have the context to know what was changed in the
  Run and though if a new event is required.

The ephemeral cache logic is largely taken from the same
functionality implemented in tektoncd/experimental/cloudevents

Fixes tektoncd#3862

Signed-off-by: Andrea Frittoli <[email protected]>
tekton-robot pushed a commit that referenced this issue Mar 14, 2022
Add the definitions for new CloudEvents for Runs.
The events are not sent yet. The event types are documented and
the cloudevent package supports creating events for Runs.

Partially addresses #3862

Signed-off-by: Andrea Frittoli <[email protected]>
afrittoli added a commit to afrittoli/pipeline that referenced this issue Mar 14, 2022
Emit CloudEvents for Runs. This is achieved by:
- add a new read-only controller for Runs
- emit CloudEvents only (no k8s events) on every reconcile of a Run
- use an ephemeral cache to store sent events across reconcile runs.
  This is required because since the Runs controller only observes
  Runs, it does not have the context to know what was changed in the
  Run and though if a new event is required.

The ephemeral cache logic is largely taken from the same
functionality implemented in tektoncd/experimental/cloudevents

Fixes tektoncd#3862

Signed-off-by: Andrea Frittoli <[email protected]>
afrittoli added a commit to afrittoli/pipeline that referenced this issue Mar 14, 2022
Emit CloudEvents for Runs. This is achieved by:
- add a new read-only controller for Runs
- emit CloudEvents only (no k8s events) on every reconcile of a Run
- use an ephemeral cache to store sent events across reconcile runs.
  This is required because since the Runs controller only observes
  Runs, it does not have the context to know what was changed in the
  Run and though if a new event is required.

The ephemeral cache logic is largely taken from the same
functionality implemented in tektoncd/experimental/cloudevents

Fixes tektoncd#3862

Signed-off-by: Andrea Frittoli <[email protected]>
afrittoli added a commit to afrittoli/pipeline that referenced this issue Mar 14, 2022
Emit CloudEvents for Runs. This is achieved by:
- add a new read-only controller for Runs
- emit CloudEvents only (no k8s events) on every reconcile of a Run
- use an ephemeral cache to store sent events across reconcile runs.
  This is required because since the Runs controller only observes
  Runs, it does not have the context to know what was changed in the
  Run and though if a new event is required.

The ephemeral cache logic is largely taken from the same
functionality implemented in tektoncd/experimental/cloudevents

Fixes tektoncd#3862

Signed-off-by: Andrea Frittoli <[email protected]>
afrittoli added a commit to afrittoli/pipeline that referenced this issue Mar 14, 2022
Emit CloudEvents for Runs. This is achieved by:
- add a new read-only controller for Runs
- emit CloudEvents only (no k8s events) on every reconcile of a Run
- use an ephemeral cache to store sent events across reconcile runs.
  This is required because since the Runs controller only observes
  Runs, it does not have the context to know what was changed in the
  Run and though if a new event is required.

The ephemeral cache logic is largely taken from the same
functionality implemented in tektoncd/experimental/cloudevents

Fixes tektoncd#3862

Signed-off-by: Andrea Frittoli <[email protected]>
tekton-robot pushed a commit that referenced this issue Mar 15, 2022
Emit CloudEvents for Runs. This is achieved by:
- add a new read-only controller for Runs
- emit CloudEvents only (no k8s events) on every reconcile of a Run
- use an ephemeral cache to store sent events across reconcile runs.
  This is required because since the Runs controller only observes
  Runs, it does not have the context to know what was changed in the
  Run and though if a new event is required.

The ephemeral cache logic is largely taken from the same
functionality implemented in tektoncd/experimental/cloudevents

Fixes #3862

Signed-off-by: Andrea Frittoli <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

7 participants