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.

Sign up for GitHub

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

ADR 39: Propose an ADR to install Knative Eventing #202

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

ADR 39: Propose an ADR to install Knative Eventing #202

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
79547fc
Propose an ADR to install Knative Eventing
skoved Jul 15, 2024
90d6558
reword ADR and provide more info about the why this is being proposed…
skoved Jul 16, 2024
0be3ce9
update ADR to more accurately reflect the purpose of the request as m…
skoved Sep 4, 2024
4f72f4d
update ADR to use correct terminology and reflect recent discussion
skoved Sep 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions ADR/0039-install-knative-eventing.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# 39. Install Knative Eventing

Created: 2024-07-15

## Status

Proposed
Copy link
Member

Choose a reason for hiding this comment

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

I think that the ADR should describe how Konflux will integrate with Kubearchive. For example how we configure, cluster wide, which resources we want to offload from the cluster what is the api for querying them.

In addition, we need to decide if it's going to be a hard dependency of Konflux (it means anyone who want to use Konflux will have to install KNative), including users that would like to try out kind on a local cluster such as Kind (for this use case we are striving to make the Konflux instllation as simple as possible).

In addition, all the requirement of Kubearchive should be listed, I believe it also requires a database (maybe other things I'm not aware of).

Copy link

Choose a reason for hiding this comment

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

@gbenhaim I advised @skoved to open an ADR just for getting Knative added to Konflux so we could weigh the merits and issues separately from KubeArchive. If you think that is the wrong approach its probably on me.

Copy link
Member

Choose a reason for hiding this comment

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

@brianwcook why should we consider Knative separately from Kubearchive? do you see it (Knative) being used elsewhere in Konflux?

Copy link

Choose a reason for hiding this comment

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

That is exactly what I want to discuss. Should knative always be deployed? Should we encourage, or at least allow, Knative to be considered in new core feature designs? Or should it be an optional component which is only used when it is dragged in by something like KubeArchive?

Copy link
Contributor

Choose a reason for hiding this comment

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

Even unused features are using resources in dev clusters, konflux is beefy enough with mandatory parts IMO.

Does it bring any benefits or other use cases except being a dependency of kubearchive?

Copy link
Author

@skoved skoved Jul 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

why should we consider Knative separately from Kubearchive? do you see it (Knative) being used elsewhere in Konflux?

Does it bring any benefits or other use cases except being a dependency of kubearchive?

Based on some of the discussion earlier today at the Community Architecture Call, if:

  1. there are components of Konflux that implement their own resource watchers
  2. there are components of Konflux that watch the same resources

Then I personally think that this a place where the use of Knative Eventing, specifically ApiServerSource, Brokers, and Triggers could:

  1. reduce the amount of resources used by the cluster on running watchers,
  2. reduce the amount of duplicate logic in Konflux to watch resources
  3. simplify the process for new components to start watching resources on the cluster
  4. simplify the process for existing components to start watching resources they weren't watching before
  5. easily determine what resources different components are watching
  6. easily determine the set of resources that Konflux as a whole watches
  7. improve debugging by being able to receive the full history of state changes on the cluster through the persistent storage of cloudevents in a broker or channel

Should knative always be deployed?

At the very least, Knative Eventing must deployed anywhere that Konflux deploys KubeArchive. I think that Konflux would benefit from using Knative Eventing itself (see my arguments above)

Copy link
Member

Choose a reason for hiding this comment

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

We do have a lot of controllers that look at PipelineRun resources. So it can make architectural sese to use Knative eventing instead of the usual controller watch loops. The watch llops have one major advantage though, they are especially easy to code when you use Kubebuider. In Konflux we are very heavy users of Kubebuilder. When I checked a while ago, I couldn't find anything in the Knative world that comes close to being a comprehensive framework like Kubebuilder. Especially for eventing.

Copy link
Author

@skoved skoved Jul 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not as familiar with the inner workings of konflux as, so thank you for providing more insight.

Knative Eventing, more specifically cloudevents, may not have something like KubeBuilder, but the cloudevents sdk provides a really easy way to start receiving events from knative eventing. This example is extremely simple and just printing out the event id but i think it demonstrates the point. Also KubeBuilder is performing a lot boilerplate needed for starting the controller and being able to register it with Kubernetes. With Knative Eventing most of the that is offloaded to the configuration of ApiServerSource, to tell it which types of resources to create events for, and Triggers/Subscriptions so that the consumer only receives events that it is interested in. This lets the application consuming the events to only worry about the logic for processing the event and performing some set of required actions based on that event. Because of this difference I think it makes sense why there is not totally comprehensive framework. I do think that the cloudevents sdk provides comprehensive tools that make it incredibly simple to start receiving events so that can focus processing and reacting to them.

I don't think that eventing can completely replace controllers because controllers and eventing are meant to solve different problems. I do think that there are areas that could benefit from Knative Eventing. For example, a controller that watches tekton PipelineRuns so that when they complete, some action can be performed like sending an email. I think that's an example where using eventing could make more sense. It would receive an event with a PipelineRun object as the payload, it would verify that the PipelineRun completed, grab what ever other data it needs from the object to fill in an email template and then send the email. A controller that has to synchronize the state of different resources on the cluster wouldn't necessarily benefit from ApiServerSource as easily. However, it could send cloudevents when it performs different actions. Other services could subscribe to these events instead of having to watch resources that this controller synchronizes.


## Context

Konflux currently uses [Tekton Results](https://tekton.dev/docs/results) to store Tekton PipelineRuns off the Kubernetes
Cluster that they were run on. There have been many performance issues that Konflux has encountered while using Tekton
Results. To side step the issues with Tekton Results, Konflux may decide to integrate with
[KubeArchive](https://github.com/kubearchive/kubearchive) to store PipelineRuns and potentially other kinds of resources
on the cluster in a database for long term storage.

Unlike Tekton Results, KubeArchive does not provide its own component to watch resources in the cluster. Instead it uses
[`ApiServerSource` from Knative Eventing](https://knative.dev/docs/eventing/sources/apiserversource) to listen to
resource specified on the cluster and create [Cloudevents](https://cloudevents.io) that contain the definition of the
object on the cluster that has changed.

Currently Tekton Results is a Konflux core component, however KubeArchive might integrate with Konflux as a Konflux
add-on. Integration with KubeArchive will mean that Knative-Eventing will be installed as a Konflux add-on.

## Decision

Knative-Eventing is a Konflux add-on. It might be installed in an instance of Konflux, but that is not guaranteed. Other
Konflux add-ons may use Knative-Eventing as a dependency. Konflux core components cannot use Knative-Eventing as a
dependency because Knative-Eventing is not guaranteed to be present in all Konflux installations.

## Consequences

If more Konflux add-ons take advantage of Knative-Eventing, duplicated Cloudevents infrastructure may appear. This
may require a centralization and standardization of the CloudEvent pipelines. As Konflux developers become more familiar
with CloudEvents, it may become apparent that there are parts of Konflux core that could benefit from the use of
CloudEvents, which would require this decision being revisited.