kube-proxy ShouldSkipService takes only one argument

instead of receiving the service name and namespace we
can obtain it from the service object directly.

Signed-off-by: Antonio Ojea <antonio.ojea.garcia@gmail.com>
This commit is contained in:
Antonio Ojea 2020-06-30 17:35:38 +02:00
parent 9fb34edded
commit a46e1f0613
5 changed files with 13 additions and 22 deletions

View File

@ -25,7 +25,7 @@ import (
"k8s.io/klog/v2" "k8s.io/klog/v2"
"k8s.io/api/core/v1" v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/sets"
"k8s.io/client-go/tools/record" "k8s.io/client-go/tools/record"
@ -307,8 +307,8 @@ func (sct *ServiceChangeTracker) serviceToServiceMap(service *v1.Service) Servic
if service == nil { if service == nil {
return nil return nil
} }
svcName := types.NamespacedName{Namespace: service.Namespace, Name: service.Name}
if utilproxy.ShouldSkipService(svcName, service) { if utilproxy.ShouldSkipService(service) {
return nil return nil
} }
@ -322,6 +322,7 @@ func (sct *ServiceChangeTracker) serviceToServiceMap(service *v1.Service) Servic
} }
serviceMap := make(ServiceMap) serviceMap := make(ServiceMap)
svcName := types.NamespacedName{Namespace: service.Namespace, Name: service.Name}
for i := range service.Spec.Ports { for i := range service.Spec.Ports {
servicePort := &service.Spec.Ports[i] servicePort := &service.Spec.Ports[i]
svcPortName := ServicePortName{NamespacedName: svcName, Port: servicePort.Name, Protocol: servicePort.Protocol} svcPortName := ServicePortName{NamespacedName: svcName, Port: servicePort.Name, Protocol: servicePort.Protocol}

View File

@ -489,12 +489,11 @@ func (proxier *Proxier) mergeService(service *v1.Service) sets.String {
if service == nil { if service == nil {
return nil return nil
} }
svcName := types.NamespacedName{Namespace: service.Namespace, Name: service.Name} if utilproxy.ShouldSkipService(service) {
if utilproxy.ShouldSkipService(svcName, service) {
klog.V(3).Infof("Skipping service %s due to clusterIP = %q", svcName, service.Spec.ClusterIP)
return nil return nil
} }
existingPorts := sets.NewString() existingPorts := sets.NewString()
svcName := types.NamespacedName{Namespace: service.Namespace, Name: service.Name}
for i := range service.Spec.Ports { for i := range service.Spec.Ports {
servicePort := &service.Spec.Ports[i] servicePort := &service.Spec.Ports[i]
serviceName := proxy.ServicePortName{NamespacedName: svcName, Port: servicePort.Name} serviceName := proxy.ServicePortName{NamespacedName: svcName, Port: servicePort.Name}
@ -551,12 +550,12 @@ func (proxier *Proxier) unmergeService(service *v1.Service, existingPorts sets.S
if service == nil { if service == nil {
return return
} }
svcName := types.NamespacedName{Namespace: service.Namespace, Name: service.Name}
if utilproxy.ShouldSkipService(svcName, service) { if utilproxy.ShouldSkipService(service) {
klog.V(3).Infof("Skipping service %s due to clusterIP = %q", svcName, service.Spec.ClusterIP)
return return
} }
staleUDPServices := sets.NewString() staleUDPServices := sets.NewString()
svcName := types.NamespacedName{Namespace: service.Namespace, Name: service.Name}
for i := range service.Spec.Ports { for i := range service.Spec.Ports {
servicePort := &service.Spec.Ports[i] servicePort := &service.Spec.Ports[i]
if existingPorts.Has(servicePort.Name) { if existingPorts.Has(servicePort.Name) {

View File

@ -35,7 +35,6 @@ go_test(
"//pkg/proxy/util/testing:go_default_library", "//pkg/proxy/util/testing:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
], ],
) )

View File

@ -146,15 +146,15 @@ func GetLocalAddrs() ([]net.IP, error) {
} }
// ShouldSkipService checks if a given service should skip proxying // ShouldSkipService checks if a given service should skip proxying
func ShouldSkipService(svcName types.NamespacedName, service *v1.Service) bool { func ShouldSkipService(service *v1.Service) bool {
// if ClusterIP is "None" or empty, skip proxying // if ClusterIP is "None" or empty, skip proxying
if !helper.IsServiceIPSet(service) { if !helper.IsServiceIPSet(service) {
klog.V(3).Infof("Skipping service %s due to clusterIP = %q", svcName, service.Spec.ClusterIP) klog.V(3).Infof("Skipping service %s in namespace %s due to clusterIP = %q", service.Name, service.Namespace, service.Spec.ClusterIP)
return true return true
} }
// Even if ClusterIP is set, ServiceTypeExternalName services don't get proxied // Even if ClusterIP is set, ServiceTypeExternalName services don't get proxied
if service.Spec.Type == v1.ServiceTypeExternalName { if service.Spec.Type == v1.ServiceTypeExternalName {
klog.V(3).Infof("Skipping service %s due to Type=ExternalName", svcName) klog.V(3).Infof("Skipping service %s in namespace %s due to Type=ExternalName", service.Name, service.Namespace)
return true return true
} }
return false return false

View File

@ -25,7 +25,6 @@ import (
v1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/sets"
fake "k8s.io/kubernetes/pkg/proxy/util/testing" fake "k8s.io/kubernetes/pkg/proxy/util/testing"
) )
@ -167,7 +166,6 @@ func TestIsProxyableHostname(t *testing.T) {
func TestShouldSkipService(t *testing.T) { func TestShouldSkipService(t *testing.T) {
testCases := []struct { testCases := []struct {
service *v1.Service service *v1.Service
svcName types.NamespacedName
shouldSkip bool shouldSkip bool
}{ }{
{ {
@ -178,7 +176,6 @@ func TestShouldSkipService(t *testing.T) {
ClusterIP: v1.ClusterIPNone, ClusterIP: v1.ClusterIPNone,
}, },
}, },
svcName: types.NamespacedName{Namespace: "foo", Name: "bar"},
shouldSkip: true, shouldSkip: true,
}, },
{ {
@ -189,7 +186,6 @@ func TestShouldSkipService(t *testing.T) {
ClusterIP: "", ClusterIP: "",
}, },
}, },
svcName: types.NamespacedName{Namespace: "foo", Name: "bar"},
shouldSkip: true, shouldSkip: true,
}, },
{ {
@ -201,7 +197,6 @@ func TestShouldSkipService(t *testing.T) {
Type: v1.ServiceTypeExternalName, Type: v1.ServiceTypeExternalName,
}, },
}, },
svcName: types.NamespacedName{Namespace: "foo", Name: "bar"},
shouldSkip: true, shouldSkip: true,
}, },
{ {
@ -213,7 +208,6 @@ func TestShouldSkipService(t *testing.T) {
Type: v1.ServiceTypeClusterIP, Type: v1.ServiceTypeClusterIP,
}, },
}, },
svcName: types.NamespacedName{Namespace: "foo", Name: "bar"},
shouldSkip: false, shouldSkip: false,
}, },
{ {
@ -225,7 +219,6 @@ func TestShouldSkipService(t *testing.T) {
Type: v1.ServiceTypeNodePort, Type: v1.ServiceTypeNodePort,
}, },
}, },
svcName: types.NamespacedName{Namespace: "foo", Name: "bar"},
shouldSkip: false, shouldSkip: false,
}, },
{ {
@ -237,13 +230,12 @@ func TestShouldSkipService(t *testing.T) {
Type: v1.ServiceTypeLoadBalancer, Type: v1.ServiceTypeLoadBalancer,
}, },
}, },
svcName: types.NamespacedName{Namespace: "foo", Name: "bar"},
shouldSkip: false, shouldSkip: false,
}, },
} }
for i := range testCases { for i := range testCases {
skip := ShouldSkipService(testCases[i].svcName, testCases[i].service) skip := ShouldSkipService(testCases[i].service)
if skip != testCases[i].shouldSkip { if skip != testCases[i].shouldSkip {
t.Errorf("case %d: expect %v, got %v", i, testCases[i].shouldSkip, skip) t.Errorf("case %d: expect %v, got %v", i, testCases[i].shouldSkip, skip)
} }