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

feat: Introduce CloudEvents to KEDA #4968

Merged
merged 69 commits into from
Nov 23, 2023

Conversation

SpiritZhou
Copy link
Contributor

@SpiritZhou SpiritZhou commented Sep 13, 2023

Checklist

  • I have verified that my change is according to the deprecations & breaking changes policy
  • Tests have been added
  • Changelog has been updated and is aligned with our changelog requirements
  • A PR is opened to update our Helm chart (repo) (if applicable, ie. when deployment manifests are modified)
  • A PR is opened to update the documentation on (repo) (if applicable)
  • Commits are signed with Developer Certificate of Origin (DCO - learn more)

Relates to #3533
Relates to kedacore/keda-docs#1227
Relates to kedacore/charts#572

Signed-off-by: SpiritZhou <[email protected]>
@github-actions
Copy link

Thank you for your contribution! 🙏 We will review your PR as soon as possible.

While you are waiting, make sure to:

Learn more about:

@tomkerkhove
Copy link
Member

@SpiritZhou if you have some time; would you mind opening a doc PR in parallel so that we can work on the end-user experience there?

@tomkerkhove
Copy link
Member

I'm sure this is on your radar but just in case it's not - Let's make sure to add some e2e tests. For plain HTTP, I think we can use a simple client which we can verify if the event was received and echo it back

@SpiritZhou
Copy link
Contributor Author

I'm sure this is on your radar but just in case it's not - Let's make sure to add some e2e tests. For plain HTTP, I think we can use a simple client which we can verify if the event was received and echo it back

Yes. This is a draft PR and I am adding some tests and improving coding now.

Signed-off-by: SpiritZhou <[email protected]>
Signed-off-by: SpiritZhou <[email protected]>
Signed-off-by: SpiritZhou <[email protected]>
Signed-off-by: SpiritZhou <[email protected]>
Signed-off-by: SpiritZhou <[email protected]>
Signed-off-by: SpiritZhou <[email protected]>
Signed-off-by: SpiritZhou <[email protected]>
Signed-off-by: SpiritZhou <[email protected]>
Signed-off-by: SpiritZhou <[email protected]>
Signed-off-by: SpiritZhou <[email protected]>
Signed-off-by: SpiritZhou <[email protected]>
Signed-off-by: SpiritZhou <[email protected]>
Signed-off-by: SpiritZhou <[email protected]>
@SpiritZhou SpiritZhou marked this pull request as ready for review September 20, 2023 08:27
@SpiritZhou SpiritZhou requested a review from a team as a code owner September 20, 2023 08:27
Signed-off-by: SpiritZhou <[email protected]>
Copy link
Member

@JorTurFer JorTurFer left a comment

Choose a reason for hiding this comment

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

You are doing an incredible work @SpiritZhou . Thanks a lot for that

CHANGELOG.md Outdated Show resolved Hide resolved
apis/keda/v1alpha1/cloudevent_types.go Outdated Show resolved Hide resolved
apis/keda/v1alpha1/cloudevent_types.go Outdated Show resolved Hide resolved
config/manager/kustomization.yaml Outdated Show resolved Hide resolved
pkg/eventemitter/eventemitter.go Outdated Show resolved Hide resolved
pkg/eventemitter/eventemitter.go Outdated Show resolved Hide resolved
pkg/eventemitter/eventemitter.go Outdated Show resolved Hide resolved
pkg/eventemitter/eventemitter.go Outdated Show resolved Hide resolved
pkg/eventemitter/eventemitter.go Outdated Show resolved Hide resolved
Signed-off-by: SpiritZhou <[email protected]>
Signed-off-by: SpiritZhou <[email protected]>
josefkarasek and others added 2 commits November 13, 2023 11:56
Signed-off-by: Josef Karasek <[email protected]>
Signed-off-by: SpiritZhou <[email protected]>
Signed-off-by: SpiritZhou <[email protected]>
Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

Looking good, just a minor stuff

CHANGELOG.md Outdated Show resolved Hide resolved
pkg/eventemitter/cloudevent_http_handler.go Show resolved Hide resolved
SpiritZhou and others added 4 commits November 17, 2023 09:52
Co-authored-by: Zbynek Roubalik <[email protected]>
Signed-off-by: SpiritZhou <[email protected]>
Signed-off-by: SpiritZhou <[email protected]>
Signed-off-by: SpiritZhou <[email protected]>
Signed-off-by: SpiritZhou <[email protected]>
@tomkerkhove
Copy link
Member

tomkerkhove commented Nov 17, 2023

/run-e2e cloudevent
Update: You can check the progress here

@tomkerkhove
Copy link
Member

@SpiritZhou Is the doc PR still up to date? I'll do another review pass

@tomkerkhove
Copy link
Member

tomkerkhove commented Nov 18, 2023

/run-e2e cloudevent
Update: You can check the progress here

@SpiritZhou
Copy link
Contributor Author

@SpiritZhou Is the doc PR still up to date? I'll do another review pass

The doc PR is up to date. And I see the e2e test failed at the keda installation process. How can I fix that?

Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

LGTM

@zroubalik
Copy link
Member

zroubalik commented Nov 20, 2023

/run-e2e cloudevent
Update: You can check the progress here

@zroubalik
Copy link
Member

@SpiritZhou is there a charts PR for this feature?

@tomkerkhove
Copy link
Member

To be clear - Zbynek is looking for changes on configuring the default cluster name to use in events + the new CRD to be added in the chart

@SpiritZhou
Copy link
Contributor Author

@zroubalik @tomkerkhove Here is the chart PR about this feature. Could you help to review? kedacore/charts#572

@tomkerkhove
Copy link
Member

Done!

@tomkerkhove tomkerkhove changed the title Introduce CloudEvents CRD to KEDA feat: Introduce CloudEvents CRD to KEDA Nov 22, 2023
@zroubalik zroubalik changed the title feat: Introduce CloudEvents CRD to KEDA feat: Introduce CloudEvents to KEDA Nov 22, 2023
@tomkerkhove
Copy link
Member

Docs + Helm are open and depend on this one so we should be able to merge, what do you think @zroubalik?

@tomkerkhove tomkerkhove merged commit 5f741ed into kedacore:main Nov 23, 2023
19 checks passed
@zroubalik
Copy link
Member

The Charts PR is still not resolved 🤷‍♂️

yoongon pushed a commit to yoongon/keda that referenced this pull request Nov 24, 2023
Co-authored-by: Jorge Turrado Ferrero <[email protected]>
Co-authored-by: Zbynek Roubalik <[email protected]>
Co-authored-by: Josef Karasek <[email protected]>
Co-authored-by: Jan Wozniak <[email protected]>
Signed-off-by: Yoon Park <[email protected]>
toniiiik pushed a commit to toniiiik/keda that referenced this pull request Jan 15, 2024
Co-authored-by: Jorge Turrado Ferrero <[email protected]>
Co-authored-by: Zbynek Roubalik <[email protected]>
Co-authored-by: Josef Karasek <[email protected]>
Co-authored-by: Jan Wozniak <[email protected]>
Signed-off-by: anton.lysina <[email protected]>
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