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

Konflux should emit cloud events. #206

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ggallen
Copy link

@ggallen ggallen commented Sep 24, 2024

No description provided.

Copy link
Member

@gbenhaim gbenhaim left a comment

Choose a reason for hiding this comment

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

LGTM. I just want to mention that Tekton has a feature for emitting cloud event (https://tekton.dev/vault/pipelines-v0.50.x-lts/additional-configs/#configuring-cloudevents-notifications), so at least reacting to the Konflux' pipelineruns and taskruns is already possible today.

@ggallen
Copy link
Author

ggallen commented Oct 10, 2024

so at least reacting to the Konflux' pipelineruns and taskruns is already possible today.

The functionality is there in Tekton, but I don't believe it is enabled by default. But one of the first things to do should this ADR be accepted is to turn on that functionality.

To support this, all Konflux components should be required to emit cloud events for signicant events. These
should be documented fully and made available for users.

Cloud event generation could be optional and that option could default to off. But users should be able to
Copy link
Contributor

Choose a reason for hiding this comment

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

This probably needs clarification on what "users" are. I'm assuming this is a Konflux deployment admin, and not a user that uses Konflux to build content.

Copy link
Member

Choose a reason for hiding this comment

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

From async conversation - I think Greg means for this to be end-users. I.e., the developers that use Konflux to build, test and release their software. He doesn't mean platform engineers (the people running a konflux instance) or other konflux controllers.

they can use these cloud events to create their own product-specific infrastructure to support their build
and release process.

To support this, all Konflux components should be required to emit cloud events for signicant events. These
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's consider if "should be required to emit" is too strong of a statement. I believe "should emit" better describes the intent.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree on the "softening"

Konflux should not emit cloud events.

Emitting cloud events would allow Konflux users to easily track what is happening in the system. In addition,
they can use these cloud events to create their own product-specific infrastructure to support their build
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use a different term other than "product-specific" throughout this document? AFAIK, Konflux is not aware of what a product is. Maybe use "application-specific"?

Copy link
Contributor

Choose a reason for hiding this comment

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

or even "team-specific" or "org-specific"

@lcarva
Copy link
Contributor

lcarva commented Oct 15, 2024

The functionality is there in Tekton, but I don't believe it is enabled by default. But one of the first things to do should this ADR be accepted is to turn on that functionality.

However, the ADR mentions:

Cloud event generation could be optional and that option could default to off.

Are you talking about turning on that functionality on a specific deployment of Konflux? That is perhaps outside the scope of these ADRs. As I see it, these are about the Konflux project, not necessarily any one deployment of Konflux.


## Consequences

Product teams can more easily build product-specific build and release infrastructure in Konflux.
Copy link
Member

Choose a reason for hiding this comment

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

Can you add more to this section to make it more clear how, when, and why cloudevents would be used as opposed to extending build, test, or release pipelines?

@ralphbean
Copy link
Member

Some notes from the architecture call today.

  • It should be possible to turn this off (and perhaps default to off) for the upstream installer to keep the footprint small.
  • Tekton already exposes CloudEvents. Turning that on should just be a configuration option that someone running the platform can do today.
  • There was some confusion about who the target audience for CloudEvents is. After discussion in the call (and some more afterwards) the intended audience is the end-user; the developer using Konflux to build their software.
  • There's a note already in the architecture constraints that says "Use tekton for anything that should be extended by the user (building, testing, releasing)." If the purpose here is to give the end user a way to extend the system, that line needs to be revised.
  • If we're revising that constraint, then we need some explanation of why, when, and how a user would choose to use cloudevents rather than extend their build, test, and release pipelines.

Arguably, anything a user might do with cloudevents they might also be able to do by extending their build, test, or release pipelines. If the idea here is that using cloudevents can be simpler for that user, I'd like to see in the Consequences section some elaboration on how that will work (what resources does the user need to create to take advantage of this, what RBAC do they need to achieve this).


## Consequences

Product teams can more easily build product-specific build and release infrastructure in Konflux.
Copy link
Member

Choose a reason for hiding this comment

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

If this is a path we want to pursue, that likely entails UI support. If we recommend users extend our platform by way of cloudevents, then their sinks and triggers should all get some representation in the UI to visualize what they've assembled and its runtime status.

If that makes sense, then that deserves some elaboration in the Consequences section here.

@scoheb
Copy link
Contributor

scoheb commented Oct 16, 2024

I think this ADR could benefit from the use cases or examples of situations where cloudevents would be helpful to users. What do you envision the users wanting to do?

I believe this would help with the question I ask myself ... "why not develop whatever feature the users is resorting to using cloudevents for?"

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.

6 participants