feat(NSC): change service.local internal traffic policy posture

Over time, feedback from users has been that our interpretation of how
the kube-router service.local annotation interacts with the internal
traffic policy has been that it is too restrictive.

It seems like tuning it to fall in line with the local internal traffic
policy is too restrictive. This commit changes that posture, by equating
the service.local annotation with External Traffic Policy Local and
Internal Traffic Policy Cluster.

This means that when service.local is set the following will be true:

* ExternalIPs / LoadBalancer IPs will only be available on a node that
  hosts the workload
* ExternalIPs / LoadBalancer IPs will only be BGP advertised (when
  enabled) by nodes that host the workload
* Services will have the same posture as External Traffic Policy set to
  local
* ClusterIPs will be available on all nodes for LoadBalancing
* ClusterIPs will only be BGP advertised (when enabled) by nodes that
  host the workload
* Cluster IP services will have the same posture as Internal Traffic
  Policy set to cluster

For anyone desiring the original functionality of the service.local
annotation that has been in place since kube-router v2.1.0, all that
would need to be done is to set `internalTrafficPolicy` to Local as
described here: https://kubernetes.io/docs/concepts/services-networking/service-traffic-policy/
This commit is contained in:
Aaron U'Ren 2024-12-03 17:43:40 -06:00 committed by Manuel Rüger
parent 1662973855
commit 44439d6069
3 changed files with 22 additions and 22 deletions

View File

@ -978,10 +978,11 @@ func (nsc *NetworkServicesController) buildServicesInfo() serviceInfoMap {
svcInfo.extTrafficPolicy = &svc.Spec.ExternalTrafficPolicy svcInfo.extTrafficPolicy = &svc.Spec.ExternalTrafficPolicy
// The kube-router.io/service.local annotation has the ability to override the internal and external traffic // 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 // policy that is set in the spec. Previously, when this was active set both to local when the annotation is
// previous functionality of the annotation is best preserved. // 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" { if svc.ObjectMeta.Annotations[svcLocalAnnotation] == "true" {
intTrafficPolicyLocal := v1.ServiceInternalTrafficPolicyLocal intTrafficPolicyLocal := v1.ServiceInternalTrafficPolicyCluster
extTrafficPolicyLocal := v1.ServiceExternalTrafficPolicyLocal extTrafficPolicyLocal := v1.ServiceExternalTrafficPolicyLocal
svcInfo.intTrafficPolicy = &intTrafficPolicyLocal svcInfo.intTrafficPolicy = &intTrafficPolicyLocal
svcInfo.extTrafficPolicy = &extTrafficPolicyLocal svcInfo.extTrafficPolicy = &extTrafficPolicyLocal

View File

@ -484,7 +484,7 @@ func (nrc *NetworkRoutingController) shouldAdvertiseService(svc *v1core.Service,
// If: // If:
// - We are assessing the clusterIP of the service (the internally facing VIP) // - 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 // - The service doesn't have any endpoints on the node we're executing on
// Then: return false // Then: return false
// We handle spec.internalTrafficPolicy different because it was introduced in v1.26 and may not be available in all // 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 { if svc.Spec.InternalTrafficPolicy != nil {
serIntTrafPol = *svc.Spec.InternalTrafficPolicy == v1core.ServiceInternalTrafficPolicyLocal serIntTrafPol = *svc.Spec.InternalTrafficPolicy == v1core.ServiceInternalTrafficPolicyLocal
} }
intLocalPol := (serIntTrafPol || svc.Annotations[svcLocalAnnotation] == "true") if isClusterIP && serIntTrafPol && !hasLocalEndpoints {
if isClusterIP && intLocalPol && !hasLocalEndpoints {
return false, nil return false, nil
} }

View File

