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

Derive nodePort from Listener port when Service is of type NodePort #2220

Closed
chromakode opened this issue Nov 20, 2023 · 15 comments
Closed

Derive nodePort from Listener port when Service is of type NodePort #2220

chromakode opened this issue Nov 20, 2023 · 15 comments
Labels
area/infra-mgr Issues related to the provisioner used for provisioning the managed Envoy Proxy fleet. help wanted Extra attention is needed kind/enhancement New feature or request

Comments

@chromakode
Copy link

chromakode commented Nov 20, 2023

Hi! When running in kind, one of the convenient ways to handle ingress to set up an external port mapping with a static port number.

I haven't found a way to specify nodePort for the gateway's service, so it's not possible to set up a port mapping because the number is dynamically assigned. It'd be useful to be able to specify the desired nodePort in the envoyService spec.

@chromakode chromakode added the kind/enhancement New feature or request label Nov 20, 2023
@zirain
Copy link
Member

zirain commented Nov 20, 2023

when running kind, we encourage use MetalLB to expose servcie. https://gateway.envoyproxy.io/v0.6.0/user/quickstart/#prerequisites

@arkodg arkodg added the kind/decision A record of a decision made by the community. label Nov 20, 2023
@arkodg
Copy link
Contributor

arkodg commented Nov 20, 2023

we've had multiple asks by users who want to be able to pin the nodePort value of the service.
kubernetes-sigs/gateway-api#2321

Here's what we can't do

  1. specify nodePort in envoyService because the ports are generated based on Gateway configs
  2. specify nodePort in https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io%2fv1.GatewayAddress because it doesn't accept ports

What we could do

  1. use gateway.spec.listeners[].port as the nodePort, and use some auto gen port as the service port
  2. users can create their own envoy services and link to a Gateway config Support User Provisioned Envoy Proxy fleet #1379

@chromakode
Copy link
Author

Thanks for the replies! I investigated MetalB, but on kind since LoadBalancers on the docker network are not easily accessible on Mac/Win docker, it didn't seem to provide any advantage vs. using kubectl port-forward.

Option 1 (gateway.spec.listeners[].port) makes sense to me as a user.

I'm also a little curious why adding it to envoyService wouldn't work -- this said as a novice to this codebase. When I was searching the code last night to figure out how ports were set, it looked like the ports and envoyService spec were both available here:

serviceSpec := resource.ExpectedServiceSpec(envoyServiceConfig)
serviceSpec.Ports = ports

Perhaps ports could be passed to resource.ExpectedServiceSpec, and it could be responsible for creating the whole serviceSpec.Ports struct, including adding nodePort if applicable?

@zirain
Copy link
Member

zirain commented Nov 21, 2023

@arkodg
Copy link
Contributor

arkodg commented Nov 22, 2023

@chromakode the ports are translated from the gateway-api Gateway config, this test case should help explain it, redefining a section of the state it in the Envoyproxy, and deriving the rest from the translation seems fragile to me (the user may not be able to always define the ports section correctly)

I think 1. is a reasonable way to achieve this

@arkodg arkodg added help wanted Extra attention is needed area/infra-mgr Issues related to the provisioner used for provisioning the managed Envoy Proxy fleet. and removed kind/decision A record of a decision made by the community. labels Nov 22, 2023
@arkodg arkodg changed the title Allow nodePort to be specified for NodePort type services Derive nodePort from Listener port when Service is of type NodePort Nov 22, 2023
@arkodg
Copy link
Contributor

arkodg commented Nov 22, 2023

@tamalsaha @tommie I believe ya'll are running in NodePort mode . Is this change of interpreting Gateway listener port as managed service Nodeport (and setting the service port same as node port or some other auto gen val, which becomes an internal implementation detail), something that makes sense for your deployments ?

@tamalsaha
Copy link
Contributor

Currently we are using annotations like below on gateway to configure node port for a listener port in our gateway.

gateway.envoy/port-mapping/<listerner-port>: <node-port>

@arkodg
Copy link
Contributor

arkodg commented Nov 22, 2023

@tamalsaha so the listener port value is a place holder, being unused, or are you counting on it for something ?

@tamalsaha
Copy link
Contributor

It is not a placeholder. We set it to the actual value of a listener port used in the gateway. Same way node port is an actual value. Effectively, we are proving a map from svc-port => node-port.

