discovery: simplify interface

There's no real need to expose a Discoverer interface/struct when the
only use of the interface (and indeed the module) is to invoke a single
function. This isn't Java, after all. So instead, simplify to Discovery
exposing just two functions: JoinCluster (i.e. what was formerly called
"discovery"), and GetCluster (hitherto "ProxyDiscovery")
This commit is contained in:
Jonathan Boulle 2014-11-05 22:45:01 -08:00
parent 6fdbb086f4
commit 68bca981de
4 changed files with 32 additions and 50 deletions

View File

@ -52,16 +52,29 @@ const (
nRetries = uint(3) nRetries = uint(3)
) )
type Discoverer interface { // JoinCluster will connect to the discovery service at the given url, and
// Discover connects to a discovery service and retrieves a string // register the server represented by the given id and config to the cluster
// describing an etcd cluster, and any error encountered. func JoinCluster(durl string, id types.ID, config string) (string, error) {
Discover() (string, error) d, err := newDiscovery(durl, id)
if err != nil {
return "", err
}
return d.joinCluster(config)
}
// GetCluster will connect to the discovery service at the given url and
// retrieve a string describing the cluster
func GetCluster(durl string) (string, error) {
d, err := newDiscovery(durl, 0)
if err != nil {
return "", err
}
return d.getCluster()
} }
type discovery struct { type discovery struct {
cluster string cluster string
id types.ID id types.ID
config string
c client.KeysAPI c client.KeysAPI
retries uint retries uint
url *url.URL url *url.URL
@ -69,27 +82,6 @@ type discovery struct {
clock clockwork.Clock clock clockwork.Clock
} }
type proxyDiscovery struct {
*discovery
}
// New returns a Discoverer which will connect to the discovery service at
// the given url, and register the server represented by the given id and
// config to the cluster during the discovery process
func New(durl string, id types.ID, config string) (Discoverer, error) {
return newDiscovery(durl, id, config)
}
// ProxyNew returns a Discoverer which will connect to the discovery service
// at the given url and retrieve a string describing the cluster
func ProxyNew(durl string) (Discoverer, error) {
d, err := newDiscovery(durl, 0, "")
if err != nil {
return nil, err
}
return &proxyDiscovery{d}, nil
}
// proxyFuncFromEnv builds a proxy function if the appropriate environment // proxyFuncFromEnv builds a proxy function if the appropriate environment
// variable is set. It performs basic sanitization of the environment variable // variable is set. It performs basic sanitization of the environment variable
// and returns any error encountered. // and returns any error encountered.
@ -119,7 +111,7 @@ func proxyFuncFromEnv() (func(*http.Request) (*url.URL, error), error) {
return http.ProxyURL(proxyURL), nil return http.ProxyURL(proxyURL), nil
} }
func newDiscovery(durl string, id types.ID, config string) (*discovery, error) { func newDiscovery(durl string, id types.ID) (*discovery, error) {
u, err := url.Parse(durl) u, err := url.Parse(durl)
if err != nil { if err != nil {
return nil, err return nil, err
@ -137,23 +129,21 @@ func newDiscovery(durl string, id types.ID, config string) (*discovery, error) {
dc := client.NewDiscoveryKeysAPI(c) dc := client.NewDiscoveryKeysAPI(c)
return &discovery{ return &discovery{
cluster: token, cluster: token,
id: id,
config: config,
c: dc, c: dc,
id: id,
url: u, url: u,
clock: clockwork.NewRealClock(), clock: clockwork.NewRealClock(),
}, nil }, nil
} }
func (d *discovery) Discover() (string, error) { func (d *discovery) joinCluster(config string) (string, error) {
// fast path: if the cluster is full, returns the error // fast path: if the cluster is full, return the error
// do not need to register itself to the cluster in this // do not need to register to the cluster in this case.
// case.
if _, _, err := d.checkCluster(); err != nil { if _, _, err := d.checkCluster(); err != nil {
return "", err return "", err
} }
if err := d.createSelf(); err != nil { if err := d.createSelf(config); err != nil {
// Fails, even on a timeout, if createSelf times out. // Fails, even on a timeout, if createSelf times out.
// TODO(barakmich): Retrying the same node might want to succeed here // TODO(barakmich): Retrying the same node might want to succeed here
// (ie, createSelf should be idempotent for discovery). // (ie, createSelf should be idempotent for discovery).
@ -173,8 +163,8 @@ func (d *discovery) Discover() (string, error) {
return nodesToCluster(all), nil return nodesToCluster(all), nil
} }
func (pd *proxyDiscovery) Discover() (string, error) { func (d *discovery) getCluster() (string, error) {
nodes, size, err := pd.checkCluster() nodes, size, err := d.checkCluster()
if err != nil { if err != nil {
if err == ErrFullCluster { if err == ErrFullCluster {
return nodesToCluster(nodes), nil return nodesToCluster(nodes), nil
@ -182,16 +172,16 @@ func (pd *proxyDiscovery) Discover() (string, error) {
return "", err return "", err
} }
all, err := pd.waitNodes(nodes, size) all, err := d.waitNodes(nodes, size)
if err != nil { if err != nil {
return "", err return "", err
} }
return nodesToCluster(all), nil return nodesToCluster(all), nil
} }
func (d *discovery) createSelf() error { func (d *discovery) createSelf(contents string) error {
ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout)
resp, err := d.c.Create(ctx, d.selfKey(), d.config, -1) resp, err := d.c.Create(ctx, d.selfKey(), contents, -1)
cancel() cancel()
if err != nil { if err != nil {
return err return err

View File

@ -322,7 +322,7 @@ func TestCreateSelf(t *testing.T) {
for i, tt := range tests { for i, tt := range tests {
d := discovery{cluster: "1000", c: tt.c} d := discovery{cluster: "1000", c: tt.c}
if err := d.createSelf(); err != tt.werr { if err := d.createSelf(""); err != tt.werr {
t.Errorf("#%d: err = %v, want %v", i, err, nil) t.Errorf("#%d: err = %v, want %v", i, err, nil)
} }
} }

View File

@ -297,11 +297,7 @@ func startProxy() error {
} }
if *durl != "" { if *durl != "" {
d, err := discovery.ProxyNew(*durl) s, err := discovery.GetCluster(*durl)
if err != nil {
return fmt.Errorf("cannot init discovery %v", err)
}
s, err := d.Discover()
if err != nil { if err != nil {
return err return err
} }

View File

@ -203,11 +203,7 @@ func NewServer(cfg *ServerConfig) (*EtcdServer, error) {
} }
m := cfg.Cluster.MemberByName(cfg.Name) m := cfg.Cluster.MemberByName(cfg.Name)
if cfg.ShouldDiscover() { if cfg.ShouldDiscover() {
d, err := discovery.New(cfg.DiscoveryURL, m.ID, cfg.Cluster.String()) s, err := discovery.JoinCluster(cfg.DiscoveryURL, m.ID, cfg.Cluster.String())
if err != nil {
return nil, fmt.Errorf("cannot init discovery %v", err)
}
s, err := d.Discover()
if err != nil { if err != nil {
return nil, err return nil, err
} }