@ -620,8 +620,8 @@ func Test_getVIPsForService(t *testing.T) {
{ {
service: getClusterSvc(), service: getClusterSvc(),
endpoints: getNoLocalAddressesEPs(), endpoints: getNoLocalAddressesEPs(),
advertisedIPs: []string{}, advertisedIPs: []string{"10.0.0.1"},
withdrawnIPs: []string{"10.0.0.1"}, withdrawnIPs: []string{},
annotations: map[string]string{ annotations: map[string]string{
svcLocalAnnotation: "true", svcLocalAnnotation: "true",
}, },
@ -629,8 +629,8 @@ func Test_getVIPsForService(t *testing.T) {
{ {
service: getExternalSvc(), service: getExternalSvc(),
endpoints: getNoLocalAddressesEPs(), endpoints: getNoLocalAddressesEPs(),
advertisedIPs: []string{}, advertisedIPs: []string{"10.0.0.1"},
withdrawnIPs: []string{"10.0.0.1", "1.1.1.1"}, withdrawnIPs: []string{"1.1.1.1"},
annotations: map[string]string{ annotations: map[string]string{
svcLocalAnnotation: "true", svcLocalAnnotation: "true",
}, },
@ -638,8 +638,8 @@ func Test_getVIPsForService(t *testing.T) {
{ {
service: getNodePortSvc(), service: getNodePortSvc(),
endpoints: getNoLocalAddressesEPs(), endpoints: getNoLocalAddressesEPs(),
advertisedIPs: []string{}, advertisedIPs: []string{"10.0.0.1"},
withdrawnIPs: []string{"10.0.0.1", "1.1.1.1"}, withdrawnIPs: []string{"1.1.1.1"},
annotations: map[string]string{ annotations: map[string]string{
svcLocalAnnotation: "true", svcLocalAnnotation: "true",
}, },
@ -647,8 +647,8 @@ func Test_getVIPsForService(t *testing.T) {
{ {
service: getLoadBalancerSvc(), service: getLoadBalancerSvc(),
endpoints: getNoLocalAddressesEPs(), endpoints: getNoLocalAddressesEPs(),
advertisedIPs: []string{}, advertisedIPs: []string{"10.0.0.1"},
withdrawnIPs: []string{"10.0.0.1", "1.1.1.1", "10.0.255.1", "10.0.255.2"}, withdrawnIPs: []string{"1.1.1.1", "10.0.255.1", "10.0.255.2"},
annotations: map[string]string{ annotations: map[string]string{
svcLocalAnnotation: "true", svcLocalAnnotation: "true",
}, },
@ -672,8 +672,8 @@ func Test_getVIPsForService(t *testing.T) {
{ {
service: getClusterSvc(), service: getClusterSvc(),
endpoints: getNoLocalAddressesEPs(), endpoints: getNoLocalAddressesEPs(),
advertisedIPs: []string{}, advertisedIPs: []string{"10.0.0.1"},
withdrawnIPs: []string{"10.0.0.1"}, withdrawnIPs: []string{},
annotations: map[string]string{ annotations: map[string]string{
svcLocalAnnotation: "true", svcLocalAnnotation: "true",
}, },
@ -681,8 +681,8 @@ func Test_getVIPsForService(t *testing.T) {
{ {
service: getExternalSvc(), service: getExternalSvc(),
endpoints: getNoLocalAddressesEPs(), endpoints: getNoLocalAddressesEPs(),
advertisedIPs: []string{}, advertisedIPs: []string{"10.0.0.1"},
withdrawnIPs: []string{"10.0.0.1", "1.1.1.1"}, withdrawnIPs: []string{"1.1.1.1"},
annotations: map[string]string{ annotations: map[string]string{
svcLocalAnnotation: "true", svcLocalAnnotation: "true",
}, },
@ -690,8 +690,8 @@ func Test_getVIPsForService(t *testing.T) {
{ {
service: getNodePortSvc(), service: getNodePortSvc(),
endpoints: getNoLocalAddressesEPs(), endpoints: getNoLocalAddressesEPs(),
advertisedIPs: []string{}, advertisedIPs: []string{"10.0.0.1"},
withdrawnIPs: []string{"10.0.0.1", "1.1.1.1"}, withdrawnIPs: []string{"1.1.1.1"},
annotations: map[string]string{ annotations: map[string]string{
svcLocalAnnotation: "true", svcLocalAnnotation: "true",
}, },
@ -699,8 +699,8 @@ func Test_getVIPsForService(t *testing.T) {
{ {
service: getLoadBalancerSvc(), service: getLoadBalancerSvc(),
endpoints: getNoLocalAddressesEPs(), endpoints: getNoLocalAddressesEPs(),
advertisedIPs: []string{}, advertisedIPs: []string{"10.0.0.1"},
withdrawnIPs: []string{"10.0.0.1", "1.1.1.1", "10.0.255.1", "10.0.255.2"}, withdrawnIPs: []string{"1.1.1.1", "10.0.255.1", "10.0.255.2"},
annotations: map[string]string{ annotations: map[string]string{
svcLocalAnnotation: "true", svcLocalAnnotation: "true",
}, },