Update some change tracker doc comments
In particular, fix the description of ServiceChangeTracker.Update's return value, and point out that it's different from EndpointsChangeTracker.EndpointSliceUpdate's (though fortunately, in a way that doesn't matter for any existing code).
This commit is contained in:
		| @@ -40,23 +40,25 @@ type EndpointsChangeTracker struct { | |||||||
| 	// lock protects lastChangeTriggerTimes | 	// lock protects lastChangeTriggerTimes | ||||||
| 	lock sync.Mutex | 	lock sync.Mutex | ||||||
|  |  | ||||||
|  | 	// processEndpointsMapChange is invoked by the apply function on every change. | ||||||
|  | 	// This function should not modify the EndpointsMaps, but just use the changes for | ||||||
|  | 	// any Proxier-specific cleanup. | ||||||
| 	processEndpointsMapChange processEndpointsMapChangeFunc | 	processEndpointsMapChange processEndpointsMapChangeFunc | ||||||
|  |  | ||||||
| 	// endpointSliceCache holds a simplified version of endpoint slices. | 	// endpointSliceCache holds a simplified version of endpoint slices. | ||||||
| 	endpointSliceCache *EndpointSliceCache | 	endpointSliceCache *EndpointSliceCache | ||||||
| 	// Map from the Endpoints namespaced-name to the times of the triggers that caused the endpoints |  | ||||||
| 	// object to change. Used to calculate the network-programming-latency. | 	// lastChangeTriggerTimes maps from the Service's NamespacedName to the times of | ||||||
|  | 	// the triggers that caused its EndpointSlice objects to change. Used to calculate | ||||||
|  | 	// the network-programming-latency metric. | ||||||
| 	lastChangeTriggerTimes map[types.NamespacedName][]time.Time | 	lastChangeTriggerTimes map[types.NamespacedName][]time.Time | ||||||
| 	// record the time when the endpointsChangeTracker was created so we can ignore the endpoints | 	// trackerStartTime is the time when the EndpointsChangeTracker was created, so | ||||||
| 	// that were generated before, because we can't estimate the network-programming-latency on those. | 	// we can avoid generating network-programming-latency metrics for changes that | ||||||
| 	// This is specially problematic on restarts, because we process all the endpoints that may have been | 	// occurred before that. | ||||||
| 	// created hours or days before. |  | ||||||
| 	trackerStartTime time.Time | 	trackerStartTime time.Time | ||||||
| } | } | ||||||
|  |  | ||||||
| type makeEndpointFunc func(info *BaseEndpointInfo, svcPortName *ServicePortName) Endpoint | type makeEndpointFunc func(info *BaseEndpointInfo, svcPortName *ServicePortName) Endpoint | ||||||
|  |  | ||||||
| // This handler is invoked by the apply function on every change. This function should not modify the |  | ||||||
| // EndpointsMap's but just use the changes for any Proxier specific cleanup. |  | ||||||
| type processEndpointsMapChangeFunc func(oldEndpointsMap, newEndpointsMap EndpointsMap) | type processEndpointsMapChangeFunc func(oldEndpointsMap, newEndpointsMap EndpointsMap) | ||||||
|  |  | ||||||
| // NewEndpointsChangeTracker initializes an EndpointsChangeTracker | // NewEndpointsChangeTracker initializes an EndpointsChangeTracker | ||||||
| @@ -69,9 +71,10 @@ func NewEndpointsChangeTracker(hostname string, makeEndpointInfo makeEndpointFun | |||||||
| 	} | 	} | ||||||
| } | } | ||||||
|  |  | ||||||
| // EndpointSliceUpdate updates given service's endpoints change map based on the <previous, current> endpoints pair. | // EndpointSliceUpdate updates the EndpointsChangeTracker by adding/updating or removing | ||||||
| // It returns true if items changed, otherwise return false. Will add/update/delete items of EndpointsChangeTracker. | // endpointSlice (depending on removeSlice). It returns true if this update contained a | ||||||
| // If removeSlice is true, slice will be removed, otherwise it will be added or updated. | // change that needs to be synced; note that this is different from the return value of | ||||||
|  | // ServiceChangeTracker.Update(). | ||||||
| func (ect *EndpointsChangeTracker) EndpointSliceUpdate(endpointSlice *discovery.EndpointSlice, removeSlice bool) bool { | func (ect *EndpointsChangeTracker) EndpointSliceUpdate(endpointSlice *discovery.EndpointSlice, removeSlice bool) bool { | ||||||
| 	if !supportedEndpointSliceAddressTypes.Has(string(endpointSlice.AddressType)) { | 	if !supportedEndpointSliceAddressTypes.Has(string(endpointSlice.AddressType)) { | ||||||
| 		klog.V(4).InfoS("EndpointSlice address type not supported by kube-proxy", "addressType", endpointSlice.AddressType) | 		klog.V(4).InfoS("EndpointSlice address type not supported by kube-proxy", "addressType", endpointSlice.AddressType) | ||||||
|   | |||||||
| @@ -36,18 +36,20 @@ type ServiceChangeTracker struct { | |||||||
| 	lock sync.Mutex | 	lock sync.Mutex | ||||||
| 	// items maps a service to its serviceChange. | 	// items maps a service to its serviceChange. | ||||||
| 	items map[types.NamespacedName]*serviceChange | 	items map[types.NamespacedName]*serviceChange | ||||||
| 	// makeServiceInfo allows proxier to inject customized information when processing service. |  | ||||||
| 	makeServiceInfo         makeServicePortFunc |  | ||||||
| 	processServiceMapChange processServiceMapChangeFunc |  | ||||||
| 	ipFamily                v1.IPFamily |  | ||||||
|  |  | ||||||
|  | 	// makeServiceInfo allows the proxier to inject customized information when | ||||||
|  | 	// processing services. | ||||||
|  | 	makeServiceInfo makeServicePortFunc | ||||||
|  | 	// processServiceMapChange is invoked by the apply function on every change. This | ||||||
|  | 	// function should not modify the ServicePortMaps, but just use the changes for | ||||||
|  | 	// any Proxier-specific cleanup. | ||||||
|  | 	processServiceMapChange processServiceMapChangeFunc | ||||||
|  |  | ||||||
|  | 	ipFamily v1.IPFamily | ||||||
| 	recorder events.EventRecorder | 	recorder events.EventRecorder | ||||||
| } | } | ||||||
|  |  | ||||||
| type makeServicePortFunc func(*v1.ServicePort, *v1.Service, *BaseServicePortInfo) ServicePort | type makeServicePortFunc func(*v1.ServicePort, *v1.Service, *BaseServicePortInfo) ServicePort | ||||||
|  |  | ||||||
| // This handler is invoked by the apply function on every change. This function should not modify the |  | ||||||
| // ServicePortMap's but just use the changes for any Proxier specific cleanup. |  | ||||||
| type processServiceMapChangeFunc func(previous, current ServicePortMap) | type processServiceMapChangeFunc func(previous, current ServicePortMap) | ||||||
|  |  | ||||||
| // serviceChange contains all changes to services that happened since proxy rules were synced.  For a single object, | // serviceChange contains all changes to services that happened since proxy rules were synced.  For a single object, | ||||||
| @@ -69,16 +71,11 @@ func NewServiceChangeTracker(makeServiceInfo makeServicePortFunc, ipFamily v1.IP | |||||||
| 	} | 	} | ||||||
| } | } | ||||||
|  |  | ||||||
| // Update updates given service's change map based on the <previous, current> service pair.  It returns true if items changed, | // Update updates the ServiceChangeTracker based on the <previous, current> service pair | ||||||
| // otherwise return false.  Update can be used to add/update/delete items of ServiceChangeMap.  For example, | // (where either previous or current, but not both, can be nil). It returns true if sct | ||||||
| // Add item | // contains changes that need to be synced (whether or not those changes were caused by | ||||||
| //   - pass <nil, service> as the <previous, current> pair. | // this update); note that this is different from the return value of | ||||||
| // | // EndpointChangeTracker.EndpointSliceUpdate(). | ||||||
| // Update item |  | ||||||
| //   - pass <oldService, service> as the <previous, current> pair. |  | ||||||
| // |  | ||||||
| // Delete item |  | ||||||
| //   - pass <service, nil> as the <previous, current> pair. |  | ||||||
| func (sct *ServiceChangeTracker) Update(previous, current *v1.Service) bool { | func (sct *ServiceChangeTracker) Update(previous, current *v1.Service) bool { | ||||||
| 	// This is unexpected, we should return false directly. | 	// This is unexpected, we should return false directly. | ||||||
| 	if previous == nil && current == nil { | 	if previous == nil && current == nil { | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Dan Winship
					Dan Winship