From 9546b606cb58a372361b39604c1ccf54ed91c7b9 Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Sat, 22 Sep 2018 15:12:02 +0100 Subject: [PATCH] K8s remove string ops (#2119) * plugin/kubernetes: remove bunch a string ops This removes a bunch of appends to where not needed, makes dnsutil.Join take variadic args which removes the need to wrap in a new string slice. Signed-off-by: Miek Gieben * Fix calls to dnsutil.Join Signed-off-by: Miek Gieben * Revert these Signed-off-by: Miek Gieben --- plugin/backend_lookup.go | 2 +- plugin/etcd/msg/path.go | 2 +- plugin/etcd/stub.go | 2 +- plugin/federation/federation.go | 2 +- plugin/file/dname.go | 2 +- plugin/kubernetes/controller.go | 30 +++++++----------------------- plugin/kubernetes/federation.go | 4 ++-- plugin/kubernetes/kubernetes.go | 18 +++++++++++------- plugin/loop/setup.go | 2 +- plugin/pkg/dnsutil/join.go | 8 +++----- plugin/pkg/dnsutil/join_test.go | 3 ++- 11 files changed, 31 insertions(+), 44 deletions(-) diff --git a/plugin/backend_lookup.go b/plugin/backend_lookup.go index 65cdcbaf5..77e08174f 100644 --- a/plugin/backend_lookup.go +++ b/plugin/backend_lookup.go @@ -446,7 +446,7 @@ func checkForApex(b ServiceBackend, zone string, state request.Request, opt Opti // this is equivalent to the NS search code. old := state.QName() state.Clear() - state.Req.Question[0].Name = dnsutil.Join([]string{"apex.dns", zone}) + state.Req.Question[0].Name = dnsutil.Join("apex.dns", zone) services, err := b.Services(state, false, opt) if err == nil { diff --git a/plugin/etcd/msg/path.go b/plugin/etcd/msg/path.go index 2abdec0fe..dc7494782 100644 --- a/plugin/etcd/msg/path.go +++ b/plugin/etcd/msg/path.go @@ -26,7 +26,7 @@ func Domain(s string) string { for i, j := 1, len(l)-1; i < j; i, j = i+1, j-1 { l[i], l[j] = l[j], l[i] } - return dnsutil.Join(l[1 : len(l)-1]) + return dnsutil.Join(l[1 : len(l)-1]...) } // PathWithWildcard ascts as Path, but if a name contains wildcards (* or any), the name will be diff --git a/plugin/etcd/stub.go b/plugin/etcd/stub.go index c270bcd2b..37e426ac4 100644 --- a/plugin/etcd/stub.go +++ b/plugin/etcd/stub.go @@ -61,7 +61,7 @@ Services: // Chop of left most label, because that is used as the nameserver place holder // and drop the right most labels that belong to zone. // We must *also* chop of dns.stub. which means cutting two more labels. - domain = dnsutil.Join(labels[1 : len(labels)-dns.CountLabel(z)-2]) + domain = dnsutil.Join(labels[1 : len(labels)-dns.CountLabel(z)-2]...) if domain == z { log.Warningf("Skipping nameserver for domain we are authoritative for: %s", domain) continue Services diff --git a/plugin/federation/federation.go b/plugin/federation/federation.go index e968aa668..779636b6a 100644 --- a/plugin/federation/federation.go +++ b/plugin/federation/federation.go @@ -130,7 +130,7 @@ func (f *Federation) isNameFederation(name, zone string) (string, string) { fed := labels[ll-2] if _, ok := f.f[fed]; ok { - without := dnsutil.Join(labels[:ll-2]) + labels[ll-1] + "." + zone + without := dnsutil.Join(labels[:ll-2]...) + labels[ll-1] + "." + zone return without, fed } return "", "" diff --git a/plugin/file/dname.go b/plugin/file/dname.go index f552bfdfd..58351a350 100644 --- a/plugin/file/dname.go +++ b/plugin/file/dname.go @@ -14,7 +14,7 @@ func substituteDNAME(qname, owner, target string) string { labels := dns.SplitDomainName(qname) labels = append(labels[0:len(labels)-dns.CountLabel(owner)], dns.SplitDomainName(target)...) - return dnsutil.Join(labels) + return dnsutil.Join(labels...) } return "" diff --git a/plugin/kubernetes/controller.go b/plugin/kubernetes/controller.go index 286f87d8e..db231c89d 100644 --- a/plugin/kubernetes/controller.go +++ b/plugin/kubernetes/controller.go @@ -72,7 +72,7 @@ type dnsControl struct { svcLister cache.Indexer podLister cache.Indexer epLister cache.Indexer - nsLister storeToNamespaceLister + nsLister cache.Store // stopLock is used to enforce only a single call to Stop is active. // Needed because we allow stopping through an http endpoint and @@ -146,7 +146,7 @@ func newdnsController(kubeClient *kubernetes.Clientset, opts dnsControlOpts) *dn cache.Indexers{epNameNamespaceIndex: epNameNamespaceIndexFunc, epIPIndex: epIPIndexFunc}) } - dns.nsLister.Store, dns.nsController = cache.NewInformer( + dns.nsLister, dns.nsController = cache.NewInformer( &cache.ListWatch{ ListFunc: namespaceListFunc(dns.client, dns.selector), WatchFunc: namespaceWatchFunc(dns.client, dns.selector), @@ -156,11 +156,6 @@ func newdnsController(kubeClient *kubernetes.Clientset, opts dnsControlOpts) *dn return &dns } -// storeToNamespaceLister makes a Store that lists Namespaces. -type storeToNamespaceLister struct { - cache.Store -} - func podIPIndexFunc(obj interface{}) ([]string, error) { p, ok := obj.(*api.Pod) if !ok { @@ -311,9 +306,8 @@ func namespaceWatchFunc(c *kubernetes.Clientset, s labels.Selector) func(options } } -func (dns *dnsControl) SetWatchChan(c dnswatch.Chan) { - dns.watchChan = c -} +func (dns *dnsControl) SetWatchChan(c dnswatch.Chan) { dns.watchChan = c } +func (dns *dnsControl) StopWatching(qname string) { delete(dns.watched, qname) } func (dns *dnsControl) Watch(qname string) error { if dns.watchChan == nil { @@ -323,10 +317,6 @@ func (dns *dnsControl) Watch(qname string) error { return nil } -func (dns *dnsControl) StopWatching(qname string) { - delete(dns.watched, qname) -} - // Stop stops the controller. func (dns *dnsControl) Stop() error { dns.stopLock.Lock() @@ -621,15 +611,9 @@ func (dns *dnsControl) sendUpdates(oldObj, newObj interface{}) { } } -func (dns *dnsControl) Add(obj interface{}) { - dns.sendUpdates(nil, obj) -} -func (dns *dnsControl) Delete(obj interface{}) { - dns.sendUpdates(obj, nil) -} -func (dns *dnsControl) Update(oldObj, newObj interface{}) { - dns.sendUpdates(oldObj, newObj) -} +func (dns *dnsControl) Add(obj interface{}) { dns.sendUpdates(nil, obj) } +func (dns *dnsControl) Delete(obj interface{}) { dns.sendUpdates(obj, nil) } +func (dns *dnsControl) Update(oldObj, newObj interface{}) { dns.sendUpdates(oldObj, newObj) } // subsetsEquivalent checks if two endpoint subsets are significantly equivalent // I.e. that they have the same ready addresses, host names, ports (including protocol diff --git a/plugin/kubernetes/federation.go b/plugin/kubernetes/federation.go index 78e31668b..bf169b911 100644 --- a/plugin/kubernetes/federation.go +++ b/plugin/kubernetes/federation.go @@ -44,8 +44,8 @@ func (k *Kubernetes) Federations(state request.Request, fname, fzone string) (ms } if r.endpoint == "" { - return msg.Service{Host: dnsutil.Join([]string{r.service, r.namespace, fname, r.podOrSvc, lz, lr, fzone})}, nil + return msg.Service{Host: dnsutil.Join(r.service, r.namespace, fname, r.podOrSvc, lz, lr, fzone)}, nil } - return msg.Service{Host: dnsutil.Join([]string{r.endpoint, r.service, r.namespace, fname, r.podOrSvc, lz, lr, fzone})}, nil + return msg.Service{Host: dnsutil.Join(r.endpoint, r.service, r.namespace, fname, r.podOrSvc, lz, lr, fzone)}, nil } diff --git a/plugin/kubernetes/kubernetes.go b/plugin/kubernetes/kubernetes.go index 4bdcd4877..9eac7d467 100644 --- a/plugin/kubernetes/kubernetes.go +++ b/plugin/kubernetes/kubernetes.go @@ -296,14 +296,18 @@ func (k *Kubernetes) Records(state request.Request, exact bool) ([]msg.Service, // serviceFQDN returns the k8s cluster dns spec service FQDN for the service (or endpoint) object. func serviceFQDN(obj meta.Object, zone string) string { - return dnsutil.Join(append([]string{}, obj.GetName(), obj.GetNamespace(), Svc, zone)) + return dnsutil.Join(obj.GetName(), obj.GetNamespace(), Svc, zone) } // podFQDN returns the k8s cluster dns spec FQDN for the pod. func podFQDN(p *api.Pod, zone string) string { - name := strings.Replace(p.Status.PodIP, ".", "-", -1) - name = strings.Replace(name, ":", "-", -1) - return dnsutil.Join(append([]string{}, name, p.GetNamespace(), Pod, zone)) + if strings.Contains(p.Status.PodIP, ".") { + name := strings.Replace(p.Status.PodIP, ".", "-", -1) + return dnsutil.Join(name, p.GetNamespace(), Pod, zone) + } + + name := strings.Replace(p.Status.PodIP, ":", "-", -1) + return dnsutil.Join(name, p.GetNamespace(), Pod, zone) } // endpointFQDN returns a list of k8s cluster dns spec service FQDNs for each subset in the endpoint. @@ -311,7 +315,7 @@ func endpointFQDN(ep *api.Endpoints, zone string, endpointNameMode bool) []strin var names []string for _, ss := range ep.Subsets { for _, addr := range ss.Addresses { - names = append(names, dnsutil.Join(append([]string{}, endpointHostname(addr, endpointNameMode), serviceFQDN(ep, zone)))) + names = append(names, dnsutil.Join(endpointHostname(addr, endpointNameMode), serviceFQDN(ep, zone))) } } return names @@ -319,7 +323,7 @@ func endpointFQDN(ep *api.Endpoints, zone string, endpointNameMode bool) []strin func endpointHostname(addr api.EndpointAddress, endpointNameMode bool) string { if addr.Hostname != "" { - return strings.ToLower(addr.Hostname) + return addr.Hostname } if endpointNameMode && addr.TargetRef != nil && addr.TargetRef.Name != "" { return addr.TargetRef.Name @@ -328,7 +332,7 @@ func endpointHostname(addr api.EndpointAddress, endpointNameMode bool) string { return strings.Replace(addr.IP, ".", "-", -1) } if strings.Contains(addr.IP, ":") { - return strings.ToLower(strings.Replace(addr.IP, ":", "-", -1)) + return strings.Replace(addr.IP, ":", "-", -1) } return "" } diff --git a/plugin/loop/setup.go b/plugin/loop/setup.go index 28829fcb4..93a8bb13b 100644 --- a/plugin/loop/setup.go +++ b/plugin/loop/setup.go @@ -85,7 +85,7 @@ func qname(zone string) string { l1 := strconv.Itoa(r.Int()) l2 := strconv.Itoa(r.Int()) - return dnsutil.Join([]string{l1, l2, zone}) + return dnsutil.Join(l1, l2, zone) } var r = rand.New(rand.NewSource(time.Now().UnixNano())) diff --git a/plugin/pkg/dnsutil/join.go b/plugin/pkg/dnsutil/join.go index 515bf3dad..b3a40db42 100644 --- a/plugin/pkg/dnsutil/join.go +++ b/plugin/pkg/dnsutil/join.go @@ -8,12 +8,10 @@ import ( // Join joins labels to form a fully qualified domain name. If the last label is // the root label it is ignored. Not other syntax checks are performed. -func Join(labels []string) string { +func Join(labels ...string) string { ll := len(labels) if labels[ll-1] == "." { - s := strings.Join(labels[:ll-1], ".") - return dns.Fqdn(s) + return strings.Join(labels[:ll-1], ".") + "." } - s := strings.Join(labels, ".") - return dns.Fqdn(s) + return dns.Fqdn(strings.Join(labels, ".")) } diff --git a/plugin/pkg/dnsutil/join_test.go b/plugin/pkg/dnsutil/join_test.go index 26eeb5897..1a50a3c99 100644 --- a/plugin/pkg/dnsutil/join_test.go +++ b/plugin/pkg/dnsutil/join_test.go @@ -9,11 +9,12 @@ func TestJoin(t *testing.T) { }{ {[]string{"bla", "bliep", "example", "org"}, "bla.bliep.example.org."}, {[]string{"example", "."}, "example."}, + {[]string{"example", "org."}, "example.org."}, // technically we should not be called like this. {[]string{"."}, "."}, } for i, tc := range tests { - if x := Join(tc.in); x != tc.out { + if x := Join(tc.in...); x != tc.out { t.Errorf("Test %d, expected %s, got %s", i, tc.out, x) } }