allow exposing gateway on nodeport service - status updates#1392
allow exposing gateway on nodeport service - status updates#1392zirain merged 4 commits intoenvoyproxy:mainfrom
Conversation
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
Codecov Report
@@ Coverage Diff @@
## main #1392 +/- ##
==========================================
- Coverage 62.40% 62.39% -0.01%
==========================================
Files 78 79 +1
Lines 10953 11033 +80
==========================================
+ Hits 6835 6884 +49
- Misses 3667 3696 +29
- Partials 451 453 +2
|
internal/provider/utils/store.go
Outdated
| internalIP = addr.Address | ||
| } | ||
| } | ||
| if externalIP != "" { |
There was a problem hiding this comment.
maybe add a comment and rename nodeDetails.externalIP to something else, since we are assigning the NodeInternalIP to it
There was a problem hiding this comment.
yea, specifically - in my kind cluster there was no externalIP on the node obj, only internalIP. So I converted internal to external.
There was a problem hiding this comment.
yeah so if we can convert that assumption - use internalIP when external is empty, which might be routable from client to the gateway into a comment, would be easy to understand why this logic was added
and should be field be called nodeIP or just address instead ?
|
I'll add more tests if we're good with the general implementation of this. |
| } | ||
|
|
||
| if svc.Spec.Type == corev1.ServiceTypeNodePort { | ||
| addresses = nodeAddresses |
There was a problem hiding this comment.
should we also append the NodePort to this address, and expose it in the status ?
There was a problem hiding this comment.
looks like we cannot expose the port in a clean way, because there will be a unique node port per listener port
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
| - apiGroups: | ||
| - "" | ||
| resources: | ||
| - nodes |
There was a problem hiding this comment.
geo-check is failing because this also need to be part of rbac.go
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
| if spec.Provider.Kubernetes != nil && spec.Provider.Kubernetes.EnvoyService != nil { | ||
| if serviceType := spec.Provider.Kubernetes.EnvoyService.Type; serviceType != nil { | ||
| if *serviceType != ServiceTypeLoadBalancer && *serviceType != ServiceTypeClusterIP { | ||
| if *serviceType != ServiceTypeLoadBalancer && |
There was a problem hiding this comment.
can you simply this lines with a function in a followup
What this PR does / why we need it:
This PR allows for users to expose Gateways on a service of type NodePort.
If the service of type NodePort, the exposed Gateway gets the addresses of the Nodes, in the Gateway status section.
In any case a Node is created/updated/deleted, the Gateway addresses status must be updated, hence the additional watch and cache logic.
Resolves #1378