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.
This commit is contained in:
Dan Winship 2025-03-12 12:25:50 -04:00
parent 19952a2b7b
commit ff640c3679
2 changed files with 10 additions and 9 deletions

View File

@ -26,8 +26,8 @@ import (
// - The service's usable Cluster-traffic-policy endpoints (taking topology into account, if // - 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. // 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 // - The service's usable Local-traffic-policy endpoints. This will be nil if the
// relevant). This will be nil if the service does not ever use Local traffic policy. // service does not ever use Local traffic policy.
// //
// - The combined list of all endpoints reachable from this node (which is the union of the // - 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 // 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 // - An indication of whether the service has any endpoints reachable from anywhere in the
// cluster. (This may be true even if allReachableEndpoints is empty.) // 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) { func CategorizeEndpoints(endpoints []Endpoint, svcInfo ServicePort, nodeLabels map[string]string) (clusterEndpoints, localEndpoints, allReachableEndpoints []Endpoint, hasAnyEndpoints bool) {
var useTopology, useServingTerminatingEndpoints bool var useTopology, useServingTerminatingEndpoints bool
@ -50,9 +54,10 @@ func CategorizeEndpoints(endpoints []Endpoint, svcInfo ServicePort, nodeLabels m
return true return true
}) })
// if there are 0 cluster-wide endpoints, we can try to fallback to any terminating endpoints that are ready. // If we didn't get any endpoints, try again using terminating endpoints.
// When falling back to terminating endpoints, we do NOT consider topology aware routing since this is a best // (Note that we would already have chosen to ignore topology if there
// effort attempt to avoid dropping connections. // 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 { if len(clusterEndpoints) == 0 {
clusterEndpoints = filterEndpoints(endpoints, func(ep Endpoint) bool { clusterEndpoints = filterEndpoints(endpoints, func(ep Endpoint) bool {
if ep.IsServing() && ep.IsTerminating() { if ep.IsServing() && ep.IsTerminating() {

View File

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