From 4203e7ab6d05af3d1cc38e7284473daa50eabaa5 Mon Sep 17 00:00:00 2001
From: Ryan Uber <ru@ryanuber.com>
Date: Thu, 9 Oct 2014 10:25:53 -0700
Subject: [PATCH] consul: clean up comments, fix globalRPC tests

---
 command/agent/keyring.go    |  4 ----
 consul/internal_endpoint.go |  6 ++---
 consul/server_test.go       | 48 ++++++++++---------------------------
 3 files changed, 15 insertions(+), 43 deletions(-)

diff --git a/command/agent/keyring.go b/command/agent/keyring.go
index 66ca7e2239..51a4666536 100644
--- a/command/agent/keyring.go
+++ b/command/agent/keyring.go
@@ -118,10 +118,6 @@ func loadKeyringFile(c *serf.Config) error {
 // keyringProcess is used to abstract away the semantic similarities in
 // performing various operations on the encryption keyring.
 func (a *Agent) keyringProcess(args *structs.KeyringRequest) (*structs.KeyringResponses, error) {
-	// Allow any server to handle the request, since this is
-	// done over the gossip protocol.
-	args.AllowStale = true
-
 	var reply structs.KeyringResponses
 	if a.server == nil {
 		return nil, fmt.Errorf("keyring operations must run against a server node")
diff --git a/consul/internal_endpoint.go b/consul/internal_endpoint.go
index c27a078370..3032e0b031 100644
--- a/consul/internal_endpoint.go
+++ b/consul/internal_endpoint.go
@@ -64,19 +64,19 @@ func (m *Internal) EventFire(args *structs.EventFireRequest,
 	return m.srv.UserEvent(args.Name, args.Payload)
 }
 
-// KeyringOperation will query the WAN and LAN gossip keyrings of all nodes,
-// adding results into a collective response as we go. It can describe requests
-// for all keyring-related operations.
+// KeyringOperation will query the WAN and LAN gossip keyrings of all nodes.
 func (m *Internal) KeyringOperation(
 	args *structs.KeyringRequest,
 	reply *structs.KeyringResponses) error {
 
+	// Only perform WAN keyring querying and RPC forwarding once
 	if !args.Forwarded {
 		args.Forwarded = true
 		m.executeKeyringOp(args, reply, true)
 		return m.srv.globalRPC("Internal.KeyringOperation", args, reply)
 	}
 
+	// Query the LAN keyring of this node's DC
 	m.executeKeyringOp(args, reply, false)
 	return nil
 }
diff --git a/consul/server_test.go b/consul/server_test.go
index 9a6c311239..0d0d1d5889 100644
--- a/consul/server_test.go
+++ b/consul/server_test.go
@@ -10,7 +10,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/hashicorp/consul/consul/structs"
 	"github.com/hashicorp/consul/testutil"
 )
 
@@ -475,46 +474,23 @@ func TestServer_BadExpect(t *testing.T) {
 	})
 }
 
-func TestServer_globalRPC(t *testing.T) {
+type fakeGlobalResp struct{}
+
+func (r *fakeGlobalResp) Add(interface{}) {
+	return
+}
+
+func (r *fakeGlobalResp) New() interface{} {
+	return struct{}{}
+}
+
+func TestServer_globalRPCErrors(t *testing.T) {
 	dir1, s1 := testServerDC(t, "dc1")
 	defer os.RemoveAll(dir1)
 	defer s1.Shutdown()
 
-	dir2, s2 := testServerDC(t, "dc2")
-	defer os.RemoveAll(dir2)
-	defer s2.Shutdown()
-
-	// Try to join
-	addr := fmt.Sprintf("127.0.0.1:%d",
-		s1.config.SerfWANConfig.MemberlistConfig.BindPort)
-	if _, err := s2.JoinWAN([]string{addr}); err != nil {
-		t.Fatalf("err: %v", err)
-	}
-
-	// Check the members
-	testutil.WaitForResult(func() (bool, error) {
-		members := len(s1.WANMembers())
-		return members == 2, fmt.Errorf("expected 2 members, got %d", members)
-	}, func(err error) {
-		t.Fatalf(err.Error())
-	})
-
-	// Wait for leader election
-	testutil.WaitForLeader(t, s1.RPC, "dc1")
-
-	// Check that replies from each gossip pool come in
-	resp := &structs.KeyringResponses{}
-	args := &structs.KeyringRequest{Operation: structs.KeyringList}
-	if err := s1.globalRPC("Internal.KeyringOperation", args, resp); err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	if len(resp.Responses) != 3 {
-		t.Fatalf("bad: %#v", resp.Responses)
-	}
-
 	// Check that an error from a remote DC is returned
-	resp = &structs.KeyringResponses{}
-	err := s1.globalRPC("Bad.Method", nil, resp)
+	err := s1.globalRPC("Bad.Method", nil, &fakeGlobalResp{})
 	if err == nil {
 		t.Fatalf("should have errored")
 	}