From 9281ed80070e5d9a65c4b837ca0eca6e23e53b45 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Sat, 24 Oct 2026 06:58:16 -0700 Subject: [PATCH] master: never auto-admit raft peers on a single-node configuration (#12021) * master: never auto-admit raft peers on a single-node configuration A master started without cluster peers (peers=none, or no peers so the list holds only itself) declares a one-node cluster, yet any foreign master that registered over KeepConnected was admitted as a second voter. The resulting two-voter cluster cannot reach quorum when the foreign peer is unreachable, wedging a previously healthy single master. Gate raft admission on having been configured with cluster peers; masters started with a peer list keep the announce-to-join flow used to grow the cluster. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * master: source SingleMaster from -peers=none, not the peer count A master started with an empty peer list ends up with a one-peer map containing itself, and NewMasterServer inferred declared single-master mode from that count. The master still mints a raft cluster through the bootstrap path, but AdmitRaftPeer then ignores every later joiner, so a deployment that grows from one master to several can never form a quorum. isSingleMasterMode already spells out the declared case (-peers=none); populate MasterOption.SingleMaster from it instead. * master: cover clustered raft admission with a real in-memory leader The previous clustered test had no raft leader and asserted nothing, so an AdmitRaftPeer that always returned early still passed. Bootstrap a single-node hashicorp raft over the in-memory transport, wait for leadership, and assert the leader reports and admits a missing peer. * master: return the raft admission result and assert committed join AdmitRaftPeer ran AddVoter on a detached goroutine, so the only observable state was an in-flight marker deleted on success and failure alike, and the clustered test could never tell whether the join worked. Admission now hands back a result channel; the test runs a connected second in-memory node and asserts the peer sees itself a committed voter. --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- weed/command/master.go | 1 + weed/server/master_server.go | 23 +++-- weed/server/master_server_admit_test.go | 119 ++++++++++++++++++++++++ 3 files changed, 135 insertions(+), 8 deletions(-) create mode 100644 weed/server/master_server_admit_test.go diff --git a/weed/command/master.go b/weed/command/master.go index b12f0a6210..d205c3a6a9 100644 --- a/weed/command/master.go +++ b/weed/command/master.go @@ -491,5 +491,6 @@ func (m *MasterOptions) toMasterOption(whiteList []string) *weed_server.MasterOp MetricsIntervalSec: *m.metricsIntervalSec, TelemetryUrl: *m.telemetryUrl, TelemetryEnabled: *m.telemetryEnabled, + SingleMaster: isSingleMasterMode(*m.peers), } } diff --git a/weed/server/master_server.go b/weed/server/master_server.go index dec88ab154..1e6cb58df9 100644 --- a/weed/server/master_server.go +++ b/weed/server/master_server.go @@ -66,6 +66,7 @@ type MasterOption struct { MetricsAddress string MetricsIntervalSec int IsFollower bool + SingleMaster bool TelemetryUrl string TelemetryEnabled bool VolumeGrowthDisabled bool @@ -106,7 +107,6 @@ type MasterServer struct { } func NewMasterServer(r *mux.Router, option *MasterOption, peers map[string]pb.ServerAddress) *MasterServer { - v := util.GetViper() signingKey := v.GetString("jwt.signing.key") v.SetDefault("jwt.signing.expires_after_seconds", 10) @@ -601,18 +601,20 @@ func (ms *MasterServer) OnPeerUpdate(update *master_pb.ClusterNodeUpdate, startF // AdmitRaftPeer pulls a master into the raft quorum, if we are the leader and // do not have it yet. A master that starts with no raft state cannot campaign -// under either implementation, so this is its only way in. -func (ms *MasterServer) AdmitRaftPeer(peerAddress pb.ServerAddress) { - if peerAddress.ToHttpAddress() == ms.option.Master.ToHttpAddress() { - return +// under either implementation, so this is its only way in. The returned +// channel resolves with the admission result, or nil when no admission ran. +func (ms *MasterServer) AdmitRaftPeer(peerAddress pb.ServerAddress) <-chan error { + if ms.option.SingleMaster || peerAddress.ToHttpAddress() == ms.option.Master.ToHttpAddress() { + return nil } peerName, ok := ms.missingRaftPeerName(peerAddress) if !ok { - return + return nil } - if _, alreadyAdmitting := ms.raftPeerAdmissions.LoadOrStore(peerName, struct{}{}); alreadyAdmitting { - return + admission := make(chan error, 1) + if inFlight, alreadyAdmitting := ms.raftPeerAdmissions.LoadOrStore(peerName, admission); alreadyAdmitting { + return inFlight.(chan error) } glog.V(0).Infof("adding new raft server: %s", peerName) // The join commits through raft, which waits on the other peers, so keep it @@ -620,10 +622,15 @@ func (ms *MasterServer) AdmitRaftPeer(peerAddress pb.ServerAddress) { // handler that a joining master is still blocked in. go func() { defer ms.raftPeerAdmissions.Delete(peerName) + defer close(admission) if err := ms.raftAddServer(peerName, peerAddress.ToGrpcAddress(), true); err != nil { glog.Warningf("failed adding raft server %s: %v", peerName, err) + admission <- err + return } + admission <- nil }() + return admission } // missingRaftPeerName returns the raft id to admit peerAddress under, and diff --git a/weed/server/master_server_admit_test.go b/weed/server/master_server_admit_test.go new file mode 100644 index 0000000000..012985b5cf --- /dev/null +++ b/weed/server/master_server_admit_test.go @@ -0,0 +1,119 @@ +package weed_server + +import ( + "testing" + "time" + + hashicorpRaft "github.com/hashicorp/raft" + "github.com/seaweedfs/seaweedfs/weed/pb" + "github.com/seaweedfs/seaweedfs/weed/topology" +) + +func TestSingleMasterNeverAdmitsRaftPeers(t *testing.T) { + ms := &MasterServer{option: &MasterOption{SingleMaster: true, Master: "localhost:9333"}} + + // A foreign master announcing itself must not touch the admission path: + // ms.Topo is deliberately nil so any admission attempt panics the test. + if done := ms.AdmitRaftPeer("10.0.0.1:9333"); done != nil { + t.Fatal("single-master mode admitted a foreign raft peer") + } +} + +func TestClusteredLeaderAdmitsMissingPeer(t *testing.T) { + topo := topology.NewTopology("topo", nil, 128*1024*1024, 0, false) + leader, leaderTransport := newInmemRaftLeader(t, "localhost:9333") + topo.HashicorpRaft = leader + ms := &MasterServer{ + option: &MasterOption{Master: "localhost:9333"}, + Topo: topo, + } + + peer := pb.ServerAddress("10.0.0.1:9333") + name, missing := ms.missingRaftPeerName(peer) + if !missing || name != "10.0.0.1:9333" { + t.Fatalf("clustered leader did not report the missing raft peer: %q %v", name, missing) + } + peerRaft := newInmemRaftFollower(t, peer, leaderTransport) + + done := ms.AdmitRaftPeer(peer) + if done == nil { + t.Fatal("clustered leader skipped admission of a missing raft peer") + } + if err := <-done; err != nil { + t.Fatalf("raft peer admission failed: %v", err) + } + + // The joined node's own configuration is the committed view: it only + // learns the quorum it was admitted into through replication. + for deadline := time.Now().Add(10 * time.Second); ; { + if raftHasVoter(peerRaft, string(peer)) { + return + } + if time.Now().After(deadline) { + t.Fatalf("admitted peer never saw itself in the quorum: %v", + peerRaft.GetConfiguration().Configuration().Servers) + } + time.Sleep(10 * time.Millisecond) + } +} + +func raftHasVoter(r *hashicorpRaft.Raft, id string) bool { + for _, server := range r.GetConfiguration().Configuration().Servers { + if server.Suffrage == hashicorpRaft.Voter && string(server.ID) == id { + return true + } + } + return false +} + +func newInmemRaftFollower(t *testing.T, self pb.ServerAddress, peerTransport *hashicorpRaft.InmemTransport) *hashicorpRaft.Raft { + t.Helper() + conf := hashicorpRaft.DefaultConfig() + conf.LocalID = hashicorpRaft.ServerID(self) + addr, transport := hashicorpRaft.NewInmemTransport(hashicorpRaft.ServerAddress(self.ToGrpcAddress())) + transport.Connect(peerTransport.LocalAddr(), peerTransport) + peerTransport.Connect(addr, transport) + r, err := hashicorpRaft.NewRaft(conf, + &StateMachine{topo: topology.NewTopology("fsm", nil, 128*1024*1024, 0, false)}, + hashicorpRaft.NewInmemStore(), hashicorpRaft.NewInmemStore(), + hashicorpRaft.NewInmemSnapshotStore(), transport) + if err != nil { + t.Fatalf("new raft: %v", err) + } + t.Cleanup(func() { r.Shutdown() }) + return r +} + +func newInmemRaftLeader(t *testing.T, self string) (*hashicorpRaft.Raft, *hashicorpRaft.InmemTransport) { + t.Helper() + conf := hashicorpRaft.DefaultConfig() + conf.LocalID = hashicorpRaft.ServerID(self) + conf.HeartbeatTimeout = 50 * time.Millisecond + conf.ElectionTimeout = 50 * time.Millisecond + conf.LeaderLeaseTimeout = 25 * time.Millisecond + conf.CommitTimeout = 5 * time.Millisecond + addr, transport := hashicorpRaft.NewInmemTransport(hashicorpRaft.ServerAddress(self)) + r, err := hashicorpRaft.NewRaft(conf, + &StateMachine{topo: topology.NewTopology("fsm", nil, 128*1024*1024, 0, false)}, + hashicorpRaft.NewInmemStore(), hashicorpRaft.NewInmemStore(), + hashicorpRaft.NewInmemSnapshotStore(), transport) + if err != nil { + t.Fatalf("new raft: %v", err) + } + t.Cleanup(func() { r.Shutdown() }) + + if err := r.BootstrapCluster(hashicorpRaft.Configuration{ + Servers: []hashicorpRaft.Server{{Suffrage: hashicorpRaft.Voter, ID: conf.LocalID, Address: addr}}, + }).Error(); err != nil { + t.Fatalf("bootstrap cluster: %v", err) + } + for deadline := time.Now().Add(10 * time.Second); ; { + if r.State() == hashicorpRaft.Leader { + return r, transport + } + if time.Now().After(deadline) { + t.Fatal("in-memory raft did not elect a leader") + } + time.Sleep(10 * time.Millisecond) + } +}