diff --git a/src/StackExchange.Redis/ClusterTopology.cs b/src/StackExchange.Redis/ClusterTopology.cs index a6ae7e3c2..71bcb9b02 100644 --- a/src/StackExchange.Redis/ClusterTopology.cs +++ b/src/StackExchange.Redis/ClusterTopology.cs @@ -110,6 +110,13 @@ internal ClusterTopologyNode(ClusterSlotNode node, string? primaryNodeId) /// public IReadOnlyList Identities => _identities ?? []; + /// + /// The endpoint form the deployment announced first (cluster-preferred-endpoint-type), which is the + /// form the client prefers to dial: a certificate validates against a name, and a companion address may not + /// even be routable from the client. The other are names the node also answers to. + /// + 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..ae02b8b74 100644 --- a/src/StackExchange.Redis/ConnectionMultiplexer.cs +++ b/src/StackExchange.Redis/ConnectionMultiplexer.cs @@ -1057,6 +1057,29 @@ internal async Task ApplyTopologyGenerationAsync(ClusterTopology topology, ILogg if (survivor is not null) { + // the identity list is ordered with the deployment's stated dial preference first; when that + // form is a hostname and the survivor dials something else, the node is held by a companion + // address - typically the pod IP from CLUSTER NODES, which may be unreachable from the + // client or reused after a restart (#3309) - and the hostname is also the form TLS can + // validate (#2826). Re-home it: adopt a server at the preferred form and retire the one + // dialing the companion address. Running here, on every applied topology, is what makes the + // preference hold however the companion server came to exist (a MOVED redirect, a NODES-only + // refresh, state inherited from an earlier preference) - not just on first discovery. A + // configured endpoint stays: the user's explicit seed may never be retired. + if (node.PreferredDialEndPoint is DnsEndPoint preferred + && survivor.Provenance != ServerProvenance.Configured + && !Equals(survivor.EndPoint, preferred)) + { + // exactForm: the preferred name may still be aliased to the companion server - that + // alias is the state being repaired, so resolution must not follow it here + var adopted = GetServerEndPoint(preferred, ServerProvenance.ClusterTopology, log, exactForm: true); + if (!ReferenceEquals(adopted, survivor)) + { + (merge ??= new()).Add(new DuplicateServers(adopted, survivor)); + survivor = adopted; + } + } + survivor.OnSeenInTopology(generation); seen.Add(survivor); @@ -1186,10 +1209,14 @@ internal void RegisterServerIdentities(ClusterTopology topology) EndPoint? endpoint, ServerProvenance provenance, ILogger? log = null, - bool activate = true) + bool activate = true, + bool exactForm = false) { if (endpoint == null) return null; - var server = (ServerEndPoint?)servers[endpoint] ?? TryResolveServerEndPoint(endpoint); + + // exactForm: a server keyed at this endpoint, created if absent, never one reached through an + // identity alias - for the caller whose job is to repair what an alias currently points at + var server = (ServerEndPoint?)servers[endpoint] ?? (exactForm ? null : TryResolveServerEndPoint(endpoint)); if (server == null) { bool isNew = false; @@ -2249,6 +2276,7 @@ public EndPoint[] GetEndPoints(bool configuredOnly = false) => var topology = ClusterTopology.From(slots); var clusterEndpoints = new EndPointCollection(); HashSet? listedInSlots = null; + Dictionary? preferredDialForm = null; if (topology is not null) { @@ -2263,7 +2291,17 @@ public EndPoint[] GetEndPoints(bool configuredOnly = false) => // every name the node answers to, not just the one chosen above: NODES may well // report it under a different form than SLOTS did - foreach (var identity in node.Identities) listedInSlots.Add(identity); + foreach (var identity in node.Identities) + { + listedInSlots.Add(identity); + + // ...and remember the stated dial preference for each secondary name, so the + // NODES loop below can create a missing node at the form the deployment announced + if (node.PreferredDialEndPoint is DnsEndPoint preferred && !Equals(identity, preferred)) + { + (preferredDialForm ??= new())[identity] = preferred; + } + } } } @@ -2289,9 +2327,13 @@ public EndPoint[] GetEndPoints(bool configuredOnly = false) => // 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); + // it waited on but undialled, for the whole ConnectTimeout (#3232). + // Created at the announced dial form where the deployment prefers hostnames: the form + // NODES reports here may be a companion pod IP not reachable from the client (#3309) + EndPoint dial = node.EndPoint; + if (preferredDialForm is not null && preferredDialForm.TryGetValue(dial, out var announced)) dial = announced; + log?.LogInformationRegisteringSlotMapNode(new(dial)); + GetServerEndPoint(dial, ServerProvenance.ClusterTopology, log); } else { diff --git a/tests/StackExchange.Redis.Tests/ClusterTopologyUnitTests.cs b/tests/StackExchange.Redis.Tests/ClusterTopologyUnitTests.cs index dae5641e9..6bd236fb9 100644 --- a/tests/StackExchange.Redis.Tests/ClusterTopologyUnitTests.cs +++ b/tests/StackExchange.Redis.Tests/ClusterTopologyUnitTests.cs @@ -101,6 +101,22 @@ 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); + } + [Fact] public async Task ShadowTopologyAgreesWithTheRoutingView() { @@ -196,6 +212,90 @@ public async Task HostnamePreferredClusterRoutesWithoutDuplicatingEndpoints() Assert.Equal(2, conn.GetEndPoints().Length); } + [Fact] + public async Task HostnameAnnouncedShardIsDiscoveredByHostname() + { + // the scenario from #3309: discover a new shard from a single seed. SLOTS names it by hostname while + // NODES reports its companion IP - often a pod address that is not reachable from the client - so + // discovery must dial the hostname and keep the IP only as an alias + using var server = CreateServer(log, ClusterEndpointType.Hostname); + GetHost(server.DefaultEndPoint, out var port); + var companionIp = new IPEndPoint(IPAddress.Loopback, port + 1); + var other = server.AddEmptyNode(companionIp); + server.SetHostname(other, "host-2.redis.example.com"); + server.Migrate((RedisKey)"hostname-routed-key", other); + + 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); + Assert.True( + await Poll.UntilAsync(() => conn.GetEndPoints().Contains(hostnameEndpoint)), + $"{hostnameEndpoint} was never discovered"); + + // one explicit pass, so the assertions observe an applied topology rather than racing the + // passes that ran during connect and redirect handling + await ApplyTopologyAsync(conn, server.DefaultEndPoint); + + Assert.DoesNotContain(companionIp, conn.GetEndPoints()); + Assert.Equal(hostnameEndpoint, conn.GetServer(companionIp).EndPoint); // the IP resolves as an alias + + foreach (var ep in conn.GetEndPoints()) + { + log.WriteLine($"endpoint: {ep}"); + } + EndpointResolutionUnitTests.AssertOneEndpointPerNode(conn, log); + } + + [Fact] + public async Task ShardHeldByAddressMovesToItsAnnouncedHostname() + { + // the durable half of #3309: the announced dial preference is applied on *every* topology + // application, not only at first discovery. A node already held by its companion address - learned + // while the deployment still preferred IPs, or created by a MOVED redirect or a NODES-only refresh - + // is re-homed to the hostname once the topology announces it + using var server = CreateServer(log, ClusterEndpointType.Ip); + GetHost(server.DefaultEndPoint, out var port); + var companionIp = new IPEndPoint(IPAddress.Loopback, port + 1); + var other = server.AddEmptyNode(companionIp); + server.SetHostname(other, "host-2.redis.example.com"); + server.Migrate((RedisKey)"hostname-routed-key", other); + + await using var conn = await server.ConnectAsync(defaultOnly: true); + Assert.True( + await Poll.UntilAsync(() => conn.GetEndPoints().Contains(companionIp)), + $"{companionIp} was never discovered"); + + // the deployment flips to hostname announcement... + server.PreferredEndpointType = ClusterEndpointType.Hostname; + await ApplyTopologyAsync(conn, server.DefaultEndPoint); + + // ...and the shard moves to its hostname, with the address kept as an alias + var hostnameEndpoint = new DnsEndPoint("host-2.redis.example.com", port + 1); + Assert.Contains(hostnameEndpoint, conn.GetEndPoints()); + Assert.DoesNotContain(companionIp, conn.GetEndPoints()); + Assert.Equal(hostnameEndpoint, conn.GetServer(companionIp).EndPoint); + + // the configured seed is exempt: an explicit endpoint is never re-homed from under the caller + Assert.Contains(server.DefaultEndPoint, conn.GetEndPoints()); + + EndpointResolutionUnitTests.AssertOneEndpointPerNode(conn, log); + } + + /// + /// Applies one topology pass - slot map and generation, as the production path does - from a fresh + /// CLUSTER SLOTS call, so a test can observe the next application without waiting for one. + /// + private static async Task ApplyTopologyAsync(IConnectionMultiplexer conn, EndPoint askOf) + { + var mux = (ConnectionMultiplexer)conn; + var topology = await GetShadowAsync(conn, askOf); + mux.UpdateClusterRange(topology); + await mux.ApplyTopologyGenerationAsync(topology); + } + [Fact] public async Task SlotLessNodesAreKnownButNotConnected() {