Change kube-proxy behaviour to consider hints when ServiceTrafficDistribution feature gate is enabled
This commit is contained in:
		@@ -137,22 +137,28 @@ func CategorizeEndpoints(endpoints []Endpoint, svcInfo ServicePort, nodeLabels m
 | 
				
			|||||||
	return
 | 
						return
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
// canUseTopology returns true if topology aware routing is enabled and properly configured
 | 
					// canUseTopology returns true if topology aware routing is enabled and properly
 | 
				
			||||||
// in this cluster. That is, it checks that:
 | 
					// configured in this cluster. That is, it checks that:
 | 
				
			||||||
// * The TopologyAwareHints feature is enabled
 | 
					//   - The TopologyAwareHints or ServiceTrafficDistribution feature is enabled.
 | 
				
			||||||
// * The "service.kubernetes.io/topology-aware-hints" annotation on this Service is set to "Auto"
 | 
					//   - If ServiceTrafficDistribution feature gate is not enabled, then the
 | 
				
			||||||
// * The node's labels include "topology.kubernetes.io/zone"
 | 
					//     hintsAnnotation should represent an enabled value.
 | 
				
			||||||
// * All of the endpoints for this Service have a topology hint
 | 
					//   - The node's labels include "topology.kubernetes.io/zone".
 | 
				
			||||||
// * At least one endpoint for this Service is hinted for this node's zone.
 | 
					//   - All of the endpoints for this Service have a topology hint.
 | 
				
			||||||
 | 
					//   - At least one endpoint for this Service is hinted for this node's zone.
 | 
				
			||||||
func canUseTopology(endpoints []Endpoint, svcInfo ServicePort, nodeLabels map[string]string) bool {
 | 
					func canUseTopology(endpoints []Endpoint, svcInfo ServicePort, nodeLabels map[string]string) bool {
 | 
				
			||||||
	if !utilfeature.DefaultFeatureGate.Enabled(features.TopologyAwareHints) {
 | 
						if !utilfeature.DefaultFeatureGate.Enabled(features.TopologyAwareHints) && !utilfeature.DefaultFeatureGate.Enabled(features.ServiceTrafficDistribution) {
 | 
				
			||||||
		return false
 | 
							return false
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
	// Any non-empty and non-disabled values for the hints annotation are acceptable.
 | 
					
 | 
				
			||||||
 | 
						// Ignore value of hintsAnnotation if the ServiceTrafficDistribution feature
 | 
				
			||||||
 | 
						// gate is enabled.
 | 
				
			||||||
 | 
						if !utilfeature.DefaultFeatureGate.Enabled(features.ServiceTrafficDistribution) {
 | 
				
			||||||
 | 
							// If the hintsAnnotation has a disabled value, we do not consider hints for route programming.
 | 
				
			||||||
		hintsAnnotation := svcInfo.HintsAnnotation()
 | 
							hintsAnnotation := svcInfo.HintsAnnotation()
 | 
				
			||||||
		if hintsAnnotation == "" || hintsAnnotation == "disabled" || hintsAnnotation == "Disabled" {
 | 
							if hintsAnnotation == "" || hintsAnnotation == "disabled" || hintsAnnotation == "Disabled" {
 | 
				
			||||||
			return false
 | 
								return false
 | 
				
			||||||
		}
 | 
							}
 | 
				
			||||||
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	zone, ok := nodeLabels[v1.LabelTopologyZone]
 | 
						zone, ok := nodeLabels[v1.LabelTopologyZone]
 | 
				
			||||||
	if !ok || zone == "" {
 | 
						if !ok || zone == "" {
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -49,6 +49,7 @@ func TestCategorizeEndpoints(t *testing.T) {
 | 
				
			|||||||
	testCases := []struct {
 | 
						testCases := []struct {
 | 
				
			||||||
		name                      string
 | 
							name                      string
 | 
				
			||||||
		hintsEnabled              bool
 | 
							hintsEnabled              bool
 | 
				
			||||||
 | 
							trafficDistFeatureEnabled bool
 | 
				
			||||||
		pteEnabled                bool
 | 
							pteEnabled                bool
 | 
				
			||||||
		nodeLabels                map[string]string
 | 
							nodeLabels                map[string]string
 | 
				
			||||||
		serviceInfo               ServicePort
 | 
							serviceInfo               ServicePort
 | 
				
			||||||
@@ -130,9 +131,38 @@ func TestCategorizeEndpoints(t *testing.T) {
 | 
				
			|||||||
		},
 | 
							},
 | 
				
			||||||
		clusterEndpoints: sets.New[string]("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"),
 | 
							clusterEndpoints: sets.New[string]("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"),
 | 
				
			||||||
		localEndpoints:   nil,
 | 
							localEndpoints:   nil,
 | 
				
			||||||
 | 
						}, {
 | 
				
			||||||
 | 
							name:                      "hints, hints annotation empty but trafficDist feature gate enabled, hints are not ignored",
 | 
				
			||||||
 | 
							hintsEnabled:              true,
 | 
				
			||||||
 | 
							trafficDistFeatureEnabled: true,
 | 
				
			||||||
 | 
							nodeLabels:                map[string]string{v1.LabelTopologyZone: "zone-a"},
 | 
				
			||||||
 | 
							serviceInfo:               &BaseServicePortInfo{},
 | 
				
			||||||
 | 
							endpoints: []Endpoint{
 | 
				
			||||||
 | 
								&BaseEndpointInfo{endpoint: "10.1.2.3:80", zoneHints: sets.New[string]("zone-a"), ready: true},
 | 
				
			||||||
 | 
								&BaseEndpointInfo{endpoint: "10.1.2.4:80", zoneHints: sets.New[string]("zone-b"), ready: true},
 | 
				
			||||||
 | 
								&BaseEndpointInfo{endpoint: "10.1.2.5:80", zoneHints: sets.New[string]("zone-c"), ready: true},
 | 
				
			||||||
 | 
								&BaseEndpointInfo{endpoint: "10.1.2.6:80", zoneHints: sets.New[string]("zone-a"), ready: true},
 | 
				
			||||||
 | 
							},
 | 
				
			||||||
 | 
							clusterEndpoints: sets.New[string]("10.1.2.3:80", "10.1.2.6:80"),
 | 
				
			||||||
 | 
							localEndpoints:   nil,
 | 
				
			||||||
 | 
						}, {
 | 
				
			||||||
 | 
							name:                      "hints disabled, trafficDist feature gate enabled, hints are not ignored",
 | 
				
			||||||
 | 
							hintsEnabled:              false,
 | 
				
			||||||
 | 
							trafficDistFeatureEnabled: true,
 | 
				
			||||||
 | 
							nodeLabels:                map[string]string{v1.LabelTopologyZone: "zone-a"},
 | 
				
			||||||
 | 
							serviceInfo:               &BaseServicePortInfo{},
 | 
				
			||||||
 | 
							endpoints: []Endpoint{
 | 
				
			||||||
 | 
								&BaseEndpointInfo{endpoint: "10.1.2.3:80", zoneHints: sets.New[string]("zone-a"), ready: true},
 | 
				
			||||||
 | 
								&BaseEndpointInfo{endpoint: "10.1.2.4:80", zoneHints: sets.New[string]("zone-b"), ready: true},
 | 
				
			||||||
 | 
								&BaseEndpointInfo{endpoint: "10.1.2.5:80", zoneHints: sets.New[string]("zone-c"), ready: true},
 | 
				
			||||||
 | 
								&BaseEndpointInfo{endpoint: "10.1.2.6:80", zoneHints: sets.New[string]("zone-a"), ready: true},
 | 
				
			||||||
 | 
							},
 | 
				
			||||||
 | 
							clusterEndpoints: sets.New[string]("10.1.2.3:80", "10.1.2.6:80"),
 | 
				
			||||||
 | 
							localEndpoints:   nil,
 | 
				
			||||||
	}, {
 | 
						}, {
 | 
				
			||||||
		name:                      "externalTrafficPolicy: Local, topology ignored for Local endpoints",
 | 
							name:                      "externalTrafficPolicy: Local, topology ignored for Local endpoints",
 | 
				
			||||||
		hintsEnabled:              true,
 | 
							hintsEnabled:              true,
 | 
				
			||||||
 | 
							trafficDistFeatureEnabled: true,
 | 
				
			||||||
		nodeLabels:                map[string]string{v1.LabelTopologyZone: "zone-a"},
 | 
							nodeLabels:                map[string]string{v1.LabelTopologyZone: "zone-a"},
 | 
				
			||||||
		serviceInfo:               &BaseServicePortInfo{externalPolicyLocal: true, nodePort: 8080, hintsAnnotation: "auto"},
 | 
							serviceInfo:               &BaseServicePortInfo{externalPolicyLocal: true, nodePort: 8080, hintsAnnotation: "auto"},
 | 
				
			||||||
		endpoints: []Endpoint{
 | 
							endpoints: []Endpoint{
 | 
				
			||||||
@@ -147,6 +177,7 @@ func TestCategorizeEndpoints(t *testing.T) {
 | 
				
			|||||||
	}, {
 | 
						}, {
 | 
				
			||||||
		name:                      "internalTrafficPolicy: Local, topology ignored for Local endpoints",
 | 
							name:                      "internalTrafficPolicy: Local, topology ignored for Local endpoints",
 | 
				
			||||||
		hintsEnabled:              true,
 | 
							hintsEnabled:              true,
 | 
				
			||||||
 | 
							trafficDistFeatureEnabled: true,
 | 
				
			||||||
		nodeLabels:                map[string]string{v1.LabelTopologyZone: "zone-a"},
 | 
							nodeLabels:                map[string]string{v1.LabelTopologyZone: "zone-a"},
 | 
				
			||||||
		serviceInfo:               &BaseServicePortInfo{internalPolicyLocal: true, hintsAnnotation: "auto", externalPolicyLocal: false, nodePort: 8080},
 | 
							serviceInfo:               &BaseServicePortInfo{internalPolicyLocal: true, hintsAnnotation: "auto", externalPolicyLocal: false, nodePort: 8080},
 | 
				
			||||||
		endpoints: []Endpoint{
 | 
							endpoints: []Endpoint{
 | 
				
			||||||
@@ -458,6 +489,7 @@ func TestCategorizeEndpoints(t *testing.T) {
 | 
				
			|||||||
	for _, tc := range testCases {
 | 
						for _, tc := range testCases {
 | 
				
			||||||
		t.Run(tc.name, func(t *testing.T) {
 | 
							t.Run(tc.name, func(t *testing.T) {
 | 
				
			||||||
			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.TopologyAwareHints, tc.hintsEnabled)()
 | 
								defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.TopologyAwareHints, tc.hintsEnabled)()
 | 
				
			||||||
 | 
								defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ServiceTrafficDistribution, tc.trafficDistFeatureEnabled)()
 | 
				
			||||||
 | 
					
 | 
				
			||||||
			clusterEndpoints, localEndpoints, allEndpoints, hasAnyEndpoints := CategorizeEndpoints(tc.endpoints, tc.serviceInfo, tc.nodeLabels)
 | 
								clusterEndpoints, localEndpoints, allEndpoints, hasAnyEndpoints := CategorizeEndpoints(tc.endpoints, tc.serviceInfo, tc.nodeLabels)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user