diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index 1c497df9673..6f2e6668427 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/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 -} - // is updated by this function (based on the given 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. diff --git a/pkg/proxy/userspace/proxier.go b/pkg/proxy/userspace/proxier.go index 846915f7fbd..7d3502ce79d 100644 --- a/pkg/proxy/userspace/proxier.go +++ b/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" diff --git a/pkg/proxy/util/BUILD b/pkg/proxy/util/BUILD index 01c12f6e8d5..9cbb8476aac 100644 --- a/pkg/proxy/util/BUILD +++ b/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", ], diff --git a/pkg/proxy/util/utils.go b/pkg/proxy/util/utils.go new file mode 100644 index 00000000000..81734f54892 --- /dev/null +++ b/pkg/proxy/util/utils.go @@ -0,0 +1,58 @@ +/* +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 +} diff --git a/pkg/proxy/util/utils_test.go b/pkg/proxy/util/utils_test.go new file mode 100644 index 00000000000..b57850ae5f9 --- /dev/null +++ b/pkg/proxy/util/utils_test.go @@ -0,0 +1,111 @@ +/* +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) + } + } +}