From 6a78c24d67e7a7965bf2e2df396fa17a7373130f Mon Sep 17 00:00:00 2001 From: Matt Keeler Date: Tue, 14 Apr 2020 11:54:27 -0400 Subject: [PATCH] =?UTF-8?q?Update=20the=20Client=20code=20to=20use=20the?= =?UTF-8?q?=20common=20version=20checking=20infra=E2=80=A6=20(#7558)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also reduce the log level of some version checking messages on the server as they can be pretty noisy during upgrades and really are more for debugging purposes. --- agent/consul/acl_client.go | 24 ++++++++---------------- agent/consul/acl_server.go | 4 ++-- agent/consul/leader.go | 3 +++ agent/consul/leader_test.go | 22 ++++++++++++++++++++++ agent/consul/util.go | 9 +++++++++ 5 files changed, 44 insertions(+), 18 deletions(-) diff --git a/agent/consul/acl_client.go b/agent/consul/acl_client.go index d5cce8d5da..e2e14dca3b 100644 --- a/agent/consul/acl_client.go +++ b/agent/consul/acl_client.go @@ -5,10 +5,8 @@ import ( "time" "github.com/hashicorp/consul/acl" - "github.com/hashicorp/consul/agent/metadata" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/lib" - "github.com/hashicorp/serf/serf" ) var clientACLCacheConfig *structs.ACLCachesConfig = &structs.ACLCachesConfig{ @@ -36,22 +34,11 @@ func (c *Client) UseLegacyACLs() bool { func (c *Client) monitorACLMode() { waitTime := aclModeCheckMinInterval for { - canUpgrade := false - for _, member := range c.LANMembers() { - if valid, parts := metadata.IsConsulServer(member); valid && parts.Status == serf.StatusAlive { - if parts.ACLs != structs.ACLModeEnabled { - canUpgrade = false - break - } else { - canUpgrade = true - } - } - } - - if canUpgrade { + foundServers, mode, _ := ServersGetACLMode(c, "", c.config.Datacenter) + if foundServers && mode == structs.ACLModeEnabled { c.logger.Debug("transitioned out of legacy ACL mode") + c.updateSerfTags("acls", string(structs.ACLModeEnabled)) atomic.StoreInt32(&c.useNewACLs, 1) - lib.UpdateSerfTag(c.serf, "acls", string(structs.ACLModeEnabled)) return } @@ -130,3 +117,8 @@ func (c *Client) ResolveTokenAndDefaultMeta(token string, entMeta *structs.Enter return authz, err } + +func (c *Client) updateSerfTags(key, value string) { + // Update the LAN serf + lib.UpdateSerfTag(c.serf, key, value) +} diff --git a/agent/consul/acl_server.go b/agent/consul/acl_server.go index 8f65b234d5..cec67062b1 100644 --- a/agent/consul/acl_server.go +++ b/agent/consul/acl_server.go @@ -111,7 +111,7 @@ func (s *Server) canUpgradeToNewACLs(isLeader bool) bool { if !s.InACLDatacenter() { foundServers, mode, _ := ServersGetACLMode(s, "", s.config.ACLDatacenter) if mode != structs.ACLModeEnabled || !foundServers { - s.logger.Info("Cannot upgrade to new ACLs, servers in acl datacenter are not yet upgraded", "ACLDatacenter", s.config.ACLDatacenter, "mode", mode, "found", foundServers) + s.logger.Debug("Cannot upgrade to new ACLs, servers in acl datacenter are not yet upgraded", "ACLDatacenter", s.config.ACLDatacenter, "mode", mode, "found", foundServers) return false } } @@ -128,7 +128,7 @@ func (s *Server) canUpgradeToNewACLs(isLeader bool) bool { } } - s.logger.Info("Cannot upgrade to new ACLs", "leaderMode", leaderMode, "mode", mode, "found", foundServers, "leader", leaderAddr) + s.logger.Debug("Cannot upgrade to new ACLs", "leaderMode", leaderMode, "mode", mode, "found", foundServers, "leader", leaderAddr) return false } diff --git a/agent/consul/leader.go b/agent/consul/leader.go index 63c8e936a3..3f851baad9 100644 --- a/agent/consul/leader.go +++ b/agent/consul/leader.go @@ -662,6 +662,9 @@ func (s *Server) initializeACLs(upgrade bool) error { if s.IsACLReplicationEnabled() { s.startLegacyACLReplication() } + // return early as we don't want to start new ACL replication + // or ACL token reaping as these are new ACL features. + return nil } if upgrade { diff --git a/agent/consul/leader_test.go b/agent/consul/leader_test.go index 00e3e37f84..8c2dbb81ea 100644 --- a/agent/consul/leader_test.go +++ b/agent/consul/leader_test.go @@ -1213,3 +1213,25 @@ func TestLeader_ConfigEntryBootstrap_Fail(t *testing.T) { result := <-ch require.Empty(t, result) } + +func TestLeader_ACLLegacyReplication(t *testing.T) { + t.Parallel() + + // This test relies on configuring a secondary DC with no route to the primary DC + // Having no route will cause the ACL mode checking of the primary to "fail". In this + // scenario legacy ACL replication should be enabled without also running new ACL + // replication routines. + cb := func(c *Config) { + c.Datacenter = "dc2" + c.ACLTokenReplication = true + } + dir, srv := testACLServerWithConfig(t, cb, true) + defer os.RemoveAll(dir) + defer srv.Shutdown() + waitForLeaderEstablishment(t, srv) + + require.True(t, srv.leaderRoutineManager.IsRunning(legacyACLReplicationRoutineName)) + require.False(t, srv.leaderRoutineManager.IsRunning(aclPolicyReplicationRoutineName)) + require.False(t, srv.leaderRoutineManager.IsRunning(aclRoleReplicationRoutineName)) + require.False(t, srv.leaderRoutineManager.IsRunning(aclTokenReplicationRoutineName)) +} diff --git a/agent/consul/util.go b/agent/consul/util.go index e8e461b4bd..c4e31500e4 100644 --- a/agent/consul/util.go +++ b/agent/consul/util.go @@ -363,6 +363,15 @@ func (s *Server) CheckServers(datacenter string, fn func(*metadata.Server) bool) } } +// CheckServers implements the checkServersProvider interface for the Client +func (c *Client) CheckServers(datacenter string, fn func(*metadata.Server) bool) { + if datacenter != c.config.Datacenter { + return + } + + c.routers.CheckServers(fn) +} + type serversACLMode struct { // leader is the address of the leader leader string