diff --git a/command/agent/consul/service_client.go b/command/agent/consul/service_client.go index 8d157d0d4..929973b65 100644 --- a/command/agent/consul/service_client.go +++ b/command/agent/consul/service_client.go @@ -17,6 +17,7 @@ import ( "github.com/hashicorp/consul/api" "github.com/hashicorp/go-hclog" "github.com/hashicorp/nomad/client/serviceregistration" + "github.com/hashicorp/nomad/helper" "github.com/hashicorp/nomad/helper/envoy" "github.com/hashicorp/nomad/nomad/structs" "golang.org/x/exp/slices" @@ -104,14 +105,16 @@ type NamespaceAPI interface { // - agent:read // - service:write type AgentAPI interface { - ServicesWithFilterOpts(filter string, q *api.QueryOptions) (map[string]*api.AgentService, error) - ChecksWithFilterOpts(filter string, q *api.QueryOptions) (map[string]*api.AgentCheck, error) CheckRegister(check *api.AgentCheckRegistration) error CheckDeregisterOpts(checkID string, q *api.QueryOptions) error - Self() (map[string]map[string]interface{}, error) + ChecksWithFilterOpts(filter string, q *api.QueryOptions) (map[string]*api.AgentCheck, error) + UpdateTTLOpts(id, output, status string, q *api.QueryOptions) error + ServiceRegister(service *api.AgentServiceRegistration) error ServiceDeregisterOpts(serviceID string, q *api.QueryOptions) error - UpdateTTLOpts(id, output, status string, q *api.QueryOptions) error + ServicesWithFilterOpts(filter string, q *api.QueryOptions) (map[string]*api.AgentService, error) + + Self() (map[string]map[string]interface{}, error) } // ConfigAPI is the consul/api.ConfigEntries API subset used by Nomad Server. @@ -215,7 +218,7 @@ func different(wanted *api.AgentServiceRegistration, existing *api.AgentService, return true case !reflect.DeepEqual(wanted.TaggedAddresses, existing.TaggedAddresses): return true - case tagsDifferent(wanted.Tags, existing.Tags): + case !helper.SliceSetEq(wanted.Tags, existing.Tags): return true case connectSidecarDifferent(wanted, sidecar): return true @@ -223,20 +226,6 @@ func different(wanted *api.AgentServiceRegistration, existing *api.AgentService, return false } -func tagsDifferent(a, b []string) bool { - if len(a) != len(b) { - return true - } - - for i, valueA := range a { - if b[i] != valueA { - return true - } - } - - return false -} - // sidecarTagsDifferent includes the special logic for comparing sidecar tags // from Nomad vs. Consul perspective. Because Consul forces the sidecar tags // to inherit the parent service tags if the sidecar tags are unset, we need to @@ -244,9 +233,9 @@ func tagsDifferent(a, b []string) bool { // comparing them to the parent service tags. func sidecarTagsDifferent(parent, wanted, sidecar []string) bool { if len(wanted) == 0 { - return tagsDifferent(parent, sidecar) + return !helper.SliceSetEq(parent, sidecar) } - return tagsDifferent(wanted, sidecar) + return !helper.SliceSetEq(wanted, sidecar) } // proxyUpstreamsDifferent determines if the sidecar_service.proxy.upstreams @@ -366,7 +355,7 @@ func (o *operations) empty() bool { } } -func (o operations) String() string { +func (o *operations) String() string { return fmt.Sprintf("<%d, %d, %d, %d>", len(o.regServices), len(o.regChecks), len(o.deregServices), len(o.deregChecks)) } diff --git a/command/agent/consul/service_client_test.go b/command/agent/consul/service_client_test.go index a89ed1b0b..e87845932 100644 --- a/command/agent/consul/service_client_test.go +++ b/command/agent/consul/service_client_test.go @@ -270,39 +270,6 @@ func TestSyncLogic_agentServiceUpdateRequired(t *testing.T) { }) } -func TestSyncLogic_tagsDifferent(t *testing.T) { - ci.Parallel(t) - - t.Run("nil nil", func(t *testing.T) { - require.False(t, tagsDifferent(nil, nil)) - }) - - t.Run("empty nil", func(t *testing.T) { - // where reflect.DeepEqual does not work - require.False(t, tagsDifferent([]string{}, nil)) - }) - - t.Run("empty empty", func(t *testing.T) { - require.False(t, tagsDifferent([]string{}, []string{})) - }) - - t.Run("set empty", func(t *testing.T) { - require.True(t, tagsDifferent([]string{"A"}, []string{})) - }) - - t.Run("set nil", func(t *testing.T) { - require.True(t, tagsDifferent([]string{"A"}, nil)) - }) - - t.Run("different content", func(t *testing.T) { - require.True(t, tagsDifferent([]string{"A"}, []string{"B"})) - }) - - t.Run("different lengths", func(t *testing.T) { - require.True(t, tagsDifferent([]string{"A"}, []string{"A", "B"})) - }) -} - func TestSyncLogic_sidecarTagsDifferent(t *testing.T) { ci.Parallel(t) @@ -386,7 +353,7 @@ func TestSyncLogic_maybeTweakTags_emptySC(t *testing.T) { existing := &api.AgentService{Tags: []string{"a", "b"}} sidecar := &api.AgentService{Tags: []string{"a", "b"}} maybeTweakTags(asr, existing, sidecar) - require.False(t, !tagsDifferent([]string{"original"}, asr.Tags)) + must.NotEq(t, []string{"original"}, asr.Tags) } try(&api.AgentServiceRegistration{