dirsigler / uptime-kuma-helm

This Helm Chart installs Uptime-Kuma from @louislam to your Kubernetes Cluster.
https://helm.irsigler.cloud
GNU General Public License v3.0
156 stars 54 forks source link

Removed misleading autoscaling option from the values and docs #96

Closed CommanderStorm closed 1 year ago

CommanderStorm commented 1 year ago

Description of the change

This PR removes the autoscaling option from this chart. The reason for it is, that it is unused => does not work

The reason for this is, that uptime-kuma currently does not scale to more than one instance. (local sqlite+ no multi-instance scheduling) I would like to discuss if we should also remove the replicas option

Benefits

This removes a non-working/misleading option

Possible drawbacks

In future iterations of uptime kuma, this might be a thing that

Applicable issues

/

Additional information

/

Checklist

dirsigler commented 1 year ago

Hey @CommanderStorm

thanks for the change proposal.

Some little history about the confusing state of the autoscaling and the configuration of a Deployment + Statefulset: There were some changes proposed to the upstream Uptime-Kuma project about adding support for MySQL or Postgres instances as the SQL backend. Furthermore they thought about testing the whole autoscaling scenario and making the application more compatible for that.

As a preparation for these features it was suggested to implement a Statefulset to the Helm Chart and in the future also add some other Charts like the Bitnami MySQL/Postgres to the setup.

As far as I know the whole topic is not finalized or still in discussion, but I agree that having a Statefulset and a Deployment is confusing.

dirsigler commented 1 year ago

@CommanderStorm this will now need a Rebase and another bump of the Chart version as #97 was merged. Thank you!

CommanderStorm commented 1 year ago

@CommanderStorm this will now need a Rebase and another bump of the Chart version as #97 was merged. Thank you!

Done. Should I also remove replicaCount in this PR?

dirsigler commented 1 year ago

@CommanderStorm this will now need a Rebase and another bump of the Chart version as #97 was merged. Thank you!

Done. Should I also remove replicaCount in this PR?

Saw the message too late I think. You can do it in an additional PR then if you like. Thank you!