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

Disable ingress per default #150

Merged
merged 1 commit into from
Feb 7, 2022
Merged

Disable ingress per default #150

merged 1 commit into from
Feb 7, 2022

Conversation

Zelldon
Copy link
Member

@Zelldon Zelldon commented Feb 3, 2022

Deploying ingress resource per default doesn't make much sense, since not everyone has an ingress controller deployed. We decided to disable it per default.

Like its done in other charts:

Grafana
https://github.com/grafana/helm-charts/blob/main/charts/grafana/values.yaml#L183-L184

Kibana:
https://github.com/elastic/helm-charts/blob/main/kibana/values.yaml#L142-L143

Elasticsearch
https://github.com/elastic/helm-charts/blob/main/elasticsearch/values.yaml#L250-L251

BTW @menski I saw that the previous full chart deployed an ingress nginx controller https://github.com/camunda-community-hub/camunda-cloud-helm/blob/main/charts/zeebe-full-helm/Chart.yaml#L19-L21 but tbh I'm not sure whether we really want that. Plus it didn't work #146

related to #124

Deploying ingress resource per default doesn't make much sense, since not everyone has an ingress controller deployed.
@Zelldon Zelldon requested a review from menski February 3, 2022 06:44
@Zelldon Zelldon mentioned this pull request Feb 3, 2022
25 tasks
@Sijoma
Copy link

Sijoma commented Feb 6, 2022

I approve! Who wants to deploy an nginx bundled with a helm-chart anyway? :D

@Zelldon have you found how I get on the list of reviewers?

@Zelldon Zelldon requested a review from Sijoma February 6, 2022 20:06
Copy link
Contributor

@menski menski left a comment

Choose a reason for hiding this comment

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

As we discussed before, I think it should be optional as people have to do other stuff before it works, and no I don't think we should deploy an ingress controller with our helm charts, this is out of scope for me

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.

None yet

3 participants