diff --git a/agent/configentry/compare.go b/agent/configentry/compare.go new file mode 100644 index 0000000000..f28d5c9b03 --- /dev/null +++ b/agent/configentry/compare.go @@ -0,0 +1,37 @@ +package configentry + +import ( + "sort" + + "github.com/hashicorp/consul/agent/structs" +) + +func SortSlice(configs []structs.ConfigEntry) { + sort.SliceStable(configs, func(i, j int) bool { + return Less(configs[i], configs[j]) + }) +} + +func Less(first structs.ConfigEntry, second structs.ConfigEntry) bool { + if first.GetKind() < second.GetKind() { + return true + } + if first.GetKind() > second.GetKind() { + return false + } + + if first.GetEnterpriseMeta().LessThan(second.GetEnterpriseMeta()) { + return true + } + if second.GetEnterpriseMeta().LessThan(first.GetEnterpriseMeta()) { + return false + } + + return first.GetName() < second.GetName() +} + +func EqualID(e1, e2 structs.ConfigEntry) bool { + return e1.GetKind() == e2.GetKind() && + e1.GetEnterpriseMeta().IsSame(e2.GetEnterpriseMeta()) && + e1.GetName() == e2.GetName() +} diff --git a/agent/configentry/compare_test.go b/agent/configentry/compare_test.go new file mode 100644 index 0000000000..fcb63d0fa8 --- /dev/null +++ b/agent/configentry/compare_test.go @@ -0,0 +1,89 @@ +package configentry + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/hashicorp/consul/agent/structs" +) + +func TestSortSlice(t *testing.T) { + newDefaults := func(name, protocol string) *structs.ServiceConfigEntry { + return &structs.ServiceConfigEntry{ + Kind: structs.ServiceDefaults, + Name: name, + Protocol: protocol, + } + } + newResolver := func(name string, timeout time.Duration) *structs.ServiceResolverConfigEntry { + return &structs.ServiceResolverConfigEntry{ + Kind: structs.ServiceResolver, + Name: name, + ConnectTimeout: timeout, + } + } + + type testcase struct { + configs []structs.ConfigEntry + expect []structs.ConfigEntry + } + + cases := map[string]testcase{ + "none": {}, + "one": { + configs: []structs.ConfigEntry{ + newDefaults("web", "grpc"), + }, + expect: []structs.ConfigEntry{ + newDefaults("web", "grpc"), + }, + }, + "just kinds": { + configs: []structs.ConfigEntry{ + newResolver("web", 33*time.Second), + newDefaults("web", "grpc"), + }, + expect: []structs.ConfigEntry{ + newDefaults("web", "grpc"), + newResolver("web", 33*time.Second), + }, + }, + "just names": { + configs: []structs.ConfigEntry{ + newDefaults("db", "grpc"), + newDefaults("api", "http2"), + }, + expect: []structs.ConfigEntry{ + newDefaults("api", "http2"), + newDefaults("db", "grpc"), + }, + }, + "all": { + configs: []structs.ConfigEntry{ + newResolver("web", 33*time.Second), + newDefaults("web", "grpc"), + newDefaults("db", "grpc"), + newDefaults("api", "http2"), + }, + expect: []structs.ConfigEntry{ + newDefaults("api", "http2"), + newDefaults("db", "grpc"), + newDefaults("web", "grpc"), + newResolver("web", 33*time.Second), + }, + }, + } + + for name, tc := range cases { + tc := tc + t.Run(name, func(t *testing.T) { + SortSlice(tc.configs) + require.Equal(t, tc.expect, tc.configs) + // and it should be stable + SortSlice(tc.configs) + require.Equal(t, tc.expect, tc.configs) + }) + } +} diff --git a/agent/consul/config_replication.go b/agent/consul/config_replication.go index a7bb714be2..1b9ac02849 100644 --- a/agent/consul/config_replication.go +++ b/agent/consul/config_replication.go @@ -6,25 +6,19 @@ package consul import ( "context" "fmt" - "sort" "time" "github.com/armon/go-metrics" "github.com/hashicorp/go-hclog" "github.com/hashicorp/go-multierror" + "github.com/hashicorp/consul/agent/configentry" "github.com/hashicorp/consul/agent/structs" ) -func configSort(configs []structs.ConfigEntry) { - sort.SliceStable(configs, func(i, j int) bool { - return cmpConfigLess(configs[i], configs[j]) - }) -} - func diffConfigEntries(local []structs.ConfigEntry, remote []structs.ConfigEntry, lastRemoteIndex uint64) ([]structs.ConfigEntry, []structs.ConfigEntry) { - configSort(local) - configSort(remote) + configentry.SortSlice(local) + configentry.SortSlice(remote) var ( deletions []structs.ConfigEntry @@ -33,7 +27,7 @@ func diffConfigEntries(local []structs.ConfigEntry, remote []structs.ConfigEntry remoteIdx int ) for localIdx, remoteIdx = 0, 0; localIdx < len(local) && remoteIdx < len(remote); { - if configSameID(local[localIdx], remote[remoteIdx]) { + if configentry.EqualID(local[localIdx], remote[remoteIdx]) { // config is in both the local and remote state - need to check raft indices if remote[remoteIdx].GetRaftIndex().ModifyIndex > lastRemoteIndex { updates = append(updates, remote[remoteIdx]) @@ -41,7 +35,7 @@ func diffConfigEntries(local []structs.ConfigEntry, remote []structs.ConfigEntry // increment both indices when equal localIdx += 1 remoteIdx += 1 - } else if cmpConfigLess(local[localIdx], remote[remoteIdx]) { + } else if configentry.Less(local[localIdx], remote[remoteIdx]) { // config no longer in remoted state - needs deleting deletions = append(deletions, local[localIdx]) @@ -67,30 +61,6 @@ func diffConfigEntries(local []structs.ConfigEntry, remote []structs.ConfigEntry return deletions, updates } -func cmpConfigLess(first structs.ConfigEntry, second structs.ConfigEntry) bool { - if first.GetKind() < second.GetKind() { - return true - } - if first.GetKind() > second.GetKind() { - return false - } - - if first.GetEnterpriseMeta().LessThan(second.GetEnterpriseMeta()) { - return true - } - if second.GetEnterpriseMeta().LessThan(first.GetEnterpriseMeta()) { - return false - } - - return first.GetName() < second.GetName() -} - -func configSameID(e1, e2 structs.ConfigEntry) bool { - return e1.GetKind() == e2.GetKind() && - e1.GetEnterpriseMeta().IsSame(e2.GetEnterpriseMeta()) && - e1.GetName() == e2.GetName() -} - func (s *Server) reconcileLocalConfig(ctx context.Context, configs []structs.ConfigEntry, op structs.ConfigEntryOp) (bool, error) { ticker := time.NewTicker(time.Second / time.Duration(s.config.ConfigReplicationApplyLimit)) defer ticker.Stop() diff --git a/agent/consul/config_replication_test.go b/agent/consul/config_replication_test.go index 9e3d236d82..a0eb8cf52a 100644 --- a/agent/consul/config_replication_test.go +++ b/agent/consul/config_replication_test.go @@ -8,7 +8,6 @@ import ( "fmt" "os" "testing" - "time" "github.com/stretchr/testify/require" @@ -17,85 +16,6 @@ import ( "github.com/hashicorp/consul/testrpc" ) -func TestReplication_ConfigSort(t *testing.T) { - newDefaults := func(name, protocol string) *structs.ServiceConfigEntry { - return &structs.ServiceConfigEntry{ - Kind: structs.ServiceDefaults, - Name: name, - Protocol: protocol, - } - } - newResolver := func(name string, timeout time.Duration) *structs.ServiceResolverConfigEntry { - return &structs.ServiceResolverConfigEntry{ - Kind: structs.ServiceResolver, - Name: name, - ConnectTimeout: timeout, - } - } - - type testcase struct { - configs []structs.ConfigEntry - expect []structs.ConfigEntry - } - - cases := map[string]testcase{ - "none": {}, - "one": { - configs: []structs.ConfigEntry{ - newDefaults("web", "grpc"), - }, - expect: []structs.ConfigEntry{ - newDefaults("web", "grpc"), - }, - }, - "just kinds": { - configs: []structs.ConfigEntry{ - newResolver("web", 33*time.Second), - newDefaults("web", "grpc"), - }, - expect: []structs.ConfigEntry{ - newDefaults("web", "grpc"), - newResolver("web", 33*time.Second), - }, - }, - "just names": { - configs: []structs.ConfigEntry{ - newDefaults("db", "grpc"), - newDefaults("api", "http2"), - }, - expect: []structs.ConfigEntry{ - newDefaults("api", "http2"), - newDefaults("db", "grpc"), - }, - }, - "all": { - configs: []structs.ConfigEntry{ - newResolver("web", 33*time.Second), - newDefaults("web", "grpc"), - newDefaults("db", "grpc"), - newDefaults("api", "http2"), - }, - expect: []structs.ConfigEntry{ - newDefaults("api", "http2"), - newDefaults("db", "grpc"), - newDefaults("web", "grpc"), - newResolver("web", 33*time.Second), - }, - }, - } - - for name, tc := range cases { - tc := tc - t.Run(name, func(t *testing.T) { - configSort(tc.configs) - require.Equal(t, tc.expect, tc.configs) - // and it should be stable - configSort(tc.configs) - require.Equal(t, tc.expect, tc.configs) - }) - } -} - func TestReplication_ConfigEntries(t *testing.T) { if testing.Short() { t.Skip("too slow for testing.Short")