Minor updates to kube-proxy topology code

Clarify the comments around terminating endpoints.

Remove stale references to the ProxyTerminatingEndpoints feature gate
in the unit tests.

Author: danwinship

pkg/proxy/topology.go

@@ -26,15 +26,19 @@ import (
 //   - The service's usable Cluster-traffic-policy endpoints (taking topology into account, if
 //     relevant). This will be nil if the service does not ever use Cluster traffic policy.
 //
-//   - The service's usable Local-traffic-policy endpoints (including terminating endpoints, if
-//     relevant). This will be nil if the service does not ever use Local traffic policy.
+//   - The service's usable Local-traffic-policy endpoints. This will be nil if the
+//     service does not ever use Local traffic policy.
 //
 //   - The combined list of all endpoints reachable from this node (which is the union of the
 //     previous two lists, but in the case where it is identical to one or the other, we avoid
 //     allocating a separate list).
 //
 //   - An indication of whether the service has any endpoints reachable from anywhere in the
 //     cluster. (This may be true even if allReachableEndpoints is empty.)
+//
+// "Usable endpoints" means Ready endpoints by default, but will fall back to
+// Serving-Terminating endpoints (independently for Cluster and Local) if no Ready
+// endpoints are available.
 func CategorizeEndpoints(endpoints []Endpoint, svcInfo ServicePort, nodeLabels map[string]string) (clusterEndpoints, localEndpoints, allReachableEndpoints []Endpoint, hasAnyEndpoints bool) {
 	var useTopology, useServingTerminatingEndpoints bool
 
@@ -50,9 +54,10 @@ func CategorizeEndpoints(endpoints []Endpoint, svcInfo ServicePort, nodeLabels m
 			return true
 		})
 
-		// if there are 0 cluster-wide endpoints, we can try to fallback to any terminating endpoints that are ready.
-		// When falling back to terminating endpoints, we do NOT consider topology aware routing since this is a best
-		// effort attempt to avoid dropping connections.
+		// If we didn't get any endpoints, try again using terminating endpoints.
+		// (Note that we would already have chosen to ignore topology if there
+		// were no ready endpoints for the given topology, so the problem at this
+		// point must be that there are no ready endpoints anywhere.)
 		if len(clusterEndpoints) == 0 {
 			clusterEndpoints = filterEndpoints(endpoints, func(ep Endpoint) bool {
 				if ep.IsServing() && ep.IsTerminating() {

pkg/proxy/topology_test.go

@@ -45,7 +45,6 @@ func checkExpectedEndpoints(expected sets.Set[string], actual []Endpoint) error
 func TestCategorizeEndpoints(t *testing.T) {
 	testCases := []struct {
 		name        string
-		pteEnabled  bool
 		nodeLabels  map[string]string
 		serviceInfo ServicePort
 		endpoints   []Endpoint
@@ -240,7 +239,6 @@ func TestCategorizeEndpoints(t *testing.T) {
 		localEndpoints:   nil,
 	}, {
 		name:        "Cluster traffic policy, all endpoints are terminating",
-		pteEnabled:  true,
 		serviceInfo: &BaseServicePortInfo{},
 		endpoints: []Endpoint{
 			&BaseEndpointInfo{endpoint: "10.0.0.0:80", ready: false, serving: true, terminating: true, isLocal: true},
@@ -290,7 +288,6 @@ func TestCategorizeEndpoints(t *testing.T) {
 		allEndpoints:     sets.New[string]("10.0.0.0:80", "10.0.0.1:80"),
 	}, {
 		name:        "iTP: Local, eTP: Local, all endpoints remote and terminating",
-		pteEnabled:  true,
 		serviceInfo: &BaseServicePortInfo{internalPolicyLocal: true, externalPolicyLocal: true, nodePort: 8080},
 		endpoints: []Endpoint{
 			&BaseEndpointInfo{endpoint: "10.0.0.0:80", ready: false, serving: true, terminating: true, isLocal: false},
@@ -302,7 +299,6 @@ func TestCategorizeEndpoints(t *testing.T) {
 		onlyRemoteEndpoints: true,
 	}, {
 		name:        "iTP: Cluster, eTP: Local, with terminating endpoints",
-		pteEnabled:  true,
 		serviceInfo: &BaseServicePortInfo{internalPolicyLocal: false, externalPolicyLocal: true, nodePort: 8080},
 		endpoints: []Endpoint{
 			&BaseEndpointInfo{endpoint: "10.0.0.0:80", ready: true, isLocal: false},