[bitnami/discourse] discourse helm chart

[paulczar]

Signed-off-by: Paul Czarkowski username.taken@gmail.com

Adding discourse helm chart

see https://github.com/bitnami/bitnami-docker-discourse/issues/132

Benefits

Improve experience for people trying to install discourse to kubernetes

Possible drawbacks

More stuff for Bitnami to support

• Chart version bumped in Chart.yaml according to semver.
• Variables are documented in the README.md
• Title of the PR starts with chart name (e.g. [bitnami/chart])
• If the chart contains a values-production.yaml apart from values.yaml, ensure that you implement the changes in both files

[stale]

This Pull Request has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thank you for your contribution.

[carrodher]

With this label, the PR wouldn't be marked as stale anymore

[LucaPrete]

@carrodher FYI I'm sending the first basic mods as PR here: https://github.com/paulczar/bitnami-charts/pulls.
Any review from your side -considering the PRs above- would be much appreciated!

[carrodher]

Hi @LucaPrete @paulczar, thanks for your efforts. I have been talking with my team and we've reserved some time for reviewing the PR deeply during our next iteration (starting on Monday). So we will provide valuable feedback soon.

[LucaPrete]

Great, thanks! Looking forward to it.

…

-Luca Il giorno gio 9 apr 2020 alle ore 18:21 Carlos Rodríguez Hernández < notifications@github.com> ha scritto:

Hi @LucaPrete <https://github.com/LucaPrete> @paulczar <https://github.com/paulczar>, thanks for your efforts. I have been talking with my team and we've reserved some time for reviewing the PR deeply during our next iteration (starting on Monday). So we will provide valuable feedback soon. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2072 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AARY7UD43L6VTNVWERUPENTRLXY2PANCNFSM4LN2QK2A> .

[javsalgar]

Thank you so much for contributing with this amazing PR. We are very sorry for the delay but here you have some notes for a first iteration of improvements. It's gonna be awesome and our users will greatly benefit from it :)

[javsalgar]

We've changed this to something like this

      {{- if .Values.affinity }}
      affinity: {{- include "discourse.tplValue" (dict "value" .Values.affinity "context" $) | nindent 8 }}
      {{- end }}

I suggest you to check our WordPress deployment example so you see an example on how we are doing it

[LucaPrete]

done

[javsalgar]

I think that the httpGet liveness and readinessProbe fits better. Also, add the variables for setting the different probe settings (already suggested in the yaml)

[paulczar]

I don't believe the sidekiq had a http endpoint to httpGet against.

[LucaPrete]

Not sure I've understood what you guys exactly suggest to do here. Also, as Paul mentions, are we sure sidekiq has a get endpoint to use?

[javsalgar]

Sorry, mistook it with the default discourse

[LucaPrete]

done

[javsalgar]

As you are using - and indent you can put it in the same line as resources

resources: {{- toYaml .Values.resources | nindent 12 }}

Looks much better

[LucaPrete]

Done. Should we still keep the nindent in this case?

[javsalgar]

Yes, please keep it

[LucaPrete]

done

[javsalgar]

Most of the changes in the sidekiq deployment apply here

[javsalgar]

Also, don't deploy the pod if the hostname is not set (check the Magento example)

[LucaPrete]

Done.
Please, check if the syntax I've used is correct:
{{- if and (include "discourse.host" .) (.Values.postgresql.enabled) -}}
Also, do we want to put in and also redis?

I've done it also for sidekiq. I don't think it makes much sense to deploy it if the former is not.

[javsalgar]

Check the Magento notes and add something similar here. We want to warn the users that they should upgrade their deployments if they didn't set the hostname. It makes sense to deploy it in two steps so they get the LoadBalancer IPs from the cloud provider.

[javsalgar]

Ping on this one

[LucaPrete]

Done in my latest patch (still needs to be approved) here: paulczar#11

[javsalgar]

Hi,

Just pinging to know if you could work on the requested changes. If you don't have time now, we would like to continue applying the changes ourselves (while maintaining you as the contributor). Would that be ok with you?

[LucaPrete]

Hello Javier, Thanks for the comments! I wasn't notified by GitHub...apologies for that! I should be able to take a look at it in the second half of this week. Would that be okay with you? Of course, no need to say that any help is absolutely appreciated in any case! :) Il giorno mar 28 apr 2020 alle ore 11:11 Javier J. Salmerón-García < notifications@github.com> ha scritto:

