proposal: remove postgres subchart

[bo0tzz]

Having the postgres subchart makes for an easy deployment, but has caused several annoying issues around upgrades in the past (see eg #55, #125). Being a bitnami chart, it also does a bunch of custom stuff that doesn't align with the pgvecto.rs image that Immich needs, and while it still seems to work I feel a bit uneasy about that.

Running a "bare" postgresql container in kubernetes like this also isn't the best approach, with it being preferable to use an operator such as https://github.com/cloudnative-pg/cloudnative-pg or https://github.com/CrunchyData/postgres-operator. That gives easier management, better resilience to runtime problems, and more.

I propose to remove the dependency on bitnami/postgresql and require users to manually set connection details for a database instead. That'll simplify and remove risk from chart upgrades, and make users' postgres deployments a bit more explicitly managed (which I think is a good think). We can provide some pointers in the README for different methods like separately installing bitnami/postgresql or using an operator. In the future, the chart could potentially include CRDs for common operators that users can enable through the values, for example setting database.cnpg.enabled: true.

[alexbarcelo]

I am used to Helm charts that self-contain (at least optionally) the database. That is true for most old k8s-at-home charts and old helm charts repo. Authentik and infisical (to name two that I use personally) official helm charts also follow this strategy.

I understand the issues that have appeared due to vecto.rs, but leaving Postgres doesn't seem a good idea IMHO. The existence of Potgres operators is not exactly a strong argument, as there are also Redis operator out there and the same argument could be made for Redis (the difference being that Redis have not presented issues in the past AFAIK, but that could change if there are some breaking changes with Redis).

Having this immich-chart with sensible defaults and ease of deployment is the strong point, at least from my PoV. I am quite happy with the current state (even with the occasional breaking changes: self-hosting is a hobby, Immich and this chart is not a commercial product, I know!). I am not an active collaborator, so I cannot ethically lobby a lot, but those are my 2c. Anyhow, great work and let's hope that everything ends up good at the end :)

[martimors]

Hey, thanks for the fix, and sorry for causing issues recently around this. I agree that using the bitnami chart as a dependency is a bit much in many cases, and the incompatibilities with the vecto images are unpredictable, especially because the chart isn't tested with the official postgresql image (on which pg-vecto-rs images are based), but with bitnamis own image. I can't remember completely but I have struggled with the official image in the past because it really wants to be god-admin, and that's not allowed on openshift and other k8s distros with a more strict security setup.

I'd say remove the chart, and replace it with some barebones pg-vecto.rs setup (just a statefulset, a service and a pvc + some env vars would be fine). State that this is meant for testing and that a production setup should use some more hardened persistence including backups and replication. Then if someone wants the more advanced features given by the bitnami chart they can just deploy this alongside, I do this anyways many cases because many projects don't have a nicely maintained chart like immich does (jellyfin, paperless-ngx, seafile...).

I can probably find some time to help out too.

[bo0tzz]

Thanks for the feedback!

The existence of Potgres operators is not exactly a strong argument, as there are also Redis operator out there and the same argument could be made for Redis

I don't entirely agree with this point - Redis isn't holding any important state and needs basically no management, upgrades are seamless, etc. None of that goes for postgres.

I do definitely see the argument for having the chart be "plug and play". I think the main issue there (and maybe in general) is that I'm currently the only maintainer for the chart, and I'm unable to provide that level of ease, so I feel like removing this regular source of problems would mean less load for users even if it is a bit more work up front.

sorry for causing issues recently around this

I don't blame you; it was a trivial change and the fault is on me for not testing it properly (and for not having a good process/automated tests in place).

some barebones pg-vecto.rs setup (just a statefulset, a service and a pvc + some env vars would be fine). State that this is meant for testing and that a production setup should use some more hardened persistence including backups and replication.

This seems like a reasonable middle ground. The one issue I see with it is the chance that people will end up relying on it anyways (and running into problems like data loss), or on the flip side that we're telling people not to use it and so it doesn't really add much anyways. It'll also need some discipline in scoping because there will definitely be requests for embellishments like automated backups.

[martimors]

Another option is to extend the bitnami postgres image with the vecto.rs extensions, and hosting that alongside the other immich official images. That'd make the image compatible with the chart, but managing the lifecycle of a docker image is another headache I guess.

[bo0tzz]

That is indeed another option, and even has a PR open for it (#61), but like you mention it's another thing to maintain - and I'd like to maintain less stuff if I can :P

[PixelJonas]

Hey - my 2 cents.

I really like having the postgres chart as a subchart to immich and for everyone installing Immich using this chart this is awesome.

I think the main problem arises when new major postgres versions are introduced as postgres does not support live major upgrades (it's a pain for a couple of apps I'm hosting). This resulted in this chart shipping with an old postgres version as we could brick everyone who did not override the version tag.

It comes up a couple of times by either someone pointing out that we ship an old image or by us updating the image and breaking peoples instances.

I don't feel Immich should invest time&ressources in mainting their own Bitnami+vecto.rs Image as this is not the focus of this project. At least as there is not a group of people volunteering for maintaining this - and even so, I'd rather have this in a different org.

My suggestion would be removing the postgres sub-chart. If there is any way for us for throwing a decent error-message if someone did not specify any DB_URL this would be awesome. This way we could remove the chart and point users (at install-time) towards setting up a postgres database

[bo0tzz]

If there is any way for us for throwing a decent error-message if someone did not specify any DB_URL this would be awesome.

There definitely is, we already do something like it for the main library volume. That also removes a worry I had, which was that people might upgrade without knowing what was coming and accidentally destroy their postgres database.

[PixelJonas]

Can we only fail, or is there way to introduce a warning. This way we could announce the change prior to actually implementing it

[bo0tzz]

Doesn't seem like it, unfortunately. There's an open request at helm/helm#12937.

[morotsgurka]

My initial thought was similar to @alexbarcelo, that I am used to Helm being the compose of Kubernetes. Add values and hit install.
But reading through this, I understand some of the issues. My vote is to remove the Bitnami chart BUT add some written documentation/ReadMe for new users like:

• How to setup Postgres for Immich/this chart(Bare pod, Operator and or Bitnami chart)

• Common pitfalls with Immich and Postgresql (Maybe link to old issues on GitHub that people have had?)

• Postgres upgrade tips/information (for example like @PixelJonas mentioned)

as postgres does not support live major upgrades

(I did not know this for example)

• A note in new Releases if/what version of postgres is recommended/need

[rjbez17]

Postgresql is already disabled by default, so IMO stronger documentation around other alternatives is a better approach. I'd argue that the majority of deployments are on the smaller end and increasing the barrier to entry will harm the project.

[bo0tzz]

I've settled on removing the postgres chart, and providing some suggestions in the README for ways to deploy postgres instead. If needed, in the future the chart could contain templated CRDs for the popular postgres operators, but that's one I'll leave for later.

The one prerequisite now is to improve the release pipeline to properly handle and surface changelogs/release notes. Any help with that would be very welcome :)

[willianpaixao]

@bo0tzz I would like to help on that. Where is a good place to contact you?

[bo0tzz]

Ideally on our discord server (https://discord.immich.app). But if you prefer to create an issue or discussion on this repo, that's fine too.