[stable/postgresql] PostgreSQL maintenance proposal

[carrodher]

What this PR does / why we need it:

This PR is intended in case the PostgreSQL maintenance proposal (#7956) is accepted.
In this way, you can see some differences between both charts, as well as work on applying new features or improve existing ones.

Differences between upstream PostgreSQL chart and the Bitnami one:

• Currently, stable version is 9.6.2 from 6 months ago while Bitnami version is 10.5.0 from 1 month ago.
  In fact, the stable chart is not up-to-date in the 9.6 branch because the current minor is 9.6.10 according to https://www.postgresql.org/support/versioning/.
  Bitnami has different systems that check upstream versions and updates the images following an automated test + release process.
• Bitnami one has replication (disabled by default), the stable one doesn't include this feature.
• Bitnami one has Pod Security Context, the stable one doesn't include this feature.
• Bitnami one has values.yaml and values-production.yaml, the stable only values.yaml.
• Upstream one has NetworkPolicy, the Bitnami one doesn't include this feature (but we can add it).
• Both use metrics exporter, secrets and PVC.

Which issue this PR fixes

• fixes [stable/postgresql] PostgreSQL maintenance proposal #7956

Checklist

• DCO signed
• Chart Version bumped
• Variables are documented in the README.md

[carrodher]

/hold

[carrodher]

/hold cancel

[javsalgar]

Small changes, the rest LGTM

[javsalgar]

Please test this. I think it may not work

[carrodher]

Hi all @scottrigby @prydonius @tompizmor @juan131 @javsalgar

I just implemented the requested changes (and merged latest features), can you take a look? Thanks!

[desaintmartin]

Do you intend to keep support somehow of pg_hba.conf?

[carrodher]

Hi! At this moment the chart supports custom configuration using postgresql.conf, but I am working to provide support for both (postgresql.conf and pg_hba.conf). But this change not only requires modifications in the chart (this is the easy part), it also requires changes in the Docker image.

TL;DR I am working to modify the Docker image to support it.

Do you think it is a blocking issue to merge it or can we merge the PR and send a new one with the ph_hba.conf changes after some days?

[desaintmartin]

It is blocking for me (I have a dozen of postgres running using that feature) for the long term but I guess it can wait for a few days with no harm and in case of problem I can use my own fork.
Tell me if I can help.

[carrodher]

Thanks!
I just added some logic to support it at chart level 92498ba, feel free to review it!
In the other hand I continue working on the Docker image stuff

[juan131]

LGTM

[tompizmor]

/ok-to-test

[tompizmor]

/approve
/lgtm

[sameersbn]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: carrodher, sameersbn, tompizmor

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [sameersbn]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Remigius2011]

Hi,

idsclaimer: I just dropped here as I have noted that my approach of deploying postgresql via the non-Bitnami helm chart is broken. so please direct me to the location where I can report my problems if this isn't the right one.

I am deploying a small-scale postgresql database on a rancher 2.0 based k8s cluster using the helm chart (last version used: 0.19.0). I have discovered that two features I am using are missing in version 2.x:

• use of an existing secret (parameter existingSecret)
• use if an existing pvc (paramteter persistence.existingClaim)

furthermore, the pod's and service's name has changed to a include a prefix or suffix postgres (in my case it is now postgres-postgres and I haven't investigated whether it is a prefix or suffix yet) - this must be fixed by changing the host name in all locations where other resources access the database - but in a running cluster this might still be difficult to achieve and mandate downtime.

As I have also tought this to the participants of my rancher workshop, my students's workloads / future experiments are affected as well.

While I absolutely welcome the initiative to take properly care ot the postgresql helm chart, I think that it would be better to avoid disruption where possible - even if this may be expected by people familiar with the concept of semver.

Should I post a new issue or is there a chance at all that these concerns will be addressed? As of now, the only chance I see is sticking with v0.19 of the chart.

[desaintmartin]

I believe the need of using an existing claim is gone with the switch to a StatefulSet (you can create the PVC ahead of time corresponding to what the volumeClaimTemplate wants, and it will work).

I let other people answer for existing secret.

About incompatible changes, I absolutely agree with you, this is why I proposed to use another, new chart for this work (although I realize it would be misleading for most people). As for now, I'm going to use my own fork of this chart until I have time to upgrade all (production) projects with downtime. I suppose there is no other way than doing what is being done right now, since it follows a needed Kubernetes change (deployment -> statefulset) with unavoidable downtime.

Maybe the service names can be the sames than the old ones.

[Remigius2011]

@desaintmartin In my view, the introduction of a new postgresql image (bitnami instead of the 'official' one) with different features, base images, tagging and release cycle as well as a different set of k8s objects to be created would be reason enough to create another helm chart - even though this might confuse a few, it might be less disrupting for those who already use the old helm chart (like me). The more I'm looking at the new helm chart version, it looks to me like a completely different animal instead of an evolution from the old one. Nevertheless I fully agree that introducing a StatefulSet and support for clustered deployments (using streaming replication) is clearly an asset - but migration to it should imho be a conscious decision and not an accident (sorry for the clear wording I use here).

People who upgrade to a new version of the helm chart (which is in rancher - conveniently but dangerously - just a couple of mouse clicks away, and as it often works, people get used to it) will most likely end up with a failed deployment which they may or may not be able to rollback (I haven't tested this - I was so far lucky enough to deploy the new chart only on a freshly created cluster).

I don't know how a StatefulSet deals with a pre-existing pvc (the documentation says 'or pre-provisioned by an admin' but I dod not find out how to 'pre-provision' storage for a StatefulSet). If you think about upgrading an existing deployment, a full database migration (i.e. dump, transfer the dump and restore it) seems unavoidable to me. Maybe it would help if you could describe exactly how to create a pre-existing pvc and use it for deploying this helm chart.

Support for pre-existing secrets should imho be easy to implement - just take the existing one instead of creating a new one.

Maybe adding a clearly visible warning at the top of the description might help even though not everybody might look at it.

[cliedeman]

Hello,

I started work on a PR to add existing secret back #9532. Would be great if @carrodher could take a look.
The change is mostly trivial, the only problematic part is the notes file.

A section on how to migrate from pre version 2 would probably be a good idea.

Ciaran

[Remigius2011]

A section on how to migrate from pre version 2 would probably be a good idea.

this and a description / facility to use pv / pvc storage (instead of storage classes).

[arianitu]

I have no idea how this was accepted. How can you claim stable/postgresql and suddenly change the underlying container entirely.

My own images were based off the official postgres image and no longer work with this chart because bitnami is entirely different.

I noticed replication was added, but it was added through a new container, meaning I can't even upgrade to this chart.

Helm is definitely off my list of things I would use in production. You guys should have made an entirely different chart, postgres-bitnami or something...

[carrodher]

I have no idea how this was accepted. How can you claim stable/postgresql and suddenly change the underlying container entirely.

My own images were based off the official postgres image and no longer work with this chart because bitnami is entirely different.

I noticed replication was added, but it was added through a new container, meaning I can't even upgrade to this chart.

Helm is definitely off my list of things I would use in production. You guys should have made an entirely different chart, postgres-bitnami or something...

Hi, I just provide an answer in the issue that you have opened (#10451), to avoid duplicate conversations in different places (I have also seen a similar comment in another issue #8735 (comment)), do you think it is good to continue the discussion in a single thread (#10451)?

[Remigius2011]

i second @arianitu 's comment about the disruption of the helm chart and also cannot understand how the (new) authors of the "official" postgresql helm chart could get away with changing it to be unusable with the official (this time without quotes) postgresql docker image by replacing it with a decommoditized one (namely the bitnami image, for which the official postgresql docker image is evidently not a drop-in replacement - even if this may be the case in the other direction). while using the bitnami image may undoubtedly have its benefits, it is imho unappropriate to force people to use it in place of the official one (which has also its benefits - e.g. early availability of new postgresql versions, comprehensible buildup etc..). what concerns me most is the disruption of operations caused by the upgrade of the helm chart from earlier versions. this has cost me - fortunately - only a few hours work, as i have first performed it on an experimental (not even test) environment, which was disposable, but this course of action has most likely cost some people who were performing the upgrade on a non-disposable environment severe headache and significant wasteful labor - which would have been absolutely avoidable.

while i think it would be appropriate to keep the "official" helm chart downwards compatible in the first place (which i myself tend to do / enforce / support in contexts where i have some control), the current situation - after a flood of new versions in short sequence - is probably already too much progressed in the new direction as to change the policy back to using the official postgresql docker image. however, it might be feasible to create a postgresql helm chart for the official docker image under a new name (maybe postgresql-official???) that forks off the last "usable" version (i am still at 0.19 and did not venture upgrading it to anything newer, as i have a productive environment running - i can still upgrade the docker image to newer official versions).

@arianitu , i don't think that helm itself is unusable, it has some benefits as it allows to create templates that bundle artifacts for complex deployments. this here is (imho) an accident that does not occur at large in the k8s/helm ecosystem (as far as i have observed). my lesson learned from it (which is not new, actually) is that upgrading helm charts lightheartedly should be avoided at any rate. instead it is advisable to perform experiments in a disposable environment first - which you must have at hand when you do anything serious anyway. this could happen in any situation where you change a component / artifact of your system, even when the evolution of components is performed as diligently as possible (this only lowers the likelyhood of disruption by some orders of magnitude instead of cuasing disruption cousciously).

@carrodher your question was not addressing me, but here's my two cents: i don't care where this discussion takes place (normally newer issues are designated as dupplicates for old ones, but there may be reasons to deviate from this practice), but my main concern is to keep up a (hopefully maintained) helm chart that supports the official postgresql docker image (i have currently no insight into the process of creating / maintaining "official" helm charts - probably it is documented just a few clicks away from here). obviously it would be advantageous for users of the old line of postgresql helm charts to have an upgrade path that avoids a complete reconstruction of the deployment (i.e. deploy new helm chart, migrate data and references from old to new etc.). if a new "official" image will not be made available, i will probably resort to forking off my own one as soon as the version i am currently using is not working any more (e.g. due to changes in k8s/helm).