charts icon indicating copy to clipboard operation
charts copied to clipboard

feat(cluster): Recovery using pg_basebackup

Open Pionerd opened this issue 1 year ago • 5 comments

An implementation of the pb_basebackup recovery method.

Tested this helm chart using the existingSecret option and was able to successfully bootstrap a cluster using this method. The TLS implementation I'm unable to test, but it is relatively straightforward, based on the docs.

I assume you can automatically update the values schema / docs? Let me know if anything else is required.

Pionerd avatar Apr 09 '24 17:04 Pionerd

This is great work and we'll merge it, but I will request some changes. I'll allocate some time to do a detailed review later this week.

itay-grudev avatar Apr 10 '24 09:04 itay-grudev

One discussion point: do you think it may have been a misnomer on my side to consider pg_basebackup a recovery mode? We could alternatively put that as another .Values.mode.

@Pionerd @phisco

itay-grudev avatar Apr 10 '24 09:04 itay-grudev

When working on this, I didn't want to refactor the whole thing straight away, but indeed, to me it makes more sense to take it out of the recovery section

Pionerd avatar Apr 10 '24 09:04 Pionerd

Don't refactor it yet. Let's hear what @phisco thinks as well. But if we want to make that change it should be now, otherwise the API will be fixed for reasons of backwards compatibility.

itay-grudev avatar Apr 10 '24 09:04 itay-grudev

Also, we are testing "bootstrapping" databases using https://cloudnative-pg.io/documentation/1.22/database_import/. E.g.

apiVersion: postgresql.cnpg.io/v1
kind: Cluster
metadata:
  name: cluster-microservice
spec:
  instances: 3

  bootstrap:
    initdb:
      import:
        type: microservice
        databases:
          - angus
        source:
          externalCluster: cluster-pg96
        #postImportApplicationSQL:
        #- |
        #  INSERT YOUR SQL QUERIES HERE
  storage:
    size: 1Gi
  externalClusters:
    - name: cluster-pg96
      connectionParameters:
        # Use the correct IP or host name for the source database
        host: pg96.local
        user: postgres
        dbname: postgres
      password:
        name: cluster-pg96-superuser
        key: password

Currently, I can specify the initdb part, but the externalClusters part would also need to be added. However, to me it feels a bit strange to add another part in the bootstrap where externalClusters are defined. Defining the externalClusters in the values.yaml and then using them one on one feels more natural to me, but maybe this is exactly the logic you want to abstract away with this helm chart. What do you think?

Pionerd avatar Apr 11 '24 15:04 Pionerd

@Pionerd Sorry it took so long. This was a great work. I added some tests and changed the API so it feels more consistent with the rest of the chart. If you are already using this on production you may need to update your YAML a tiny bit.

itay-grudev avatar Aug 28 '24 17:08 itay-grudev