woodpecker-ci / helm

This repo contains the helm charts of the Woodpecker project.
Apache License 2.0
21 stars 16 forks source link

add Postgresql subchart for integrated db deployment #143

Closed psych0d0g closed 8 months ago

psych0d0g commented 8 months ago

to easily spin up a woodpecker deployment in k8s i added the bitnami postgresql chart as a subchart to the server chart. that way enabling it via values (defaults to true) it deploys a pgsql alongside woodpecker-server and configures the environment of the server accordingly

pat-s commented 8 months ago

We could certainly do this. Yet, I would always highly recommend to maintain/deploy your own (central) PG instance instead of using a sub-chart dependency of a chart - or even better, use a managed DB in the cloud. Why? You have an independent PG instance and avoid having multiple DBs for different charts. If there's one thing worth paying for in the cloud, then it's a managed DB.

I also maintain the Gitea helm chart, and we have the Bitnami PG one as a sub-chart there and while it's convenient for users to some degree, problems arise when it comes to (major) updates of the chart and other maintenance related tasks.

I am certainly not deciding this one here on my own, but my suggestion would be to only make recommendations what to do/which chart to use but not bundle PG as a sub-chart dependency (WRT to the arguments mentioned above).

One thing that is certainly missing right now is a note on not relying on the built-in SQLITE DB for production use. While this might be known by many people anyhow, we should still point it out.

@woodpecker-ci/maintainers Any thoughts to the request and my 2 cents above?

psych0d0g commented 8 months ago

central DBs are imho a good use for non-cloud deployments, when it comes to k8s workloads i am a fan of 1 deployment - 1 pgsql database (that way your deployment is independent from others) - coming back to your argument of having troubles when it comes to major DB upgrades: that problem gets worse when you have multiple deployments depending on a central PGSQL cluster where not all deployments can agree on a pgsql version. with pgsql instances besides each deployment you are free to run several different versions, tied and optimized towards the deployment they are running for. it would be a totally diffrent topic for mysql or mariadb in my opinion since they come with a way bigger overhead then pgsql.

on a note regarding the implementation: i would suggest, and if agreed on implement, a way to give the user a choice, either provide the sql connect string yourself (for an already existing database), or use the subchart which handles that part of configuration itself.

as an example, i did it that way (even leaving the choice between pgsql, sqlite AND mariadb, either bundled or dedicated and independent) in my romm chart which can be examined here: https://github.com/CrystalNET-org/helm-romm

pat-s commented 8 months ago

central DBs are imho a good use for non-cloud deployments, when it comes to k8s workloads i am a fan of 1 deployment

There are arguments for both sides, surely. I see your points.

that problem gets worse when you have multiple deployments depending on a central PGSQL cluster where not all deployments can agree on a pgsql version.

Sure, this applies to any DB/application. Despite that for PG, the compatibility range is usually quite big, i.e. as of today most apps usually support PG11+ with PG16 being the latest one. But yeah, I see your point. When using a central DB you need to check for comp across all apps which use it before upgrading.

i would suggest, and if agreed on implement, a way to give the user a choice, either provide the sql connect string yourself (for an already existing database), or use the subchart which handles that part of configuration itself.

Oh totally, this is standard practice.


WRT to upgrade issues: in other charts I see groups of users complaining about update issues with DBs as they expect the parent chart to apply some magic for major upgrades (via init containers etc.) instead of being left alone and/or pinning the major version themselves. There are of course others which don't expect this and know that DB upgrade management is not implied if the DB is a sub-chart but it always sparks discussions. So if we do it here, I would heavily document the fact that upgrading is on the user side and the chart just goes with the rolling sub-chart releases and major version pinning is encouraged.