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.

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

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

Closed
anthonysomerset opened this issue May 3, 2024 · 4 comments · Fixed by #232
Closed

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

anthonysomerset opened this issue May 3, 2024 · 4 comments · Fixed by #232
Labels
bug Something isn't working

Comments

@anthonysomerset
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

Xtigyro commented May 7, 2024

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

@anthonysomerset
Copy link
Contributor Author

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 ;)

anthonysomerset added a commit to anthonysomerset/puppetserver-helm-chart that referenced this issue May 9, 2024
Xtigyro pushed a commit that referenced this issue May 9, 2024
@cpiment
Copy link
Contributor

cpiment commented May 24, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants