Check membership before transfering leadership

Updated `handle_leader` to include a cluster membership check of target `ServerId` before initiating leader transfer and return an error if the target server is not a voter.
rabbitmq · Dec 3, 2024 · 5270889 · 5270889
1 parent cba3581
commit 5270889
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 12 deletions.
diff --git a/src/ra_server.erl b/src/ra_server.erl
@@ -827,17 +827,22 @@ handle_leader({transfer_leadership, Member},
            [LogId, Member]),
     {leader, State, [{reply, {error, unknown_member}}]};
 handle_leader({transfer_leadership, ServerId},
-              #{cfg := #cfg{log_id = LogId}} = State) ->
-    ?DEBUG("~ts: transfer leadership to ~w requested",
-           [LogId, ServerId]),
-    %% TODO find a timeout
-    gen_statem:cast(ServerId, try_become_leader),
-    {await_condition,
-     State#{condition =>
-            #{predicate_fun => fun transfer_leadership_condition/2,
-              timeout => #{effects => [],
-                           transition_to => leader}}},
-     [{reply, ok}]};
+              #{cfg := #cfg{log_id = LogId},
+                cluster := Cluster} = State) ->
+    case Cluster of
+        #{ServerId := #{voter_status := #{membership := Membership}}} when Membership =/= voter ->
+            ?DEBUG("~ts: transfer leadership requested but non-voter member ~w", [LogId, ServerId]),
+            {leader, State, [{reply, {error, non_voter_member}}]};
+        _ ->
+            ?DEBUG("~ts: transfer leadership to ~w requested", [LogId, ServerId]),
+            %% TODO find a timeout
+            gen_statem:cast(ServerId, try_become_leader),
+            {await_condition,
+             State#{condition =>
+                        #{predicate_fun => fun transfer_leadership_condition/2,
+                          timeout => #{effects => [], transition_to => leader}}},
+             [{reply, ok}]}
+    end;
 handle_leader({register_external_log_reader, Pid}, #{log := Log0} = State) ->
     {Log, Effs} = ra_log:register_reader(Pid, Log0),
     {leader, State#{log => Log}, Effs};

diff --git a/test/ra_SUITE.erl b/test/ra_SUITE.erl
@@ -1226,7 +1226,15 @@ transfer_leadership(Config) ->
     ?assertEqual(NewLeader, NextInLine),
     ?assertEqual(already_leader, ra:transfer_leadership(NewLeader, NewLeader)),
     ?assertEqual({error, unknown_member}, ra:transfer_leadership(NewLeader, {unknown, node()})),
-    terminate_cluster(Members).
+    NonVoterMember =
+        #{id => NonVoterMemberId = {n4, node()},
+          uid => ra:new_uid(ra_lib:to_binary(Name)),
+          membership => non_voter},
+    {ok, _, _} = ra:add_member(NewLeader, NonVoterMember),
+    ok = ra:start_server(default, Name, NonVoterMember, add_machine(), Members),
+    ct:pal("Transferring leadership from ~p to ~p", [NewLeader, NonVoterMemberId]),
+    ?assertEqual({error, non_voter_member}, ra:transfer_leadership(NewLeader, NonVoterMemberId)),
+    terminate_cluster([NonVoterMemberId | Members]).
 
 transfer_leadership_two_node(Config) ->
     Name = ?config(test_name, Config),