woodpecker-ci / helm

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

Rewrite the chart for woodpecker-server for a better UX #4

Closed gapodo closed 1 year ago

gapodo commented 1 year ago

Clear and concise description of the problem

The helm chart currently requires manual creation of secrets, copying env vars, and knowing which options clash (i.e. 2 Forges simultaneously)

Suggested solution

Switching the woodpecker-server chart from "copy these env into your file" to just "set gitlab, gitea, github or whatever to true and add the values defined in the respective section".

This would mean providing simple settings in the values.yaml, possibly verifying those settings, and therefore abstracting the actual configuration from the user.

Implementing this would allow us to change parameters, option orders, etc. in the non-user facing part and therefore providing a hands-off upgrade approach on the chart based installations.

Alternative

In this case it is pretty much do it, or document it. Documentation might be useful, but does not solve the problem of issues / support effort when users upgrade in a breaking way

Additional context

It might not be strictly necessary, but I would be open to implement it.

Validations

pat-s commented 1 year ago

The helm chart currently requires manual creation of secrets, copying env vars,

IMO this is "good enough", there are various ways how one can insert secrets in a "proper" way in helm charts without the chart needing to provide a skeleton for it.

knowing which options clash (i.e. 2 Forges simultaneously)

This can certainly be done in the chart via some pre-checks or a schema. Yet these are often details which most people lack the motivation to do so after getting the chart working in the first place - as most users get it right on their side anyhow and you only "save a few" who don't. But certainly a valid point.

pat-s commented 1 year ago

@gapodo Are you still interested in tackling this rewrite? I am not unhappy with env vars so to me this is something totally optional but if you're motivated to do it... :)

anbraten commented 1 year ago

In terms of maintaining I think the current approach is much easier as you don't have to update the helm charts each time a new config option is changed.

pat-s commented 1 year ago

Good point! I agree. Let's keep it as is then.

-> Hence closing here. @gapodo I hope this is understandable for you? Let us know if you differing thoughts on this one.

gapodo commented 1 year ago

Sorry, life has been quite busy...

as you don't have to update the helm charts each time a new config option is changed.

True, the motivation for the proposal came, when such changes were introduced (env renaming) and broke my setup (as in helm went through and the result was a downtime).

The motivation behind using helm (at least to me), is making the install and update process as robust, seamless and hands off as possible. Many available helm charts abstract settings, so they can be (internally) renamed,... while being verifyable (as in the chart being able to check basic configs, and potential breakages due to updates, preventing breaking rollouts) and typically not requiring user intervention.

Good point! I agree. Let's keep it as is then.

-> Hence closing here. @gapodo I hope this is understandable for you? Let us know if you differing thoughts on this one.

I'm fine with this, sadly I'm lacking the time to tackle it, so it would just be laying around.

Though I think keeping the reasoning / idea in mind would be a good idea (don't let a updates break stuff, either by increasing the helm chart major, when breaking config changes are introduced in woodpecker, or by handling them in the chart, by warning or auto migrating).