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

Allow custom (-options in the) Service object - allocateLoadBalancerNodePorts: false #1772

Closed
dihmandrake opened this issue Aug 7, 2023 · 9 comments · Fixed by #1893
Closed
Assignees
Labels
area/api API-related issues area/infra-mgr Issues related to the provisioner used for provisioning the managed Envoy Proxy fleet. kind/enhancement New feature or request
Milestone

Comments

@dihmandrake
Copy link

dihmandrake commented Aug 7, 2023

Description:

Describe the desired behavior, what scenario it enables and how it
would be used.

Allow for custom Service (https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#servicespec-v1-core) objects to be provided in KubernetesServiceSpec (https://gateway.envoyproxy.io/latest/api/config_types.html#kubernetesservicespec) instead of only ServiceType.
The only aspect overridden by Envoy Gateway would need be the selector object in the Service.
Having the Service object exposed to configuration allows for all sorts of custom deployments .

Specifically, I am looking for a way to support allocateLoadBalancerNodePorts: false.
Likely others will also start to look into custom: loadBalancerClass or other options.

[optional Relevant Links:]

Any extra documentation required to understand the issue.

See: https://kubernetes.io/docs/concepts/services-networking/service/#load-balancer-nodeport-allocation

@dihmandrake dihmandrake added the kind/enhancement New feature or request label Aug 7, 2023
@arkodg
Copy link
Contributor

arkodg commented Aug 7, 2023

thanks for raising this issue @dihmandrake, seeing 2 ways to solve your specific case of setting allocateLoadBalancerNodePorts: false when managed Envoy Proxy Service is of type LoadBalancer

  • Can all external load balancer implementations (AWS, GKE etc) work with allocateLoadBalancerNodePorts: false ? If yes, we can just set this by default, and a user can use service type NodePort to enable node ports
  • start supported BYO Envoy Proxy service and deployment sooner than later Support User Provisioned Envoy Proxy fleet #1379

@dihmandrake
Copy link
Author

Thanks for getting back on this so quickly.

In regards to option 1 (changing default): I would recommend avoiding setting this as a default. It would help me personally, since I am using Ciliums LB, but the default in k8s has always been to allocate these ports. If one (silently-) changes this, it will likely cause a bunch of issues with other load balancers eventually.

In regards to option 2: My assumption is that this will be difficult to support in the long run. People (including myself) will wrongly configure the BYO and it will cause overhead for the maintainers to support all sorts of custom setups. If you chose to support BYO setups, I would strongly advise to create a single source for the deployments. I.e. helm generated deployments also used in the operator. Otherwise, there will be two different deployment mechanisms, which eventually diverge and one always ends up being less tested and supported).

I would still ask you to reconsider the options of overriding defaults in the deployment or service via i.e. KubernetesServiceSpec. It is quite common to support a good amount of options in Helm charts (see Cilium or CoreDNS) to adjust for more customized setups. I am aware that Envoy is different since it is actually configured via CRDs and not via helm. For the record, it is easier to customize helm outputs via other tools (i.e. kustomize patches).

The options available for Envoy in the CRDs could be extended over time when needed by users or regularly pulled in from the official k8s API. I think this would avoid a lot of maintenance issues in the long run and still enable more customization in the deployment.

@arkodg
Copy link
Contributor

arkodg commented Aug 8, 2023

@dihmandrake whether allocateLoadBalancerNodePorts makes it into https://gateway.envoyproxy.io/v0.5.0/api/config_types.html#kubernetesservicespec, is tied to how many users using this project need to configure this. lets leave this issue open so we can track this.

this is where BYO solves the project maintenance / support problem of providing an alternative to enable all user needs w/o having to maintain the API / logic in the project. I do agree, this approach comes with a lot of gotchas

@arkodg
Copy link
Contributor

arkodg commented Aug 22, 2023

this was discussed in the community meeting today, and no one opposed adding this field, so adding this issue to our v0.6.0 milestone

@arkodg arkodg added help wanted Extra attention is needed area/api API-related issues area/infra-mgr Issues related to the provisioner used for provisioning the managed Envoy Proxy fleet. labels Aug 22, 2023
@arkodg arkodg added this to the 0.6.0-rc1 milestone Aug 22, 2023
@zirain
Copy link
Contributor

zirain commented Aug 23, 2023

do EG need check the version of kubernetes cluster?

@arkodg
Copy link
Contributor

arkodg commented Aug 23, 2023

dont think so, since feature is stable from v1.24 https://kubernetes.io/docs/concepts/services-networking/service/#load-balancer-nodeport-allocation

@zirain
Copy link
Contributor

zirain commented Aug 23, 2023

dont think so, since feature is stable from v1.24 https://kubernetes.io/docs/concepts/services-networking/service/#load-balancer-nodeport-allocation

that sounds good.

@den3tsou
Copy link
Contributor

Can I help with this please? If I understand correctly, we would like to add one additional field KubernetesServiceSpec. allocateLoadBalancerNodePorts. Is it correct?

@zirain zirain removed the help wanted Extra attention is needed label Sep 13, 2023
@zirain
Copy link
Contributor

zirain commented Sep 13, 2023

@den3tsou Thanks for the help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api API-related issues area/infra-mgr Issues related to the provisioner used for provisioning the managed Envoy Proxy fleet. kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants