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.

Sign up for GitHub

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

Gw/chart fix for external postgres #275

Closed
wants to merge 7 commits into from
Closed

Gw/chart fix for external postgres #275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4db2b00
Fix for external Postgres when PG already existing
guidowalter Feb 8, 2023
1289503
Merge branch 'main' into gw/chartFixForExternalPostgres
guidowalter Feb 8, 2023
688f1fc
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 8, 2023
8b14598
Update Chart.yaml
guidowalter Feb 8, 2023
b3bae0d
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 8, 2023
58873bd
Update values.yaml
guidowalter Feb 8, 2023
ab64352
Update README.md
guidowalter Feb 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion charts/weblate/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ appVersion: 4.15.2
description: Weblate is a free web-based translation management system.
name: weblate
type: application
version: 0.4.21
version: 0.4.22
home: https://weblate.org/
icon: https://s.weblate.org/cdn/weblate.svg
maintainers:
Expand Down
5 changes: 3 additions & 2 deletions charts/weblate/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# weblate

![Version: 0.4.21](https://img.shields.io/badge/Version-0.4.21-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: 4.15.2](https://img.shields.io/badge/AppVersion-4.15.2-informational?style=flat-square)
![Version: 0.4.22](https://img.shields.io/badge/Version-0.4.22-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: 4.15.2](https://img.shields.io/badge/AppVersion-4.15.2-informational?style=flat-square)

Weblate is a free web-based translation management system.

Expand Down Expand Up @@ -78,10 +78,11 @@ $ helm install my-release weblate/weblate
| postgresql.auth.enablePostgresUser | bool | `true` | |
| postgresql.auth.existingSecret | string | `""` | |
| postgresql.auth.postgresPassword | string | `"weblate"` | |
| postgresql.auth.postgresqlUsername | string | `""` | |
| postgresql.auth.secretKeys.userPasswordKey | string | `"postgresql-password"` | |
| postgresql.enabled | bool | `true` | |
| postgresql.postgresqlHost | string | `None` | External postgres database endpoint, to be used if `postgresql.enabled == false` |
| postgresql.service.ports.postgresql | int | `5432` | |
| postgresql.service.ports | int | `5432` | |
Copy link
Member

Choose a reason for hiding this comment

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

The template uses postgresql.service.port, shouldn't this match?

Copy link
Author

Choose a reason for hiding this comment

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

Hi,
you mean it should match the original Bitnami Postgresql Template? Only, if it should be inherited.
If so, then we need to change Line 54 in deployment.yaml (Values.postgresql.service.port) ](value: "{{ .Values.postgresql.service.port }}"), because this would create a map entry in the Postgres_Port env variable, which prevents Weblate from starting. Then we can revert the change in the values.yaml.

That said, if we want to inherit all the variables, we need to change more, as for example postgresql.auth.postgresPassword does not match with postgresql.auth.password from the Bitnami template.
I can take care of that and sync the templates :-)

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I meant this:

helm/charts/weblate/templates/deployment.yaml

Line 54 in 6687cea

value: "{{ .Values.postgresql.service.port }}"

I have no clue whether it's related, though.

My helm knowledge is nearly zero, I just try to avoid messing up the chart more than what we've already done (see #226 or #268).

Copy link
Author

Choose a reason for hiding this comment

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

Hi, exactly this is what I described. If we use

helm/charts/weblate/templates/deployment.yaml

Line 54 in 6687cea

value: "{{ .Values.postgresql.service.port }}"

in the deployment.yaml, but have

helm/charts/weblate/values.yaml

Lines 165 to 167 in 6687cea

service:
ports:
postgresql: 5432

in the values yaml, that result in a ENV entry for that Weblate Container somethin like this:
POSTGRES_PORT: map[postgresql: 5432]. Weblate does not start with this port information. I ran into this problem myself.
I decided not to change the deployment.yaml, rather than make the change in the values.yaml.

The problem with #226 is, that the structure of the postgres template changed. The only solution is to delete the deployment and to reinstall. But this results in data loss IF the persitant volume is not set to RETAIN and after the reinstallation manually bound again to the PVC of Weblate. I had to do this also manually but all data relevant PVs are set to retain. This could be fixed by having a pv.yaml and creating a cross reference between pv and pvc in the first place.

#268 might has something to do with the newly added

helm/charts/weblate/values.yaml

Line 177 in a9adb28

enabled: true

I ran into this as well when upgrading when disabling the auth for redis.

Long story short: I'll take care of the chart issues tomorrow and have look how to best sync the template values without breaking existing deployments. I will also have a look at the PV/PVC combination, so that even people need to reinstall, they can decide to keep their existing data in the future. I can also write some documentation for existing deployments that might help people out if they run into that problem. I would update this PR and then we can discuss this futher.
Would that be Ok with you?

Copy link
Member

Choose a reason for hiding this comment

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

That sounds great!

| redis.architecture | string | `"standalone"` | |
| redis.auth.enabled | bool | `true` | |
| redis.auth.existingSecret | string | `""` | |
Expand Down
2 changes: 1 addition & 1 deletion charts/weblate/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ spec:
containerPort: 8080
protocol: TCP
env:
{{- if .Values.postgresql.enabled }}
{{- if or (eq .Values.postgresql.enabled true) (ne .Values.postgresql.postgresqlHost nil) }}
- name: POSTGRES_DATABASE
value: {{ .Values.postgresql.auth.database | default (include "weblate.fullname" .) }}
- name: POSTGRES_HOST
Expand Down
6 changes: 5 additions & 1 deletion charts/weblate/templates/secret.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,12 @@ metadata:
{{ include "weblate.labels" . | indent 4 }}
type: Opaque
data:
{{- if .Values.postgresql.auth.enablePostgresUser }}
postgresql-user: {{ "postgres" | b64enc | quote }}
postgresql-password: {{ .Values.postgresql.auth.postgresPassword | b64enc | quote }}
{{- else }}
postgresql-user: {{ .Values.postgresql.auth.postgresqlUsername | b64enc | quote }}
{{- end }}
postgresql-password: {{ .Values.postgresql.auth.postgresqlPassword | b64enc | quote }}
redis-password: {{ .Values.redis.auth.password | b64enc | quote }}
email-user: {{ .Values.emailUser | b64enc | quote }}
email-password: {{ .Values.emailPassword | b64enc | quote }}
Expand Down
6 changes: 4 additions & 2 deletions charts/weblate/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,15 +156,17 @@ postgresql:
# PostgreSQL user should be a superuser to
# be able to install pg_trgm extension. Alternatively you can install it
# manually prior starting Weblate.
# To user your own PostgresUsername set enablePostgresUser: false and
# fill in postgresqlUsername
enablePostgresUser: true
postgresqlUsername: ''
postgresPassword: weblate
database: weblate
existingSecret: ''
secretKeys:
userPasswordKey: postgresql-password
service:
ports:
postgresql: 5432
ports: 5432
enabled: true
# postgresql.postgresqlHost -- External postgres database endpoint, to be
# used if `postgresql.enabled == false`
Expand Down