…

Hi, Just pinging to know if you could work on the requested changes. If you don't have time now, we would like to continue applying the changes ourselves (while maintaining you as the contributor). Would that be ok with you? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2072 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AARY7UARRVWDS7ME7OTCURDRO2MUNANCNFSM4LN2QK2A> .

[javsalgar]

Hi,

Great! We'll wait then. Also @dani8art will help with the review

[paulczar]

Hey, I've merged in most of your suggested changes. I don't have a ton of time to work on it, but feel free to hit me up @LucaPrete if you need anything.

[LucaPrete]

@joancafom thanks for the patch. I see there's lots of stuff in it, but still, checks do not pass.
I had in mind to download the chart and do some improvements, but I'd prefer you to go through it first and solve the issues.
Is there any way I can help?

[LucaPrete]

@javsalgar how do I trigger checks again? I'm suspecting the last linting check was just a temporary failure...

[joancafom]

Hi @LucaPrete !

Thanks so much for your help. I think that the issue for the checks to not pass is that discourse takes so much time to install. I have been testing the Chart in local and am able to start it. I will try to increase liveness/readiness probes timings to see if there that can help.

Yes, I do agree with you. This Chart will be released soon and you will be able to start making contributions on a separate PR. I think that will be the best way to keep things as neat as possible. Regarding checks, they are triggered every time a new commit is added.

Thanks so much 😄

[LucaPrete]

I see. I was hoping for a /recheck gitops thing... :) Well, thank you for the help! I'll let you dig into it. As soon as you need help again, please ping me here. Ciao Il giorno mer 17 giu 2020 alle ore 11:24 Jose Antonio Carmona < notifications@github.com> ha scritto:

…

Hi @LucaPrete <https://github.com/LucaPrete> ! Thanks so much for your help. I think that the issue for the checks to not pass is that discourse takes so much time to install. I have been testing the Chart in local and am able to start it. I will try to increase liveness/readiness probes timings to see if there that can help. Yes, I do agree with you. This Chart will be released soon and you will be able to start making contributions on a separate PR. I think that will be the best way to keep things as neat as possible. Regarding checks, they are triggered every time a new commit is added. Thanks so much 😄 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2072 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AARY7UAJVXYBZBT24BZRADTRXCDTRANCNFSM4LN2QK2A> .

[LucaPrete]

I guess we're almost there guys! Congrats to everyone who contributed :)

I've just submitted on @paulczar fork a patch with minor changes to the readme.

@javsalgar the last thing I see remaining is the license. I see there are no licenses in other charts. What should we do?

[joancafom]

Hi @paulczar @LucaPrete !

It looks like this Chart is ready to be landed. First of all thank you so much for your contributions, you did an amazing job here and everyone will benefit from it 😄🎉.

Regarding your question about licensing, by default we should keep a common chart/LICENSE file for all Charts. This file tells the end user all of our Charts are under Apache License v2. On behalf of the contributors, it is a common practice to add them in the maintainers section in the Chart.yaml of the specific application, where you both appear (along with your email addresses).

Thanks a lot again!

[LucaPrete]

Jose, thanks for your support! Amazing job everyone! Looking forward to using it and to my next contribution ;)

…

-Luca Il giorno mar 23 giu 2020 alle ore 18:29 Jose Antonio Carmona < notifications@github.com> ha scritto:

Hi @paulczar <https://github.com/paulczar> @LucaPrete <https://github.com/LucaPrete> ! It looks like this Chart is ready to be landed. First of all thank you so much for your contributions, you did an amazing job here and everyone will benefit from it 😄🎉. Regarding your question about licensing, by default we should keep a common chart/LICENSE file for all Charts. This file tells the end user all of our Charts are under Apache License v2. On behalf of the contributors, it is a common practice to add them in the maintainers section in the Chart.yaml of the specific application, where you both appear (along with your email addresses). Thanks a lot again! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2072 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AARY7UHHUPALL2W473LKWE3RYDJ6HANCNFSM4LN2QK2A> .

[LucaPrete]

🍾🍾🍾🍾 Il giorno mer 24 giu 2020 alle 11:53 Jose Antonio Carmona < notifications@github.com> ha scritto:

…

Merged #2072 <#2072> into master. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2072 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AARY7UAKS6QLSO7VHSDUXDTRYHEKHANCNFSM4LN2QK2A> .