Refactor of serf feature flag tags.

This refactor is to make it easier to see how serf feature flags are
encoded as serf tags, and where those feature flags are read.

- use constants for both the prefix and feature flag name. A constant
  makes it much easier for an IDE to locate the read and write location.
- isolate the feature-flag encoding logic in the metadata package, so
  that the feature flag prefix can be unexported. Only expose a function
  for encoding the flags into tags. This logic is now next to the logic
  which reads the tags.
- remove the duplicate `addEnterpriseSerfTags` functions. Both Client
  and Server structs had the same implementation. And neither
  implementation needed the method receiver.

agent/consul/client_serf.go

@@ -5,14 +5,15 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/hashicorp/go-hclog"
+	"github.com/hashicorp/serf/serf"
+
 	"github.com/hashicorp/consul/agent/metadata"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/lib"
 	libserf "github.com/hashicorp/consul/lib/serf"
 	"github.com/hashicorp/consul/logging"
 	"github.com/hashicorp/consul/types"
-	"github.com/hashicorp/go-hclog"
-	"github.com/hashicorp/serf/serf"
 )
 
 // setupSerf is used to setup and initialize a Serf
@@ -67,7 +68,7 @@ func (c *Client) setupSerf(conf *serf.Config, ch chan serf.Event, path string) (
 		return nil, err
 	}
 
-	c.addEnterpriseSerfTags(conf.Tags)
+	addEnterpriseSerfTags(conf.Tags)
 
 	conf.ReconnectTimeoutOverride = libserf.NewReconnectOverride(c.logger)
 

agent/consul/enterprise_client_oss.go

@@ -23,7 +23,3 @@ func (c *Client) handleEnterpriseUserEvents(event serf.UserEvent) bool {
 func (c *Client) enterpriseStats() map[string]map[string]string {
 	return nil
 }
-
-func (_ *Client) addEnterpriseSerfTags(_ map[string]string) {
-	// do nothing
-}

agent/consul/enterprise_server_oss.go

@@ -7,10 +7,11 @@ import (
 	"net"
 	"strings"
 
-	"github.com/hashicorp/consul/agent/pool"
-	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/go-version"
 	"github.com/hashicorp/serf/serf"
+
+	"github.com/hashicorp/consul/agent/pool"
+	"github.com/hashicorp/consul/agent/structs"
 )
 
 var (
@@ -73,7 +74,7 @@ func (s *Server) validateEnterpriseIntentionNamespace(ns string, _ bool) error {
 	return errors.New("Namespaces is a Consul Enterprise feature")
 }
 
-func (_ *Server) addEnterpriseSerfTags(_ map[string]string) {
+func addEnterpriseSerfTags(_ map[string]string) {
 	// do nothing
 }
 

agent/consul/replication.go

@@ -252,9 +252,7 @@ func (r *IndexReplicator) Replicate(ctx context.Context, lastRemoteIndex uint64,
 		return 0, false, fmt.Errorf("failed to retrieve %s: %w", r.Delegate.PluralNoun(), err)
 	}
 
-	r.Logger.Debug("finished fetching remote objects",
+	r.Logger.Debug("finished fetching remote objects", "amount", lenRemote)
-		"amount", lenRemote,
-	)
 
 	// Need to check if we should be stopping. This will be common as the fetching process is a blocking
 	// RPC which could have been hanging around for a long time and during that time leadership could

agent/consul/server_serf.go

@@ -7,16 +7,17 @@ import (
 	"strings"
 	"time"
 
+	"github.com/hashicorp/go-hclog"
+	"github.com/hashicorp/memberlist"
+	"github.com/hashicorp/raft"
+	"github.com/hashicorp/serf/serf"
+
 	"github.com/hashicorp/consul/agent/consul/wanfed"
 	"github.com/hashicorp/consul/agent/metadata"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/lib"
 	libserf "github.com/hashicorp/consul/lib/serf"
 	"github.com/hashicorp/consul/logging"
-	"github.com/hashicorp/go-hclog"
-	"github.com/hashicorp/memberlist"
-	"github.com/hashicorp/raft"
-	"github.com/hashicorp/serf/serf"
 )
 
 const (
@@ -174,7 +175,7 @@ func (s *Server) setupSerf(conf *serf.Config, ch chan serf.Event, path string, w
 
 	conf.ReconnectTimeoutOverride = libserf.NewReconnectOverride(s.logger)
 
-	s.addEnterpriseSerfTags(conf.Tags)
+	addEnterpriseSerfTags(conf.Tags)
 
 	if s.config.OverrideInitialSerfTags != nil {
 		s.config.OverrideInitialSerfTags(conf.Tags)

agent/metadata/server.go

@@ -7,9 +7,10 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/go-version"
 	"github.com/hashicorp/serf/serf"
+
+	"github.com/hashicorp/consul/agent/structs"
 )
 
 // Key is used in maps and for equality tests.  A key is based on endpoints.
@@ -120,8 +121,8 @@ func IsConsulServer(m serf.Member) (bool, *Server) {
 			segmentName := strings.TrimPrefix(name, "sl_")
 			segmentAddrs[segmentName] = addr
 			segmentPorts[segmentName] = segmentPort
-		} else if strings.HasPrefix(name, "ft_") {
+		} else if strings.HasPrefix(name, featureFlagPrefix) {
-			featureName := strings.TrimPrefix(name, "ft_")
+			featureName := strings.TrimPrefix(name, featureFlagPrefix)
 			featureState, err := strconv.Atoi(value)
 			if err != nil {
 				return false, nil
@@ -192,3 +193,14 @@ func IsConsulServer(m serf.Member) (bool, *Server) {
 	}
 	return true, parts
 }
+
+const featureFlagPrefix = "ft_"
+
+// AddFeatureFlags to the tags. The tags map is expected to be a serf.Config.Tags.
+// The feature flags are encoded in the tags so that IsConsulServer can decode them
+// and populate the Server.FeatureFlags map.
+func AddFeatureFlags(tags map[string]string, flags ...string) {
+	for _, flag := range flags {
+		tags[featureFlagPrefix+flag] = "1"
+	}
+}

agent/metadata/server_test.go

@@ -4,9 +4,10 @@ import (
 	"net"
 	"testing"
 
-	"github.com/hashicorp/consul/agent/metadata"
 	"github.com/hashicorp/serf/serf"
 	"github.com/stretchr/testify/require"
+
+	"github.com/hashicorp/consul/agent/metadata"
 )
 
 func TestServer_Key_params(t *testing.T) {
@@ -187,8 +188,10 @@ func TestIsConsulServer_Optional(t *testing.T) {
 	if parts.RaftVersion != 0 {
 		t.Fatalf("bad: %v", parts.RaftVersion)
 	}
+
 	m.Tags["bootstrap"] = "1"
 	m.Tags["disabled"] = "1"
+	m.Tags["ft_ns"] = "1"
 	ok, parts = metadata.IsConsulServer(m)
 	if !ok {
 		t.Fatalf("expected a valid consul server")
@@ -202,6 +205,9 @@ func TestIsConsulServer_Optional(t *testing.T) {
 	if parts.Version != 1 {
 		t.Fatalf("bad: %v", parts)
 	}
+	expectedFlags := map[string]int{"ns": 1}
+	require.Equal(t, expectedFlags, parts.FeatureFlags)
+
 	m.Tags["expect"] = "3"
 	delete(m.Tags, "bootstrap")
 	delete(m.Tags, "disabled")