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

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

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

Conversation

skoved
Copy link

@skoved skoved commented Jul 15, 2024

No description provided.

@skoved
Copy link
Author

skoved commented Jul 15, 2024

This would not need to be done on Konflux production clusters yet, but at least installing knative eventing on testing clusters will allow integration to start if and when Konflux decides to integrate with KubeArchive


## 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).

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?

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

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

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.

@skoved
Copy link
Author

skoved commented Jul 16, 2024

I've update the ADR to (hopefully) provide more clarity about why I'm proposing to have konflux install Knative Eventing and what Knative Eventing provides. This came from feedback during the community architecture call earlier today. I've also added additional details to what I think the consequences of installing Knative Eventing might be


## 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.

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.

ADR/0039-install-knative-eventing.md Outdated Show resolved Hide resolved
@cit1zen
Copy link

cit1zen commented Jul 18, 2024

Serverless-eventing is one of the technologies we look into as a possible future basis of MintMaker (dependency management).

I think having it available in some form could open doors to considerable performance and resource utilization improvements across Konflux, especially after people earn some experience. This option becomes part of the architecture design possibilities when designing new features or redesigning existing ones.

@skoved
Copy link
Author

skoved commented Sep 4, 2024

It's been a while since anything has happened on this PR, but I was busy with KubeArchive stuff in the second half of July and all of August.

I've updated the ADR to reflect the intent of this ADR as mentioned by @brianwcook at the July 16th Architecture meeting. My understanding of it is as follows.

To consider the usage of Knative-Eventing in Konflux separately from KubeArchive and determine if the usage of Knative-Eventing could make sense in Konflux.

If you believe that this is incorrect or incomplete, I'm willing to discuss this and update the ADR as needed. I would also appreciate if those involved in the discussion would take another look. I'm also willing to bring this as a topic for the architecture call again if folks think that it is necessary/helpful.

@ralphbean
Copy link
Member

Although we don't have it in the architecture repo docs yet, I think we are happy to use some new terms to put some boundaries around different parts of konflux:

  • Konflux core - these are components that you can assume are always installed in an instance of konflux. Other components can always depend on them being present. Example, tekton.
  • Konflux add-ons - these are components that may or may not be installed in an instance of konflux. Other components can depend on them being present, but they must also have defined behavior for when they are not present. Example: mintmaker.
  • Konflux integrations - these are systems external to Konflux that some Konflux components may refer to, if so configured. Example: trustification. If configured, the Konflux UI will link to SBOMs in a trustification UI somewhere else.

What would the # Decision section look like here if written in those terms? Are we saying one of the following things?

  • knative eventing is a konflux add-on. It might be installed. If installed, other add-ons can take advantage of it. Nothing in core is allowed to depend on it.
  • knative eventing is in konflux core. It will always be installed, and everything is permitted to depend on it.
  • knative eventing is in konflux core, and all subsystems must refactor themselves to depend on it.

@skoved
Copy link
Author

skoved commented Sep 4, 2024

knative eventing is in konflux core, and all subsystems must refactor themselves to depend on it.

I want to make clear that this is NOT what this ADR is proposing. It is not the goal to force anyone to use Knative-Eventing

With that out of the way:

What would the # Decision section look like here if written in those terms? Are we saying one of the following things?

* knative eventing is a konflux add-on. It might be installed. If installed, other add-ons can take advantage of it. Nothing in core is allowed to depend on it.

* knative eventing is in konflux core. It will always be installed, and everything is permitted to depend on it.

This is something that I was thinking about, but I'm not sure that I can answer this myself. I'm not developing for konflux (directly) and I don't have any decision making power for the architecture and direction for Konflux. I'm hopeing that this ADR can start that discussion.

I assume that baring any other decisions that get made in the future, Knative-Eventing will be installed where ever KubeArchive gets installed. My assumption is that since KubeArchive would replace Tekton Results, KubeArchive and Knative-Eventing as a dependency for KubeArchive, would fit into the same category that Tekton Results inhabits currently. The only way I would see this changing is if, for the sake of argument (idk if this is actually the case in Konflux today) Tekton Results is in the Konflux add-ons or Konflux integrations group, Konflux makes the decision that it could be useful to have Knative-Eventing available and then promote Knative-Eventing into Konflux core. In this comment I list potential benefits to Knative-Eventing, but since I'm not directly involved in Konflux development, I cannot point to specific places in Konflux core that could benefit directly from Knative-Eventing. Based on the comment from @cit1zen it seems like MintMaker would be interested in Knative-Eventing if it was made available. I'm not sure which category MintMaker is in though

@ralphbean
Copy link
Member

ralphbean commented Sep 4, 2024

So, (this is up for debate) I think we want KubeArchive to be a Konflux add-on. It might be installed in an instance of Konflux, or it might not. If it is installed, then the UI can be configured to query it for old archived resources.

Today, I think tekton-results may actually be in konflux core, but (and this is up for debate) I think we may want to push it out to be a konflux add-on.

If we assume KubeArchive is an add-on, then I think we should start with the minimalist position of characterizing Knative Eventing as an add-on, and (simply) a dependency of KubeArchive. None of the core component would be permitted to depend on it. Mintmaker may depend on it, if that team chooses; it is also an optional add-on.

@gbenhaim
Copy link
Member

gbenhaim commented Sep 8, 2024

Additional concern that I have is that the in memory channel shouldn't be used for production purposes, so it would require users to also get a Kafka instance, and I would really prefer not to have hard dependency on Kafka.

@pierDipi
Copy link

Hi everyone, I'm from Serverless/Knative Eventing team.

What I've read as concerns/points of discussions so far is:

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

For 1) we have specific configurations to deploy only what you exactly need and we can help you with setting that up so that resource usage is as minimal as possible for your use cases and we might even do this automatically in the future.

  1. Additional concern that I have is that the in memory channel shouldn't be used for production purposes, so it would require users to also get a Kafka instance, and I would really prefer not to have hard dependency on Kafka.

For 2), while that's currently true if you plan to use Broker and Triggers and have reliable distributed persistence, to use only event sources, you can just use Eventing with no other dependencies.
Related questions are: is Kafka and operating Kafka the problem or generally having another dependency? would you be more comfortable with a different system to store events? and if so, which one?

I can't comment on other use cases and whether Eventing is a good fit for other core Konflux use cases as I don't know the specifics, however, I believe having the platform evolve and rely on events will make the platform much more reliable and scalable in the long run, especially since the Eventing runtime interface is just an HTTP POST request with a couple of extra standardized metadata/headers (CloudEvents), so consuming and producing services are very decoupled from specific implementation details and can be deployed and tested independently.

I don't want to hijack the conversation but feel free to reach out to us for any questions on #forum-serverless Slack or serverless-dev mailing list / group.

@gbenhaim
Copy link
Member

  1. Additional concern that I have is that the in memory channel shouldn't be used for production purposes, so it would require users to also get a Kafka instance, and I would really prefer not to have hard dependency on Kafka.

For 2), while that's currently true if you plan to use Broker and Triggers and have reliable distributed persistence, to use only event sources, you can just use Eventing with no other dependencies. Related questions are: is Kafka and operating Kafka the problem or generally having another dependency? would you be more comfortable with a different system to store events? and if so, which one?

@pierDipi yes my concern is requiring a potential Konflux user to also maintain/use managed Kafka instance. If there
is a lighter backed that can be used it would be great.

@arewm arewm changed the title Propose an ADR to install Knative Eventing ADR 39: Propose an ADR to install Knative Eventing Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants