diff --git a/.changelog/21909.txt b/.changelog/21909.txt new file mode 100644 index 0000000000..b49562f137 --- /dev/null +++ b/.changelog/21909.txt @@ -0,0 +1,3 @@ +```release-note:bug +state: ensure that identical manual virtual IP updates result in not bumping the modify indexes +``` diff --git a/agent/consul/internal_endpoint.go b/agent/consul/internal_endpoint.go index cab2195845..6afb8405f3 100644 --- a/agent/consul/internal_endpoint.go +++ b/agent/consul/internal_endpoint.go @@ -7,15 +7,18 @@ import ( "fmt" "net" + hashstructure_v2 "github.com/mitchellh/hashstructure/v2" + "golang.org/x/exp/maps" + "github.com/hashicorp/go-bexpr" "github.com/hashicorp/go-hclog" "github.com/hashicorp/go-memdb" "github.com/hashicorp/serf/serf" - hashstructure_v2 "github.com/mitchellh/hashstructure/v2" "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/consul/state" "github.com/hashicorp/consul/agent/structs" + "github.com/hashicorp/consul/lib/stringslice" ) const MaximumManualVIPsPerService = 8 @@ -782,17 +785,38 @@ func (m *Internal) AssignManualServiceVIPs(args *structs.AssignServiceManualVIPs return fmt.Errorf("cannot associate more than %d manual virtual IPs with the same service", MaximumManualVIPsPerService) } + vipMap := make(map[string]struct{}) for _, ip := range args.ManualVIPs { parsedIP := net.ParseIP(ip) if parsedIP == nil || parsedIP.To4() == nil { return fmt.Errorf("%q is not a valid IPv4 address", parsedIP.String()) } + vipMap[ip] = struct{}{} + } + // Silently ignore duplicates. + args.ManualVIPs = maps.Keys(vipMap) + + psn := structs.PeeredServiceName{ + ServiceName: structs.NewServiceName(args.Service, &args.EnterpriseMeta), + } + + // Check to see if we can skip the raft apply entirely. + { + existingIPs, err := m.srv.fsm.State().ServiceManualVIPs(psn) + if err != nil { + return fmt.Errorf("error checking for existing manual ips for service: %w", err) + } + if existingIPs != nil && stringslice.EqualMapKeys(existingIPs.ManualIPs, vipMap) { + *reply = structs.AssignServiceManualVIPsResponse{ + Found: true, + UnassignedFrom: nil, + } + return nil + } } req := state.ServiceVirtualIP{ - Service: structs.PeeredServiceName{ - ServiceName: structs.NewServiceName(args.Service, &args.EnterpriseMeta), - }, + Service: psn, ManualIPs: args.ManualVIPs, } resp, err := m.srv.raftApplyMsgpack(structs.UpdateVirtualIPRequestType, req) diff --git a/agent/consul/internal_endpoint_test.go b/agent/consul/internal_endpoint_test.go index 91f46cb760..3c73f31c17 100644 --- a/agent/consul/internal_endpoint_test.go +++ b/agent/consul/internal_endpoint_test.go @@ -12,11 +12,11 @@ import ( "testing" "time" - "github.com/hashicorp/consul-net-rpc/net/rpc" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" msgpackrpc "github.com/hashicorp/consul-net-rpc/net-rpc-msgpackrpc" + "github.com/hashicorp/consul-net-rpc/net/rpc" "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/structs" @@ -3885,21 +3885,38 @@ func TestInternal_AssignManualServiceVIPs(t *testing.T) { require.NoError(t, msgpackrpc.CallWithCodec(codec, "Internal.AssignManualServiceVIPs", req, &resp)) type testcase struct { - name string - req structs.AssignServiceManualVIPsRequest - expect structs.AssignServiceManualVIPsResponse - expectErr string + name string + req structs.AssignServiceManualVIPsRequest + expect structs.AssignServiceManualVIPsResponse + expectAgain structs.AssignServiceManualVIPsResponse + expectErr string + expectIPs []string } - run := func(t *testing.T, tc testcase) { + + run := func(t *testing.T, tc testcase, again bool) { + if tc.expectErr != "" && again { + return // we don't retest known errors + } + var resp structs.AssignServiceManualVIPsResponse err := msgpackrpc.CallWithCodec(codec, "Internal.AssignManualServiceVIPs", tc.req, &resp) if tc.expectErr != "" { - require.Error(t, err) - require.Contains(t, err.Error(), tc.expectErr) - return + testutil.RequireErrorContains(t, err, tc.expectErr) + } else { + if again { + require.Equal(t, tc.expectAgain, resp) + } else { + require.Equal(t, tc.expect, resp) + } + + psn := structs.PeeredServiceName{ServiceName: structs.NewServiceName(tc.req.Service, nil)} + got, err := s1.fsm.State().ServiceManualVIPs(psn) + require.NoError(t, err) + require.NotNil(t, got) + require.Equal(t, tc.expectIPs, got.ManualIPs) } - require.Equal(t, tc.expect, resp) } + tcs := []testcase{ { name: "successful manual ip assignment", @@ -3907,7 +3924,19 @@ func TestInternal_AssignManualServiceVIPs(t *testing.T) { Service: "web", ManualVIPs: []string{"1.1.1.1", "2.2.2.2"}, }, - expect: structs.AssignServiceManualVIPsResponse{Found: true}, + expectIPs: []string{"1.1.1.1", "2.2.2.2"}, + expect: structs.AssignServiceManualVIPsResponse{Found: true}, + expectAgain: structs.AssignServiceManualVIPsResponse{Found: true}, + }, + { + name: "successfully ignoring duplicates", + req: structs.AssignServiceManualVIPsRequest{ + Service: "web", + ManualVIPs: []string{"1.2.3.4", "5.6.7.8", "1.2.3.4", "5.6.7.8"}, + }, + expectIPs: []string{"1.2.3.4", "5.6.7.8"}, + expect: structs.AssignServiceManualVIPsResponse{Found: true}, + expectAgain: structs.AssignServiceManualVIPsResponse{Found: true}, }, { name: "reassign existing ip", @@ -3915,6 +3944,7 @@ func TestInternal_AssignManualServiceVIPs(t *testing.T) { Service: "web", ManualVIPs: []string{"8.8.8.8"}, }, + expectIPs: []string{"8.8.8.8"}, expect: structs.AssignServiceManualVIPsResponse{ Found: true, UnassignedFrom: []structs.PeeredServiceName{ @@ -3923,6 +3953,8 @@ func TestInternal_AssignManualServiceVIPs(t *testing.T) { }, }, }, + // When we repeat this operation the second time it's a no-op. + expectAgain: structs.AssignServiceManualVIPsResponse{Found: true}, }, { name: "invalid ip", @@ -3930,13 +3962,19 @@ func TestInternal_AssignManualServiceVIPs(t *testing.T) { Service: "web", ManualVIPs: []string{"3.3.3.3", "invalid"}, }, - expect: structs.AssignServiceManualVIPsResponse{}, expectErr: "not a valid", }, } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - run(t, tc) + t.Run("initial", func(t *testing.T) { + run(t, tc, false) + }) + if tc.expectErr == "" { + t.Run("repeat", func(t *testing.T) { + run(t, tc, true) // only repeat a write if it isn't an known error + }) + } }) } } diff --git a/agent/consul/state/catalog.go b/agent/consul/state/catalog.go index dcfe4ec91f..b8588f17cc 100644 --- a/agent/consul/state/catalog.go +++ b/agent/consul/state/catalog.go @@ -8,6 +8,8 @@ import ( "fmt" "net" "reflect" + "slices" + "sort" "strings" "github.com/hashicorp/go-memdb" @@ -18,6 +20,7 @@ import ( "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/lib" "github.com/hashicorp/consul/lib/maps" + "github.com/hashicorp/consul/lib/stringslice" "github.com/hashicorp/consul/types" ) @@ -1106,6 +1109,9 @@ func (s *Store) AssignManualServiceVIPs(idx uint64, psn structs.PeeredServiceNam for _, ip := range ips { assignedIPs[ip] = struct{}{} } + + txnNeedsCommit := false + modifiedEntries := make(map[structs.PeeredServiceName]struct{}) for ip := range assignedIPs { entry, err := tx.First(tableServiceVirtualIPs, indexManualVIPs, psn.ServiceName.PartitionOrDefault(), ip) @@ -1118,7 +1124,13 @@ func (s *Store) AssignManualServiceVIPs(idx uint64, psn structs.PeeredServiceNam } newEntry := entry.(ServiceVirtualIP) - if newEntry.Service.ServiceName.Matches(psn.ServiceName) { + + var ( + thisServiceName = newEntry.Service.ServiceName + thisPeer = newEntry.Service.Peer + ) + + if thisServiceName.Matches(psn.ServiceName) && thisPeer == psn.Peer { continue } @@ -1130,6 +1142,7 @@ func (s *Store) AssignManualServiceVIPs(idx uint64, psn structs.PeeredServiceNam filteredIPs = append(filteredIPs, existingIP) } } + sort.Strings(filteredIPs) newEntry.ManualIPs = filteredIPs newEntry.ModifyIndex = idx @@ -1137,6 +1150,12 @@ func (s *Store) AssignManualServiceVIPs(idx uint64, psn structs.PeeredServiceNam return false, nil, fmt.Errorf("failed inserting service virtual IP entry: %s", err) } modifiedEntries[newEntry.Service] = struct{}{} + + if err := updateVirtualIPMaxIndexes(tx, idx, thisServiceName.PartitionOrDefault(), thisPeer); err != nil { + return false, nil, err + } + + txnNeedsCommit = true } entry, err := tx.First(tableServiceVirtualIPs, indexID, psn) @@ -1149,23 +1168,37 @@ func (s *Store) AssignManualServiceVIPs(idx uint64, psn structs.PeeredServiceNam } newEntry := entry.(ServiceVirtualIP) - newEntry.ManualIPs = ips - newEntry.ModifyIndex = idx - if err := tx.Insert(tableServiceVirtualIPs, newEntry); err != nil { - return false, nil, fmt.Errorf("failed inserting service virtual IP entry: %s", err) + // Check to see if the slice already contains the same ips. + if !stringslice.EqualMapKeys(newEntry.ManualIPs, assignedIPs) { + newEntry.ManualIPs = slices.Clone(ips) + newEntry.ModifyIndex = idx + + sort.Strings(newEntry.ManualIPs) + + if err := tx.Insert(tableServiceVirtualIPs, newEntry); err != nil { + return false, nil, fmt.Errorf("failed inserting service virtual IP entry: %s", err) + } + if err := updateVirtualIPMaxIndexes(tx, idx, psn.ServiceName.PartitionOrDefault(), psn.Peer); err != nil { + return false, nil, err + } + txnNeedsCommit = true } - if err := updateVirtualIPMaxIndexes(tx, idx, psn.ServiceName.PartitionOrDefault(), psn.Peer); err != nil { - return false, nil, err - } - if err = tx.Commit(); err != nil { - return false, nil, err + + if txnNeedsCommit { + if err = tx.Commit(); err != nil { + return false, nil, err + } } return true, maps.SliceOfKeys(modifiedEntries), nil } func updateVirtualIPMaxIndexes(txn WriteTxn, idx uint64, partition, peerName string) error { + // update global max index (for snapshots) + if err := indexUpdateMaxTxn(txn, idx, tableServiceVirtualIPs); err != nil { + return fmt.Errorf("failed while updating index: %w", err) + } // update per-partition max index if err := indexUpdateMaxTxn(txn, idx, partitionedIndexEntryName(tableServiceVirtualIPs, partition)); err != nil { return fmt.Errorf("failed while updating partitioned index: %w", err) @@ -3086,6 +3119,7 @@ func servicesVirtualIPsTxn(tx ReadTxn, ws memdb.WatchSet) (uint64, []ServiceVirt vips = append(vips, vip) } + // Pull from the global one idx := maxIndexWatchTxn(tx, nil, tableServiceVirtualIPs) return idx, vips, nil diff --git a/agent/consul/state/catalog_test.go b/agent/consul/state/catalog_test.go index cef608bc1c..8445acf987 100644 --- a/agent/consul/state/catalog_test.go +++ b/agent/consul/state/catalog_test.go @@ -13,15 +13,15 @@ import ( "testing" "time" - "github.com/hashicorp/consul/acl" - "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" - "github.com/hashicorp/go-memdb" - "github.com/hashicorp/go-uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/hashicorp/go-memdb" + "github.com/hashicorp/go-uuid" + + "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/lib/stringslice" @@ -1963,81 +1963,289 @@ func TestStateStore_AssignManualVirtualIPs(t *testing.T) { s := testStateStore(t) setVirtualIPFlags(t, s) - // Attempt to assign manual virtual IPs to a service that doesn't exist - should be a no-op. - psn := structs.PeeredServiceName{ServiceName: structs.ServiceName{Name: "foo", EnterpriseMeta: *acl.DefaultEnterpriseMeta()}} - found, svcs, err := s.AssignManualServiceVIPs(0, psn, []string{"7.7.7.7", "8.8.8.8"}) - require.NoError(t, err) - require.False(t, found) - require.Empty(t, svcs) - serviceVIP, err := s.ServiceManualVIPs(psn) - require.NoError(t, err) - require.Nil(t, serviceVIP) - - // Create the service registration. - entMeta := structs.DefaultEnterpriseMetaInDefaultPartition() - ns1 := &structs.NodeService{ - ID: "foo", - Service: "foo", - Address: "1.1.1.1", - Port: 1111, - Connect: structs.ServiceConnect{Native: true}, - EnterpriseMeta: *entMeta, + newPSN := func(name, peer string) structs.PeeredServiceName { + return structs.PeeredServiceName{ + ServiceName: structs.ServiceName{ + Name: name, + EnterpriseMeta: *acl.DefaultEnterpriseMeta(), + }, + Peer: peer, + } } - // Service successfully registers into the state store. - testRegisterNode(t, s, 0, "node1") - require.NoError(t, s.EnsureService(1, "node1", ns1)) + checkMaxIndexes := func(t *testing.T, expect, expectImported uint64) { + t.Helper() + tx := s.db.Txn(false) + defer tx.Abort() - // Make sure there's a virtual IP for the foo service. - vip, err := s.VirtualIPForService(psn) - require.NoError(t, err) - assert.Equal(t, "240.0.0.1", vip) + idx := maxIndexWatchTxn(tx, nil, tableServiceVirtualIPs) + require.Equal(t, expect, idx) - // No manual IP should be set yet. - serviceVIP, err = s.ServiceManualVIPs(psn) - require.NoError(t, err) - require.Equal(t, "0.0.0.1", serviceVIP.IP.String()) - require.Empty(t, serviceVIP.ManualIPs) + entMeta := acl.DefaultEnterpriseMeta() - // Attempt to assign manual virtual IPs again. - found, svcs, err = s.AssignManualServiceVIPs(2, psn, []string{"7.7.7.7", "8.8.8.8"}) - require.NoError(t, err) - require.True(t, found) - require.Empty(t, svcs) - serviceVIP, err = s.ServiceManualVIPs(psn) - require.NoError(t, err) - require.Equal(t, "0.0.0.1", serviceVIP.IP.String()) - require.Equal(t, serviceVIP.ManualIPs, []string{"7.7.7.7", "8.8.8.8"}) + importedIdx := maxIndexTxn(tx, partitionedIndexEntryName(tableServiceVirtualIPs+".imported", entMeta.PartitionOrDefault())) + require.Equal(t, expectImported, importedIdx) + } - // Register another service via config entry. - s.EnsureConfigEntry(3, &structs.ServiceResolverConfigEntry{ - Kind: structs.ServiceResolver, - Name: "bar", + assignManual := func( + t *testing.T, + idx uint64, + psn structs.PeeredServiceName, + ips []string, + modified ...structs.PeeredServiceName, + ) { + t.Helper() + found, svcs, err := s.AssignManualServiceVIPs(idx, psn, ips) + require.NoError(t, err) + require.True(t, found) + if len(modified) == 0 { + require.Empty(t, svcs) + } else { + require.ElementsMatch(t, modified, svcs) + } + } + + checkVIP := func( + t *testing.T, + psn structs.PeeredServiceName, + expectVIP string, + ) { + t.Helper() + // Make sure there's a virtual IP for the foo service. + vip, err := s.VirtualIPForService(psn) + require.NoError(t, err) + assert.Equal(t, expectVIP, vip) + } + + checkManualVIP := func( + t *testing.T, + psn structs.PeeredServiceName, + expectIP string, + expectManual []string, + expectIndex uint64, + ) { + t.Helper() + serviceVIP, err := s.ServiceManualVIPs(psn) + require.NoError(t, err) + require.Equal(t, expectIP, serviceVIP.IP.String()) + if len(expectManual) == 0 { + require.Empty(t, serviceVIP.ManualIPs) + } else { + require.Equal(t, expectManual, serviceVIP.ManualIPs) + } + require.Equal(t, expectIndex, serviceVIP.ModifyIndex) + } + + psn := newPSN("foo", "") + + lastIndex := uint64(0) + nextIndex := func() uint64 { + lastIndex++ + return lastIndex + } + + testutil.RunStep(t, "assign to nonexistent service is noop", func(t *testing.T) { + useIdx := nextIndex() + + // Attempt to assign manual virtual IPs to a service that doesn't exist - should be a no-op. + found, svcs, err := s.AssignManualServiceVIPs(useIdx, psn, []string{"7.7.7.7", "8.8.8.8"}) + require.NoError(t, err) + require.False(t, found) + require.Empty(t, svcs) + + serviceVIP, err := s.ServiceManualVIPs(psn) + require.NoError(t, err) + require.Nil(t, serviceVIP) + + checkMaxIndexes(t, 0, 0) }) - psn2 := structs.PeeredServiceName{ServiceName: structs.ServiceName{Name: "bar"}} - vip, err = s.VirtualIPForService(psn2) - require.NoError(t, err) - assert.Equal(t, "240.0.0.2", vip) + // Create the service registration. + var regIndex1 uint64 + testutil.RunStep(t, "create service 1", func(t *testing.T) { + useIdx := nextIndex() + regIndex1 = useIdx + + entMeta := acl.DefaultEnterpriseMeta() + ns1 := &structs.NodeService{ + ID: "foo", + Service: "foo", + Address: "1.1.1.1", + Port: 1111, + Connect: structs.ServiceConnect{Native: true}, + EnterpriseMeta: *entMeta, + } + + // Service successfully registers into the state store. + testRegisterNode(t, s, useIdx, "node1") + require.NoError(t, s.EnsureService(useIdx, "node1", ns1)) + + // Make sure there's a virtual IP for the foo service. + checkVIP(t, psn, "240.0.0.1") + + // No manual IP should be set yet. + checkManualVIP(t, psn, "0.0.0.1", []string{}, regIndex1) + + checkMaxIndexes(t, regIndex1, 0) + }) + + // Attempt to assign manual virtual IPs again. + var assignIndex1 uint64 + testutil.RunStep(t, "assign to existent service does something", func(t *testing.T) { + useIdx := nextIndex() + assignIndex1 = useIdx + + // inserting in the wrong order to test the string sort + assignManual(t, useIdx, psn, []string{"7.7.7.7", "8.8.8.8", "6.6.6.6"}) + + checkManualVIP(t, psn, "0.0.0.1", []string{ + "6.6.6.6", "7.7.7.7", "8.8.8.8", + }, assignIndex1) + + checkMaxIndexes(t, assignIndex1, 0) + }) + + psn2 := newPSN("bar", "") + + var regIndex2 uint64 + testutil.RunStep(t, "create service 2", func(t *testing.T) { + useIdx := nextIndex() + regIndex2 = useIdx + + // Register another service via config entry. + s.EnsureConfigEntry(useIdx, &structs.ServiceResolverConfigEntry{ + Kind: structs.ServiceResolver, + Name: "bar", + }) + + checkVIP(t, psn2, "240.0.0.2") + + // No manual IP should be set yet. + checkManualVIP(t, psn2, "0.0.0.2", []string{}, regIndex2) + + checkMaxIndexes(t, regIndex2, 0) + }) // Attempt to assign manual virtual IPs for bar, with one IP overlapping with foo. // This should cause the ip to be removed from foo's list of manual IPs. - found, svcs, err = s.AssignManualServiceVIPs(4, psn2, []string{"7.7.7.7", "9.9.9.9"}) - require.NoError(t, err) - require.True(t, found) - require.ElementsMatch(t, svcs, []structs.PeeredServiceName{psn}) + var assignIndex2 uint64 + testutil.RunStep(t, "assign to existent service and ip is removed from another", func(t *testing.T) { + useIdx := nextIndex() + assignIndex2 = useIdx - serviceVIP, err = s.ServiceManualVIPs(psn) - require.NoError(t, err) - require.Equal(t, "0.0.0.1", serviceVIP.IP.String()) - require.Equal(t, []string{"8.8.8.8"}, serviceVIP.ManualIPs) - require.Equal(t, uint64(4), serviceVIP.ModifyIndex) + assignManual(t, useIdx, psn2, []string{"7.7.7.7", "9.9.9.9"}, psn) - serviceVIP, err = s.ServiceManualVIPs(psn2) - require.NoError(t, err) - require.Equal(t, "0.0.0.2", serviceVIP.IP.String()) - require.Equal(t, []string{"7.7.7.7", "9.9.9.9"}, serviceVIP.ManualIPs) - require.Equal(t, uint64(4), serviceVIP.ModifyIndex) + checkManualVIP(t, psn, "0.0.0.1", []string{ + "6.6.6.6", "8.8.8.8", // 7.7.7.7 was stolen by psn2 + }, assignIndex2) + checkManualVIP(t, psn2, "0.0.0.2", []string{ + "7.7.7.7", "9.9.9.9", + }, assignIndex2) + + checkMaxIndexes(t, assignIndex2, 0) + }) + + psn3 := newPSN("gir", "peer1") + + var regIndex3 uint64 + testutil.RunStep(t, "create peered service 1", func(t *testing.T) { + useIdx := nextIndex() + regIndex3 = useIdx + + // Create the service registration. + entMetaPeer := acl.DefaultEnterpriseMeta() + nsPeer1 := &structs.NodeService{ + ID: "gir", + Service: "gir", + Address: "9.9.9.9", + Port: 2222, + PeerName: "peer1", + Connect: structs.ServiceConnect{Native: true}, + EnterpriseMeta: *entMetaPeer, + } + + // Service successfully registers into the state store. + testRegisterPeering(t, s, useIdx, "peer1") + testRegisterNodeOpts(t, s, useIdx, "node9", func(n *structs.Node) error { + n.PeerName = "peer1" + return nil + }) + require.NoError(t, s.EnsureService(useIdx, "node9", nsPeer1)) + + checkVIP(t, psn3, "240.0.0.3") + + // No manual IP should be set yet. + checkManualVIP(t, psn3, "0.0.0.3", []string{}, regIndex3) + + checkMaxIndexes(t, regIndex3, regIndex3) + }) + + // Assign manual virtual IPs to peered service. + var assignIndex3 uint64 + testutil.RunStep(t, "assign to peered service and steal from non-peered", func(t *testing.T) { + useIdx := nextIndex() + assignIndex3 = useIdx + + // 5.5.5.5 is stolen from psn + assignManual(t, useIdx, psn3, []string{"5.5.5.5", "6.6.6.6"}, psn) + + checkManualVIP(t, psn, "0.0.0.1", []string{ + "8.8.8.8", // 5.5.5.5 was stolen by psn3 + }, assignIndex3) + checkManualVIP(t, psn2, "0.0.0.2", []string{ + "7.7.7.7", "9.9.9.9", + }, assignIndex2) + checkManualVIP(t, psn3, "0.0.0.3", []string{ + "5.5.5.5", "6.6.6.6", + }, assignIndex3) + + checkMaxIndexes(t, assignIndex3, assignIndex3) + }) + + var assignIndex4 uint64 + testutil.RunStep(t, "assign to non-peered service and steal from peered", func(t *testing.T) { + useIdx := nextIndex() + assignIndex4 = useIdx + + // 6.6.6.6 is stolen from psn3 + assignManual(t, useIdx, psn2, []string{ + "7.7.7.7", "9.9.9.9", "6.6.6.6", + }, psn3) + + checkManualVIP(t, psn, "0.0.0.1", []string{ + "8.8.8.8", // 5.5.5.5 was stolen by psn3 + }, assignIndex3) + checkManualVIP(t, psn2, "0.0.0.2", []string{ + "6.6.6.6", "7.7.7.7", "9.9.9.9", + }, assignIndex4) + checkManualVIP(t, psn3, "0.0.0.3", []string{ + "5.5.5.5", + }, assignIndex4) + + checkMaxIndexes(t, assignIndex4, assignIndex4) + }) + + testutil.RunStep(t, "repeat the last write and no indexes should be bumped", func(t *testing.T) { + useIdx := nextIndex() + + assignManual(t, useIdx, psn2, []string{ + "7.7.7.7", "9.9.9.9", "6.6.6.6", + }) // no modified this time + + // no changes + checkManualVIP(t, psn, "0.0.0.1", []string{ + "8.8.8.8", + }, assignIndex3) + checkManualVIP(t, psn2, "0.0.0.2", []string{ + "6.6.6.6", "7.7.7.7", "9.9.9.9", + }, assignIndex4) + checkManualVIP(t, psn3, "0.0.0.3", []string{ + "5.5.5.5", + }, assignIndex4) + + // no change + checkMaxIndexes(t, assignIndex4, assignIndex4) + }) } func TestStateStore_EnsureService_ReassignFreedVIPs(t *testing.T) { diff --git a/lib/stringslice/stringslice.go b/lib/stringslice/stringslice.go index 7c32864b94..71e90dba2a 100644 --- a/lib/stringslice/stringslice.go +++ b/lib/stringslice/stringslice.go @@ -80,3 +80,17 @@ func CloneStringSlice(s []string) []string { copy(out, s) return out } + +// EqualMapKeys returns true if the slice equals the keys of +// the map ignoring any ordering. +func EqualMapKeys[V any](a []string, b map[string]V) bool { + if len(a) != len(b) { + return false + } + for _, ip := range a { + if _, ok := b[ip]; !ok { + return false + } + } + return true +} diff --git a/lib/stringslice/stringslice_test.go b/lib/stringslice/stringslice_test.go index dd25071757..b861d9c38a 100644 --- a/lib/stringslice/stringslice_test.go +++ b/lib/stringslice/stringslice_test.go @@ -63,3 +63,28 @@ func TestMergeSorted(t *testing.T) { }) } } + +func TestEqualMapKeys(t *testing.T) { + for _, tc := range []struct { + a []string + b map[string]int + same bool + }{ + // same + {nil, nil, true}, + {[]string{}, nil, true}, + {nil, map[string]int{}, true}, + {[]string{}, map[string]int{}, true}, + {[]string{"a"}, map[string]int{"a": 1}, true}, + {[]string{"b", "a"}, map[string]int{"a": 1, "b": 1}, true}, + // different + {[]string{"a"}, map[string]int{}, false}, + {[]string{}, map[string]int{"a": 1}, false}, + {[]string{"b", "a"}, map[string]int{"c": 1, "a": 1, "b": 1}, false}, + {[]string{"b", "a"}, map[string]int{"c": 1, "a": 1, "b": 1}, false}, + {[]string{"b", "a", "c"}, map[string]int{"a": 1, "b": 1}, false}, + } { + got := EqualMapKeys(tc.a, tc.b) + require.Equal(t, tc.same, got) + } +}