Backport of state: ensure that identical manual virtual IP updates result in not bumping the modify indexes into release/1.20.x (#21969)

The consul-k8s endpoints controller issues catalog register and manual virtual ip
updates without first checking to see if the updates would be effectively not
changing anything. This is supposed to be reasonable because the state store
functions do the check for a no-op update and should discard repeat updates so
that downstream blocking queries watching one of the resources don't fire
pointlessly (and CPU wastefully).

While this is true for the check/service/node catalog updates, it is not true for
the "manual virtual ip" updates triggered by the PUT /v1/internal/service-virtual-ip.
Forcing the connect injector pod to recycle while watching some lightly
modified FSM code can show that a lot of updates are of the update list of ips
from [A] to [A]. Immediately following this stray update you can see a lot of
activity in proxycfg and xds packages waking up due to blocking queries
triggered by this.

This PR skips updates that change nothing both:

- at the RPC layer before passing it to raft (ideally)
- if the write does make it through raft and get applied to the FSM (failsafe)

.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
+```

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{
+		Service:   psn,
-			ServiceName: structs.NewServiceName(args.Service, &args.EnterpriseMeta),
-		},
 		ManualIPs: args.ManualVIPs,
 	}
 	resp, err := m.srv.raftApplyMsgpack(structs.UpdateVirtualIPRequestType, req)

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"
@@ -3888,18 +3888,35 @@ func TestInternal_AssignManualServiceVIPs(t *testing.T) {
 		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)
+			testutil.RequireErrorContains(t, err, tc.expectErr)
-			require.Contains(t, err.Error(), tc.expectErr)
+		} else {
-			return
+			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)
+		}
+	}
+
 	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"},
 			},
+			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
+				})
+			}
 		})
 	}
 }

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
+
+	// 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 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

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,18 +1963,110 @@ func TestStateStore_AssignManualVirtualIPs(t *testing.T) {
 	s := testStateStore(t)
 	setVirtualIPFlags(t, s)
 
+	newPSN := func(name, peer string) structs.PeeredServiceName {
+		return structs.PeeredServiceName{
+			ServiceName: structs.ServiceName{
+				Name:           name,
+				EnterpriseMeta: *acl.DefaultEnterpriseMeta(),
+			},
+			Peer: peer,
+		}
+	}
+
+	checkMaxIndexes := func(t *testing.T, expect, expectImported uint64) {
+		t.Helper()
+		tx := s.db.Txn(false)
+		defer tx.Abort()
+
+		idx := maxIndexWatchTxn(tx, nil, tableServiceVirtualIPs)
+		require.Equal(t, expect, idx)
+
+		entMeta := acl.DefaultEnterpriseMeta()
+
+		importedIdx := maxIndexTxn(tx, partitionedIndexEntryName(tableServiceVirtualIPs+".imported", entMeta.PartitionOrDefault()))
+		require.Equal(t, expectImported, importedIdx)
+	}
+
+	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.
-	psn := structs.PeeredServiceName{ServiceName: structs.ServiceName{Name: "foo", EnterpriseMeta: *acl.DefaultEnterpriseMeta()}}
+		found, svcs, err := s.AssignManualServiceVIPs(useIdx, psn, []string{"7.7.7.7", "8.8.8.8"})
-	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)
 
+		checkMaxIndexes(t, 0, 0)
+	})
+
 	// Create the service registration.
-	entMeta := structs.DefaultEnterpriseMetaInDefaultPartition()
+	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",
@@ -1985,59 +2077,175 @@ func TestStateStore_AssignManualVirtualIPs(t *testing.T) {
 		}
 
 		// Service successfully registers into the state store.
-	testRegisterNode(t, s, 0, "node1")
+		testRegisterNode(t, s, useIdx, "node1")
-	require.NoError(t, s.EnsureService(1, "node1", ns1))
+		require.NoError(t, s.EnsureService(useIdx, "node1", ns1))
 
 		// Make sure there's a virtual IP for the foo service.
-	vip, err := s.VirtualIPForService(psn)
+		checkVIP(t, psn, "240.0.0.1")
-	require.NoError(t, err)
-	assert.Equal(t, "240.0.0.1", vip)
 
 		// No manual IP should be set yet.
-	serviceVIP, err = s.ServiceManualVIPs(psn)
+		checkManualVIP(t, psn, "0.0.0.1", []string{}, regIndex1)
-	require.NoError(t, err)
+
-	require.Equal(t, "0.0.0.1", serviceVIP.IP.String())
+		checkMaxIndexes(t, regIndex1, 0)
-	require.Empty(t, serviceVIP.ManualIPs)
+	})
 
 	// Attempt to assign manual virtual IPs again.
-	found, svcs, err = s.AssignManualServiceVIPs(2, psn, []string{"7.7.7.7", "8.8.8.8"})
+	var assignIndex1 uint64
-	require.NoError(t, err)
+	testutil.RunStep(t, "assign to existent service does something", func(t *testing.T) {
-	require.True(t, found)
+		useIdx := nextIndex()
-	require.Empty(t, svcs)
+		assignIndex1 = useIdx
-	serviceVIP, err = s.ServiceManualVIPs(psn)
+
-	require.NoError(t, err)
+		// inserting in the wrong order to test the string sort
-	require.Equal(t, "0.0.0.1", serviceVIP.IP.String())
+		assignManual(t, useIdx, psn, []string{"7.7.7.7", "8.8.8.8", "6.6.6.6"})
-	require.Equal(t, serviceVIP.ManualIPs, []string{"7.7.7.7", "8.8.8.8"})
+
+		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(3, &structs.ServiceResolverConfigEntry{
+		s.EnsureConfigEntry(useIdx, &structs.ServiceResolverConfigEntry{
 			Kind: structs.ServiceResolver,
 			Name: "bar",
 		})
 
-	psn2 := structs.PeeredServiceName{ServiceName: structs.ServiceName{Name: "bar"}}
+		checkVIP(t, psn2, "240.0.0.2")
-	vip, err = s.VirtualIPForService(psn2)
+
-	require.NoError(t, err)
+		// No manual IP should be set yet.
-	assert.Equal(t, "240.0.0.2", vip)
+		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"})
+	var assignIndex2 uint64
-	require.NoError(t, err)
+	testutil.RunStep(t, "assign to existent service and ip is removed from another", func(t *testing.T) {
-	require.True(t, found)
+		useIdx := nextIndex()
-	require.ElementsMatch(t, svcs, []structs.PeeredServiceName{psn})
+		assignIndex2 = useIdx
 
-	serviceVIP, err = s.ServiceManualVIPs(psn)
+		assignManual(t, useIdx, psn2, []string{"7.7.7.7", "9.9.9.9"}, 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)
 
-	serviceVIP, err = s.ServiceManualVIPs(psn2)
+		checkManualVIP(t, psn, "0.0.0.1", []string{
-	require.NoError(t, err)
+			"6.6.6.6", "8.8.8.8", // 7.7.7.7 was stolen by psn2
-	require.Equal(t, "0.0.0.2", serviceVIP.IP.String())
+		}, assignIndex2)
-	require.Equal(t, []string{"7.7.7.7", "9.9.9.9"}, serviceVIP.ManualIPs)
+		checkManualVIP(t, psn2, "0.0.0.2", []string{
-	require.Equal(t, uint64(4), serviceVIP.ModifyIndex)
+			"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) {

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

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