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

[RFC-0006] Flux-CDEvent Receiver #4534

Merged
merged 1 commit into from
Mar 13, 2024
Merged

[RFC-0006] Flux-CDEvent Receiver #4534

merged 1 commit into from
Mar 13, 2024

Conversation

adamkenihan
Copy link
Contributor

This RFC proposes to add a receiver that supports receiving and validating CDEvents in order to reconcile resources.

@stefanprodan stefanprodan added the area/rfc Feature request proposals in the RFC format label Jan 11, 2024
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

We have discussed the CDEvents proposal at Flux dev meetings and I volunteer to sponsor this RFC. Thanks @adamkenihan 🏅

Copy link
Contributor

@darkowlzz darkowlzz left a comment

Choose a reason for hiding this comment

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

Overall, the proposal is simple and clear. Left a few suggestions for minor improvements that I could think of.

rfcs/NNNN-cdevents/README.md Outdated Show resolved Hide resolved
rfcs/NNNN-cdevents/README.md Outdated Show resolved Hide resolved
rfcs/NNNN-cdevents/README.md Outdated Show resolved Hide resolved
rfcs/NNNN-cdevents/README.md Outdated Show resolved Hide resolved
Comment on lines 98 to 107
resources:
- kind: Kustomization
apiVersion: kustomize.toolkit.fluxcd.io/v1
name: podinfo
namespace: flux-system
Copy link
Member

@stefanprodan stefanprodan Jan 25, 2024

Choose a reason for hiding this comment

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

We should use here a GitRepository, triggering the Kustomization would do nothing. Receivers are for telling Flux to pull changes from upstream sources, then Flux knows how to notify the appliers (Kustomizations and HelmReleases).

Copy link
Member

Choose a reason for hiding this comment

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

For clarity, this is how Flux eventing works
Screenshot 2024-01-25 at 13 45 16

@adamkenihan
Copy link
Contributor Author

Many of the above requested changes have been added if you would like to review the RFC again.

rfcs/NNNN-cdevents/README.md Outdated Show resolved Hide resolved
rfcs/NNNN-cdevents/README.md Outdated Show resolved Hide resolved
rfcs/NNNN-cdevents/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@darkowlzz darkowlzz left a comment

Choose a reason for hiding this comment

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

Overall the proposal looks good to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem to be used anywhere.

Copy link
Member

Choose a reason for hiding this comment

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

@adamkenihan can you remove this file?

Copy link
Member

Choose a reason for hiding this comment

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

I will remove it when I mark the RFC ready for implementation.

@stefanprodan stefanprodan changed the title [RFC] Flux-CDEvent Receiver [RFC-0005] Flux-CDEvent Receiver Mar 7, 2024
@stefanprodan
Copy link
Member

@adamkenihan I have assigned 0006 for this RFC, please make the changes to the dir name and inside the markdown, then rebase with upstream main branch and force push.

@stefanprodan stefanprodan changed the title [RFC-0005] Flux-CDEvent Receiver [RFC-0006] Flux-CDEvent Receiver Mar 7, 2024
rfcs/0006-cdevents/README.md Outdated Show resolved Hide resolved
rfcs/0006-cdevents/README.md Outdated Show resolved Hide resolved
rfcs/0006-cdevents/README.md Outdated Show resolved Hide resolved
Copy link
Member

@souleb souleb left a comment

Choose a reason for hiding this comment

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

LGTM! thanks @adamkenihan

Signed-off-by: adamkenihan <adam.kenihan@est.tech>

Create CDEvents RFC

Signed-off-by: adamkenihan <adam.kenihan@est.tech>

Update README.md

Signed-off-by: adamkenihan <adam.kenihan@est.tech>

Add files via upload

Signed-off-by: adamkenihan <adam.kenihan@est.tech>

Edits to diagrams

Signed-off-by: adamkenihan <adam.kenihan@est.tech>

Update README.md

Signed-off-by: adamkenihan <adam.kenihan@est.tech>

Add files via upload

Signed-off-by: adamkenihan <adam.kenihan@est.tech>

Update README.md

Signed-off-by: adamkenihan <adam.kenihan@est.tech>

Improvements to rfcs/NNNN-cdevents/README.md

Signed-off-by: adamkenihan <adam.kenihan@est.tech>

Add CDEvents Receiver RFC

Signed-off-by: adamkenihan <adam.kenihan@est.tech>

Add CDEvents Receiver RFC

Signed-off-by: adamkenihan <adam.kenihan@est.tech>

Add CDEvents Receiver RFC

Signed-off-by: adamkenihan <adam.kenihan@est.tech>

Small tweaks to cdevents RFC

Signed-off-by: adamkenihan <adam.kenihan@est.tech>

change cdevents RFC yaml example format

Signed-off-by: adamkenihan <adam.kenihan@est.tech>

Add CDEvents Receiver RFC

Signed-off-by: adamkenihan <adam.kenihan@est.tech>

Create CDEvents RFC

Signed-off-by: adamkenihan <adam.kenihan@est.tech>

Signed-off-by: adamkenihan <adam.kenihan@est.tech>

Change RFC number

Signed-off-by: adamkenihan <adam.kenihan@est.tech>

Update rfcs/0006-cdevents/README.md

Signed-off-by: adamkenihan <adam.kenihan@est.tech>

Update README.md

Signed-off-by: adamkenihan <adam.kenihan@est.tech>

Update rfcs/0006-cdevents/README.md

Signed-off-by: adamkenihan <adam.kenihan@est.tech>

Update rfcs/0006-cdevents/README.md

Signed-off-by: adamkenihan <adam.kenihan@est.tech>
Co-Authored-By: Sunny <github@darkowlzz.space>
Co-Authored-By: souleb <bah.soule@gmail.com>
@stefanprodan stefanprodan merged commit 54a7132 into fluxcd:main Mar 13, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/rfc Feature request proposals in the RFC format
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants