From 67f95b8cd7be0af4f9129853da278a52a5420245 Mon Sep 17 00:00:00 2001 From: Artem Vovk Date: Thu, 17 Sep 2026 09:06:49 -0600 Subject: [PATCH 1/2] fix(raft): keep hostnames as given in the raft store after the raft library update, the old function normalizeRaftHostnameIP ended up storing node's IP addresses as ServerIDs in the raft store. This becomes a problem in emphemeral environments like kubernetes where hostnames remain stable but IPs may change at any point. The problem: quorum is lost when nodes of a cluster restart and change IP addresses. Repro steps: 1. Stand up orchestrator with raft on, 3+ pods, hostnames (not IPs) in RaftAdvertise/RaftNodes. 2. Check raft state, confirm everyone's happy and in quorum. 3. kubectl delete pod orchestrator-1: same hostname comes back, different IP. 4. Check raft state again, that pod can't rejoin, the old IP is still stuck in the member list; 5. restart a few more pods and the whole quorum falls apart. Fastest way to see the actual bug without spinning up k8s: just call normalizeRaftHostnameIP("localhost"); it comes back as ::1 instead of localhost. In some cases that even blows up the address string outright (too many colons in address) before you get anywhere near a quorum problem. Bonus fix: handle IPv6 normalization Signed-off-by: Artem Vovk --- go/raft/raft.go | 40 ++++++++------------- go/raft/raft_test.go | 65 +++++++++++++++++++++++++++++++++ go/raft/store_test.go | 84 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 163 insertions(+), 26 deletions(-) create mode 100644 go/raft/raft_test.go create mode 100644 go/raft/store_test.go diff --git a/go/raft/raft.go b/go/raft/raft.go index 617bea2e6..87a2a5ea6 100644 --- a/go/raft/raft.go +++ b/go/raft/raft.go @@ -21,6 +21,7 @@ import ( "fmt" "math/rand" "net" + "strconv" "strings" "sync" "sync/atomic" @@ -175,37 +176,24 @@ func getRaft() *raft.Raft { return store.raft } -func normalizeRaftHostnameIP(host string) (string, error) { - if ip := net.ParseIP(host); ip != nil { - // this is a valid IP address. - return host, nil - } - ips, err := net.LookupIP(host) - if err != nil { - // resolve failed. But we don't want to fail the entire operation for that - _ = log.Errore(err) - return host, nil - } - // resolve success! - for _, ip := range ips { - return ip.String(), nil - } - return host, fmt.Errorf("%+v resolved but no IP found", host) -} - // normalizeRaftNode attempts to make sure there's a port to the given node. -// It consults the DefaultRaftPort when there isn't +// It consults the DefaultRaftPort when there isn't. The host is kept as +// given (never resolved to an IP), since it becomes this node's persistent +// raft ServerID. func normalizeRaftNode(node string) (string, error) { - hostPort := strings.Split(node, ":") - host, err := normalizeRaftHostnameIP(hostPort[0]) + host, port, err := net.SplitHostPort(node) if err != nil { - return host, err + host, port = node, "" + } + if net.ParseIP(host) == nil { + if _, err := net.LookupHost(host); err != nil { + _ = log.Errore(err) + } } - if len(hostPort) > 1 { - return fmt.Sprintf("%s:%s", host, hostPort[1]), nil + if port != "" { + return net.JoinHostPort(host, port), nil } else if config.Config.DefaultRaftPort != 0 { - // No port specified, add one - return fmt.Sprintf("%s:%d", host, config.Config.DefaultRaftPort), nil + return net.JoinHostPort(host, strconv.Itoa(config.Config.DefaultRaftPort)), nil } else { return host, nil } diff --git a/go/raft/raft_test.go b/go/raft/raft_test.go new file mode 100644 index 000000000..15f55a02d --- /dev/null +++ b/go/raft/raft_test.go @@ -0,0 +1,65 @@ +package orcraft + +import ( + "testing" + + "github.com/proxysql/orchestrator/go/config" +) + +func TestNormalizeRaftNodePreservesHostnameWithPort(t *testing.T) { + node, err := normalizeRaftNode("orchestrator-0.orchestrator.default.svc.cluster.local:10008") + if err != nil { + t.Fatalf("normalizeRaftNode returned error: %+v", err) + } + if node != "orchestrator-0.orchestrator.default.svc.cluster.local:10008" { + t.Errorf("expected hostname:port to be preserved unresolved, got %q", node) + } +} + +func TestNormalizeRaftNodePassesThroughLiteralIP(t *testing.T) { + node, err := normalizeRaftNode("192.168.1.10:10008") + if err != nil { + t.Fatalf("normalizeRaftNode returned error: %+v", err) + } + if node != "192.168.1.10:10008" { + t.Errorf("expected literal IP to pass through unchanged, got %q", node) + } +} + +func TestNormalizeRaftNodePassesThroughLiteralIPv6(t *testing.T) { + node, err := normalizeRaftNode("[::1]:10008") + if err != nil { + t.Fatalf("normalizeRaftNode returned error: %+v", err) + } + if node != "[::1]:10008" { + t.Errorf("expected literal IPv6 to pass through unchanged, got %q", node) + } +} + +func TestNormalizeRaftNodeAddsDefaultPortToIPv6(t *testing.T) { + originalPort := config.Config.DefaultRaftPort + config.Config.DefaultRaftPort = 10008 + defer func() { config.Config.DefaultRaftPort = originalPort }() + + node, err := normalizeRaftNode("::1") + if err != nil { + t.Fatalf("normalizeRaftNode returned error: %+v", err) + } + if node != "[::1]:10008" { + t.Errorf("expected default port to be appended in bracketed form, got %q", node) + } +} + +func TestNormalizeRaftNodeAddsDefaultPort(t *testing.T) { + originalPort := config.Config.DefaultRaftPort + config.Config.DefaultRaftPort = 10008 + defer func() { config.Config.DefaultRaftPort = originalPort }() + + node, err := normalizeRaftNode("orchestrator-0.orchestrator.default.svc.cluster.local") + if err != nil { + t.Fatalf("normalizeRaftNode returned error: %+v", err) + } + if node != "orchestrator-0.orchestrator.default.svc.cluster.local:10008" { + t.Errorf("expected default port to be appended, got %q", node) + } +} diff --git a/go/raft/store_test.go b/go/raft/store_test.go new file mode 100644 index 000000000..2c6e03a88 --- /dev/null +++ b/go/raft/store_test.go @@ -0,0 +1,84 @@ +package orcraft + +import ( + "net" + "strconv" + "testing" + "time" + + "github.com/hashicorp/raft" +) + +// freeLocalPort asks the OS for a free TCP port on 127.0.0.1. +func freeLocalPort(t *testing.T) int { + t.Helper() + l, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("failed to allocate a free port: %+v", err) + } + port := l.Addr().(*net.TCPAddr).Port + if err := l.Close(); err != nil { + t.Fatalf("failed to release listener: %+v", err) + } + return port +} + +// assertSingleNodeIdentity bootstraps a single-node store advertising as +// advertiseAddr and checks that raft's persisted configuration keys the node +// by that exact address, rather than some resolved variant of it. +func assertSingleNodeIdentity(t *testing.T, advertiseAddr string) { + t.Helper() + raftDir := t.TempDir() + + raftBind, err := normalizeRaftNode(advertiseAddr) + if err != nil { + t.Fatalf("normalizeRaftNode failed: %+v", err) + } + raftAdvertise, err := normalizeRaftNode(advertiseAddr) + if err != nil { + t.Fatalf("normalizeRaftNode failed: %+v", err) + } + + s := NewStore(raftDir, raftBind, raftAdvertise, nil, nil) + if err := s.Open(nil); err != nil { + t.Fatalf("Store.Open failed: %+v", err) + } + defer s.raft.Shutdown() + + // Wait for the single-node cluster to elect itself leader. + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + if s.raft.State() == raft.Leader { + break + } + time.Sleep(50 * time.Millisecond) + } + if s.raft.State() != raft.Leader { + t.Fatalf("single-node raft store never became leader (state=%v)", s.raft.State()) + } + + future := s.raft.GetConfiguration() + if err := future.Error(); err != nil { + t.Fatalf("GetConfiguration failed: %+v", err) + } + servers := future.Configuration().Servers + if len(servers) != 1 { + t.Fatalf("expected exactly 1 server in configuration, got %d: %+v", len(servers), servers) + } + + server := servers[0] + if string(server.ID) != advertiseAddr { + t.Errorf("expected raft ServerID to be %q, got %q (identity must not be a resolved, point-in-time address)", advertiseAddr, string(server.ID)) + } + if string(server.Address) != advertiseAddr { + t.Errorf("expected raft ServerAddress to be %q, got %q", advertiseAddr, string(server.Address)) + } +} + +func TestStoreOpenKeepsHostnameAsServerIdentity(t *testing.T) { + assertSingleNodeIdentity(t, "localhost:"+strconv.Itoa(freeLocalPort(t))) +} + +func TestStoreOpenKeepsIPv6AsServerIdentity(t *testing.T) { + assertSingleNodeIdentity(t, net.JoinHostPort("::1", strconv.Itoa(freeLocalPort(t)))) +} From 9b5a3c0966ea59a5afb33397c4b656cbc32f33fb Mon Sep 17 00:00:00 2001 From: Artem Vovk Date: Thu, 17 Sep 2026 09:46:59 -0600 Subject: [PATCH 2/2] fix IPv6 handling for leader URI as well Signed-off-by: Artem Vovk --- go/raft/raft.go | 17 ++++++++------- go/raft/raft_test.go | 52 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 8 deletions(-) diff --git a/go/raft/raft.go b/go/raft/raft.go index 87a2a5ea6..bfa232ab4 100644 --- a/go/raft/raft.go +++ b/go/raft/raft.go @@ -22,7 +22,6 @@ import ( "math/rand" "net" "strconv" - "strings" "sync" "sync/atomic" "time" @@ -94,7 +93,7 @@ func FatalRaftError(err error) error { return err } -func computeLeaderURI() (uri string, err error) { +func computeLeaderURI(raftAdvertise string) (uri string, err error) { if config.Config.HTTPAdvertise != "" { // Explicitly given return config.Config.HTTPAdvertise, nil @@ -105,14 +104,16 @@ func computeLeaderURI() (uri string, err error) { scheme = "https" } - hostname := strings.Split(config.Config.RaftAdvertise, ":")[0] - listenTokens := strings.Split(config.Config.ListenAddress, ":") - if len(listenTokens) < 2 { + hostname, _, err := net.SplitHostPort(raftAdvertise) + if err != nil { + hostname = raftAdvertise + } + _, port, err := net.SplitHostPort(config.Config.ListenAddress) + if err != nil { return uri, fmt.Errorf("computeLeaderURI: cannot determine listen port out of config.Config.ListenAddress: %+v", config.Config.ListenAddress) } - port := listenTokens[1] - uri = fmt.Sprintf("%s://%s:%s", scheme, hostname, port) + uri = fmt.Sprintf("%s://%s", scheme, net.JoinHostPort(hostname, port)) return uri, nil } @@ -147,7 +148,7 @@ func Setup(applier CommandApplier, snapshotCreatorApplier SnapshotCreatorApplier return log.Errorf("failed to open raft store: %s", err.Error()) } - thisLeaderURI, err = computeLeaderURI() + thisLeaderURI, err = computeLeaderURI(raftAdvertise) if err != nil { return FatalRaftError(err) } diff --git a/go/raft/raft_test.go b/go/raft/raft_test.go index 15f55a02d..5b3064b33 100644 --- a/go/raft/raft_test.go +++ b/go/raft/raft_test.go @@ -26,6 +26,58 @@ func TestNormalizeRaftNodePassesThroughLiteralIP(t *testing.T) { } } +func withConfig(t *testing.T, httpAdvertise, listenAddress string, useSSL bool, fn func()) { + t.Helper() + origHTTPAdvertise := config.Config.HTTPAdvertise + origListenAddress := config.Config.ListenAddress + origUseSSL := config.Config.UseSSL + config.Config.HTTPAdvertise = httpAdvertise + config.Config.ListenAddress = listenAddress + config.Config.UseSSL = useSSL + defer func() { + config.Config.HTTPAdvertise = origHTTPAdvertise + config.Config.ListenAddress = origListenAddress + config.Config.UseSSL = origUseSSL + }() + fn() +} + +func TestComputeLeaderURIWithIPv6Advertise(t *testing.T) { + withConfig(t, "", "0.0.0.0:3000", false, func() { + uri, err := computeLeaderURI("[::1]:10008") + if err != nil { + t.Fatalf("computeLeaderURI returned error: %+v", err) + } + if uri != "http://[::1]:3000" { + t.Errorf("expected %q, got %q", "http://[::1]:3000", uri) + } + }) +} + +func TestComputeLeaderURIWithHostname(t *testing.T) { + withConfig(t, "", "0.0.0.0:3000", false, func() { + uri, err := computeLeaderURI("orchestrator-0.svc.cluster.local:10008") + if err != nil { + t.Fatalf("computeLeaderURI returned error: %+v", err) + } + if uri != "http://orchestrator-0.svc.cluster.local:3000" { + t.Errorf("expected %q, got %q", "http://orchestrator-0.svc.cluster.local:3000", uri) + } + }) +} + +func TestComputeLeaderURIPrefersExplicitHTTPAdvertise(t *testing.T) { + withConfig(t, "https://explicit:9999", "0.0.0.0:3000", false, func() { + uri, err := computeLeaderURI("[::1]:10008") + if err != nil { + t.Fatalf("computeLeaderURI returned error: %+v", err) + } + if uri != "https://explicit:9999" { + t.Errorf("expected %q, got %q", "https://explicit:9999", uri) + } + }) +} + func TestNormalizeRaftNodePassesThroughLiteralIPv6(t *testing.T) { node, err := normalizeRaftNode("[::1]:10008") if err != nil {