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

skip iptables sync if no endpoint changes #41223

Merged
merged 1 commit into from Feb 10, 2017
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 15 additions & 8 deletions pkg/proxy/iptables/proxier.go
Expand Up @@ -591,6 +591,7 @@ func (proxier *Proxier) OnEndpointsUpdate(allEndpoints []api.Endpoints) {
activeEndpoints := make(map[proxy.ServicePortName]bool) // use a map as a set
staleConnections := make(map[endpointServicePair]bool)
svcPortToInfoMap := make(map[proxy.ServicePortName][]hostPortInfo)
newEndpointsMap := make(map[proxy.ServicePortName][]*endpointsInfo)

// Update endpoints for services.
for i := range allEndpoints {
Expand Down Expand Up @@ -627,32 +628,38 @@ func (proxier *Proxier) OnEndpointsUpdate(allEndpoints []api.Endpoints) {
// Flatten the list of current endpoint infos to just a list of ips as strings
curEndpointIPs := flattenEndpointsInfo(curEndpoints)
if len(curEndpointIPs) != len(newEndpoints) || !slicesEquiv(slice.CopyStrings(curEndpointIPs), newEndpoints) {
glog.V(3).Infof("Setting endpoints for %q to %+v", svcPort, newEndpoints)
// Gather stale connections to removed endpoints
removedEndpoints := getRemovedEndpoints(curEndpointIPs, newEndpoints)
for _, ep := range removedEndpoints {
staleConnections[endpointServicePair{endpoint: ep, servicePortName: svcPort}] = true
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment: The only significant diff is that we were only doing the work to buildEndpointInfoList lazily, but that function only seems as expensive as flattenEndpointsInfo anyway (O(endpoints)), so I don't think we're burning any more cpu.

glog.V(3).Infof("Setting endpoints for %q to %+v", svcPort, newEndpoints)
// Once the set operations using the list of ips are complete, build the list of endpoint infos
proxier.endpointsMap[svcPort] = proxier.buildEndpointInfoList(portsToEndpoints[portname], newEndpoints)
}
// Once the set operations using the list of ips are complete, build the list of endpoint infos
newEndpointsMap[svcPort] = proxier.buildEndpointInfoList(portsToEndpoints[portname], newEndpoints)
activeEndpoints[svcPort] = true
}
}
// Remove endpoints missing from the update.
// Check stale connections against endpoints missing from the update.
for svcPort := range proxier.endpointsMap {
if !activeEndpoints[svcPort] {
glog.V(2).Infof("Removing endpoints for %q", svcPort)
// record endpoints of unactive service to stale connections
for _, ep := range proxier.endpointsMap[svcPort] {
staleConnections[endpointServicePair{endpoint: ep.ip, servicePortName: svcPort}] = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment: this version seems cleaner since we're not deleting from the map we're iterating, and there's no residual state as we start with a fresh map everytime.

}

glog.V(2).Infof("Removing endpoints for %q", svcPort)
delete(proxier.endpointsMap, svcPort)
}

proxier.updateHealthCheckEntries(svcPort.NamespacedName, svcPortToInfoMap[svcPort])
}
proxier.syncProxyRules()

if len(newEndpointsMap) != len(proxier.endpointsMap) || !reflect.DeepEqual(newEndpointsMap, proxier.endpointsMap) {
proxier.endpointsMap = newEndpointsMap
proxier.syncProxyRules()
} else {
glog.V(4).Infof("Skipping proxy iptables rule sync on endpoint update because nothing changed")
}

proxier.deleteEndpointConnections(staleConnections)
}

Expand Down