diff options
| -rw-r--r-- | src/rabbit_quorum_queue.erl | 17 | ||||
| -rw-r--r-- | test/quorum_queue_SUITE.erl | 28 |
2 files changed, 28 insertions, 17 deletions
diff --git a/src/rabbit_quorum_queue.erl b/src/rabbit_quorum_queue.erl index 76cd194d91..cbd9980491 100644 --- a/src/rabbit_quorum_queue.erl +++ b/src/rabbit_quorum_queue.erl @@ -673,7 +673,8 @@ add_member(VHost, Name, Node) -> true -> case lists:member(Node, QNodes) of true -> - {error, already_a_member}; + %% idempotent by design + ok; false -> add_member(Q, Node) end @@ -719,16 +720,12 @@ delete_member(VHost, Name, Node) -> {error, classic_queue_not_supported}; {ok, Q} when ?amqqueue_is_quorum(Q) -> QNodes = amqqueue:get_quorum_nodes(Q), - case lists:member(Node, rabbit_mnesia:cluster_nodes(running)) of + case lists:member(Node, QNodes) of false -> - {error, node_not_running}; + %% idempotent by design + ok; true -> - case lists:member(Node, QNodes) of - false -> - {error, not_a_member}; - true -> - delete_member(Q, Node) - end + delete_member(Q, Node) end; {error, not_found} = E -> E @@ -743,7 +740,7 @@ delete_member(Q, Node) when ?amqqueue_is_quorum(Q) -> %% deleting the last member is not allowed {error, last_node}; _ -> - case ra:leave_and_delete_server(ServerId) of + case ra:leave_and_delete_server(amqqueue:get_pid(Q), ServerId) of ok -> Fun = fun(Q1) -> amqqueue:set_quorum_nodes( diff --git a/test/quorum_queue_SUITE.erl b/test/quorum_queue_SUITE.erl index 02616677f3..1aa5247b57 100644 --- a/test/quorum_queue_SUITE.erl +++ b/test/quorum_queue_SUITE.erl @@ -54,8 +54,9 @@ groups() -> add_member_not_found, delete_member_not_running, delete_member_classic, - delete_member_not_found, - delete_member] + delete_member_queue_not_found, + delete_member, + delete_member_not_a_member] ++ all_tests()}, {cluster_size_2, [], memory_tests()}, {cluster_size_3, [], [ @@ -1228,7 +1229,8 @@ add_member_already_a_member(Config) -> QQ = ?config(queue_name, Config), ?assertEqual({'queue.declare_ok', QQ, 0, 0}, declare(Ch, QQ, [{<<"x-queue-type">>, longstr, <<"quorum">>}])), - ?assertEqual({error, already_a_member}, + %% idempotent by design + ?assertEqual(ok, rpc:call(Server, rabbit_quorum_queue, add_member, [<<"/">>, QQ, Server])). @@ -1266,7 +1268,8 @@ delete_member_not_running(Config) -> QQ = ?config(queue_name, Config), ?assertEqual({'queue.declare_ok', QQ, 0, 0}, declare(Ch, QQ, [{<<"x-queue-type">>, longstr, <<"quorum">>}])), - ?assertEqual({error, node_not_running}, + %% it should be possible to delete members that are not online (e.g. decomissioned) + ?assertEqual(ok, rpc:call(Server, rabbit_quorum_queue, delete_member, [<<"/">>, QQ, 'rabbit@burrow'])). @@ -1279,7 +1282,7 @@ delete_member_classic(Config) -> rpc:call(Server, rabbit_quorum_queue, delete_member, [<<"/">>, CQ, Server])). -delete_member_not_found(Config) -> +delete_member_queue_not_found(Config) -> [Server | _] = rabbit_ct_broker_helpers:get_node_configs(Config, nodename), QQ = ?config(queue_name, Config), ?assertEqual({error, not_found}, @@ -1295,12 +1298,23 @@ delete_member(Config) -> timer:sleep(100), ?assertEqual(ok, rpc:call(Server, rabbit_quorum_queue, delete_member, + [<<"/">>, QQ, Server])). + +delete_member_not_a_member(Config) -> + [Server | _] = rabbit_ct_broker_helpers:get_node_configs(Config, nodename), + Ch = rabbit_ct_client_helpers:open_channel(Config, Server), + QQ = ?config(queue_name, Config), + ?assertEqual({'queue.declare_ok', QQ, 0, 0}, + declare(Ch, QQ, [{<<"x-queue-type">>, longstr, <<"quorum">>}])), + timer:sleep(100), + ?assertEqual(ok, + rpc:call(Server, rabbit_quorum_queue, delete_member, [<<"/">>, QQ, Server])), - ?assertEqual({error, not_a_member}, + %% idempotent by design + ?assertEqual(ok, rpc:call(Server, rabbit_quorum_queue, delete_member, [<<"/">>, QQ, Server])). - cleanup_data_dir(Config) -> %% This test is slow, but also checks that we handle properly errors when %% trying to delete a queue in minority. A case clause there had gone |