puppetlabs / puppetserver-helm-chart

The Helm Chart for Puppet Server
Apache License 2.0
47 stars 55 forks source link

multiMasters enabled can cause infinite wait to init in some scenarios #228

Closed anthonysomerset closed 2 months ago

anthonysomerset commented 2 months ago

Describe the Bug

I am raising this as a bug because i have only been testing one scenario here and may need some other testing/data points to validate as well as to come up with a solution.

My Scenario is AKS + External Postgres (for PuppetDB)

I am importing my pre-existing infra certs and CA from legacy on prem into AKS and running multiMasters enabled.

with the default init-configmap that gets generated the check_for_masters.sh script waits indefinitely because /etc/puppetlabs/puppet/ssl/certs/puppet.pem is not present. it appears at least in my scenario the file that gets generated is /etc/puppetlabs/puppet/ssl/certs/puppet.puppet.svc.cluster.local.pem

I can work around the issue by executing a bash shell in the init container and then kill the script and allow it to proceed or by editing the configmap to update the PUPPET_SSL_CERT_PEM variable and triggering restart on the deployment at which point everything proceeds as normal

I don't know if its a unique to my setup thing or if the cert is always in the FQDN format and the script never worked or what.

i did try touching an empty puppet.pem file however while this makes the default configmap work it will cause the main puppetserver container to fail at the 99-log-config.sh step - https://github.com/voxpupuli/container-puppetserver/blob/main/puppetserver/docker-entrypoint.d/99-log-config.sh because the command is only expecting one non ca.pem file present and then openssl borks because the input is broken

Expected Behavior

check_for_masters.sh should check for correct cert file and pass without issue if it is present

Steps to Reproduce

Steps to reproduce the behavior: I am not sure if this is simply using multiMaster or alternateServerNames or the pregenerated import or a combination of all 3 - i have a suspicion that its the first 2.

here is a snippet of my values- domain changed to protect the innocent :D

puppetserver:
  preGeneratedCertsJob:
    enabled: true
    importPuppetdb: false
  masters:
    fqdns:
      alternateServerNames: "puppet-ca.domain.net,liq-puppet-foreman-za.domain.net"
    multiMasters:
      enabled: true
      manualScaling:
        masters: 2
  compilers:
    enabled: true
    manualScaling:
      compilers: 3
    fqdns:
      alternateServerNames: "puppet-enc.domain.net,puppet.domain.net"

Additional Context

I haven't yet created a PR here - I would be curious to find out the different possible puppserver cert names that could get generated because i think the script may need to take into account possible variations first to cover all potential scenarios - for example maybe we should attempt to read the certname field out of the puppet config and interpret correct file name from that?

anthonysomerset commented 2 months ago

when i generate config without cert import - cert generated is puppet.puppettest.svc.cluster.local.pem (assuming its namespace specific) - this might be specific to AKS though will need more data points for other environments

however i have a fix for the test that will be slightly more generic which i will open a PR for

Xtigyro commented 2 months ago

@anthonysomerset Thank you! Wanna become one of the official maintainers?

anthonysomerset commented 2 months ago

I have no objections but only if you sure! - i just needed to fix use cases specific to my scenario :)

i do want to look at making the restic compoent support azure blob storage as well ;)

cpiment commented 2 months ago

I was hitting this bug too! Thanks for fixing it :)