From f59abcc3940a719a488f33eb2d11497513108c32 Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Wed, 21 Feb 2018 12:33:22 -0600 Subject: [PATCH 1/3] Make sure revokeLeadership is called if establishLeadership errors --- agent/consul/leader.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/agent/consul/leader.go b/agent/consul/leader.go index 64b7ea1cdc60..f5a1f237e95e 100644 --- a/agent/consul/leader.go +++ b/agent/consul/leader.go @@ -123,6 +123,11 @@ RECONCILE: if !establishedLeader { if err := s.establishLeadership(); err != nil { s.logger.Printf("[ERR] consul: failed to establish leadership: %v", err) + // Immediately revoke leadership since we didn't successfully + // establish leadership. + if err := s.revokeLeadership(); err != nil { + s.logger.Printf("[ERR] consul: failed to revoke leadership: %v", err) + } goto WAIT } establishedLeader = true @@ -178,6 +183,7 @@ WAIT: // previously inflight transactions have been committed and that our // state is up-to-date. func (s *Server) establishLeadership() error { + defer metrics.MeasureSince([]string{"consul", "leader", "establish_leadership"}, time.Now()) // This will create the anonymous token and master token (if that is // configured). if err := s.initializeACL(); err != nil { @@ -213,6 +219,7 @@ func (s *Server) establishLeadership() error { // revokeLeadership is invoked once we step down as leader. // This is used to cleanup any state that may be specific to a leader. func (s *Server) revokeLeadership() error { + defer metrics.MeasureSince([]string{"consul", "leader", "revoke_leadership"}, time.Now()) // Disable the tombstone GC, since it is only useful as a leader s.tombstoneGC.SetEnabled(false) From 907b97b7f2ac07491c674424709fb1c89849d763 Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Wed, 21 Feb 2018 12:48:53 -0600 Subject: [PATCH 2/3] Unit test that calls revokeLeadership twice to make sure its idempotent --- agent/consul/server_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/agent/consul/server_test.go b/agent/consul/server_test.go index d65e2c7d7cf7..74dbf726eea4 100644 --- a/agent/consul/server_test.go +++ b/agent/consul/server_test.go @@ -761,3 +761,22 @@ func TestServer_TLSToFullVerify(t *testing.T) { t.Fatalf("bad: %v", success) } } + +func TestServer_RevokeLeadershipIdempotent(t *testing.T) { + t.Parallel() + dir1, s1 := testServer(t) + defer os.RemoveAll(dir1) + defer s1.Shutdown() + + + testrpc.WaitForLeader(t, s1.RPC, "dc1") + + err:= s1.revokeLeadership() + if err != nil { + t.Fatal(err) + } + err = s1.revokeLeadership() + if err != nil { + t.Fatal(err) + } +} From 80791d5b213f377fc153db3aca4a074e9b6c2f6b Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Wed, 21 Feb 2018 13:21:47 -0600 Subject: [PATCH 3/3] Remove extra newline --- agent/consul/server_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/agent/consul/server_test.go b/agent/consul/server_test.go index 74dbf726eea4..3a02308df210 100644 --- a/agent/consul/server_test.go +++ b/agent/consul/server_test.go @@ -767,8 +767,7 @@ func TestServer_RevokeLeadershipIdempotent(t *testing.T) { dir1, s1 := testServer(t) defer os.RemoveAll(dir1) defer s1.Shutdown() - - + testrpc.WaitForLeader(t, s1.RPC, "dc1") err:= s1.revokeLeadership()