Merge pull request #9782 from rabbitmq/join_cluster-handles-stop-start-automatically

rabbit_db: `join/2` now takes care of stopping/starting RabbitMQ

Author: michaelklishin

deps/rabbit/src/rabbit_db_cluster.erl

@@ -96,6 +96,28 @@ join(RemoteNode, NodeType)
   when is_atom(RemoteNode) andalso ?IS_NODE_TYPE(NodeType) ->
     case can_join(RemoteNode) of
         {ok, ClusterNodes} when is_list(ClusterNodes) ->
+            %% RabbitMQ and Mnesia must be stopped to modify the cluster. In
+            %% particular, we stop Mnesia regardless of the remotely selected
+            %% database because we might change it during the join.
+            RestartMnesia = rabbit_mnesia:is_running(),
+            RestartFFCtl = rabbit_ff_controller:is_running(),
+            RestartRabbit = rabbit:is_running(),
+            case RestartRabbit of
+                true ->
+                    rabbit:stop();
+                false ->
+                    case RestartFFCtl of
+                        true ->
+                            ok = rabbit_ff_controller:wait_for_task_and_stop();
+                        false ->
+                            ok
+                    end,
+                    case RestartMnesia of
+                        true  -> rabbit_mnesia:stop_mnesia();
+                        false -> ok
+                    end
+            end,
+
             ok = rabbit_db:reset(),
             rabbit_feature_flags:copy_feature_states_after_reset(RemoteNode),
 
@@ -106,6 +128,27 @@ join(RemoteNode, NodeType)
                       true  -> join_using_khepri(ClusterNodes, NodeType);
                       false -> join_using_mnesia(ClusterNodes, NodeType)
                   end,
+
+            %% Restart RabbitMQ afterwards, if it was running before the join.
+            %% Likewise for the Feature flags controller and Mnesia (if we
+            %% still need it).
+            case RestartRabbit of
+                true ->
+                    rabbit:start();
+                false ->
+                    case RestartFFCtl of
+                        true ->
+                            ok = rabbit_sup:start_child(rabbit_ff_controller);
+                        false ->
+                            ok
+                    end,
+                    NeedMnesia = not rabbit_khepri:is_enabled(),
+                    case RestartMnesia andalso NeedMnesia of
+                        true  -> rabbit_mnesia:start_mnesia(false);
+                        false -> ok
+                    end
+            end,
+
             case Ret of
                 ok ->
                     rabbit_node_monitor:notify_joined_cluster(),

deps/rabbit/src/rabbit_ff_controller.erl

@@ -49,7 +49,8 @@
          running_nodes/0,
          collect_inventory_on_nodes/1, collect_inventory_on_nodes/2,
          mark_as_enabled_on_nodes/4,
