-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
[bitnami/juypterhub] postgres image missing when postgres is disabled (fluxCD/argoCD) #28841
Conversation
Signed-off-by: Lucas Easley <[email protected]>
Signed-off-by: Lucas Easley <[email protected]>
fix formatting for postgres.image Signed-off-by: Lucas Easley <[email protected]>
fix postgres juypterhub issue
Signed-off-by: Lucas Easley <[email protected]>
Signed-off-by: Bitnami Containers <[email protected]>
Signed-off-by: Bitnami Containers <[email protected]>
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.
Nice catch!!! thanks a lot @leasley199 to work on this fix.
This is happening because the postgresql subchart is using the postgresql.enabled
value as condition:
charts/bitnami/jupyterhub/Chart.yaml
Lines 19 to 22 in 9da9f3b
- condition: postgresql.enabled | |
name: postgresql | |
repository: oci://registry-1.docker.io/bitnamicharts | |
version: 15.x.x |
Please take a glance to the solution adopted in concourse to follow the same pattern:
charts/bitnami/concourse/templates/web/deployment.yaml
Lines 70 to 76 in 9da9f3b
{{- if .Values.postgresql.enabled }} | |
# NOTE: The value postgresql.image is not available unless postgresql.enabled is not set. We could change this to use os-shell if | |
# it had the binary wait-for-port. | |
# This init container is for avoiding CrashLoopback errors in the Hub container because the PostgreSQL container is not ready | |
- name: wait-for-db | |
image: {{ include "common.images.image" (dict "imageRoot" .Values.postgresql.image "global" .Values.global) }} | |
imagePullPolicy: {{ .Values.postgresql.image.pullPolicy }} |
I think is worth to follow the same solution.
Signed-off-by: Lucas Easley <[email protected]>
Signed-off-by: Lucas Easley <[email protected]>
Signed-off-by: Lucas Easley <[email protected]>
much better approach updated my branch with the suggested changes :) many thank! |
Signed-off-by: Bitnami Containers <[email protected]>
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.
Thanks a lot @leasley199
LGTM
Description of the change
When using fluxCD or argoCD
and
using an external postgres instances, the postgres image is lost in translation and causes the following error.This creates a catchall so that the deploy user/manifest can still lint the postgres image to the init container and deploy successfully.
Benefits
fluxCD and ArgoCD deployments are happy. For some odd reason this is not happening in non CD deployments. Believe it to be due to helm rendering validation baked into flux and argo.
Possible drawbacks
none to my knowledge, if
postgres.image
is not set, uses defaults from helm chart.Applicable issues
None
Additional information
Simply just allows the deployer to specify the postgres image for the init-container, currently fails and prevents deployments in CD tools.
Checklist
Chart.yaml
according to semver. This is not necessary when the changes only affect README.md files.README.md
using readme-generator-for-helm