Change rabbit_quorum_queue:grow/3

To filter non-running nodes instead of by connection as it should
not be possible to add a quorum
queue server running on an erlang node without RabbitMQ.

[#162782801]

Author: kjnilsson

src/rabbit_quorum_queue.erl

@@ -760,32 +760,32 @@ shrink_all(Node) ->
             amqqueue:get_type(Q) == quorum,
             lists:member(Node, amqqueue:get_quorum_nodes(Q))].
 
--spec grow(node(), binary(), binary(),
-           all | even) ->
+-spec grow(node(), binary(), binary(), all | even) ->
     [{rabbit_amqqueue:name(),
       {ok, pos_integer()} | {error, pos_integer(), term()}}].
 grow(Node, VhostSpec, QueueSpec, Strategy) ->
-    ConnectedNodes = [node() | nodes()],
+    Running = rabbit_mnesia:cluster_nodes(running),
     [begin
+         Size = length(amqqueue:get_quorum_nodes(Q)),
          QName = amqqueue:get_name(Q),
          rabbit_log:info("~s: Adding member ~w",
                          [rabbit_misc:rs(QName), Node]),
-         Size = length(amqqueue:get_quorum_nodes(Q)),
          case add_member(Q, Node) of
              ok ->
                  {QName, {ok, Size + 1}};
              {error, Err} ->
-                 rabbit_log:warning("~s: Failed to add member ~w, Error ~w",
-                                    [rabbit_misc:rs(QName), Node, Err]),
+                 rabbit_log:warning(
+                   "~s: Failed to add member ~w, Error ~w",
+                   [rabbit_misc:rs(QName), Node, Err]),
                  {QName, {error, Size, Err}}
          end
      end
      || Q <- rabbit_amqqueue:list(),
         amqqueue:get_type(Q) == quorum,
         %% don't add a member if there is already one on the node
         not lists:member(Node, amqqueue:get_quorum_nodes(Q)),
-        %% if the node isn't connected, best not to add it
-        lists:member(Node, ConnectedNodes),
+        %% node needs to be running
+        lists:member(Node, Running),
         matches_strategy(Strategy, amqqueue:get_quorum_nodes(Q)),
         is_match(amqqueue:get_vhost(Q), VhostSpec) andalso
         is_match(get_resource_name(amqqueue:get_name(Q)), QueueSpec) ].

test/rabbitmq_queues_cli_integration_SUITE.erl

@@ -30,7 +30,8 @@ groups() ->
     [
         {tests, [], [
             shrink,
-            grow
+            grow,
+            grow_invalid_node_filtered
         ]}
     ].
 
@@ -99,6 +100,19 @@ grow(Config) ->
     ?assertNotMatch(#{{"/", "grow1"} := _}, parse_result(Out3)),
     ok.
 
+grow_invalid_node_filtered(Config) ->
+    NodeConfig = rabbit_ct_broker_helpers:get_node_config(Config, 2),
+    Nodename2 = ?config(nodename, NodeConfig),
+    Ch = rabbit_ct_client_helpers:open_channel(Config, Nodename2),
+    %% declare a quorum queue
+    QName = "grow-err",
+    Args = [{<<"x-quorum-initial-group-size">>, long, 1}],
+    #'queue.declare_ok'{} = declare_qq(Ch, QName, Args),
+    DummyNode = not_really_a_node@nothing,
+    {ok, Out1} = rabbitmq_queues(Config, 0, ["grow", DummyNode, "all"]),
+    ?assertNotMatch(#{{"/", "grow-err"} := _}, parse_result(Out1)),
+    ok.
+
 parse_result(S) ->
     Lines = string:split(S, "\n", all),
     maps:from_list(