OneUptime / oneuptime

OneUptime is the complete open-source observability platform.
https://oneuptime.com
Apache License 2.0
4.73k stars 209 forks source link

Enhancement: Add a way to disable chart dependencies. #829

Closed kashalls closed 3 weeks ago

kashalls commented 10 months ago

Issue cross posted from https://github.com/OneUptime/helm-chart/issues/1

Thanks for working on getting this helm chart published. It makes it easier to manage this instead of a huge glob of manifests. We do have some issues that needs to be sorted out.

  1. Some of the additional charts included in this chart need a way to be disabled.

Clickhouse, Minio, Postgres and Redis are already scaled services I run. Postgres and Redis are widely available as managed services by hosting companies so it would make sense to that over a small one in this chart. This would need to basically have an option where you can set enabled as a boolean and configure some other things on where to connect, how to connect, connect with special options (redis with sentinel?).

  1. I'm not entirely sure why we need metallb in a helmchart?

  2. There is no way to pass secret data. Environment variables like the ones in example.env, are unsettable here.

It's true that they are added here. But it simply generates a random string, unless these need to be changing every time you deploy this chart.

A good alternative would be to allow users to create a Secret/ConfigMap called oneuptime-config for the helmchart to read from instead of hardcoding stuff in helpers.tpl

  1. It might be a good idea to explore alternatives to running a nginx container in kubernetes when you can take advantage of other options like traefik, ingress-nginx, or the built-in endpoints that I haven't taken a look into.

All in all, good job on the chart! Glad it is being pushed, a lot of the OSS homelabbers will be happy to stop updating 25 oneuptime containers every day. :)

simlarsen commented 10 months ago

2 - MetalLb is needed for people self hosting this. Its optional if you're hosting the chart in cloud env. 3- secrets are autogenerated and stored in k8s secrets. 4 - Not a priority at the moment.

1 - We need to work on it, but currently low priority unless some enterprise is willing to pay for the feature.

batazor commented 10 months ago

Adding to @kashalls comment. Thanks for providing the helm chart!

It's good that there is a possibility to disable MetalLb I don't think it's necessary to configure the whole world around you, and if users need MetalLb they probably already have it installed, just leave it to the users, they will decide which volume of service they need LoadBalancer or ClusterIP.

Using ClusterIP will remove MetalLb from the dependencies and leave it up to the user.

Regarding nginx, I agree, that right now the service runs a frightening number of services for a simple task - to show the status of the service on the page. I already have istio/nginx-ingress - as most do, and using standard Ingress will ease the complexity and load on the cluster considerably.

Also in terms of customizing volumes, I would like to have more control over their types, for example, my cluster does not support ReadWriteMany

MadOtis commented 6 months ago

I'll add my +1 to this, also. I also run several of the services in my cluster alreeady (Postgres, MetalLB, Redis, hell, even some Mongo) and it would be nice to be able to set up values prior to install so the chart doesn't try to recreate these. I'm trying to adjust a helm template generated manifest to see if I can bypass these needs, currently.

brandon-parrott-conversenow commented 3 months ago

+1

I already have Redis and Postgres. To make this product more versatile, it would be nice to have the ability to pass in credentials to use these existing resources.