diff --git a/src/StackExchange.Redis/ClusterTopology.cs b/src/StackExchange.Redis/ClusterTopology.cs index a6ae7e3c2..52b549bfa 100644 --- a/src/StackExchange.Redis/ClusterTopology.cs +++ b/src/StackExchange.Redis/ClusterTopology.cs @@ -110,6 +110,12 @@ internal ClusterTopologyNode(ClusterSlotNode node, string? primaryNodeId) /// public IReadOnlyList Identities => _identities ?? []; + /// + /// The announced endpoint form to use for dialing. Companion identities such as an IP address must not be + /// registered as separate dialable servers when this preferred endpoint is a hostname. + /// + public EndPoint? PreferredDialEndPoint => Identities.Count > 0 ? Identities[0] : null; + /// The slots this node serves as primary; empty for a replica. public IReadOnlyList Slots => _slots ?? []; diff --git a/src/StackExchange.Redis/ConnectionMultiplexer.cs b/src/StackExchange.Redis/ConnectionMultiplexer.cs index 49a5682e5..b767a766e 100644 --- a/src/StackExchange.Redis/ConnectionMultiplexer.cs +++ b/src/StackExchange.Redis/ConnectionMultiplexer.cs @@ -2286,12 +2286,12 @@ public EndPoint[] GetEndPoints(bool configuredOnly = false) => else if (listedInSlots!.Contains(node.EndPoint)) { // SLOTS lists it, so it is in clusterEndpoints and we are about to connect to it and - // wait for it. Only the *existence* of a server was ever checked here, which a node - // in this state fails - a replica of a shard we have not reached yet is created by - // its primary's genealogy pass, and that has not run. Registering it inert then left - // it waited on but undialled, for the whole ConnectTimeout (#3232) - log?.LogInformationRegisteringSlotMapNode(new(node.EndPoint)); - GetServerEndPoint(node.EndPoint, ServerProvenance.ClusterTopology, log); + // wait for it. Prefer the SLOTS hostname over its companion IP from NODES: that IP + // may be an unreachable pod address using the ingress client port. + var dialEndPoint = FindPreferredDialEndPoint(topology, node.EndPoint) ?? node.EndPoint; + log?.LogInformationRegisteringSlotMapNode(new(dialEndPoint)); + GetServerEndPoint(dialEndPoint, ServerProvenance.ClusterTopology, log); + if (!Equals(dialEndPoint, node.EndPoint)) RegisterServerIdentities(topology); } else { @@ -2320,18 +2320,44 @@ public EndPoint[] GetEndPoints(bool configuredOnly = false) => return null; } - // an identity we already hold if there is one - otherwise the form the answering node advertised, - // which is what Identities is ordered by, and which is the form TLS can validate EndPoint? SelectIdentity(ClusterTopologyNode node) { + var preferred = node.PreferredDialEndPoint; + ServerEndPoint? preferredKnown = null, dnsKnown = null, anyKnown = null; + foreach (var identity in node.Identities) { - if (TryResolveServerEndPoint(identity) is { } known) return known.EndPoint; + if (TryResolveServerEndPoint(identity) is not { } known) continue; + anyKnown ??= known; + if (preferred is not null && Equals(known.EndPoint, preferred)) preferredKnown = known; + if (known.EndPoint is DnsEndPoint) dnsKnown ??= known; + } + + if (preferred is DnsEndPoint) + { + return (preferredKnown ?? dnsKnown)?.EndPoint ?? preferred; } - return node.Identities.Count > 0 ? node.Identities[0] : null; + + return (preferredKnown ?? anyKnown)?.EndPoint ?? preferred; } } + /// + /// Finds the SLOTS preferred dial endpoint for a node that also answers at + /// (typically the IP form from CLUSTER NODES). + /// + private static EndPoint? FindPreferredDialEndPoint(ClusterTopology topology, EndPoint identity) + { + foreach (var node in topology.Nodes) + { + foreach (var id in node.Identities) + { + if (Equals(id, identity)) return node.PreferredDialEndPoint; + } + } + return null; + } + private void ResetAllNonConnected() { var snapshot = GetServerSnapshot(); @@ -2552,8 +2578,8 @@ internal void UpdateClusterRange(ClusterTopology topology) { if (node.IsReplica || node.Slots.Count == 0) continue; - // resolve by *any* identity the node answers to before falling back to the form this reply - // happened to use; that is what keeps one node from becoming two ServerEndPoints + // Resolve the preferred server first, and do not adopt a companion IP server when hostnames + // are preferred. var server = ResolveOrCreate(node); if (server is null) continue; @@ -2565,15 +2591,36 @@ internal void UpdateClusterRange(ClusterTopology topology) ServerEndPoint? ResolveOrCreate(ClusterTopologyNode node) { + var preferred = node.PreferredDialEndPoint; + ServerEndPoint? preferredKnown = null, dnsKnown = null, anyKnown = null; + foreach (var identity in node.Identities) { - if (TryResolveServerEndPoint(identity) is { } known) return known; + if (TryResolveServerEndPoint(identity) is not { } known) continue; + anyKnown ??= known; + if (preferred is not null && Equals(known.EndPoint, preferred)) preferredKnown = known; + if (known.EndPoint is DnsEndPoint) dnsKnown ??= known; } - // unknown node: dial the form the answering node *advertised*, which Identities is ordered by. - // Not the address by preference: a certificate validates against a name, and where hostnames - // are preferred the advertised address may not even be routable (#2826) - return node.Identities.Count > 0 ? GetServerEndPoint(node.Identities[0], ServerProvenance.ClusterTopology) : null; + if (preferred is DnsEndPoint) + { + // Hostname-routed TLS/SNI clusters: never adopt a companion IP ServerEndPoint for + // the slot map. That IP is often the pod address with the ingress client port and + // does not survive pod restart. + if (preferredKnown is not null) return preferredKnown; + if (dnsKnown is not null) return dnsKnown; + return preferred is not null + ? GetServerEndPoint(preferred, ServerProvenance.ClusterTopology) + : null; + } + + if (preferredKnown is not null) return preferredKnown; + if (anyKnown is not null) return anyKnown; + + // Unknown node: dial the form the answering node advertised (#2826). + return preferred is not null + ? GetServerEndPoint(preferred, ServerProvenance.ClusterTopology) + : null; } } diff --git a/tests/StackExchange.Redis.Tests/ClusterTopologyUnitTests.cs b/tests/StackExchange.Redis.Tests/ClusterTopologyUnitTests.cs index dae5641e9..164815622 100644 --- a/tests/StackExchange.Redis.Tests/ClusterTopologyUnitTests.cs +++ b/tests/StackExchange.Redis.Tests/ClusterTopologyUnitTests.cs @@ -101,6 +101,23 @@ public async Task ShadowTopologyKnowsBothIdentities(ClusterEndpointType preferre Assert.Contains(new DnsEndPoint(Hostname, port), node.Identities); } + [Theory] + [InlineData(ClusterEndpointType.Ip)] + [InlineData(ClusterEndpointType.Hostname)] + public async Task PreferredDialEndPointFollowsAnnouncedForm(ClusterEndpointType preferred) + { + using var server = CreateServer(log, preferred); + await using var conn = await server.ConnectAsync(defaultOnly: true); + + var node = Assert.Single((await GetShadowAsync(conn, server.DefaultEndPoint)).Nodes); + GetHost(server.DefaultEndPoint, out var port); + EndPoint expected = preferred == ClusterEndpointType.Hostname + ? new DnsEndPoint(Hostname, port) + : new IPEndPoint(IPAddress.Loopback, port); + Assert.Equal(expected, node.PreferredDialEndPoint); + Assert.Equal(node.Identities[0], node.PreferredDialEndPoint); + } + [Fact] public async Task ShadowTopologyAgreesWithTheRoutingView() { @@ -173,19 +190,25 @@ public async Task SlotMapIsDrivenByTheSlotsView() [Fact] public async Task HostnamePreferredClusterRoutesWithoutDuplicatingEndpoints() { - // the case the flip exists for: SLOTS names every node by hostname while NODES names them by - // address, so a slot map fed from SLOTS must still resolve to the servers we already hold + // Discover the new shard from a single seed. SLOTS names it by hostname, while NODES reports its + // companion IP; routing and registration must use the hostname and retain the IP only as an alias. using var server = CreateServer(log, ClusterEndpointType.Hostname); GetHost(server.DefaultEndPoint, out var port); var other = server.AddEmptyNode(new IPEndPoint(IPAddress.Loopback, port + 1)); server.SetHostname(other, "host-2.redis.example.com"); server.Migrate((RedisKey)"hostname-routed-key", other); - await using var conn = await server.ConnectAsync(); + await using var conn = await server.ConnectAsync(defaultOnly: true); var db = conn.GetDatabase(); await db.StringSetAsync("hostname-routed-key", "value"); Assert.Equal("value", await db.StringGetAsync("hostname-routed-key")); + var hostnameEndpoint = new DnsEndPoint("host-2.redis.example.com", port + 1); + var companionIp = new IPEndPoint(IPAddress.Loopback, port + 1); + Assert.Contains(hostnameEndpoint, conn.GetEndPoints()); + Assert.DoesNotContain(companionIp, conn.GetEndPoints()); + Assert.Equal(hostnameEndpoint, conn.GetServer(companionIp).EndPoint); + foreach (var ep in conn.GetEndPoints()) { log.WriteLine($"endpoint: {ep}");