From 2c0fc3182caa2d76a2c83a2a3b85ec5c5fa8f915 Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Thu, 3 Aug 2017 23:14:11 -0700 Subject: [PATCH] middleware/kubernetes: cleanup (#818) Drop the interfaceAddr interfaces and just use a function. Cleanup all fallout from that. Remove the use of global variables and cleanup the tests a bit. --- middleware/kubernetes/federation.go | 13 +++---------- middleware/kubernetes/federation_test.go | 7 ++----- middleware/kubernetes/handler_test.go | 2 +- middleware/kubernetes/kubernetes.go | 15 ++++++--------- middleware/kubernetes/kubernetes_test.go | 1 + middleware/kubernetes/ns.go | 22 ++++++---------------- middleware/kubernetes/ns_test.go | 10 ++-------- middleware/kubernetes/setup.go | 8 ++++---- 8 files changed, 25 insertions(+), 53 deletions(-) diff --git a/middleware/kubernetes/federation.go b/middleware/kubernetes/federation.go index 4c058b839..6f7e5f122 100644 --- a/middleware/kubernetes/federation.go +++ b/middleware/kubernetes/federation.go @@ -13,10 +13,6 @@ type Federation struct { zone string } -var localNodeName string -var federationZone string -var federationRegion string - const ( // TODO: Do not hardcode these labels. Pull them out of the API instead. // @@ -80,21 +76,18 @@ func (k *Kubernetes) federationCNAMERecord(r recordRequest) msg.Service { } func (k *Kubernetes) localNodeName() string { - if localNodeName != "" { - return localNodeName - } - localIP := k.localPodIP() + localIP := k.interfaceAddrsFunc() if localIP == nil { return "" } + // Find endpoint matching localIP endpointsList := k.APIConn.EndpointsList() for _, ep := range endpointsList.Items { for _, eps := range ep.Subsets { for _, addr := range eps.Addresses { if localIP.Equal(net.ParseIP(addr.IP)) { - localNodeName = *addr.NodeName - return localNodeName + return *addr.NodeName } } } diff --git a/middleware/kubernetes/federation_test.go b/middleware/kubernetes/federation_test.go index a938159ff..95dbc9024 100644 --- a/middleware/kubernetes/federation_test.go +++ b/middleware/kubernetes/federation_test.go @@ -88,11 +88,9 @@ func TestFederationCNAMERecord(t *testing.T) { k := Kubernetes{Zones: []string{"inter.webs"}} k.Federations = []Federation{{name: "fed", zone: "era.tion.com"}} k.APIConn = apiConnFedTest{} + k.interfaceAddrsFunc = func() net.IP { return net.ParseIP("10.9.8.7") } - var r recordRequest - - r, _ = k.parseRequest("s1.ns.fed.svc.inter.webs", dns.TypeA) - localPodIP = net.ParseIP("10.9.8.7") + r, _ := k.parseRequest("s1.ns.fed.svc.inter.webs", dns.TypeA) testFederationCNAMERecord(t, k, r, msg.Service{Key: "/coredns/webs/inter/svc/fed/ns/s1", Host: "s1.ns.fed.svc.fd-az.fd-r.era.tion.com"}) r, _ = k.parseRequest("ep1.s1.ns.fed.svc.inter.webs", dns.TypeA) @@ -100,5 +98,4 @@ func TestFederationCNAMERecord(t *testing.T) { r, _ = k.parseRequest("ep1.s1.ns.foo.svc.inter.webs", dns.TypeA) testFederationCNAMERecord(t, k, r, msg.Service{Key: "", Host: ""}) - } diff --git a/middleware/kubernetes/handler_test.go b/middleware/kubernetes/handler_test.go index 8d9417b8d..9e38bc59f 100644 --- a/middleware/kubernetes/handler_test.go +++ b/middleware/kubernetes/handler_test.go @@ -269,7 +269,7 @@ func TestServeDNS(t *testing.T) { k.APIConn = &APIConnServeTest{} k.AutoPath.Enabled = true k.AutoPath.HostSearchPath = []string{"hostdom.test"} - //k.Proxy = test.MockHandler(nextMWMap) + k.interfaceAddrsFunc = localPodIP k.Next = testHandler(nextMWMap) ctx := context.TODO() diff --git a/middleware/kubernetes/kubernetes.go b/middleware/kubernetes/kubernetes.go index 58d3656ae..8a87322e8 100644 --- a/middleware/kubernetes/kubernetes.go +++ b/middleware/kubernetes/kubernetes.go @@ -46,7 +46,7 @@ type Kubernetes struct { ReverseCidrs []net.IPNet Fallthrough bool AutoPath - interfaceAddrs interfaceAddrser + interfaceAddrsFunc func() net.IP } type AutoPath struct { @@ -98,8 +98,6 @@ type recordRequest struct { federation string } -var localPodIP net.IP - var ( errNoItems = errors.New("no items found") errNsNotExposed = errors.New("namespace is not exposed") @@ -651,11 +649,11 @@ func symbolContainsWildcard(symbol string) bool { return (symbol == "*" || symbol == "any") } -func (k *Kubernetes) localPodIP() net.IP { - if localPodIP != nil { - return localPodIP +func localPodIP() net.IP { + addrs, err := net.InterfaceAddrs() + if err != nil { + return nil } - addrs, _ := k.interfaceAddrs.interfaceAddrs() for _, addr := range addrs { ip, _, _ := net.ParseCIDR(addr.String()) @@ -663,8 +661,7 @@ func (k *Kubernetes) localPodIP() net.IP { if ip == nil || ip.IsLoopback() { continue } - localPodIP = ip - return localPodIP + return ip } return nil } diff --git a/middleware/kubernetes/kubernetes_test.go b/middleware/kubernetes/kubernetes_test.go index 530e2234e..c93270cbe 100644 --- a/middleware/kubernetes/kubernetes_test.go +++ b/middleware/kubernetes/kubernetes_test.go @@ -432,6 +432,7 @@ func TestServices(t *testing.T) { k := Kubernetes{Zones: []string{"interwebs.test"}} k.Federations = []Federation{{name: "fed", zone: "era.tion.com"}} + k.interfaceAddrsFunc = localPodIP k.APIConn = &APIConnServiceTest{} type svcAns struct { diff --git a/middleware/kubernetes/ns.go b/middleware/kubernetes/ns.go index af6f83f21..488713d8b 100644 --- a/middleware/kubernetes/ns.go +++ b/middleware/kubernetes/ns.go @@ -41,25 +41,15 @@ func isDefaultNS(name string, r recordRequest) bool { } func (k *Kubernetes) coreDNSRecord() dns.A { - var localIP net.IP - var svcName string - var svcNamespace string - var dnsIP net.IP + var ( + svcName string + svcNamespace string + dnsIP net.IP + ) if len(corednsRecord.Hdr.Name) == 0 || corednsRecord.A == nil { // get local Pod IP - addrs, _ := k.interfaceAddrs.interfaceAddrs() - - for _, addr := range addrs { - ip, _, _ := net.ParseCIDR(addr.String()) - ip = ip.To4() - - if ip == nil || ip.IsLoopback() { - continue - } - localIP = ip - break - } + localIP := k.interfaceAddrsFunc() // Find endpoint matching IP to get service and namespace endpointsList := k.APIConn.EndpointsList() diff --git a/middleware/kubernetes/ns_test.go b/middleware/kubernetes/ns_test.go index c7500a3fc..1aa4f910d 100644 --- a/middleware/kubernetes/ns_test.go +++ b/middleware/kubernetes/ns_test.go @@ -96,19 +96,13 @@ func (APIConnTest) EndpointsList() api.EndpointsList { func (APIConnTest) GetNodeByName(name string) (api.Node, error) { return api.Node{}, nil } -type interfaceAddrsTest struct{} - -func (i interfaceAddrsTest) interfaceAddrs() ([]net.Addr, error) { - _, ipnet, _ := net.ParseCIDR("172.0.40.10/32") - return []net.Addr{ipnet}, nil -} - func TestDoCoreDNSRecord(t *testing.T) { corednsRecord = dns.A{} k := Kubernetes{Zones: []string{"inter.webs.test"}} - k.interfaceAddrs = &interfaceAddrsTest{} + k.interfaceAddrsFunc = func() net.IP { return net.ParseIP("172.0.40.10") } + k.APIConn = &APIConnTest{} cdr := k.coreDNSRecord() diff --git a/middleware/kubernetes/setup.go b/middleware/kubernetes/setup.go index 08bc9b02d..a12673a13 100644 --- a/middleware/kubernetes/setup.go +++ b/middleware/kubernetes/setup.go @@ -56,10 +56,10 @@ func setup(c *caddy.Controller) error { func kubernetesParse(c *caddy.Controller) (*Kubernetes, error) { k8s := &Kubernetes{ - ResyncPeriod: defaultResyncPeriod, - interfaceAddrs: &interfaceAddrs{}, - PodMode: PodModeDisabled, - Proxy: proxy.Proxy{}, + ResyncPeriod: defaultResyncPeriod, + interfaceAddrsFunc: localPodIP, + PodMode: PodModeDisabled, + Proxy: proxy.Proxy{}, } for c.Next() {