Skip to content

Commit

Permalink
For review - 2
Browse files Browse the repository at this point in the history
  • Loading branch information
hongliangl committed Apr 28, 2021
1 parent 6b28de5 commit bc95f63
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 21 deletions.
2 changes: 1 addition & 1 deletion pkg/agent/proxy/endpoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func (t *endpointsChangesTracker) OnEndpointUpdate(previous, current *corev1.End
return len(t.changes) > 0
}

// EndpointSliceUpdate updates the given service's endpoints change map based on the <previous, current> endpoints pair.
// OnEndpointSliceUpdate updates the given service's endpoints change map based on the <previous, current> endpoints pair.
// It returns true if items changed, otherwise it returns false. Will add/update/delete items of endpointsChange Map.
// If removeSlice is true, slice will be removed, otherwise it will be added or updated.
func (t *endpointsChangesTracker) OnEndpointSliceUpdate(endpointSlice *discovery.EndpointSlice, removeSlice bool) bool {
Expand Down
40 changes: 20 additions & 20 deletions pkg/agent/proxy/proxier.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,38 +269,38 @@ func (p *proxier) installServices() {
}

var endpointUpdateList []k8sproxy.Endpoint
for _, endpoint := range endpoints { // Check if there is any installed Endpoint which is not expected anymore.
if _, ok := endpointsInstalled[endpoint.String()]; !ok { // There is an expected Endpoint which is not installed.
needUpdateEndpoints = true
}
endpointUpdateList = append(endpointUpdateList, endpoint)
}

// If the length of endpointUpdateList > maxEndpoints, endpointUpdateList should be cut. However, the iteration
// of map in Golang is random, so endpointUpdateList are not always in the same order. If endpointUpdateList is
// cut directly without any sorting, some Endpoints may not be installed on the cut endpointUpdateList(Last sync,
// these Endpoints may be cut off from endpointUpdateList, they are of course not installed.) So cutting
// endpointUpdateList after sorting can avoid this situation in some degree.
if len(endpointUpdateList) > maxEndpoints {
if len(endpoints) > maxEndpoints {
if !p.oversizeServiceSet.Has(svcPortName.String()) {
klog.Warningf("Since Endpoints of Service %s exceeds %d, extra Endpoints will be dropped", svcPortName.String(), maxEndpoints)
p.oversizeServiceSet.Insert(svcPortName.String())
}
// If the length of endpoints > maxEndpoints, endpoints should be cut. However, endpoints is a map. Therefore,
// iterate the map and append every Endpoint to a slice endpointList. Since the iteration order of map in
// Golang is random, if cut directly without any sorting, some Endpoints may not be installed. So cutting
// slice endpointList after sorting can avoid this situation in some degree.
var endpointList []k8sproxy.Endpoint
for _, endpoint := range endpoints {
endpointList = append(endpointList, endpoint)
}
sort.Sort(byEndpoint(endpointList))
endpointList = endpointList[:maxEndpoints]

needUpdateEndpoints = false
sort.Sort(byEndpoint(endpointUpdateList))
endpointUpdateList = endpointUpdateList[:maxEndpoints]
// Check if the cut endpointUpdateList are all installed.
for _, endpoint := range endpointUpdateList {
if _, ok := endpointsInstalled[endpoint.String()]; !ok {
for _, endpoint := range endpointList { // Check if there is any installed Endpoint which is not expected anymore.
if _, ok := endpointsInstalled[endpoint.String()]; !ok { // There is an expected Endpoint which is not installed.
needUpdateEndpoints = true
break
}
endpointUpdateList = append(endpointUpdateList, endpoint)
}
} else {
if p.oversizeServiceSet.Has(svcPortName.String()) {
p.oversizeServiceSet.Delete(svcPortName.String())
}
for _, endpoint := range endpoints { // Check if there is any installed Endpoint which is not expected anymore.
if _, ok := endpointsInstalled[endpoint.String()]; !ok { // There is an expected Endpoint which is not installed.
needUpdateEndpoints = true
}
endpointUpdateList = append(endpointUpdateList, endpoint)
}
}

if len(endpoints) < len(endpointsInstalled) { // There are Endpoints which expired.
Expand Down

0 comments on commit bc95f63

Please sign in to comment.