Skip to content

Commit d3ab0c7

Browse files
committed
fix: parse CLUSTER NODES endpoints IPv6-aware in stale-address detection
CLUSTER NODES renders IPv6 endpoints bracketed ([fd00::2]:6379@16379). Slicing the endpoint at its last colon kept the brackets, so a failing IPv6 peer at its current address never compared equal to the bare pod IP Kubernetes reports - misclassifying it as moved and issuing spurious CLUSTER MEETs plus requeues on IPv6 clusters. Extract the host with net.SplitHostPort (after trimming the @cport and optional ,hostname suffix), which unbrackets IPv6 and leaves IPv4 untouched. Signed-off-by: Matan David <matan.david@eon.io>
1 parent 5c88340 commit d3ab0c7

2 files changed

Lines changed: 82 additions & 5 deletions

File tree

internal/valkey/clusterstate.go

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"crypto/tls"
2222
"fmt"
23+
"net"
2324
"slices"
2425
"strconv"
2526
"strings"
@@ -396,6 +397,27 @@ func (s *ClusterState) FindNodeById(id string) *NodeState {
396397
return nil
397398
}
398399

400+
// hostFromClusterNodesEndpoint extracts the bare host from a CLUSTER NODES
401+
// endpoint field (<ip:port@cport[,hostname]>). IPv6 hosts appear bracketed
402+
// ([fd00::2]:6379@16379); net.SplitHostPort unbrackets them so the result
403+
// compares equal to the bare pod IP Kubernetes reports. Returns "" when the
404+
// field has no parsable host:port part.
405+
func hostFromClusterNodesEndpoint(endpoint string) string {
406+
// Drop the cluster-bus suffix and the optional ,hostname after it.
407+
if i := strings.Index(endpoint, "@"); i != -1 {
408+
endpoint = endpoint[:i]
409+
}
410+
if host, _, err := net.SplitHostPort(endpoint); err == nil {
411+
return host
412+
}
413+
// Fallback for entries with no port (shouldn't occur in CLUSTER NODES,
414+
// but keep the previous last-colon behavior rather than dropping them).
415+
if i := strings.LastIndex(endpoint, ":"); i != -1 {
416+
return strings.Trim(endpoint[:i], "[]")
417+
}
418+
return ""
419+
}
420+
399421
// StaleAddressPeer pairs a viewer node with a live cluster member whose
400422
// address in the viewer's node table is outdated.
401423
type StaleAddressPeer struct {
@@ -442,11 +464,7 @@ func (s *ClusterState) FindStaleAddressPeers() []StaleAddressPeer {
442464
if !ok {
443465
continue
444466
}
445-
idx := strings.LastIndex(fields[1], ":")
446-
if idx == -1 {
447-
continue
448-
}
449-
if address := fields[1][:idx]; address != peer.Address {
467+
if address := hostFromClusterNodesEndpoint(fields[1]); address != "" && address != peer.Address {
450468
stale = append(stale, StaleAddressPeer{Viewer: viewer, Live: peer})
451469
}
452470
}

internal/valkey/clusterstate_test.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -640,3 +640,62 @@ func TestClusterState_FindNodeById(t *testing.T) {
640640
t.Error("expected nil for unknown id")
641641
}
642642
}
643+
644+
func TestHostFromClusterNodesEndpoint(t *testing.T) {
645+
cases := []struct{ in, want string }{
646+
{"10.0.1.2:6379@16379", "10.0.1.2"},
647+
{"10.0.1.2:6379@16379,node-1.example", "10.0.1.2"},
648+
{"[fd00::2]:6379@16379", "fd00::2"},
649+
{"[fd00::2]:6379@16379,node-1.example", "fd00::2"},
650+
{"[fd00::2]:6379", "fd00::2"},
651+
{"10.0.1.2:6379", "10.0.1.2"},
652+
{"garbage", ""},
653+
}
654+
for _, c := range cases {
655+
if got := hostFromClusterNodesEndpoint(c.in); got != c.want {
656+
t.Errorf("hostFromClusterNodesEndpoint(%q) = %q, want %q", c.in, got, c.want)
657+
}
658+
}
659+
}
660+
661+
func TestClusterState_FindStaleAddressPeers_IPv6(t *testing.T) {
662+
node := func(id, address, clusterNodes string) *NodeState {
663+
return &NodeState{Id: id, Address: address, ClusterNodes: clusterNodes}
664+
}
665+
666+
t.Run("failing IPv6 peer at its current address is not stale", func(t *testing.T) {
667+
// Bracketed table entry vs bare pod IP must compare equal.
668+
a := node("aaa", "fd00::1",
669+
"aaa [fd00::1]:6379@16379 myself,master - 0 0 1 connected 0-8191\n"+
670+
"bbb [fd00::2]:6379@16379 master,fail? - 0 0 2 connected 8192-16383\n")
671+
b := node("bbb", "fd00::2",
672+
"bbb [fd00::2]:6379@16379 myself,master - 0 0 2 connected 8192-16383\n"+
673+
"aaa [fd00::1]:6379@16379 master - 0 0 1 connected 0-8191\n")
674+
state := &ClusterState{Shards: []*ShardState{
675+
{Id: "s1", PrimaryId: "aaa", Nodes: []*NodeState{a}},
676+
{Id: "s2", PrimaryId: "bbb", Nodes: []*NodeState{b}},
677+
}}
678+
679+
if stale := state.FindStaleAddressPeers(); len(stale) != 0 {
680+
t.Fatalf("expected no stale pairs for current bracketed IPv6 address, got %d", len(stale))
681+
}
682+
})
683+
684+
t.Run("moved IPv6 peer is stale", func(t *testing.T) {
685+
a := node("aaa", "fd00::11",
686+
"aaa [fd00::11]:6379@16379 myself,master - 0 0 1 connected 0-8191\n"+
687+
"bbb [fd00::2]:6379@16379 master,fail? - 0 0 2 connected 8192-16383\n")
688+
b := node("bbb", "fd00::12",
689+
"bbb [fd00::12]:6379@16379 myself,master - 0 0 2 connected 8192-16383\n"+
690+
"aaa [fd00::11]:6379@16379 master - 0 0 1 connected 0-8191\n")
691+
state := &ClusterState{Shards: []*ShardState{
692+
{Id: "s1", PrimaryId: "aaa", Nodes: []*NodeState{a}},
693+
{Id: "s2", PrimaryId: "bbb", Nodes: []*NodeState{b}},
694+
}}
695+
696+
stale := state.FindStaleAddressPeers()
697+
if len(stale) != 1 || stale[0].Viewer.Id != "aaa" || stale[0].Live.Id != "bbb" {
698+
t.Fatalf("expected exactly aaa->bbb, got %v", stale)
699+
}
700+
})
701+
}

0 commit comments

Comments
 (0)