Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
3 tasks done
gapodo opened this issue Oct 30, 2022 · 5 comments
Closed
3 tasks done

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

gapodo opened this issue Oct 30, 2022 · 5 comments
Labels
enhancement 👆️ Enhance existing feature

Comments

@gapodo
Copy link

gapodo commented Oct 30, 2022

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

@6543 6543 added the enhancement 👆️ Enhance existing feature label Nov 6, 2022
@6543 6543 transferred this issue from woodpecker-ci/woodpecker Feb 22, 2023
@pat-s
Copy link
Collaborator

pat-s commented May 28, 2023

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
Copy link
Collaborator

pat-s commented Aug 5, 2023

@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
Copy link
Member

anbraten commented Aug 5, 2023

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
Copy link
Collaborator

pat-s commented Aug 12, 2023

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.

@pat-s pat-s closed this as completed Aug 12, 2023
@gapodo
Copy link
Author

gapodo commented Aug 12, 2023

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 👆️ Enhance existing feature
Projects
None yet
Development

No branches or pull requests

4 participants