Skip to content

Commit

Permalink
Fix etcdv3 client won't return error when no endpoint is available (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
wayjam committed Sep 13, 2020
1 parent 6860cdd commit 4133f7f
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 2 deletions.
12 changes: 10 additions & 2 deletions sd/etcdv3/client.go
Expand Up @@ -8,6 +8,7 @@ import (

"go.etcd.io/etcd/clientv3"
"go.etcd.io/etcd/pkg/transport"
"google.golang.org/grpc"
)

var (
Expand Down Expand Up @@ -73,8 +74,14 @@ type ClientOptions struct {
CACert string
DialTimeout time.Duration
DialKeepAlive time.Duration
Username string
Password string

// DialOptions is a list of dial options for the gRPC client (e.g., for interceptors).
// For example, pass grpc.WithBlock() to block until the underlying connection is up.
// Without this, Dial returns immediately and connecting the server happens in background.
DialOptions []grpc.DialOption

Username string
Password string
}

// NewClient returns Client with a connection to the named machines. It will
Expand Down Expand Up @@ -107,6 +114,7 @@ func NewClient(ctx context.Context, machines []string, options ClientOptions) (C
Endpoints: machines,
DialTimeout: options.DialTimeout,
DialKeepAliveTime: options.DialKeepAlive,
DialOptions: options.DialOptions,
TLS: tlscfg,
Username: options.Username,
Password: options.Password,
Expand Down
80 changes: 80 additions & 0 deletions sd/etcdv3/client_test.go
@@ -0,0 +1,80 @@
package etcdv3

import (
"context"
"testing"
"time"

"google.golang.org/grpc"
)

const (
// irrelevantEndpoint is an address which does not exists.
irrelevantEndpoint = "http://irrelevant:12345"
)

func TestNewClient(t *testing.T) {
client, err := NewClient(
context.Background(),
[]string{irrelevantEndpoint},
ClientOptions{
DialTimeout: 3 * time.Second,
DialKeepAlive: 3 * time.Second,
},
)
if err != nil {
t.Fatalf("unexpected error creating client: %v", err)
}
if client == nil {
t.Fatal("expected new Client, got nil")
}
}

func TestClientOptions(t *testing.T) {
client, err := NewClient(
context.Background(),
[]string{},
ClientOptions{
Cert: "",
Key: "",
CACert: "",
DialTimeout: 3 * time.Second,
DialKeepAlive: 3 * time.Second,
},
)
if err == nil {
t.Errorf("expected error: %v", err)
}
if client != nil {
t.Fatalf("expected client to be nil on failure")
}

_, err = NewClient(
context.Background(),
[]string{irrelevantEndpoint},
ClientOptions{
Cert: "does-not-exist.crt",
Key: "does-not-exist.key",
CACert: "does-not-exist.CACert",
DialTimeout: 3 * time.Second,
DialKeepAlive: 3 * time.Second,
},
)
if err == nil {
t.Errorf("expected error: %v", err)
}

client, err = NewClient(
context.Background(),
[]string{irrelevantEndpoint},
ClientOptions{
DialOptions: []grpc.DialOption{grpc.WithBlock()},
},
)
if err == nil {
t.Errorf("expected connection should fail")
}
if client != nil {
t.Errorf("expected client to be nil on failure")
}
}
4 changes: 4 additions & 0 deletions sd/etcdv3/example_test.go
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/go-kit/kit/log"
"github.com/go-kit/kit/sd"
"github.com/go-kit/kit/sd/lb"
"google.golang.org/grpc"
)

func Example() {
Expand Down Expand Up @@ -44,6 +45,9 @@ func Example() {

// If DialKeepAlive is 0, it defaults to 3s
DialKeepAlive: time.Second * 3,

// If passing `grpc.WithBlock`, dial connection will block until success.
DialOptions: []grpc.DialOption{grpc.WithBlock()},
}

// Build the client.
Expand Down

0 comments on commit 4133f7f

Please sign in to comment.