Skip to content

Commit

Permalink
do not resolve redis IP, but rather pass hostname to redis.Client opts (
Browse files Browse the repository at this point in the history
  • Loading branch information
nonsense authored May 28, 2020
1 parent 3bcbe05 commit f97396e
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 63 deletions.
54 changes: 13 additions & 41 deletions sync/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package sync
import (
"context"
"fmt"
"net"
"os"
"strconv"
"sync"
Expand All @@ -18,10 +17,8 @@ import (
const (
RedisPayloadKey = "p"

EnvRedisHost = "REDIS_HOST"
EnvRedisPort = "REDIS_PORT"
RedisHostname = "testground-redis"
HostHostname = "host.docker.internal"
EnvRedisHost = "REDIS_HOST"
EnvRedisPort = "REDIS_PORT"
)

// ErrNoRunParameters is returned by the generic client when an unbound context
Expand Down Expand Up @@ -189,44 +186,19 @@ func redisClient(ctx context.Context, log *zap.SugaredLogger) (client *redis.Cli
}
}

var tryHosts []string
if host == "" {
// Try to resolve the "testground-redis" host from Docker's DNS.
//
// Fall back to attempting to use `host.docker.internal` which
// is only available in macOS and Windows.
// Finally, falling back on localhost (for local:exec)
tryHosts = []string{RedisHostname, HostHostname, "localhost"}
} else {
tryHosts = []string{host}
}
log.Debugw("trying redis host", "host", host, "port", port)

for _, h := range tryHosts {
log.Debugw("resolving redis host", "host", h)
opts := DefaultRedisOpts
opts.Addr = fmt.Sprintf("%s:%d", host, port)
client = redis.NewClient(&opts).WithContext(ctx)

addrs, err := net.DefaultResolver.LookupIPAddr(ctx, h)
if err != nil {
log.Debugw("failed to resolve redis host", "host", h, "error", err)
continue
}
for _, addr := range addrs {
log.Debugw("trying redis host", "host", h, "address", addr, "error", err)
opts := DefaultRedisOpts // copy to be safe.
// Use TCPAddr to properly handle IPv6 addresses.
opts.Addr = (&net.TCPAddr{IP: addr.IP, Zone: addr.Zone, Port: port}).String()
client = redis.NewClient(&opts).WithContext(ctx)

// PING redis to make sure we're alive.
if err := client.Ping().Err(); err != nil {
_ = client.Close()
log.Debugw("failed to ping redis host", "host", h, "address", addr, "error", err)
continue
}
if err := client.Ping().Err(); err != nil {
_ = client.Close()
log.Errorw("failed to ping redis host", "host", host, "port", port, "error", err)
return nil, err
}

log.Debugw("redis ping OK", "opts", opts)
log.Debugw("redis ping OK", "opts", opts)

return client, nil
}
}
return nil, fmt.Errorf("no viable redis host found")
return client, nil
}
23 changes: 1 addition & 22 deletions sync/redis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package sync

import (
"context"
"net"
"os"
"testing"
"time"
Expand Down Expand Up @@ -39,29 +38,9 @@ func TestRedisHost(t *testing.T) {
_ = os.Setenv(EnvRedisHost, realHost)
client, err = redisClient(ctx, zap.S())
if err != nil {
t.Errorf("should have found the redis host, failed with: %s", err)
t.Errorf("expected to establish connection to redis, but failed with: %s", err)
}
addr := client.Options().Addr
_ = client.Close()
host, _, err := net.SplitHostPort(addr)
if err != nil {
t.Fatal(err)
}
hostIP := net.ParseIP(host)
if hostIP == nil {
t.Fatal("expected host to be an IP")
}
addrs, err := net.LookupIP(realHost)
if err != nil {
t.Fatal("failed to resolve redis host")
}
for _, a := range addrs {
if a.Equal(hostIP) {
// Success!
return
}
}
t.Fatal("redis address not found in list of addresses")
}

func TestConnUnblock(t *testing.T) {
Expand Down

0 comments on commit f97396e

Please sign in to comment.