From f8f89d74903c07a2d0b12e1a8e645ac25154ffb7 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Thu, 7 Jun 2018 16:22:31 -0400 Subject: [PATCH] move logic for testing equality for vault config --- command/agent/command.go | 30 +++++++++--------------------- nomad/vault.go | 5 +++++ nomad/vault_test.go | 25 +++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 21 deletions(-) diff --git a/command/agent/command.go b/command/agent/command.go index 3ed6e9624..256914e21 100644 --- a/command/agent/command.go +++ b/command/agent/command.go @@ -740,28 +740,16 @@ func (c *Command) handleReload() { } } - var shouldReloadVault bool - switch { - case c.agent.config.Vault == nil && newConf.Vault != nil: - fallthrough - case c.agent.config.Vault != nil && newConf.Vault == nil: - fallthrough - case c.agent.config.Vault != nil && !c.agent.config.Vault.IsEqual(newConf.Vault): - shouldReloadVault = true - } - - if shouldReloadRPC || shouldReloadVault { - if s := c.agent.Server(); s != nil { - sconf, err := convertServerConfig(newConf, c.logOutput) - c.agent.logger.Printf("[DEBUG] agent: starting reload of server config") - if err != nil { - c.agent.logger.Printf("[ERR] agent: failed to convert server config: %v", err) + if s := c.agent.Server(); s != nil { + sconf, err := convertServerConfig(newConf, c.logOutput) + c.agent.logger.Printf("[DEBUG] agent: starting reload of server config") + if err != nil { + c.agent.logger.Printf("[ERR] agent: failed to convert server config: %v", err) + return + } else { + if err := s.Reload(sconf); err != nil { + c.agent.logger.Printf("[ERR] agent: reloading server config failed: %v", err) return - } else { - if err := s.Reload(sconf); err != nil { - c.agent.logger.Printf("[ERR] agent: reloading server config failed: %v", err) - return - } } } } diff --git a/nomad/vault.go b/nomad/vault.go index ca1486ebf..fc60075fb 100644 --- a/nomad/vault.go +++ b/nomad/vault.go @@ -316,6 +316,11 @@ func (v *vaultClient) SetConfig(config *config.VaultConfig) error { v.l.Lock() defer v.l.Unlock() + // If reloading the same config, no-op + if v.config.IsEqual(config) { + return nil + } + // Kill any background routines if v.running { // Stop accepting any new request diff --git a/nomad/vault_test.go b/nomad/vault_test.go index 687395a0d..2c165fb6a 100644 --- a/nomad/vault_test.go +++ b/nomad/vault_test.go @@ -437,6 +437,31 @@ func TestVaultClient_SetConfig(t *testing.T) { if client.tokenData == nil || len(client.tokenData.Policies) != 3 { t.Fatalf("unexpected token: %v", client.tokenData) } + + // test that when SetConfig is called with the same configuration, it is a + // no-op + failCh := make(chan struct{}, 1) + go func() { + tomb := client.tomb + select { + case <-tomb.Dying(): + close(failCh) + case <-time.After(1 * time.Second): + return + } + }() + + // Update the config + if err := client.SetConfig(v2.Config); err != nil { + t.Fatalf("SetConfig failed: %v", err) + } + + select { + case <-failCh: + t.Fatalf("Tomb shouldn't have exited") + case <-time.After(1 * time.Second): + return + } } // Test that we can disable vault