From 61fc672e1939df3dd365c9d836427d8eec1e5e7d Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Wed, 23 Aug 2017 07:19:41 +0100 Subject: [PATCH] mw/kubernetes: remove kPod and kServices (#969) Based up on: #939, but redone in a new PR with some cherry-picked commits: aacb91ef0b927683b21d6ee39dbddbd001334854 5dc34247b7d0136d9fe035f6b10d6b3e14ee7f2c This removes kPod and Kservice and creates []msg.Service from k.findPods and k.findServices. Updated few tests which I *think* are correct; they look correct to me. --- middleware/kubernetes/federation.go | 2 +- middleware/kubernetes/handler_test.go | 29 +++- middleware/kubernetes/kubernetes.go | 185 +++++++------------------- middleware/kubernetes/parse.go | 2 +- middleware/kubernetes/parse_test.go | 8 +- middleware/kubernetes/reverse.go | 35 +++++ test/kubernetes_test.go | 2 +- 7 files changed, 110 insertions(+), 153 deletions(-) diff --git a/middleware/kubernetes/federation.go b/middleware/kubernetes/federation.go index 0861f90a4..96ce22730 100644 --- a/middleware/kubernetes/federation.go +++ b/middleware/kubernetes/federation.go @@ -29,7 +29,7 @@ func (k *Kubernetes) Federations(state request.Request, fname, fzone string) (ms if err != nil { return msg.Service{}, err } - r, err := k.parseRequest(state) + r, err := parseRequest(state) lz := node.Labels[LabelZone] lr := node.Labels[LabelRegion] diff --git a/middleware/kubernetes/handler_test.go b/middleware/kubernetes/handler_test.go index b866ce78c..292ffdf3a 100644 --- a/middleware/kubernetes/handler_test.go +++ b/middleware/kubernetes/handler_test.go @@ -47,7 +47,7 @@ var dnsTestCases = map[string](test.Case){ }, "SRV Service Not udp/tcp": { Qname: "*._not-udp-or-tcp.svc1.testns.svc.cluster.local.", Qtype: dns.TypeSRV, - Rcode: dns.RcodeSuccess, + Rcode: dns.RcodeNameError, Ns: []dns.RR{ test.SOA("cluster.local. 300 IN SOA ns.dns.cluster.local. hostmaster.cluster.local. 1499347823 7200 1800 86400 60"), }, @@ -91,16 +91,21 @@ var dnsTestCases = map[string](test.Case){ }, "AAAA Service (existing service)": { Qname: "svc1.testns.svc.cluster.local.", Qtype: dns.TypeAAAA, - Rcode: dns.RcodeSuccess, - Answer: []dns.RR{}, + Rcode: dns.RcodeSuccess, Ns: []dns.RR{ test.SOA("cluster.local. 300 IN SOA ns.dns.cluster.local. hostmaster.cluster.local. 1499347823 7200 1800 86400 60"), }, }, "AAAA Service (non-existing service)": { Qname: "svc0.testns.svc.cluster.local.", Qtype: dns.TypeAAAA, - Rcode: dns.RcodeNameError, - Answer: []dns.RR{}, + Rcode: dns.RcodeNameError, + Ns: []dns.RR{ + test.SOA("cluster.local. 300 IN SOA ns.dns.cluster.local. hostmaster.cluster.local. 1499347823 7200 1800 86400 60"), + }, + }, + "A Service (non-existing service)": { + Qname: "svc0.testns.svc.cluster.local.", Qtype: dns.TypeA, + Rcode: dns.RcodeNameError, Ns: []dns.RR{ test.SOA("cluster.local. 300 IN SOA ns.dns.cluster.local. hostmaster.cluster.local. 1499347823 7200 1800 86400 60"), }, @@ -112,6 +117,20 @@ var dnsTestCases = map[string](test.Case){ test.TXT("dns-version.cluster.local 28800 IN TXT 1.0.1"), }, }, + "A Service (Headless) does not exist": { + Qname: "bogusendpoint.hdls1.testns.svc.cluster.local.", Qtype: dns.TypeA, + Rcode: dns.RcodeNameError, + Ns: []dns.RR{ + test.SOA("cluster.local. 300 IN SOA ns.dns.cluster.local. hostmaster.cluster.local. 1499347823 7200 1800 86400 60"), + }, + }, + "A Service does not exist": { + Qname: "bogusendpoint.svc0.testns.svc.cluster.local.", Qtype: dns.TypeA, + Rcode: dns.RcodeNameError, + Ns: []dns.RR{ + test.SOA("cluster.local. 300 IN SOA ns.dns.cluster.local. hostmaster.cluster.local. 1499347823 7200 1800 86400 60"), + }, + }, } var podModeDisabledCases = map[string](test.Case){ diff --git a/middleware/kubernetes/kubernetes.go b/middleware/kubernetes/kubernetes.go index d5b2fb47f..26d784e70 100644 --- a/middleware/kubernetes/kubernetes.go +++ b/middleware/kubernetes/kubernetes.go @@ -70,27 +70,6 @@ const ( DNSSchemaVersion = "1.0.1" ) -type endpoint struct { - addr api.EndpointAddress - port api.EndpointPort -} - -// kService is a service as retrieved via the k8s API. -type kService struct { - name string - namespace string - addr string - ports []api.ServicePort - endpoints []endpoint -} - -// kPod is a pod as retrieved via the k8s API. -type kPod struct { - name string - namespace string - addr string -} - var ( errNoItems = errors.New("no items found") errNsNotExposed = errors.New("namespace is not exposed") @@ -286,7 +265,7 @@ func (k *Kubernetes) initKubeCache(opts dnsControlOpts) (err error) { // Records looks up services in kubernetes. func (k *Kubernetes) Records(state request.Request, exact bool) ([]msg.Service, error) { - r, e := k.parseRequest(state) + r, e := parseRequest(state) if e != nil { return nil, e } @@ -295,16 +274,13 @@ func (k *Kubernetes) Records(state request.Request, exact bool) ([]msg.Service, return nil, errNsNotExposed } - services, pods, err := k.get(r) - if err != nil { - return nil, err - } - if len(services) == 0 && len(pods) == 0 { - return nil, errNoItems + if r.podOrSvc == Pod { + pods, err := k.findPods(r, state.Zone) + return pods, err } - records := k.getRecordsForK8sItems(services, pods, state.Zone) - return records, nil + services, err := k.findServices(r, state.Zone) + return services, err } func endpointHostname(addr api.EndpointAddress) string { @@ -320,51 +296,17 @@ func endpointHostname(addr api.EndpointAddress) string { return "" } -func (k *Kubernetes) getRecordsForK8sItems(services []kService, pods []kPod, zone string) (records []msg.Service) { - zonePath := msg.Path(zone, "coredns") - - for _, svc := range services { - if svc.addr == api.ClusterIPNone || len(svc.endpoints) > 0 { - // This is a headless service or endpoints are present, create records for each endpoint - for _, ep := range svc.endpoints { - s := msg.Service{Host: ep.addr.IP, Port: int(ep.port.Port)} - s.Key = strings.Join([]string{zonePath, Svc, svc.namespace, svc.name, endpointHostname(ep.addr)}, "/") - - records = append(records, s) - } - continue - } - - // Create records for each exposed port... - for _, p := range svc.ports { - s := msg.Service{Host: svc.addr, Port: int(p.Port)} - s.Key = strings.Join([]string{zonePath, Svc, svc.namespace, svc.name}, "/") - - records = append(records, s) - } - // If the addr is not an IP (i.e. an external service), add the record ... - s := msg.Service{Key: strings.Join([]string{zonePath, Svc, svc.namespace, svc.name}, "/"), Host: svc.addr} - if t, _ := s.HostType(); t == dns.TypeCNAME { - s.Key = strings.Join([]string{zonePath, Svc, svc.namespace, svc.name}, "/") - - records = append(records, s) - } - } - - for _, p := range pods { - s := msg.Service{Key: strings.Join([]string{zonePath, Pod, p.namespace, p.name}, "/"), Host: p.addr} - records = append(records, s) - } - - return records -} - -func (k *Kubernetes) findPods(namespace, podname string) (pods []kPod, err error) { +func (k *Kubernetes) findPods(r recordRequest, zone string) (pods []msg.Service, err error) { if k.podMode == podModeDisabled { - return pods, errPodsDisabled + return nil, errPodsDisabled } - var ip string + namespace := r.namespace + podname := r.service + zonePath := msg.Path(zone, "coredns") + ip := "" + err = errNoItems + if strings.Count(podname, "-") == 3 && !strings.Contains(podname, "--") { ip = strings.Replace(podname, "-", ".", -1) } else { @@ -372,9 +314,7 @@ func (k *Kubernetes) findPods(namespace, podname string) (pods []kPod, err error } if k.podMode == podModeInsecure { - s := kPod{name: podname, namespace: namespace, addr: ip} - pods = append(pods, s) - return pods, nil + return []msg.Service{{Key: strings.Join([]string{zonePath, Pod, namespace, podname}, "/"), Host: ip}}, nil } // PodModeVerified @@ -391,29 +331,20 @@ func (k *Kubernetes) findPods(namespace, podname string) (pods []kPod, err error } // check for matching ip and namespace if ip == p.Status.PodIP && match(namespace, p.Namespace) { - s := kPod{name: podname, namespace: namespace, addr: ip} + s := msg.Service{Key: strings.Join([]string{zonePath, Pod, namespace, podname}, "/"), Host: ip} pods = append(pods, s) - return pods, nil + + err = nil } } - return pods, nil + return pods, err } -// get retrieves matching data from the cache. -func (k *Kubernetes) get(r recordRequest) (services []kService, pods []kPod, err error) { - switch r.podOrSvc { - case Pod: - pods, err = k.findPods(r.namespace, r.service) - return nil, pods, err - default: - services, err = k.findServices(r) - return services, nil, err - } -} - -func (k *Kubernetes) findServices(r recordRequest) ([]kService, error) { +// findServices returns the services matching r from the cache. +func (k *Kubernetes) findServices(r recordRequest, zone string) (services []msg.Service, err error) { serviceList := k.APIConn.ServiceList() - var resultItems []kService + zonePath := msg.Path(zone, "coredns") + err = errNoItems // Set to errNoItems to signal really nothing found, gets reset when name is matched. for _, svc := range serviceList { if !(match(r.namespace, svc.Namespace) && match(r.service, svc.Name)) { @@ -425,11 +356,9 @@ func (k *Kubernetes) findServices(r recordRequest) ([]kService, error) { if wildcard(r.namespace) && !k.namespaceExposed(svc.Namespace) { continue } - s := kService{name: svc.Name, namespace: svc.Namespace} // Endpoint query or headless service if svc.Spec.ClusterIP == api.ClusterIPNone || r.endpoint != "" { - s.addr = svc.Spec.ClusterIP endpointsList := k.APIConn.EndpointsList() for _, ep := range endpointsList.Items { if ep.ObjectMeta.Name != svc.Name || ep.ObjectMeta.Namespace != svc.Namespace { @@ -451,36 +380,47 @@ func (k *Kubernetes) findServices(r recordRequest) ([]kService, error) { if !(match(r.port, p.Name) && match(r.protocol, string(p.Protocol))) { continue } - s.endpoints = append(s.endpoints, endpoint{addr: addr, port: p}) + s := msg.Service{Host: addr.IP, Port: int(p.Port)} + s.Key = strings.Join([]string{zonePath, Svc, svc.Namespace, svc.Name, endpointHostname(addr)}, "/") + + err = nil + + services = append(services, s) } } } } - if len(s.endpoints) > 0 { - resultItems = append(resultItems, s) - } continue } // External service if svc.Spec.ExternalName != "" { - s.addr = svc.Spec.ExternalName - resultItems = append(resultItems, s) - continue + s := msg.Service{Key: strings.Join([]string{zonePath, Svc, svc.Namespace, svc.Name}, "/"), Host: svc.Spec.ExternalName} + if t, _ := s.HostType(); t == dns.TypeCNAME { + s.Key = strings.Join([]string{zonePath, Svc, svc.Namespace, svc.Name}, "/") + services = append(services, s) + + err = nil + + continue + } } // ClusterIP service - s.addr = svc.Spec.ClusterIP for _, p := range svc.Spec.Ports { if !(match(r.port, p.Name) && match(r.protocol, string(p.Protocol))) { continue } - s.ports = append(s.ports, p) - } - resultItems = append(resultItems, s) + err = nil + + s := msg.Service{Host: svc.Spec.ClusterIP, Port: int(p.Port)} + s.Key = strings.Join([]string{zonePath, Svc, svc.Namespace, svc.Name}, "/") + + services = append(services, s) + } } - return resultItems, nil + return services, err } // match checks if a and b are equal taking wildcards into account. @@ -499,39 +439,6 @@ func wildcard(s string) bool { return s == "*" || s == "any" } -// serviceRecordForIP gets a service record with a cluster ip matching the ip argument -// If a service cluster ip does not match, it checks all endpoints -func (k *Kubernetes) serviceRecordForIP(ip, name string) []msg.Service { - // First check services with cluster ips - svcList := k.APIConn.ServiceList() - - for _, service := range svcList { - if (len(k.Namespaces) > 0) && !k.namespaceExposed(service.Namespace) { - continue - } - if service.Spec.ClusterIP == ip { - domain := dnsutil.Join([]string{service.Name, service.Namespace, Svc, k.primaryZone()}) - return []msg.Service{{Host: domain}} - } - } - // If no cluster ips match, search endpoints - epList := k.APIConn.EndpointsList() - for _, ep := range epList.Items { - if (len(k.Namespaces) > 0) && !k.namespaceExposed(ep.ObjectMeta.Namespace) { - continue - } - for _, eps := range ep.Subsets { - for _, addr := range eps.Addresses { - if addr.IP == ip { - domain := dnsutil.Join([]string{endpointHostname(addr), ep.ObjectMeta.Name, ep.ObjectMeta.Namespace, Svc, k.primaryZone()}) - return []msg.Service{{Host: domain}} - } - } - } - } - return nil -} - // namespaceExposed returns true when the namespace is exposed. func (k *Kubernetes) namespaceExposed(namespace string) bool { _, ok := k.Namespaces[namespace] diff --git a/middleware/kubernetes/parse.go b/middleware/kubernetes/parse.go index 4b09b15f9..cadaaa3a1 100644 --- a/middleware/kubernetes/parse.go +++ b/middleware/kubernetes/parse.go @@ -26,7 +26,7 @@ type recordRequest struct { // parseRequest parses the qname to find all the elements we need for querying k8s. Anything // that is not parsed will have the wildcard "*" value (except r.endpoint). // Potential underscores are stripped from _port and _protocol. -func (k *Kubernetes) parseRequest(state request.Request) (r recordRequest, err error) { +func parseRequest(state request.Request) (r recordRequest, err error) { // 3 Possible cases: // 1. _port._protocol.service.namespace.pod|svc.zone // 2. (endpoint): endpoint.service.namespace.pod|svc.zone diff --git a/middleware/kubernetes/parse_test.go b/middleware/kubernetes/parse_test.go index e44fb09f4..06d5a2aaa 100644 --- a/middleware/kubernetes/parse_test.go +++ b/middleware/kubernetes/parse_test.go @@ -9,8 +9,6 @@ import ( ) func TestParseRequest(t *testing.T) { - k := New([]string{zone}) - tests := []struct { query string expected string // output from r.String() @@ -27,7 +25,7 @@ func TestParseRequest(t *testing.T) { m.SetQuestion(tc.query, dns.TypeA) state := request.Request{Zone: zone, Req: m} - r, e := k.parseRequest(state) + r, e := parseRequest(state) if e != nil { t.Errorf("Test %d, expected no error, got '%v'.", i, e) } @@ -39,8 +37,6 @@ func TestParseRequest(t *testing.T) { } func TestParseInvalidRequest(t *testing.T) { - k := New([]string{zone}) - invalid := []string{ "webs.mynamespace.pood.inter.webs.test.", // Request must be for pod or svc subdomain. "too.long.for.what.I.am.trying.to.pod.inter.webs.tests.", // Too long. @@ -51,7 +47,7 @@ func TestParseInvalidRequest(t *testing.T) { m.SetQuestion(query, dns.TypeA) state := request.Request{Zone: zone, Req: m} - if _, e := k.parseRequest(state); e == nil { + if _, e := parseRequest(state); e == nil { t.Errorf("Test %d: expected error from %s, got none", i, query) } } diff --git a/middleware/kubernetes/reverse.go b/middleware/kubernetes/reverse.go index 62b3a7043..eac5a0b04 100644 --- a/middleware/kubernetes/reverse.go +++ b/middleware/kubernetes/reverse.go @@ -1,6 +1,8 @@ package kubernetes import ( + "strings" + "github.com/coredns/coredns/middleware" "github.com/coredns/coredns/middleware/etcd/msg" "github.com/coredns/coredns/middleware/pkg/dnsutil" @@ -18,3 +20,36 @@ func (k *Kubernetes) Reverse(state request.Request, exact bool, opt middleware.O records := k.serviceRecordForIP(ip, state.Name()) return records, nil, nil } + +// serviceRecordForIP gets a service record with a cluster ip matching the ip argument +// If a service cluster ip does not match, it checks all endpoints +func (k *Kubernetes) serviceRecordForIP(ip, name string) []msg.Service { + // First check services with cluster ips + svcList := k.APIConn.ServiceList() + + for _, service := range svcList { + if (len(k.Namespaces) > 0) && !k.namespaceExposed(service.Namespace) { + continue + } + if service.Spec.ClusterIP == ip { + domain := strings.Join([]string{service.Name, service.Namespace, Svc, k.primaryZone()}, ".") + return []msg.Service{{Host: domain}} + } + } + // If no cluster ips match, search endpoints + epList := k.APIConn.EndpointsList() + for _, ep := range epList.Items { + if (len(k.Namespaces) > 0) && !k.namespaceExposed(ep.ObjectMeta.Namespace) { + continue + } + for _, eps := range ep.Subsets { + for _, addr := range eps.Addresses { + if addr.IP == ip { + domain := strings.Join([]string{endpointHostname(addr), ep.ObjectMeta.Name, ep.ObjectMeta.Namespace, Svc, k.primaryZone()}, ".") + return []msg.Service{{Host: domain}} + } + } + } + } + return nil +} diff --git a/test/kubernetes_test.go b/test/kubernetes_test.go index 870f55e9d..881394c00 100644 --- a/test/kubernetes_test.go +++ b/test/kubernetes_test.go @@ -214,7 +214,7 @@ var dnsTestCases = []test.Case{ }, { Qname: "*._not-udp-or-tcp.svc-1-a.test-1.svc.cluster.local.", Qtype: dns.TypeSRV, - Rcode: dns.RcodeSuccess, + Rcode: dns.RcodeNameError, Ns: []dns.RR{ test.SOA("cluster.local. 300 IN SOA ns.dns.cluster.local. hostmaster.cluster.local. 1499347823 7200 1800 86400 60"), },