Prefer announce hostname when dialing cluster node on reconnect

src/StackExchange.Redis/ClusterTopology.cs

@@ -110,6 +110,13 @@ internal ClusterTopologyNode(ClusterSlotNode node, string? primaryNodeId)
     /// </summary>
     public IReadOnlyList<EndPoint> Identities => _identities ?? [];
 
+    /// <summary>
+    /// The endpoint form the deployment announced first (<c>cluster-preferred-endpoint-type</c>), 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 <see cref="Identities"/> are names the node also answers to.
+    /// </summary>
+    public EndPoint? PreferredDialEndPoint => Identities.Count > 0 ? Identities[0] : null;
+
     /// <summary>The slots this node serves as primary; empty for a replica.</summary>
     public IReadOnlyList<SlotRange> Slots => _slots ?? [];
 

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<EndPoint>? listedInSlots = null;
+                Dictionary<EndPoint, EndPoint>? 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
                     {

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);
+    }
+
+    /// <summary>
+    /// Applies one topology pass - slot map and generation, as the production path does - from a fresh
+    /// <c>CLUSTER SLOTS</c> call, so a test can observe the next application without waiting for one.
+    /// </summary>
+    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()
     {