restrict externalTrafficPolicy=Local interpretation only to NodePort and LoadBalancer services #819
Conversation
There was a problem hiding this comment.
When I tested this in our cluster I found that while the IPVS endpoints were getting created on the node after this change, they weren't populated with any endpoints (I commented in the code where I think the issue with the endpoints is coming from). I think that where we want to try to get to is:
service.local service on a node WITH an endpoint:
NodeIP (if exists): Is configured
ExternalIP (if exists): Is configured
ClusterIP: Is configured
IPVS Routing: Configured with ONLY pod endpoints that are local to the host
service.local service on a node WITHOUT an endpoint:
NodeIP (if exists): Not configured (external traffic should not be ingressing on this node)
ExternalIP (if exists): Not configured (external traffic should not be ingressing on this node)
ClusterIP: Not BGP announced (internal traffic that is trying to route natively should not be ingressing on this node)
IPVS Routing: Configured with ALL pods endpoints of the service (so non-BGP announced cluster IP traffic can route from this node)
| @@ -910,6 +905,11 @@ func (nsc *NetworkServicesController) syncIpvsServices(serviceInfoMap serviceInf | |||
| var clusterServiceId = generateIpPortId(svc.clusterIP.String(), svc.protocol, strconv.Itoa(svc.port)) | |||
| activeServiceEndpointMap[clusterServiceId] = make([]string, 0) | |||
|
|
|||
| if svc.local && !hasActiveEndpoints(svc, endpoints) { | |||
| glog.V(1).Infof("Skipping NodePort/LoadBalancer service %s/%s as it does not have active endpoints\n", svc.namespace, svc.name) | |||
| continue | |||
There was a problem hiding this comment.
I think that the problem is that if you continue here, then you end up with an IPVS service without any endpoints in it as endpoints are added below in the other section you modified.
So you need to make it to the section below, but you don't want to run anything imbetween. Probably the easiest way would be to abstract the nodeport and externalIP stuff into their own methods, then gate them by a variation of this if statement.
murali-reddy
force-pushed
the
exttrafficpolicy
branch
from
b12e497
to
a6bd111
Compare
|
@aauren thanks for the review. I have addressed the issue you raised. I have also taken this opportunity to restructure the sync logic to be more modular PTAL |
|
Accidentally merge and closed this PR. I have opened new PR #836 |
clusterIPserviceFixes #818