From 4d99696f024e081c29b7d272a74e1a34e5f06841 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Tue, 20 Feb 2018 15:51:59 -0800 Subject: [PATCH 1/2] Improve autopilot shutdown to be idempotent --- agent/consul/autopilot/autopilot.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/agent/consul/autopilot/autopilot.go b/agent/consul/autopilot/autopilot.go index d6efd11e7..b50252ce5 100644 --- a/agent/consul/autopilot/autopilot.go +++ b/agent/consul/autopilot/autopilot.go @@ -38,8 +38,10 @@ type Autopilot struct { clusterHealth OperatorHealthReply clusterHealthLock sync.RWMutex + enabled bool removeDeadCh chan struct{} shutdownCh chan struct{} + shutdownLock sync.Mutex waitGroup sync.WaitGroup } @@ -62,6 +64,14 @@ func NewAutopilot(logger *log.Logger, delegate Delegate, interval, healthInterva } func (a *Autopilot) Start() { + a.shutdownLock.Lock() + defer a.shutdownLock.Unlock() + + // Nothing to do + if a.enabled { + return + } + a.shutdownCh = make(chan struct{}) a.waitGroup = sync.WaitGroup{} a.clusterHealth = OperatorHealthReply{} @@ -69,11 +79,21 @@ func (a *Autopilot) Start() { a.waitGroup.Add(2) go a.run() go a.serverHealthLoop() + a.enabled = true } func (a *Autopilot) Stop() { + a.shutdownLock.Lock() + defer a.shutdownLock.Unlock() + + // Nothing to do + if !a.enabled { + return + } + close(a.shutdownCh) a.waitGroup.Wait() + a.enabled = false } // run periodically looks for nonvoting servers to promote and dead servers to remove. From 535842004c0808858e447b844848110036e613be Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 21 Feb 2018 10:19:30 -0800 Subject: [PATCH 2/2] Test autopilots start/stop idempotency --- agent/consul/autopilot_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/agent/consul/autopilot_test.go b/agent/consul/autopilot_test.go index f7be3273d..ea88dcca3 100644 --- a/agent/consul/autopilot_test.go +++ b/agent/consul/autopilot_test.go @@ -11,6 +11,20 @@ import ( "github.com/hashicorp/serf/serf" ) +func TestAutopilot_IdempotentShutdown(t *testing.T) { + dir1, s1 := testServerWithConfig(t, nil) + defer os.RemoveAll(dir1) + defer s1.Shutdown() + retry.Run(t, func(r *retry.R) { r.Check(waitForLeader(s1)) }) + + s1.autopilot.Start() + s1.autopilot.Start() + s1.autopilot.Start() + s1.autopilot.Stop() + s1.autopilot.Stop() + s1.autopilot.Stop() +} + func TestAutopilot_CleanupDeadServer(t *testing.T) { t.Parallel() for i := 1; i <= 3; i++ {