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 Aug 30, 2020
1 parent 5561005 commit d75700a
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 2 deletions.
13 changes: 11 additions & 2 deletions sd/etcdv3/client.go
Expand Up @@ -6,6 +6,8 @@ import (
"errors"
"time"

"google.golang.org/grpc"

"go.etcd.io/etcd/clientv3"
"go.etcd.io/etcd/pkg/transport"
)
Expand Down Expand Up @@ -73,8 +75,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 +115,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")
}
}
5 changes: 5 additions & 0 deletions sd/etcdv3/example_test.go
Expand Up @@ -5,6 +5,8 @@ import (
"io"
"time"

"google.golang.org/grpc"

"github.com/go-kit/kit/endpoint"
"github.com/go-kit/kit/log"
"github.com/go-kit/kit/sd"
Expand Down Expand Up @@ -44,6 +46,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 d75700a

Please sign in to comment.