Allocate virtual ip for resolver/router/splitter config entries (#16760)

agent/consul/fsm/snapshot_test.go

@@ -518,6 +518,16 @@ func TestFSM_SnapshotRestore_OSS(t *testing.T) {
 		},
 	}))
 
+	// Add a service-resolver entry to get a virtual IP for service foo
+	resolverEntry := &structs.ServiceResolverConfigEntry{
+		Kind: structs.ServiceResolver,
+		Name: "foo",
+	}
+	require.NoError(t, fsm.state.EnsureConfigEntry(34, resolverEntry))
+	vip, err = fsm.state.VirtualIPForService(structs.PeeredServiceName{ServiceName: structs.NewServiceName("foo", nil)})
+	require.NoError(t, err)
+	require.Equal(t, vip, "240.0.0.3")
+
 	// Snapshot
 	snap, err := fsm.Snapshot()
 	require.NoError(t, err)
@@ -621,6 +631,10 @@ func TestFSM_SnapshotRestore_OSS(t *testing.T) {
 	vip, err = fsm2.state.VirtualIPForService(psn)
 	require.NoError(t, err)
 	require.Equal(t, vip, "240.0.0.2")
+	psn = structs.PeeredServiceName{ServiceName: structs.NewServiceName("foo", nil)}
+	vip, err = fsm2.state.VirtualIPForService(psn)
+	require.NoError(t, err)
+	require.Equal(t, vip, "240.0.0.3")
 
 	// Verify key is set
 	_, d, err := fsm2.state.KVSGet(nil, "/test", nil)

agent/consul/state/catalog.go

@@ -2016,6 +2016,33 @@ func freeServiceVirtualIP(
 		return nil
 	}
 
+	// Don't deregister the virtual IP if at least one instance of this service still exists.
+	q := Query{
+		Value:          psn.ServiceName.Name,
+		EnterpriseMeta: psn.ServiceName.EnterpriseMeta,
+		PeerName:       psn.Peer,
+	}
+	if remainingService, err := tx.First(tableServices, indexService, q); err == nil {
+		if remainingService != nil {
+			return nil
+		}
+	} else {
+		return fmt.Errorf("failed service lookup for %q: %s", psn.ServiceName.Name, err)
+	}
+
+	// Don't deregister the virtual IP if at least one resolver/router/splitter config entry still
+	// references this service.
+	configEntryVIPKinds := []string{structs.ServiceResolver, structs.ServiceRouter, structs.ServiceSplitter}
+	for _, kind := range configEntryVIPKinds {
+		_, entry, err := configEntryTxn(tx, nil, kind, psn.ServiceName.Name, &psn.ServiceName.EnterpriseMeta)
+		if err != nil {
+			return fmt.Errorf("failed config entry lookup for %s/%s: %s", kind, psn.ServiceName.Name, err)
+		}
+		if entry != nil {
+			return nil
+		}
+	}
+
 	// Don't deregister the virtual IP if at least one terminating gateway still references this service.
 	termGatewaySupported, err := terminatingGatewayVirtualIPsSupported(tx, nil)
 	if err != nil {

agent/consul/state/config_entry.go

@@ -455,6 +455,15 @@ func deleteConfigEntryTxn(tx WriteTxn, idx uint64, kind, name string, entMeta *a
 		return fmt.Errorf("failed updating index: %s", err)
 	}
 
+	// If this is a resolver/router/splitter, attempt to delete the virtual IP associated
+	// with this service.
+	if kind == structs.ServiceResolver || kind == structs.ServiceRouter || kind == structs.ServiceSplitter {
+		psn := structs.PeeredServiceName{ServiceName: sn}
+		if err := freeServiceVirtualIP(tx, idx, psn, nil); err != nil {
+			return fmt.Errorf("failed to clean up virtual IP for %q: %v", psn.String(), err)
+		}
+	}
+
 	return nil
 }
 
@@ -465,14 +474,15 @@ func insertConfigEntryWithTxn(tx WriteTxn, idx uint64, conf structs.ConfigEntry)
 
 	// If the config entry is for a terminating or ingress gateway we update the memdb table
 	// that associates gateways <-> services.
-	if conf.GetKind() == structs.TerminatingGateway || conf.GetKind() == structs.IngressGateway {
+	kind := conf.GetKind()
+	if kind == structs.TerminatingGateway || kind == structs.IngressGateway {
 		err := updateGatewayServices(tx, idx, conf, conf.GetEnterpriseMeta())
 		if err != nil {
 			return fmt.Errorf("failed to associate services to gateway: %v", err)
 		}
 	}
 
-	switch conf.GetKind() {
+	switch kind {
 	case structs.ServiceDefaults:
 		if conf.(*structs.ServiceConfigEntry).Destination != nil {
 			sn := structs.ServiceName{Name: conf.GetName(), EnterpriseMeta: *conf.GetEnterpriseMeta()}
@@ -499,6 +509,15 @@ func insertConfigEntryWithTxn(tx WriteTxn, idx uint64, conf structs.ConfigEntry)
 		if err != nil {
 			return err
 		}
+	case structs.ServiceResolver:
+		fallthrough
+	case structs.ServiceRouter:
+		fallthrough
+	case structs.ServiceSplitter:
+		psn := structs.PeeredServiceName{ServiceName: structs.NewServiceName(conf.GetName(), conf.GetEnterpriseMeta())}
+		if _, err := assignServiceVirtualIP(tx, idx, psn); err != nil {
+			return err
+		}
 	}
 
 	// Insert the config entry and update the index

agent/consul/state/config_entry_test.go

@@ -1,6 +1,7 @@
 package state
 
 import (
+	"fmt"
 	"testing"
 	"time"
 
@@ -3032,3 +3033,149 @@ func TestStore_ValidateServiceIntentionsErrorOnIncompatibleProtocols(t *testing.
 		})
 	}
 }
+
+func TestStateStore_ConfigEntry_VirtualIP(t *testing.T) {
+	createServiceInstance := func(t *testing.T, s *Store, name string) {
+		ns1 := &structs.NodeService{
+			ID:      name,
+			Service: name,
+			Address: "1.1.1.1",
+			Port:    1111,
+			Connect: structs.ServiceConnect{Native: true},
+		}
+		require.NoError(t, s.EnsureService(0, "node1", ns1))
+	}
+	deleteServiceInstance := func(t *testing.T, s *Store, name string) {
+		require.NoError(t, s.DeleteService(0, "node1", name, nil, ""))
+	}
+	createServiceResolver := func(t *testing.T, s *Store, name string) {
+		require.NoError(t, s.EnsureConfigEntry(0, &structs.ServiceResolverConfigEntry{
+			Kind: structs.ServiceResolver,
+			Name: name,
+		}))
+	}
+	createServiceRouter := func(t *testing.T, s *Store, name string) {
+		require.NoError(t, s.EnsureConfigEntry(0, &structs.ServiceRouterConfigEntry{
+			Kind: structs.ServiceRouter,
+			Name: name,
+		}))
+	}
+	createServiceSplitter := func(t *testing.T, s *Store, name string) {
+		require.NoError(t, s.EnsureConfigEntry(0, &structs.ServiceSplitterConfigEntry{
+			Kind: structs.ServiceSplitter,
+			Name: name,
+			Splits: []structs.ServiceSplit{
+				{Weight: 100},
+			},
+		}))
+	}
+	deleteConfigEntry := func(t *testing.T, s *Store, kind, name string) {
+		require.NoError(t, s.DeleteConfigEntry(0, kind, name, nil))
+	}
+	ensureVirtualIP := func(t *testing.T, s *Store, service string, value string) {
+		vip, err := s.VirtualIPForService(structs.PeeredServiceName{ServiceName: structs.ServiceName{Name: service}})
+		require.NoError(t, err)
+		require.Equal(t, value, vip)
+	}
+
+	testVIPStateStore := func(t *testing.T) *Store {
+		s := testStateStore(t)
+		setVirtualIPFlags(t, s)
+		testRegisterNode(t, s, 0, "node1")
+		s.EnsureConfigEntry(0, &structs.ProxyConfigEntry{
+			Kind: structs.ProxyDefaults,
+			Name: structs.ProxyConfigGlobal,
+			Config: map[string]interface{}{
+				"protocol": "http",
+			},
+		})
+		return s
+	}
+
+	cases := []struct {
+		kind       string
+		createFunc func(*testing.T, *Store, string)
+	}{
+		{
+			kind:       structs.ServiceResolver,
+			createFunc: createServiceResolver,
+		},
+		{
+			kind:       structs.ServiceRouter,
+			createFunc: createServiceRouter,
+		},
+		{
+			kind:       structs.ServiceSplitter,
+			createFunc: createServiceSplitter,
+		},
+	}
+	for _, tc := range cases {
+		t.Run(fmt.Sprintf("create and delete %s with no service instances", tc.kind), func(t *testing.T) {
+			s := testVIPStateStore(t)
+
+			// Create unrelated service instance
+			createServiceInstance(t, s, "unrelated")
+
+			// Create the config entry and make sure a virtual ip is allocated
+			ensureVirtualIP(t, s, "foo", "")
+			tc.createFunc(t, s, "foo")
+			ensureVirtualIP(t, s, "foo", "240.0.0.2")
+
+			// Delete the config entry and make sure the virtual ip is freed and reused
+			ensureVirtualIP(t, s, "bar", "")
+			deleteConfigEntry(t, s, tc.kind, "foo")
+			ensureVirtualIP(t, s, "foo", "")
+			tc.createFunc(t, s, "bar")
+			ensureVirtualIP(t, s, "bar", "240.0.0.2")
+		})
+
+		t.Run(fmt.Sprintf("create and delete %s with service instances", tc.kind), func(t *testing.T) {
+			s := testVIPStateStore(t)
+
+			// Create a foo service instance and an unrelated service instance
+			createServiceInstance(t, s, "foo")
+
+			// Creating the config entry should not affect the service virtual IP
+			ensureVirtualIP(t, s, "foo", "240.0.0.1")
+			tc.createFunc(t, s, "foo")
+			ensureVirtualIP(t, s, "foo", "240.0.0.1")
+
+			// Deleting should also not affect the service virtual IP because there are still existing
+			// service instances that need the VIP.
+			deleteConfigEntry(t, s, tc.kind, "foo")
+			ensureVirtualIP(t, s, "foo", "240.0.0.1")
+
+			// Now delete the service instance, which should free up the virtual IP
+			deleteServiceInstance(t, s, "foo")
+			ensureVirtualIP(t, s, "foo", "")
+
+			// Make sure the free address can be reused
+			tc.createFunc(t, s, "bar")
+			ensureVirtualIP(t, s, "bar", "240.0.0.1")
+		})
+
+		t.Run(fmt.Sprintf("create and delete service instance while %s still exists", tc.kind), func(t *testing.T) {
+			s := testVIPStateStore(t)
+
+			// Create the config entry to get the virtual IP
+			tc.createFunc(t, s, "foo")
+			ensureVirtualIP(t, s, "foo", "240.0.0.1")
+
+			// Creating service instance should not affect virtual IP
+			createServiceInstance(t, s, "foo")
+			ensureVirtualIP(t, s, "foo", "240.0.0.1")
+
+			// Deleting should also not affect the service virtual IP because the config entry still exists.
+			deleteServiceInstance(t, s, "foo")
+			ensureVirtualIP(t, s, "foo", "240.0.0.1")
+
+			// Now delete the config entry, which should free up the ip
+			deleteConfigEntry(t, s, tc.kind, "foo")
+			ensureVirtualIP(t, s, "foo", "")
+
+			// Make sure the free address can be reused
+			tc.createFunc(t, s, "bar")
+			ensureVirtualIP(t, s, "bar", "240.0.0.1")
+		})
+	}
+}