-         wait_for_task_and_stop/0]).
+         wait_for_task_and_stop/0,
+         is_running/0]).
 
 %% gen_statem callbacks.
 -export([callback_mode/0,
@@ -79,7 +80,13 @@ start_link() ->
     gen_statem:start_link({local, ?LOCAL_NAME}, ?MODULE, none, []).
 
 wait_for_task_and_stop() ->
-    gen_statem:stop(?LOCAL_NAME).
+    case erlang:whereis(rabbit_sup) of
+        undefined -> gen_statem:stop(?LOCAL_NAME);
+        _         -> rabbit_sup:stop_child(?LOCAL_NAME)
+    end.
+
+is_running() ->
+    is_pid(erlang:whereis(?LOCAL_NAME)).
 
 is_supported(FeatureNames) ->
     is_supported(FeatureNames, ?TIMEOUT).
@@ -176,6 +183,9 @@ callback_mode() ->
     state_functions.
 
 init(_Args) ->
+    ?LOG_DEBUG(
+       "Feature flags: controller standing by",
+       #{domain => ?RMQLOG_DOMAIN_FEAT_FLAGS}),
     {ok, standing_by, none}.
 
 standing_by(

deps/rabbit/src/rabbit_mnesia.erl

@@ -177,7 +177,6 @@ create_cluster_callback(RemoteNode, NodeType) ->
                         -> {ok, [node()]} | {ok, already_member} | {error, {inconsistent_cluster, string()}}.
 
 can_join_cluster(DiscoveryNode) ->
-    ensure_mnesia_not_running(),
     ensure_mnesia_dir(),
     case is_only_clustered_disc_node() of
         true  -> e(clustering_only_disc_node);
@@ -225,6 +224,7 @@ join_cluster(ClusterNodes, NodeType) when is_list(ClusterNodes) ->
 join_cluster(DiscoveryNode, NodeType) when is_atom(DiscoveryNode) ->
     %% Code to remain compatible with `change_cluster_node_type/1' and older
     %% CLI.
+    ensure_mnesia_not_running(),
     case can_join_cluster(DiscoveryNode) of
         {ok, ClusterNodes} when is_list(ClusterNodes) ->
             ok = reset_gracefully(),

deps/rabbit/test/clustering_management_SUITE.erl

@@ -269,7 +269,30 @@ join_and_part_cluster(Config) ->
 
     stop_reset_start(Config, Hare),
     assert_not_clustered(Hare),
-    assert_not_clustered(Bunny).
+    assert_not_clustered(Bunny),
+
+    %% Using `join_cluster` is allowed without stopping `rabbit` first since
+    %% 3.13.0. It will only work if all nodes support it: check if they all
+    %% expose `rabbit_ff_controller:is_running/0`.
+    Supported = lists:all(
+                  fun(R) -> R end,
+                  rabbit_ct_broker_helpers:rpc_all(
+                    Config, erlang, function_exported,
+                    [rabbit_ff_controller, is_running, 0])),
+    ct:pal(
+      "Do all nodes support `join_cluster` without stopping `rabbit` "
+      "first? ~p",
+      [Supported]),
+    case Supported of
+        true ->
+            ?assertEqual(ok, join_cluster(Config, Rabbit, Bunny)),
+            assert_clustered([Rabbit, Bunny]),
+
+            ?assertEqual(ok, join_cluster(Config, Hare, Bunny)),
+            assert_clustered([Rabbit, Bunny, Hare]);
+        false ->
+            ok
+    end.
 
 stop_start_cluster_node(Config) ->
     [Rabbit, Hare] = cluster_members(Config),
@@ -326,7 +349,15 @@ join_and_part_cluster_in_khepri(Config) ->
 
     stop_reset_start(Config, Rabbit),
     assert_not_clustered(Rabbit),
-    assert_not_clustered(Hare).
+    assert_not_clustered(Hare),
+
+    %% Using `join_cluster` is allowed without stopping `rabbit` first since
+    %% 3.13.0.
+    ?assertEqual(ok, join_cluster(Config, Rabbit, Bunny)),
+    assert_clustered([Rabbit, Bunny]),
+
+    ?assertEqual(ok, join_cluster(Config, Hare, Bunny)),
+    assert_clustered([Rabbit, Bunny, Hare]).
 
 join_cluster_bad_operations(Config) ->
     [Rabbit, Hare, Bunny] = cluster_members(Config),
@@ -342,10 +373,6 @@ join_cluster_bad_operations(Config) ->
     ok = start_app(Config, Rabbit),
     assert_not_clustered(Rabbit),
 
-    %% Trying to cluster with mnesia running
-    assert_failure(fun () -> join_cluster(Config, Rabbit, Bunny) end),
-    assert_not_clustered(Rabbit),
-
     %% Trying to cluster the node with itself
     ok = stop_app(Config, Rabbit),
     assert_failure(fun () -> join_cluster(Config, Rabbit, Rabbit) end),
@@ -379,18 +406,14 @@ join_cluster_bad_operations(Config) ->
     ok.
 
 join_cluster_bad_operations_in_khepri(Config) ->
-    [Rabbit, _Hare, Bunny] = cluster_members(Config),
+    [Rabbit, _Hare, _Bunny] = cluster_members(Config),
 
     %% Nonexistent node
     ok = stop_app(Config, Rabbit),
     assert_failure(fun () -> join_cluster(Config, Rabbit, non@existent) end),
     ok = start_app(Config, Rabbit),
     assert_not_clustered(Rabbit),
 
-    %% Trying to cluster with mnesia running
-    assert_failure(fun () -> join_cluster(Config, Rabbit, Bunny) end),
-    assert_not_clustered(Rabbit),
-
     %% Trying to cluster the node with itself
     ok = stop_app(Config, Rabbit),
     assert_failure(fun () -> join_cluster(Config, Rabbit, Rabbit) end),

deps/rabbit_common/mk/rabbitmq-run.mk

@@ -392,12 +392,8 @@ start-brokers start-cluster: $(DIST_TARGET)
 	for n in $$(seq $(NODES)); do \
 		nodename="rabbit-$$n@$(HOSTNAME)"; \
 		if test '$@' = 'start-cluster' && test "$$nodename1"; then \
-			ERL_LIBS="$(DIST_ERL_LIBS)" \
-			  $(RABBITMQCTL) -n "$$nodename" stop_app; \
 			ERL_LIBS="$(DIST_ERL_LIBS)" \
 			  $(RABBITMQCTL) -n "$$nodename" join_cluster "$$nodename1"; \
-			ERL_LIBS="$(DIST_ERL_LIBS)" \
-			  $(RABBITMQCTL) -n "$$nodename" start_app; \
 		else \
 			nodename1=$$nodename; \
 		fi; \

deps/rabbitmq_cli/lib/rabbitmq/cli/ctl/commands/join_cluster_command.ex

@@ -28,8 +28,6 @@ defmodule RabbitMQ.CLI.Ctl.Commands.JoinClusterCommand do
   def validate([_], _), do: :ok
   def validate(_, _), do: {:validation_failure, :too_many_args}
 
-  use RabbitMQ.CLI.Core.RequiresRabbitAppStopped
-
   def run([target_node], %{node: node_name, ram: ram, disc: disc} = opts) do
     node_type =
       case {ram, disc} do