feat: add httproute from gateway-api to create chart template#12912
feat: add httproute from gateway-api to create chart template#12912
Conversation
57dc6e0 to
4ee174f
Compare
gjenkins8
left a comment
There was a problem hiding this comment.
nice, thanks for the PR.
A suggestion: perhaps add an example on how to access the installed deployment into NOTES.txt (https://github.com/helm/helm/blob/main/pkg/chartutil/create.go#L438) ?
(I'm not familiar enough with gateway API to know if this is entirely reasonable; but if so, might be helpful to chart consumers/authors?)
Thanks, and great suggestion. I added some infos to the NOTES.txt, but since configuration in the gateway-api are set in With With Namespaces and Paths are all tempalted form the values/release namespace |
|
Looking good. Thanks for working on this :) |
sabre1041
left a comment
There was a problem hiding this comment.
Reviewed the content of the PR and here are some comments:
- Let's align the default hostname among
IngressandHTTPRouteso that there is pariety. Yes, you could potentially install both, but in most cases, you would pick one or the other. So set the default hostname of theHTTPRouteto bechart-example.local - Support for
v1beta1. While the Gateway API is now GA, most who are currently using it is making use of the beta version. Similar toIngress, there should be a version check to determine which API Group/Version to apply - Overall thoughts: I am hesitant about adding this to the default scaffolded chart. This is mainly from a user experience perspective as we would want the getting started experience to be as smooth as possible. Most users will not have the Gateway API installed and there is a chance that they would face errors when attempting to leverage some of the features that are available in the default scaffolded chart.
|
Thanks for the feedback @sabre1041
Sure will do!
I thought so too at first but then the question was raised if it is used that much by people, so I removed it. I can revert that without a problem but you also mentioned user experience. I will make the assumption that the people using
I understand that concern. My goal was to make using the GatewayAPI easy because at fist I struggled transforming something usable into helm. To not disturb the user experience the gateway stuff is disabled by default, you have to enable it explicitly and then it should provide a good starting point for your first routes. I see it similar to Ingress and HorizontalPodAutoscaler. A lot of people don't use these and just delete the template but if you need them it is nice to have a good starting point. |
The biggest concern with including an |
|
The Kubernetes folks intend for the gateway api to replace the ingress api though. There is a chicken and the egg problem though. Enough software needs to adopt the ability to use the gateway api before enough clusters deploy it. Clusters are reluctant to deploy it unless software uses it. Software developers are less likely to support it if they don't have a good example of how to do it. Hence this pr's goal to help break the deadlock. |
416d8d6 to
d549cf6
Compare
|
Changed the default Rebased from main |
Any thoughts about supporting older versions of the Gateway API pre GA? |
|
Seems like there a no real opinions on that. Also asked in the k8s gateway-api slack channel. Given that |
|
There was a concern raised (on the Helm weekly developer call), that as the gateway API is not part of the core/standard API. Rather, that is an extension installed via optional CRDs. Helm shouldn't include a "non-standard" (non-core) API as part of a standard chart definition. (I didn't realize when first seeing this PR, that the API would remain non-core for the foreseeable future, iiuc) Personally, the Gateway API seems to much closer to being a part of core Kubernetes, than many other CRDs e.g. cert-manager's. But strictly, while it is optionally installed via CRDs. It is never going to be part of a "default" Kubernetes API set. I specifically asked what Helm maintainers see the function of the |
|
So, a few comments from the Gateway API maintainer POV (that's me!):
I'm happy to come to a Helm community call or something at some point to discuss this in a higher-bandwidth way, or to meet up at Kubecon NA and talk there too. There are two Helm-related problems that have been on my mind for some time:
|
|
Apologies for the belated reply. I agree with the above. And I would like to work out how to move forward. Overall, I'm not exactly sure what the answers are. I think the question is less "how does Helm handle HTTPRoute and gateway API?", and more: "How does Helm (and other tooling) handle 'out-of-tree' types?" I do think worthy to try and sync at KubeCon. I will be there, as well as other Helm maintainers. And if possible, jumping on the Helm developer call (9:30am Pacific, Thursdays; see #helm-dev on Kubernetes slack for zoom link and password) to try and prompt some attention might help too. |
gjenkins8
left a comment
There was a problem hiding this comment.
I approve, but other maintainers may still have differing opinions
|
Thanks for approving and your feedback @gjenkins8. I will update the comment tomorrow and will also rebase. Just out of interest: Did the topic of any none core kubernetes-apis come up at kubecon? I was not able to attend, but am quite interested if there where opinions exchanged. Regarding the role of helm and gateway-api |
|
Whats left to get this in? |
sabre1041
left a comment
There was a problem hiding this comment.
Logic wide, it looks good. Just a couple cosmetic changes since they are applied by default, to all newly created charts
37970d8 to
d058ec1
Compare
|
Implemented the requested changes to values.yaml template |
|
@hegerdes -- could you fix the conflicts please. I didn't see Andy's prior approval (otherwise I would have merged). Once conflicts are fixed, we should be able to do a quick round of re-approvals and get this merged. |
Adds the HTTPRoute from https://gateway-api.sigs.k8s.io/reference/spec/ to the example getting started chart. This closes helm#12603 Signed-off-by: Henrik Gerdes <hegerdes@outlook.de>
Signed-off-by: Henrik Gerdes <hegerdes@outlook.de>
Signed-off-by: Henrik Gerdes <hegerdes@outlook.de>
Signed-off-by: Henrik Gerdes <hegerdes@outlook.de>
Signed-off-by: Henrik Gerdes <hegerdes@outlook.de>
Co-authored-by: George Jenkins <gvjenkins@gmail.com> Signed-off-by: Henrik Gerdes <hegerdes@outlook.de>
Signed-off-by: Henrik Gerdes <hegerdes@outlook.de>
d058ec1 to
597c358
Compare
|
@gjenkins8 No problem! No code changes required, git handled the modified repo structure just fine in a simple rebase |
This closes #12603
What this PR does / why we need it:
This PR adds the HTTPRoute from gateway-api to the example getting started chart.
It is disabled by default, just like ingress.
Adding the resource will make helm charts future-proof for the gateway-api and may also increase its adoption rate.
Special notes for your reviewer:
If enabled, the via
httpRoute.enabled: true, it will produce a validHTTPRoutebut it will not route real traffic since aGatewaymust be installed and configured which is in the responsibility of the cluster admin, just like ingress-controller.The gateway api is still quite young and the gateway-api CRDs are not yet part of a default k8s installation. They currently have to be installed separably with
kubectl apply -f https://github.com/kubernetes-sigs/gateway-api/releases/download/v1.0.0/standard-install.yaml.The template checks if thev1or thev1alpha2version is installed and sets the api version accordingly.If the CRDs are missing the installation fails.
If applicable: