Merge pull request #1626 from jonboulle/proxy_stuff

discovery: simplify interface

discovery/discovery.go

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

discovery/discovery_test.go

@@ -322,7 +322,7 @@ func TestCreateSelf(t *testing.T) {
 
 	for i, tt := range tests {
 		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)
 		}
 	}

etcdmain/etcd.go

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

etcdserver/server.go

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