move IsLocalIP() and ShouldSkipService() to pkg/proxy/util

pkg/proxy/iptables/proxier.go

@@ -687,20 +687,6 @@ func (proxier *Proxier) OnServiceSynced() {
 	proxier.syncProxyRules()
 }
 
-func shouldSkipService(svcName types.NamespacedName, service *api.Service) bool {
-	// if ClusterIP is "None" or empty, skip proxying
-	if !helper.IsServiceIPSet(service) {
-		glog.V(3).Infof("Skipping service %s due to clusterIP = %q", svcName, service.Spec.ClusterIP)
-		return true
-	}
-	// Even if ClusterIP is set, ServiceTypeExternalName services don't get proxied
-	if service.Spec.Type == api.ServiceTypeExternalName {
-		glog.V(3).Infof("Skipping service %s due to Type=ExternalName", svcName)
-		return true
-	}
-	return false
-}
-
 // <serviceMap> is updated by this function (based on the given changes).
 // <changes> map is cleared after applying them.
 func updateServiceMap(
@@ -894,7 +880,7 @@ func serviceToServiceMap(service *api.Service) proxyServiceMap {
 		return nil
 	}
 	svcName := types.NamespacedName{Namespace: service.Namespace, Name: service.Name}
-	if shouldSkipService(svcName, service) {
+	if utilproxy.ShouldSkipService(svcName, service) {
 		return nil
 	}
 
@@ -1211,7 +1197,7 @@ func (proxier *Proxier) syncProxyRules() {
 			// If the "external" IP happens to be an IP that is local to this
 			// machine, hold the local port open so no other process can open it
 			// (because the socket might open but it would never work).
-			if local, err := isLocalIP(externalIP); err != nil {
+			if local, err := utilproxy.IsLocalIP(externalIP); err != nil {
 				glog.Errorf("can't determine if IP is local, assuming not: %v", err)
 			} else if local {
 				lp := localPort{
@@ -1665,23 +1651,6 @@ func writeLine(buf *bytes.Buffer, words ...string) {
 	}
 }
 
-func isLocalIP(ip string) (bool, error) {
-	addrs, err := net.InterfaceAddrs()
-	if err != nil {
-		return false, err
-	}
-	for i := range addrs {
-		intf, _, err := net.ParseCIDR(addrs[i].String())
-		if err != nil {
-			return false, err
-		}
-		if net.ParseIP(ip).Equal(intf) {
-			return true, nil
-		}
-	}
-	return false, nil
-}
-
 func openLocalPort(lp *localPort) (closeable, error) {
 	// For ports on node IPs, open the actual port and hold it, even though we
 	// use iptables to redirect traffic.

pkg/proxy/userspace/proxier.go

@@ -583,7 +583,7 @@ func (proxier *Proxier) openPortal(service proxy.ServicePortName, info *ServiceI
 }
 
 func (proxier *Proxier) openOnePortal(portal portal, protocol api.Protocol, proxyIP net.IP, proxyPort int, name proxy.ServicePortName) error {
-	if local, err := isLocalIP(portal.ip); err != nil {
+	if local, err := utilproxy.IsLocalIP(portal.ip.String()); err != nil {
 		return fmt.Errorf("can't determine if IP %s is local, assuming not: %v", portal.ip, err)
 	} else if local {
 		err := proxier.claimNodePort(portal.ip, portal.port, protocol, name)
@@ -761,7 +761,7 @@ func (proxier *Proxier) closePortal(service proxy.ServicePortName, info *Service
 func (proxier *Proxier) closeOnePortal(portal portal, protocol api.Protocol, proxyIP net.IP, proxyPort int, name proxy.ServicePortName) []error {
 	el := []error{}
 
-	if local, err := isLocalIP(portal.ip); err != nil {
+	if local, err := utilproxy.IsLocalIP(portal.ip.String()); err != nil {
 		el = append(el, fmt.Errorf("can't determine if IP %s is local, assuming not: %v", portal.ip, err))
 	} else if local {
 		if err := proxier.releaseNodePort(portal.ip, portal.port, protocol, name); err != nil {
@@ -832,23 +832,6 @@ func (proxier *Proxier) closeNodePort(nodePort int, protocol api.Protocol, proxy
 	return el
 }
 
-func isLocalIP(ip net.IP) (bool, error) {
-	addrs, err := net.InterfaceAddrs()
-	if err != nil {
-		return false, err
-	}
-	for i := range addrs {
-		intf, _, err := net.ParseCIDR(addrs[i].String())
-		if err != nil {
-			return false, err
-		}
-		if ip.Equal(intf) {
-			return true, nil
-		}
-	}
-	return false, nil
-}
-
 // See comments in the *PortalArgs() functions for some details about why we
 // use two chains for portals.
 var iptablesContainerPortalChain iptables.Chain = "KUBE-PORTALS-CONTAINER"

pkg/proxy/util/BUILD

@@ -2,16 +2,31 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
 
 go_library(
     name = "go_default_library",
-    srcs = ["conntrack.go"],
+    srcs = [
+        "conntrack.go",
+        "utils.go",
+    ],
     visibility = ["//visibility:public"],
-    deps = ["//vendor/k8s.io/utils/exec:go_default_library"],
+    deps = [
+        "//pkg/api:go_default_library",
+        "//pkg/api/helper:go_default_library",
+        "//vendor/github.com/golang/glog:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
+        "//vendor/k8s.io/utils/exec:go_default_library",
+    ],
 )
 
 go_test(
     name = "go_default_test",
-    srcs = ["conntrack_test.go"],
+    srcs = [
+        "conntrack_test.go",
+        "utils_test.go",
+    ],
     library = ":go_default_library",
     deps = [
+        "//pkg/api:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
         "//vendor/k8s.io/utils/exec:go_default_library",
         "//vendor/k8s.io/utils/exec/testing:go_default_library",
     ],

pkg/proxy/util/utils.go

+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package util
+
+import (
+	"net"
+
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/api/helper"
+
+	"github.com/golang/glog"
+)
+
+func IsLocalIP(ip string) (bool, error) {
+	addrs, err := net.InterfaceAddrs()
+	if err != nil {
+		return false, err
+	}
+	for i := range addrs {
+		intf, _, err := net.ParseCIDR(addrs[i].String())
+		if err != nil {
+			return false, err
+		}
+		if net.ParseIP(ip).Equal(intf) {
+			return true, nil
+		}
+	}
+	return false, nil
+}
+
+func ShouldSkipService(svcName types.NamespacedName, service *api.Service) bool {
+	// if ClusterIP is "None" or empty, skip proxying
+	if !helper.IsServiceIPSet(service) {
+		glog.V(3).Infof("Skipping service %s due to clusterIP = %q", svcName, service.Spec.ClusterIP)
+		return true
+	}
+	// Even if ClusterIP is set, ServiceTypeExternalName services don't get proxied
+	if service.Spec.Type == api.ServiceTypeExternalName {
+		glog.V(3).Infof("Skipping service %s due to Type=ExternalName", svcName)
+		return true
+	}
+	return false
+}

pkg/proxy/util/utils_test.go

+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package util
+
+import (
+	"testing"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/kubernetes/pkg/api"
+)
+
+func TestShouldSkipService(t *testing.T) {
+	testCases := []struct {
+		service    *api.Service
+		svcName    types.NamespacedName
+		shouldSkip bool
+	}{
+		{
+			// Cluster IP is None
+			service: &api.Service{
+				ObjectMeta: metav1.ObjectMeta{Namespace: "foo", Name: "bar"},
+				Spec: api.ServiceSpec{
+					ClusterIP: api.ClusterIPNone,
+				},
+			},
+			svcName:    types.NamespacedName{Namespace: "foo", Name: "bar"},
+			shouldSkip: true,
+		},
+		{
+			// Cluster IP is empty
+			service: &api.Service{
+				ObjectMeta: metav1.ObjectMeta{Namespace: "foo", Name: "bar"},
+				Spec: api.ServiceSpec{
+					ClusterIP: "",
+				},
+			},
+			svcName:    types.NamespacedName{Namespace: "foo", Name: "bar"},
+			shouldSkip: true,
+		},
+		{
+			// ExternalName type service
+			service: &api.Service{
+				ObjectMeta: metav1.ObjectMeta{Namespace: "foo", Name: "bar"},
+				Spec: api.ServiceSpec{
+					ClusterIP: "1.2.3.4",
+					Type:      api.ServiceTypeExternalName,
+				},
+			},
+			svcName:    types.NamespacedName{Namespace: "foo", Name: "bar"},
+			shouldSkip: true,
+		},
+		{
+			// ClusterIP type service with ClusterIP set
+			service: &api.Service{
+				ObjectMeta: metav1.ObjectMeta{Namespace: "foo", Name: "bar"},
+				Spec: api.ServiceSpec{
+					ClusterIP: "1.2.3.4",
+					Type:      api.ServiceTypeClusterIP,
+				},
+			},
+			svcName:    types.NamespacedName{Namespace: "foo", Name: "bar"},
+			shouldSkip: false,
+		},
+		{
+			// NodePort type service with ClusterIP set
+			service: &api.Service{
+				ObjectMeta: metav1.ObjectMeta{Namespace: "foo", Name: "bar"},
+				Spec: api.ServiceSpec{
+					ClusterIP: "1.2.3.4",
+					Type:      api.ServiceTypeNodePort,
+				},
+			},
+			svcName:    types.NamespacedName{Namespace: "foo", Name: "bar"},
+			shouldSkip: false,
+		},
+		{
+			// LoadBalancer type service with ClusterIP set
+			service: &api.Service{
+				ObjectMeta: metav1.ObjectMeta{Namespace: "foo", Name: "bar"},
+				Spec: api.ServiceSpec{
+					ClusterIP: "1.2.3.4",
+					Type:      api.ServiceTypeLoadBalancer,
+				},
+			},
+			svcName:    types.NamespacedName{Namespace: "foo", Name: "bar"},
+			shouldSkip: false,
+		},
+	}
+
+	for i := range testCases {
+		skip := ShouldSkipService(testCases[i].svcName, testCases[i].service)
+		if skip != testCases[i].shouldSkip {
+			t.Errorf("case %d: expect %v, got %v", i, testCases[i].shouldSkip, skip)
+		}
+	}
+}