puppetlabs/puppetserver-helm-chart

Chart only works if releasename is puppetserver

bjvrielink opened this issue · 4 comments

bjvrielink commented

Describe the Bug

Puppetdb expects to find the Postgress service as 'puppetserver-postgresql', but the Postgress chart uses {{ .Release.Name }}-postgresql, which may a different value if a release name other than puppetserver is used.

Expected Behavior

A working puppetdb that can find its Postgress, irregardless of the Helm release name.

Steps to Reproduce

Steps to reproduce the behavior:

  1. helm repo add puppet https://puppetlabs.github.io/puppetserver-helm-chart
  2. helm install --namespace puppetserver --name puppet puppet/puppetserver --set puppetserver.puppeturl='https://github.com/$SOMEUSER/control-repo.git'
  3. kubectl get service puppet-postgresql
  4. kubectl logs -l app.kubernetes.io/component=puppetdb -c puppetdb
  5. Notice the error:
    (/docker-entrypoint.d/10-wait-for-hosts.sh) Error: dependent service at puppetserver-postgresql cannot be resolved or contacted

Environment

  • Version 6.8.1
  • Platform kubernetes 1.22, Helm v3.10.3, ArgoCD v2.5.2+148d8da

Additional Context

By default:

  • In templates/puppetdb-deployment.yaml the environment variable PUPPETDB_POSTGRES_HOSTNAME is set to "{{ template "puppetserver.name" . }}-postgresql"
  • In templates/_helpers.tpl puppetserver.name is set to .Chart.Name
  • In charts/postgresql/templates/svc.yaml the name of the service is set to common.names.fullname
  • In charts/postgresql/charts/common/templates/_names.tpl common.names.fullname is set to .Release.Name-$name

Setting postgresql.fullnameOverride to "puppetserver-postgresql" in Puppetserver's values.yaml avoids this bug.

davidphay commented

Thanks for the proposition, @bjvrielink ! Please feel free to send our way a PR - I'll be happy to review and merge it, afterwards!

davidphay commented

Ok I just test and I understand the issue. indeed your workaround work.

I will merge your PR, but we should fix the problem in the helm chart directly because we can't use any external db with this. and we can have some issue because a lot of ressource have a static name.

2 updates:
we should using .Release.Name instead of .Chart.Name .
we should add .Release.Name` when the ressource have a static name.

What are you thinking @Xtigyro ?

bjvrielink commented

Refactoring the helm chart to use .Release.Name instead of .Chart.Name is what I would advice. This is what Helm also uses to differentiate between different installs of the same chart.
In theory, it should be possible to install the same chart twice in the same namespace, when using 2 different release names. This is not a feature I would expect from the puppetserver chart, but it indicates the level of flexibility helm users may expect.

BTW. Thank you for merging.

davidphay commented

I close the issue since I fixed the issue in release 7.0.0.