Skip to content

[TEP-0137] Activate formats field in config-events#9776

Open
afrittoli wants to merge 1 commit intomainfrom
worktree-tep-0137-pr6-formats
Open

[TEP-0137] Activate formats field in config-events#9776
afrittoli wants to merge 1 commit intomainfrom
worktree-tep-0137-pr6-formats

Conversation

@afrittoli
Copy link
Copy Markdown
Member

@afrittoli afrittoli commented Apr 15, 2026

Dependencies (stacked):

Once the dependencies are merged, the PR will contain 1 commit only.

Future PRs

  • Add CDEvents support
  • Add Tekton v2 support (with v1 resources)

Changes

Activate the formats field in config-events by making EmitCloudEvents format-aware.

  • Refactor EmitCloudEvents to read cfg.Events.Formats from context and iterate over configured formats, dispatching to per-format senders. The L1 object-level cache check runs once before the format loop, not once per format.
  • Extract dispatchCloudEvent as the shared delivery core for all formats: L2 event-level cache check, goroutine dispatch, exponential-backoff retries, and Kubernetes Event recording (CloudEventSent/CloudEventFailed). PR7 format senders will call this without duplicating any delivery logic.
  • Add sendTektonV1Event as the first format-specific sender; it builds the tektonv1 event and delegates to dispatchCloudEvent.
  • Slim SendCloudEventWithRetries to a thin wrapper over dispatchCloudEvent; its public contract is unchanged.
  • EmitCloudEventsWhenConditionChange is unchanged (calls SendCloudEventWithRetries directly; remains deprecated).
  • Remove stale TODO comment from validFormats in pkg/apis/config/events.go.
  • Add "Event formats" section to docs/events.md; add one clarifying sentence to docs/additional-configs.md.

Adding a future format (tektonv2, cdevents) requires only a new eventForRunObject variant, a sendFormat function, and a case in the EmitCloudEvents switch — no changes to dispatchCloudEvent or ReconcileRunObject.

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

  • Has Docs if any changes are user facing, including updates to minimum requirements e.g. Kubernetes version bumps
  • Has Tests included if any functionality added or changed
  • Follows the commit message standard
  • Meets the Tekton contributor standards (including functionality, content, code)
  • Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
  • Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings). See some examples of good release notes.
  • Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

The formats field in config-events is now active. The default value is
tektonv1, which preserves existing behaviour. Setting an invalid or
unrecognised format value logs a warning and suppresses event emission
for that format.

/kind feature

@tekton-robot tekton-robot added kind/feature Categorizes issue or PR as related to a new feature. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Apr 15, 2026
@tekton-robot tekton-robot requested a review from abayer April 15, 2026 14:34
@tekton-robot tekton-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 15, 2026
@afrittoli afrittoli force-pushed the worktree-tep-0137-pr6-formats branch from 5757238 to def5911 Compare April 15, 2026 15:03
@tekton-robot tekton-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 15, 2026
@afrittoli afrittoli force-pushed the worktree-tep-0137-pr6-formats branch from def5911 to 176da7a Compare April 27, 2026 12:06
@tekton-robot
Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please ask for approval from afrittoli after the PR has been reviewed.

The full list of commands accepted by this bot can be found here.

Details 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 size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 27, 2026
@twoGiants twoGiants self-assigned this Apr 28, 2026
Copy link
Copy Markdown
Member

@twoGiants twoGiants left a comment

Choose a reason for hiding this comment

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

Just a first question so far. Tests review is pending ... ⌚ 😺

//
// Note: this function bypasses the L1 object-level cache check performed by
// EmitCloudEvents. Callers that want full deduplication should use EmitCloudEvents
// instead. This function is retained for callers that manage their own gating
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

`This function is retained for callers that manage their own gating (e.g. EmitCloudEventsWhenConditionChange).

Will this function also be remove once EmitCloudEventsWhenConditionChange will be removed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good catch, I should deprecate this function as well, so it will be removed once EmitCloudEventsWhenConditionChangeis removed.

Refactor EmitCloudEvents to read cfg.Events.Formats and dispatch to
per-format senders, with a single L1 cache check before the format loop.
Extract dispatchCloudEvent as the shared delivery core (L2 cache, goroutine
dispatch, retries, k8s Event recording) reused by all formats. Slim
SendCloudEventWithRetries to a thin wrapper. Add sendTektonV1Event as
the first format-specific sender. EmitCloudEventsWhenConditionChange is
unchanged (calls SendCloudEventWithRetries directly, remains deprecated).

Adding a future format (tektonv2, cdevents) requires only a new
eventForRunObject variant, a sendFormat function, and a case in the
EmitCloudEvents switch — no changes to dispatchCloudEvent or ReconcileRunObject.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Andrea Frittoli <andrea.frittoli@uk.ibm.com>
@afrittoli afrittoli force-pushed the worktree-tep-0137-pr6-formats branch from 176da7a to 8144b6b Compare April 30, 2026 14:19
@github-actions
Copy link
Copy Markdown

Merging this branch will increase overall coverage

Impacted Packages Coverage Δ 🤖
github.com/tektoncd/pipeline/pkg/apis/config 87.87% (ø)
github.com/tektoncd/pipeline/pkg/reconciler/events/cloudevent 65.18% (+2.07%) 👍
github.com/tektoncd/pipeline/pkg/reconciler/notifications 100.00% (ø)
github.com/tektoncd/pipeline/pkg/reconciler/notifications/pipelinerun 92.31% (ø)
github.com/tektoncd/pipeline/pkg/reconciler/notifications/taskrun 92.31% (ø)

Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/tektoncd/pipeline/pkg/apis/config/events.go 98.15% (ø) 54 53 1
github.com/tektoncd/pipeline/pkg/reconciler/events/cloudevent/cloud_event_controller.go 82.72% (+1.76%) 81 (+18) 67 (+16) 14 (+2) 👍

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files

  • github.com/tektoncd/pipeline/pkg/reconciler/events/cloudevent/cloud_event_controller_test.go
  • github.com/tektoncd/pipeline/pkg/reconciler/notifications/pipelinerun/reconciler_test.go
  • github.com/tektoncd/pipeline/pkg/reconciler/notifications/runtimeobject_test.go
  • github.com/tektoncd/pipeline/pkg/reconciler/notifications/taskrun/reconciler_test.go

@afrittoli
Copy link
Copy Markdown
Member Author

/retest

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. release-note Denotes a PR that will be considered when it comes time to generate release notes. 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.

3 participants