diff --git a/pkg/controllers/proxy/network_services_controller.go b/pkg/controllers/proxy/network_services_controller.go index bca0e30fa..aaf8015fc 100644 --- a/pkg/controllers/proxy/network_services_controller.go +++ b/pkg/controllers/proxy/network_services_controller.go @@ -978,10 +978,11 @@ func (nsc *NetworkServicesController) buildServicesInfo() serviceInfoMap { svcInfo.extTrafficPolicy = &svc.Spec.ExternalTrafficPolicy // The kube-router.io/service.local annotation has the ability to override the internal and external traffic - // policy that is set in the spec. In this case we set both to local when the annotation is true so that - // previous functionality of the annotation is best preserved. + // policy that is set in the spec. Previously, when this was active set both to local when the annotation is + // true so that previous functionality of the annotation is best preserved. However, this has proved to not + // be a good fit for ClusterIP traffic, so we retain cluster for internal traffic policy. if svc.ObjectMeta.Annotations[svcLocalAnnotation] == "true" { - intTrafficPolicyLocal := v1.ServiceInternalTrafficPolicyLocal + intTrafficPolicyLocal := v1.ServiceInternalTrafficPolicyCluster extTrafficPolicyLocal := v1.ServiceExternalTrafficPolicyLocal svcInfo.intTrafficPolicy = &intTrafficPolicyLocal svcInfo.extTrafficPolicy = &extTrafficPolicyLocal diff --git a/pkg/controllers/routing/ecmp_vip.go b/pkg/controllers/routing/ecmp_vip.go index 16f95bce4..2b9b72dcc 100644 --- a/pkg/controllers/routing/ecmp_vip.go +++ b/pkg/controllers/routing/ecmp_vip.go @@ -484,7 +484,7 @@ func (nrc *NetworkRoutingController) shouldAdvertiseService(svc *v1core.Service, // If: // - We are assessing the clusterIP of the service (the internally facing VIP) - // - The service has an internal traffic policy of "local" or the service has the service.local annotation on it + // - The service has an internal traffic policy of "local" // - The service doesn't have any endpoints on the node we're executing on // Then: return false // We handle spec.internalTrafficPolicy different because it was introduced in v1.26 and may not be available in all @@ -493,8 +493,7 @@ func (nrc *NetworkRoutingController) shouldAdvertiseService(svc *v1core.Service, if svc.Spec.InternalTrafficPolicy != nil { serIntTrafPol = *svc.Spec.InternalTrafficPolicy == v1core.ServiceInternalTrafficPolicyLocal } - intLocalPol := (serIntTrafPol || svc.Annotations[svcLocalAnnotation] == "true") - if isClusterIP && intLocalPol && !hasLocalEndpoints { + if isClusterIP && serIntTrafPol && !hasLocalEndpoints { return false, nil } diff --git a/pkg/controllers/routing/ecmp_vip_test.go b/pkg/controllers/routing/ecmp_vip_test.go index 5fa53bb60..e9011a39f 100644 --- a/pkg/controllers/routing/ecmp_vip_test.go +++ b/pkg/controllers/routing/ecmp_vip_test.go @@ -620,8 +620,8 @@ func Test_getVIPsForService(t *testing.T) { { service: getClusterSvc(), endpoints: getNoLocalAddressesEPs(), - advertisedIPs: []string{}, - withdrawnIPs: []string{"10.0.0.1"}, + advertisedIPs: []string{"10.0.0.1"}, + withdrawnIPs: []string{}, annotations: map[string]string{ svcLocalAnnotation: "true", }, @@ -629,8 +629,8 @@ func Test_getVIPsForService(t *testing.T) { { service: getExternalSvc(), endpoints: getNoLocalAddressesEPs(), - advertisedIPs: []string{}, - withdrawnIPs: []string{"10.0.0.1", "1.1.1.1"}, + advertisedIPs: []string{"10.0.0.1"}, + withdrawnIPs: []string{"1.1.1.1"}, annotations: map[string]string{ svcLocalAnnotation: "true", }, @@ -638,8 +638,8 @@ func Test_getVIPsForService(t *testing.T) { { service: getNodePortSvc(), endpoints: getNoLocalAddressesEPs(), - advertisedIPs: []string{}, - withdrawnIPs: []string{"10.0.0.1", "1.1.1.1"}, + advertisedIPs: []string{"10.0.0.1"}, + withdrawnIPs: []string{"1.1.1.1"}, annotations: map[string]string{ svcLocalAnnotation: "true", }, @@ -647,8 +647,8 @@ func Test_getVIPsForService(t *testing.T) { { service: getLoadBalancerSvc(), endpoints: getNoLocalAddressesEPs(), - advertisedIPs: []string{}, - withdrawnIPs: []string{"10.0.0.1", "1.1.1.1", "10.0.255.1", "10.0.255.2"}, + advertisedIPs: []string{"10.0.0.1"}, + withdrawnIPs: []string{"1.1.1.1", "10.0.255.1", "10.0.255.2"}, annotations: map[string]string{ svcLocalAnnotation: "true", }, @@ -672,8 +672,8 @@ func Test_getVIPsForService(t *testing.T) { { service: getClusterSvc(), endpoints: getNoLocalAddressesEPs(), - advertisedIPs: []string{}, - withdrawnIPs: []string{"10.0.0.1"}, + advertisedIPs: []string{"10.0.0.1"}, + withdrawnIPs: []string{}, annotations: map[string]string{ svcLocalAnnotation: "true", }, @@ -681,8 +681,8 @@ func Test_getVIPsForService(t *testing.T) { { service: getExternalSvc(), endpoints: getNoLocalAddressesEPs(), - advertisedIPs: []string{}, - withdrawnIPs: []string{"10.0.0.1", "1.1.1.1"}, + advertisedIPs: []string{"10.0.0.1"}, + withdrawnIPs: []string{"1.1.1.1"}, annotations: map[string]string{ svcLocalAnnotation: "true", }, @@ -690,8 +690,8 @@ func Test_getVIPsForService(t *testing.T) { { service: getNodePortSvc(), endpoints: getNoLocalAddressesEPs(), - advertisedIPs: []string{}, - withdrawnIPs: []string{"10.0.0.1", "1.1.1.1"}, + advertisedIPs: []string{"10.0.0.1"}, + withdrawnIPs: []string{"1.1.1.1"}, annotations: map[string]string{ svcLocalAnnotation: "true", }, @@ -699,8 +699,8 @@ func Test_getVIPsForService(t *testing.T) { { service: getLoadBalancerSvc(), endpoints: getNoLocalAddressesEPs(), - advertisedIPs: []string{}, - withdrawnIPs: []string{"10.0.0.1", "1.1.1.1", "10.0.255.1", "10.0.255.2"}, + advertisedIPs: []string{"10.0.0.1"}, + withdrawnIPs: []string{"1.1.1.1", "10.0.255.1", "10.0.255.2"}, annotations: map[string]string{ svcLocalAnnotation: "true", },