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

Add keda http add-on operator charts #137

Merged
merged 13 commits into from
May 12, 2021
Merged

Add keda http add-on operator charts #137

merged 13 commits into from
May 12, 2021

Conversation

khaosdoctor
Copy link
Contributor

Moving HTTP add-on charts here

Checklist

Fixes kedacore/http-add-on#104

Signed-off-by: Lucas Santos <lhs.santoss@gmail.com>
Signed-off-by: Lucas Santos <lhs.santoss@gmail.com>
Copy link
Member

@tomkerkhove tomkerkhove left a comment

Choose a reason for hiding this comment

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

A good start but we should rename the Helm chart

http-add-on/Chart.yaml Outdated Show resolved Hide resolved
http-add-on/Chart.yaml Outdated Show resolved Hide resolved
http-add-on/templates/deployment.yaml Outdated Show resolved Hide resolved
http-add-on/templates/deployment.yaml Show resolved Hide resolved
http-add-on/templates/keda-http-operator.yml Outdated Show resolved Hide resolved
http-add-on/templates/keda-http-operator.yml Outdated Show resolved Hide resolved
http-add-on/templates/svc.yaml Outdated Show resolved Hide resolved
http-add-on/values.yaml Show resolved Hide resolved
http-add-on/templates/deployment.yaml Outdated Show resolved Hide resolved
@tomkerkhove
Copy link
Member

@khaosdoctor Please move the CRDs as per #140 as well, Helm doesn't handle them well :/

@tomkerkhove
Copy link
Member

tomkerkhove commented Apr 11, 2021

Let's use crds.install as a parameter name

@khaosdoctor khaosdoctor requested a review from tomkerkhove April 27, 2021 17:50
Signed-off-by: Lucas Santos <lhs.santoss@gmail.com>
@khaosdoctor
Copy link
Contributor Author

@tomkerkhove applied the changes from your review, could you please review again?

Signed-off-by: Lucas Santos <lhs.santoss@gmail.com>
@khaosdoctor
Copy link
Contributor Author

Added change from kedacore/http-add-on#115 to setup the chart

Signed-off-by: Lucas Santos <lhs.santoss@gmail.com>
Copy link
Collaborator

@arschles arschles left a comment

Choose a reason for hiding this comment

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

I have a single nitpick change, but +1 to @tomkerkhove's requested changes too

http-add-on/values.yaml Show resolved Hide resolved
Signed-off-by: Lucas Santos <lhs.santoss@gmail.com>
Signed-off-by: Lucas Santos <lhs.santoss@gmail.com>
Signed-off-by: Lucas Santos <lhs.santoss@gmail.com>
Signed-off-by: Lucas Santos <lhs.santoss@gmail.com>
@khaosdoctor khaosdoctor requested a review from arschles May 6, 2021 20:53
@khaosdoctor
Copy link
Contributor Author

@tomkerkhove @arschles can you PTAL?

http-add-on/values.yaml Show resolved Hide resolved
Copy link
Member

@tomkerkhove tomkerkhove 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 only need two last changes:

  1. Fix the app version references below
  2. Provide a new CI workflow for this chart, similar to KEDA Core CI

http-add-on/Chart.yaml Outdated Show resolved Hide resolved
http-add-on/templates/deployment.yaml Outdated Show resolved Hide resolved
http-add-on/templates/deployment.yaml Outdated Show resolved Hide resolved
http-add-on/templates/deployment.yaml Outdated Show resolved Hide resolved
Signed-off-by: Lucas Santos <lhs.santoss@gmail.com>
@khaosdoctor
Copy link
Contributor Author

khaosdoctor commented May 10, 2021

Suggestions done, let me create the CI

@khaosdoctor khaosdoctor requested a review from tomkerkhove May 10, 2021 17:53
Copy link
Member

@tomkerkhove tomkerkhove left a comment

Choose a reason for hiding this comment

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

Thank you for working on the CI!

http-add-on/Chart.yaml Outdated Show resolved Hide resolved
http-add-on/Chart.yaml Outdated Show resolved Hide resolved
http-add-on/templates/deployment.yaml Outdated Show resolved Hide resolved
http-add-on/templates/deployment.yaml Outdated Show resolved Hide resolved
http-add-on/templates/deployment.yaml Outdated Show resolved Hide resolved
Signed-off-by: Lucas Santos <lhs.santoss@gmail.com>
Signed-off-by: Lucas Santos <lhs.santoss@gmail.com>
Signed-off-by: Lucas Santos <lhs.santoss@gmail.com>
@khaosdoctor khaosdoctor requested a review from tomkerkhove May 11, 2021 19:04
@khaosdoctor
Copy link
Contributor Author

@tomkerkhove I added the CI pipeline based on the one that's already here, can you PTAL to check if that's useful?

@tomkerkhove
Copy link
Member

THank you @khaosdoctor! I'll clean it up in a separate PR!

@tomkerkhove tomkerkhove merged commit 7886040 into kedacore:master May 12, 2021
@tomkerkhove tomkerkhove mentioned this pull request May 12, 2021
3 tasks
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.

Deploy helm charts to a repository
3 participants