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

audit kgateway helm install UX #10640

Open
Tracked by #10441
lgadban opened this issue Feb 17, 2025 · 3 comments
Open
Tracked by #10441

audit kgateway helm install UX #10640

lgadban opened this issue Feb 17, 2025 · 3 comments
Milestone

Comments

@lgadban
Copy link
Contributor

lgadban commented Feb 17, 2025

  • CRD installation
    • Separate CRD chart?
  • What installs the default GW?
    • As I mentioned before I think there are major long term UX trade-offs to include it in the templating in the chart directly. ~ @josh-pritchard
  • Revisit default GW Params
    • IMO you should never need this by default. It should be for overrides and customization. The fact that all of that is required in the CR right now shows me the feature was probably originally rushed. ~ @josh-pritchard
  • Audit helm values
@timflannagan
Copy link
Member

+1 on removing the need for default GW Params object.

re:default GW. @josh-pritchard do you remember the concerns you had in this approach (vs. having the controller roll it out for us?).

@lgadban
Copy link
Contributor Author

lgadban commented Feb 17, 2025

+1 on removing the need for default GW Params object.

I'm also a heavy +1 on this, but it will require a lot more work than the other items called out.
The internal helm chart for dynamic resources and the deployer semantics will need to be revisited entirely to break this out.
To be clear, I think it is worth it, but the timing will be difficult for 2.0.0 at least.

re:default GW. @josh-pritchard do you remember the concerns you had in this approach (vs. having the controller roll it out for us?).

I am also in favor of this, my reasons are:

  • not installing via helm is one less cluster-scoped resource installed, which means we can explore namespaced installs for niche use-cases

Other reasons I can recall being mentioned:

  • we need to effectively recreate the GwClass API in helm if we install it via helm
    • IMO this reasoning is a bit flimsy
  • If users modify it after the fact in cluster, there is no reconciliation and will be blown away by e.g. helm upgrade

@jenshu
Copy link
Contributor

jenshu commented Feb 18, 2025

re: default gatewayparameters, if we keep it in the chart, we should revisit how it is linked to the gatewayclass and which fields you can override. does it have to exist in the install namespace?

see a previous bug that was uncovered with hardcoded namespace #10653

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Planned
Development

No branches or pull requests

3 participants