allow exposing gateway on nodeport service - status updates #1392
Conversation
Signed-off-by: Shubham Chauhan <[email protected]>
Signed-off-by: Shubham Chauhan <[email protected]>
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. |
| @@ -50,6 +50,10 @@ func UpdateGatewayStatusProgrammedCondition(gw *gwapiv1b1.Gateway, svc *corev1.S | |||
| } | |||
| } | |||
|
|
|||
| 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 <[email protected]>
| @@ -8,6 +8,7 @@ rules: | |||
| - 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 <[email protected]>
| @@ -74,7 +74,9 @@ func validateServiceType(spec *EnvoyProxySpec) []error { | |||
| var errs []error | |||
| 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
qicz
added
kind/enhancement
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