From 9f4970250967bd2f59dd3db5795fcc4c720590c4 Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Tue, 21 Dec 2021 21:12:17 +0100 Subject: [PATCH 1/6] grpcutil.Resolver.Resolve: Take a service parameter instead of hardcoding grpclb MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Peter Štibraný Signed-off-by: Arve Knudsen --- CHANGELOG.md | 1 + grpcutil/dns_resolver.go | 38 ++++++++++++++++++++++++-------------- grpcutil/naming.go | 2 +- 3 files changed, 26 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e6a276960..55bcd009f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ * [CHANGE] Remove package `math`. #104 * [CHANGE] time: Remove time package. #103 * [CHANGE] grpcutil: Convert Resolver into concrete type. #105 +* [CHANGE] grpcutil.Resolver.Resolve: Take a service parameter. #102 * [ENHANCEMENT] Add middleware package. #38 * [ENHANCEMENT] Add the ring package #45 * [ENHANCEMENT] Add limiter package. #41 diff --git a/grpcutil/dns_resolver.go b/grpcutil/dns_resolver.go index f36ed9b93..f26718ec6 100644 --- a/grpcutil/dns_resolver.go +++ b/grpcutil/dns_resolver.go @@ -101,8 +101,11 @@ func parseTarget(target string) (host, port string, err error) { return "", "", fmt.Errorf("invalid target address %v", target) } -// Resolve creates a watcher that watches the name resolution of the target. -func (r *Resolver) Resolve(target string) (Watcher, error) { +// Resolve creates a watcher that watches the SRV/hostname record resolution of the target. +// +// If service is not empty, the watcher will first attempt to resolve an SRV record. +// If that fails, or service is empty, hostname record resolution is attempted instead. +func (r *Resolver) Resolve(target, service string) (Watcher, error) { host, port, err := parseTarget(target) if err != nil { return nil, err @@ -119,22 +122,24 @@ func (r *Resolver) Resolve(target string) (Watcher, error) { ctx, cancel := context.WithCancel(context.Background()) return &dnsWatcher{ - r: r, - logger: r.logger, - host: host, - port: port, - ctx: ctx, - cancel: cancel, - t: time.NewTimer(0), + r: r, + logger: r.logger, + host: host, + port: port, + service: service, + ctx: ctx, + cancel: cancel, + t: time.NewTimer(0), }, nil } // dnsWatcher watches for the name resolution update for a specific target type dnsWatcher struct { r *Resolver - logger log.Logger - host string - port string + logger log.Logger + host string + port string + service string // The latest resolved address set curAddrs map[string]*Update ctx context.Context @@ -203,8 +208,13 @@ func (w *dnsWatcher) compileUpdate(newAddrs map[string]*Update) []*Update { } func (w *dnsWatcher) lookupSRV() map[string]*Update { + if w.service == "" { + level.Debug(w.logger).Log("msg", "not looking up DNS SRV record since w.service is empty") + return nil + } + newAddrs := make(map[string]*Update) - _, srvs, err := lookupSRV(w.ctx, "grpclb", "tcp", w.host) + _, srvs, err := lookupSRV(w.ctx, w.service, "tcp", w.host) if err != nil { level.Info(w.logger).Log("msg", "failed DNS SRV record lookup", "err", err) return nil @@ -251,7 +261,7 @@ func (w *dnsWatcher) lookupHost() map[string]*Update { func (w *dnsWatcher) lookup() []*Update { newAddrs := w.lookupSRV() if newAddrs == nil { - // If failed to get any balancer address (either no corresponding SRV for the + // If we failed to get any balancer address (either no corresponding SRV for the // target, or caused by failure during resolution/parsing of the balancer target), // return any A record info available. newAddrs = w.lookupHost() diff --git a/grpcutil/naming.go b/grpcutil/naming.go index 8a1458690..68a52a663 100644 --- a/grpcutil/naming.go +++ b/grpcutil/naming.go @@ -24,7 +24,7 @@ type Update struct { Metadata interface{} } -// Watcher watches for the updates on the specified target. +// Watcher watches for the SRV updates on the specified target. type Watcher interface { // Next blocks until an update or error happens. It may return one or more // updates. The first call should get the full set of the results. It should From 4678df8595afb47aa126897eb6201ae6b8224add Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Thu, 23 Dec 2021 16:59:06 +0100 Subject: [PATCH 2/6] grpcutil: Remove gRPC load balancer metadata Signed-off-by: Arve Knudsen --- CHANGELOG.md | 1 + grpcutil/dns_resolver.go | 23 +---------------------- 2 files changed, 2 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 55bcd009f..9a1a8b54a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ * [CHANGE] time: Remove time package. #103 * [CHANGE] grpcutil: Convert Resolver into concrete type. #105 * [CHANGE] grpcutil.Resolver.Resolve: Take a service parameter. #102 +* [CHANGE] grpcutil.Update: Remove gRPC LB related metadata. #102 * [ENHANCEMENT] Add middleware package. #38 * [ENHANCEMENT] Add the ring package #45 * [ENHANCEMENT] Add limiter package. #41 diff --git a/grpcutil/dns_resolver.go b/grpcutil/dns_resolver.go index f26718ec6..f20359106 100644 --- a/grpcutil/dns_resolver.go +++ b/grpcutil/dns_resolver.go @@ -169,26 +169,6 @@ func (i *ipWatcher) Close() { close(i.updateChan) } -// AddressType indicates the address type returned by name resolution. -type AddressType uint8 - -const ( - // Backend indicates the server is a backend server. - Backend AddressType = iota - // GRPCLB indicates the server is a grpclb load balancer. - GRPCLB -) - -// AddrMetadataGRPCLB contains the information the name resolver for grpclb should provide. The -// name resolver used by the grpclb balancer is required to provide this type of metadata in -// its address updates. -type AddrMetadataGRPCLB struct { - // AddrType is the type of server (grpc load balancer or backend). - AddrType AddressType - // ServerName is the name of the grpc load balancer. Used for authentication. - ServerName string -} - // compileUpdate compares the old resolved addresses and newly resolved addresses, // and generates an update list func (w *dnsWatcher) compileUpdate(newAddrs map[string]*Update) []*Update { @@ -232,8 +212,7 @@ func (w *dnsWatcher) lookupSRV() map[string]*Update { continue } addr := a + ":" + strconv.Itoa(int(s.Port)) - newAddrs[addr] = &Update{Addr: addr, - Metadata: AddrMetadataGRPCLB{AddrType: GRPCLB, ServerName: s.Target}} + newAddrs[addr] = &Update{Addr: addr} } } return newAddrs From a76324f94b1f8ddfb99c5c9398a8cd68772a196e Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Thu, 23 Dec 2021 17:11:30 +0100 Subject: [PATCH 3/6] grpcutil.Resolver.Resolve: Improve doc comment Signed-off-by: Arve Knudsen --- grpcutil/dns_resolver.go | 1 + 1 file changed, 1 insertion(+) diff --git a/grpcutil/dns_resolver.go b/grpcutil/dns_resolver.go index f20359106..03382328e 100644 --- a/grpcutil/dns_resolver.go +++ b/grpcutil/dns_resolver.go @@ -105,6 +105,7 @@ func parseTarget(target string) (host, port string, err error) { // // If service is not empty, the watcher will first attempt to resolve an SRV record. // If that fails, or service is empty, hostname record resolution is attempted instead. +// If target can be parsed as an IP address, the watcher will return it, and will not send any more updates afterwards. func (r *Resolver) Resolve(target, service string) (Watcher, error) { host, port, err := parseTarget(target) if err != nil { From 18bbc5eac184d7115a77d7fa3e453b3da41ba62b Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Thu, 23 Dec 2021 17:14:59 +0100 Subject: [PATCH 4/6] grpcutil: Remove load balancer references Signed-off-by: Arve Knudsen --- grpcutil/dns_resolver.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/grpcutil/dns_resolver.go b/grpcutil/dns_resolver.go index 03382328e..0c6f2f05b 100644 --- a/grpcutil/dns_resolver.go +++ b/grpcutil/dns_resolver.go @@ -136,7 +136,7 @@ func (r *Resolver) Resolve(target, service string) (Watcher, error) { // dnsWatcher watches for the name resolution update for a specific target type dnsWatcher struct { - r *Resolver + r *Resolver logger log.Logger host string port string @@ -201,12 +201,12 @@ func (w *dnsWatcher) lookupSRV() map[string]*Update { return nil } for _, s := range srvs { - lbAddrs, err := lookupHost(w.ctx, s.Target) + addrs, err := lookupHost(w.ctx, s.Target) if err != nil { - level.Warn(w.logger).Log("msg", "failed load balancer address DNS lookup", "err", err) + level.Warn(w.logger).Log("msg", "failed SRV target DNS lookup", "target", s.Target, "err", err) continue } - for _, a := range lbAddrs { + for _, a := range addrs { a, ok := formatIP(a) if !ok { level.Error(w.logger).Log("failed IP parsing", "err", err) @@ -241,8 +241,7 @@ func (w *dnsWatcher) lookupHost() map[string]*Update { func (w *dnsWatcher) lookup() []*Update { newAddrs := w.lookupSRV() if newAddrs == nil { - // If we failed to get any balancer address (either no corresponding SRV for the - // target, or caused by failure during resolution/parsing of the balancer target), + // If we failed to get any valid addresses from SRV record lookup, // return any A record info available. newAddrs = w.lookupHost() } From b0d3ac660de646e238dc8e680c4359e6ad52435e Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Tue, 4 Jan 2022 16:10:29 +0100 Subject: [PATCH 5/6] grpcutil: Remove debug log message Signed-off-by: Arve Knudsen --- grpcutil/dns_resolver.go | 1 - 1 file changed, 1 deletion(-) diff --git a/grpcutil/dns_resolver.go b/grpcutil/dns_resolver.go index 0c6f2f05b..ef9c63989 100644 --- a/grpcutil/dns_resolver.go +++ b/grpcutil/dns_resolver.go @@ -190,7 +190,6 @@ func (w *dnsWatcher) compileUpdate(newAddrs map[string]*Update) []*Update { func (w *dnsWatcher) lookupSRV() map[string]*Update { if w.service == "" { - level.Debug(w.logger).Log("msg", "not looking up DNS SRV record since w.service is empty") return nil } From 1fb802dc9dcbacfcf9cbaf3b3d667458c51889a5 Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Tue, 4 Jan 2022 16:44:19 +0100 Subject: [PATCH 6/6] grpcutil: Improve comment Signed-off-by: Arve Knudsen --- grpcutil/naming.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/grpcutil/naming.go b/grpcutil/naming.go index 68a52a663..802932440 100644 --- a/grpcutil/naming.go +++ b/grpcutil/naming.go @@ -24,7 +24,7 @@ type Update struct { Metadata interface{} } -// Watcher watches for the SRV updates on the specified target. +// Watcher watches for SRV updates on the specified target. type Watcher interface { // Next blocks until an update or error happens. It may return one or more // updates. The first call should get the full set of the results. It should