Update the Client code to use the common version checking infra… (#7558)

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

@@ -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
+		foundServers, mode, _ := ServersGetACLMode(c, "", c.config.Datacenter)
-		for _, member := range c.LANMembers() {
+		if foundServers && mode == structs.ACLModeEnabled {
-			if valid, parts := metadata.IsConsulServer(member); valid && parts.Status == serf.StatusAlive {
-				if parts.ACLs != structs.ACLModeEnabled {
-					canUpgrade = false
-					break
-				} else {
-					canUpgrade = true
-				}
-			}
-		}
-
-		if canUpgrade {
 			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)
+}

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
 }
 

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 {

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))
+}

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