diff --git a/pkg/proxy/topology.go b/pkg/proxy/topology.go index 76a3dff328a..2a514b0816f 100644 --- a/pkg/proxy/topology.go +++ b/pkg/proxy/topology.go @@ -26,8 +26,8 @@ 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 @@ -35,6 +35,10 @@ import ( // // - 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() { diff --git a/pkg/proxy/topology_test.go b/pkg/proxy/topology_test.go index 276ba9cb06d..96d0cd549fd 100644 --- a/pkg/proxy/topology_test.go +++ b/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},