-
Notifications
You must be signed in to change notification settings - Fork 19
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 helm chart #150
base: master
Are you sure you want to change the base?
Add helm chart #150
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well done @jbagot, I like it :)
Just a few things to make k8s v1.18+ compatible and fix the possibility to deploy it (some bugs, env vars and some patchs)
I'm going to submit some changes to make it work
command: | ||
- invoke | ||
- celery-queues | ||
envFrom: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe liveness tests could be implemented via celery -b XXX inspect ping
command (or something similar)
helm/apf/templates/secrets.yaml
Outdated
release: {{ .Release.Name | quote }} | ||
heritage: {{ .Release.Service | quote }} | ||
data: | ||
BASE_URL: {{ .Values.baseurl | b64enc | quote }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why all those vars are stored as a Secret? Is there any added value that justifies it?
tls: | ||
- secretName: {{ $fullName }}-cert | ||
{{- end }} | ||
rules: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Todo: review traefik
helm/apf/templates/ingress.yaml
Outdated
pathType: Prefix | ||
backend: | ||
service: | ||
name: {{ $fullName }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not k8s 1.18+ compatible
helm/apf/values.yaml
Outdated
@@ -0,0 +1,79 @@ | |||
baseurl: https://nemperfeina.cat |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The case is not correct
helm/apf/templates/secrets.yaml
Outdated
POSTGRES_PASSWORD: {{ required "postgres password is needed" .Values.postgresql.postgresqlPassword | b64enc | quote }} | ||
# Celery | ||
CELERY_BROKER_PROTOCOL: {{ .Values.celery.broker.protocol | b64enc | quote }} | ||
CELERY_BROKER_HOST: {{ printf "%s-redis-master" (include "apf.name" .) | b64enc | quote }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be apf.fullname
helm/apf/templates/secrets.yaml
Outdated
# Postgres | ||
POSTGRES_DB: {{ .Values.postgresql.postgresqlDatabase | b64enc | quote }} | ||
POSTGRES_USER: {{ .Values.postgresql.postgresqlUsername | b64enc | quote }} | ||
POSTGRES_HOST: {{ printf "%s-postgresql" (include "apf.name" .) | b64enc | quote }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be apf.fullname
helm/apf/templates/secrets.yaml
Outdated
# Telegram | ||
TELEGRAM_TOKEN: {{ .Values.notifications.telegram.token | b64enc | quote }} | ||
NOTIF_TELEGRAM_ENABLED: {{ .Values.notifications.telegram.enabled | b64enc | quote }} | ||
TELEGRAM_CHAT_IDS: {{ .Values.notifications.telegram.chatIds | b64enc | quote }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are more env vars needed to deploy the NPF (twitter, sentry, ...)
@@ -52,8 +52,8 @@ def uwsgi( | |||
|
|||
command_args = [ | |||
"uwsgi", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uWSGI is not currently integrated into the env, maybe it's better to handle it in another PR
I commited some changes to your PR to make it works, feel free to rollback or continue patching it @jbagot :) When this PR is ready, a massive |
Description
Fix #97
Add helm chart. You need a myvalues.yaml file with some secrets to execute it.
Now uses uwsgi.
Postgresql and redis as a depencies charts.
Code formating
The code has to be well formatted following the formatting rules. For example in Python the code has to follow the PEP8.
Before create your PR, please make sure your code is well formatted and styled doing:
$ >> pre-commit run --all