gateway.envoy/port-mapping/5432: 35342

@arkodg
Copy link
Contributor

arkodg commented Nov 22, 2023

It is not a placeholder. We set it to the actual value of a listener port used in the gateway. Same way node port is an actual value. Effectively, we are proving a map from svc-port => node-port.

gateway.envoy/port-mapping/5432: 35342

if the listener port directly mapped to the nodePort (only for NodePort type svc). you wouldn't need the annotation, correct ?

@tamalsaha
Copy link
Contributor

tamalsaha commented Nov 22, 2023

if the listener port directly mapped to the nodePort

What do you mean by that? In our case the listener port becomes the envoy service port. We make the envoy service a NodePort type service using EnvoyProxy CR. Then these gateway annotations are used to specify which node port to use for a given envoy svc port. We needed the annotation because there is no place to so that in the Listener block for now.

@arkodg
Copy link
Contributor

arkodg commented Nov 22, 2023

@tamalsaha

  1. if a user setup a NodePort type svc
apiVersion: gateway.networking.k8s.io/v1
kind: GatewayClass
metadata:
  name: eg
spec:
  controllerName: gateway.envoyproxy.io/gatewayclass-controller
  parametersRef:
    group: gateway.envoyproxy.io
    kind: EnvoyProxy
    name: custom-proxy-config
    namespace: envoy-gateway-system
---

apiVersion: gateway.envoyproxy.io/v1alpha1
kind: EnvoyProxy
metadata:
  name: custom-proxy-config
  namespace: envoy-gateway-system
spec:
  provider:
    type: Kubernetes
    kubernetes:
      envoyService:
        type: NodePort  
  1. And created a Gateway
apiVersion: gateway.networking.k8s.io/v1
kind: Gateway
metadata:
  name: eg
spec:
  gatewayClassName: eg
  listeners:
    - name: http
      protocol: HTTP
      port: 35342

then the generated Envoy service would have a spec that looks like

apiVersion: v1
kind: Service
metadata:
  labels:
    app.kubernetes.io/name: envoy
    app.kubernetes.io/component: proxy
    app.kubernetes.io/managed-by: envoy-gateway
    gateway.envoyproxy.io/owning-gateway-name: default
    gateway.envoyproxy.io/owning-gateway-namespace: default
  name: envoy-default-37a8eec1
  namespace: envoy-gateway-system
spec:
  ports:
    - name: envoy-........
      nodePort: 35342
      port: 35342
      protocol: TCP
      targetPort: 35342
  selector:
    app.kubernetes.io/name: envoy
    app.kubernetes.io/component: proxy
    app.kubernetes.io/managed-by: envoy-gateway
    gateway.envoyproxy.io/owning-gateway-name: eg
    gateway.envoyproxy.io/owning-gateway-namespace: default
  sessionAffinity: None
  type: NodePort

@tamalsaha
Copy link
Contributor

I see. Here the svc port and node port are same. I don't know if that will always be a preferred option especially for HTTP/S type workloads.

@tommie
Copy link
Contributor

tommie commented Nov 22, 2023

In my case, I just want to set up a small k8s from nothing, while being able to scale it seamlessly. For that, MetalLB is way overkill: just start Envoy on a couple of nodes, and have a DNS controller that reads the Envoy services and updates records. Since I don't run a dedicated LB, there's no point in adding yet another indirection, because the traffic has already arrived at my cluster. (Hence my original aversion against requiring MetalLB rather than supporting NodePort natively.)

Is this change of interpreting Gateway listener port as managed service Nodeport (and setting the service port same as node port or some other auto gen val, which becomes an internal implementation detail), something that makes sense for your deployments ?

(I feel I'm missing some context and, uhm, knowledge, to answer. It's been a while since I was playing with Envoy. Got stuck on an unrelated task.)

Yes, I think your example makes sense to support using Envoy as the cluster LB, though I don't remember how that differs from now. (IIRC, this part of EG ended up working well for me, but haven't run the test code since summer.)

@arkodg
Copy link
Contributor

arkodg commented Feb 29, 2024

Users should be able to now specify an explicit nodePort using #2719, closing this one

@arkodg arkodg closed this as completed Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/infra-mgr Issues related to the provisioner used for provisioning the managed Envoy Proxy fleet. help wanted Extra attention is needed kind/enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants