From 3a4fcdf2b596a9e7e98615767b06bf33acaf7cdf Mon Sep 17 00:00:00 2001 From: Diana Parra Corbacho Date: Mon, 18 Nov 2024 09:56:33 +0100 Subject: [PATCH 01/14] Tests: mqtt_shared_SUITE skip check for previous connections The test checks later based on clientId --- deps/rabbitmq_mqtt/test/shared_SUITE.erl | 1 - 1 file changed, 1 deletion(-) diff --git a/deps/rabbitmq_mqtt/test/shared_SUITE.erl b/deps/rabbitmq_mqtt/test/shared_SUITE.erl index 6b7d7fa80255..2101d9039c26 100644 --- a/deps/rabbitmq_mqtt/test/shared_SUITE.erl +++ b/deps/rabbitmq_mqtt/test/shared_SUITE.erl @@ -1235,7 +1235,6 @@ management_plugin_connection(Config) -> eventually(?_assertEqual([], all_connection_pids(Config)), 500, 3). management_plugin_enable(Config) -> - ?assertEqual(0, length(http_get(Config, "/connections"))), ok = rabbit_ct_broker_helpers:disable_plugin(Config, 0, rabbitmq_management), ok = rabbit_ct_broker_helpers:disable_plugin(Config, 0, rabbitmq_management_agent), From bfca5c211be03660e9ad319fb5da9cff0e6784bd Mon Sep 17 00:00:00 2001 From: Diana Parra Corbacho Date: Mon, 18 Nov 2024 10:43:34 +0100 Subject: [PATCH 02/14] tests: clustering_SUITE set small metrics gc interval --- deps/rabbitmq_management/test/clustering_SUITE.erl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/deps/rabbitmq_management/test/clustering_SUITE.erl b/deps/rabbitmq_management/test/clustering_SUITE.erl index 0e9039cc3675..ed0ccc9eef9c 100644 --- a/deps/rabbitmq_management/test/clustering_SUITE.erl +++ b/deps/rabbitmq_management/test/clustering_SUITE.erl @@ -68,7 +68,8 @@ merge_app_env(Config) -> Config1 = rabbit_ct_helpers:merge_app_env( Config, {rabbit, [ {collect_statistics, fine}, - {collect_statistics_interval, ?STATS_INTERVAL} + {collect_statistics_interval, ?STATS_INTERVAL}, + {core_metrics_gc_interval, 500} ]}), rabbit_ct_helpers:merge_app_env(Config1, {rabbitmq_management_agent, [ From 2813fc643159303a2e9727c7b446ee442e57ca30 Mon Sep 17 00:00:00 2001 From: Diana Parra Corbacho Date: Mon, 18 Nov 2024 15:24:51 +0100 Subject: [PATCH 03/14] tests: v5_SUITE wait longer to receive messages --- deps/rabbitmq_mqtt/test/v5_SUITE.erl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/deps/rabbitmq_mqtt/test/v5_SUITE.erl b/deps/rabbitmq_mqtt/test/v5_SUITE.erl index ed22a599280d..8b252003b3c3 100644 --- a/deps/rabbitmq_mqtt/test/v5_SUITE.erl +++ b/deps/rabbitmq_mqtt/test/v5_SUITE.erl @@ -1071,15 +1071,15 @@ send(Parent, Client, Topic, NumSent) -> end. assert_received_no_duplicates() -> - assert_received_no_duplicates0(#{}). + assert_received_no_duplicates0(#{}, 30000). -assert_received_no_duplicates0(Received) -> +assert_received_no_duplicates0(Received, Timeout) -> receive {publish, #{payload := P}} -> case maps:is_key(P, Received) of true -> ct:fail("Received ~p twice", [P]); - false -> assert_received_no_duplicates0(maps:put(P, ok, Received)) + false -> assert_received_no_duplicates0(maps:put(P, ok, Received), 500) end - after 500 -> + after Timeout -> %% Check that we received at least one message. ?assertNotEqual(0, maps:size(Received)) end. From 3076c73f425f2f5c358365c991bc24c64ad7ae18 Mon Sep 17 00:00:00 2001 From: Diana Parra Corbacho Date: Mon, 18 Nov 2024 16:08:52 +0100 Subject: [PATCH 04/14] Tests: clustering_prop_SUITE retry whole cleanup --- deps/rabbitmq_management/test/clustering_prop_SUITE.erl | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/deps/rabbitmq_management/test/clustering_prop_SUITE.erl b/deps/rabbitmq_management/test/clustering_prop_SUITE.erl index e006bad9077b..df27571f043a 100644 --- a/deps/rabbitmq_management/test/clustering_prop_SUITE.erl +++ b/deps/rabbitmq_management/test/clustering_prop_SUITE.erl @@ -121,9 +121,11 @@ prop_connection_channel_counts(Config) -> 60), cleanup(Cons), rabbit_ct_helpers:await_condition( - fun () -> validate_counts(Config, []) end, + fun () -> + cleanup(Cons), + force_stats(Config), + validate_counts(Config, []) end, 60000), - force_stats(Config), Res end). From 5f54e4c5f752ee78424da9d6c24f8c2a7793e60a Mon Sep 17 00:00:00 2001 From: Diana Parra Corbacho Date: Tue, 19 Nov 2024 18:02:12 +0100 Subject: [PATCH 05/14] Tests: auth_SUITE close all connections in end_per_testcase --- deps/rabbitmq_mqtt/test/auth_SUITE.erl | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/deps/rabbitmq_mqtt/test/auth_SUITE.erl b/deps/rabbitmq_mqtt/test/auth_SUITE.erl index d151af003a71..72a9be726090 100644 --- a/deps/rabbitmq_mqtt/test/auth_SUITE.erl +++ b/deps/rabbitmq_mqtt/test/auth_SUITE.erl @@ -393,6 +393,7 @@ end_per_testcase(Testcase, Config) when Testcase == ssl_user_auth_success; Testcase == ssl_user_auth_failure; Testcase == ssl_user_vhost_not_allowed -> delete_cert_user(Config), + close_all_connections(Config), rabbit_ct_helpers:testcase_finished(Config, Testcase); end_per_testcase(TestCase, Config) when TestCase == ssl_user_vhost_parameter_mapping_success; TestCase == ssl_user_vhost_parameter_mapping_not_allowed -> @@ -400,14 +401,17 @@ end_per_testcase(TestCase, Config) when TestCase == ssl_user_vhost_parameter_map VhostForCertUser = ?config(temp_vhost_for_ssl_user, Config), ok = rabbit_ct_broker_helpers:delete_vhost(Config, VhostForCertUser), ok = rabbit_ct_broker_helpers:clear_global_parameter(Config, mqtt_default_vhosts), + close_all_connections(Config), rabbit_ct_helpers:testcase_finished(Config, TestCase); end_per_testcase(user_credentials_auth, Config) -> User = ?config(new_user, Config), {ok,_} = rabbit_ct_broker_helpers:rabbitmqctl(Config, 0, ["delete_user", User]), + close_all_connections(Config), rabbit_ct_helpers:testcase_finished(Config, user_credentials_auth); end_per_testcase(ssl_user_vhost_parameter_mapping_vhost_does_not_exist, Config) -> delete_cert_user(Config), ok = rabbit_ct_broker_helpers:clear_global_parameter(Config, mqtt_default_vhosts), + close_all_connections(Config), rabbit_ct_helpers:testcase_finished(Config, ssl_user_vhost_parameter_mapping_vhost_does_not_exist); end_per_testcase(Testcase, Config) when Testcase == port_vhost_mapping_success; Testcase == port_vhost_mapping_not_allowed; @@ -417,11 +421,13 @@ end_per_testcase(Testcase, Config) when Testcase == port_vhost_mapping_success; VHost = ?config(temp_vhost_for_port_mapping, Config), ok = rabbit_ct_broker_helpers:delete_vhost(Config, VHost), ok = rabbit_ct_broker_helpers:clear_global_parameter(Config, mqtt_port_to_vhost_mapping), + close_all_connections(Config), rabbit_ct_helpers:testcase_finished(Config, Testcase); end_per_testcase(T = port_vhost_mapping_vhost_does_not_exist, Config) -> User = <<"guest">>, ok = set_full_permissions(Config, User, <<"/">>), ok = rabbit_ct_broker_helpers:clear_global_parameter(Config, mqtt_port_to_vhost_mapping), + close_all_connections(Config), rabbit_ct_helpers:testcase_finished(Config, T); end_per_testcase(T = ssl_user_cert_vhost_mapping_takes_precedence_over_port_vhost_mapping, Config) -> delete_cert_user(Config), @@ -432,6 +438,7 @@ end_per_testcase(T = ssl_user_cert_vhost_mapping_takes_precedence_over_port_vhos VHostForPortVHostMapping = ?config(temp_vhost_for_port_mapping, Config), ok = rabbit_ct_broker_helpers:delete_vhost(Config, VHostForPortVHostMapping), ok = rabbit_ct_broker_helpers:clear_global_parameter(Config, mqtt_port_to_vhost_mapping), + close_all_connections(Config), rabbit_ct_helpers:testcase_finished(Config, T); end_per_testcase(T, Config) when T == queue_bind_permission; T == queue_unbind_permission; @@ -459,6 +466,8 @@ end_per_testcase(T, Config) when T == queue_bind_permission; %% And provide an empty log file for the next test in this group file:write_file(?config(log_location, Config), <<>>), + close_all_connections(Config), + rabbit_ct_helpers:testcase_finished(Config, T); end_per_testcase(T, Config) @@ -469,11 +478,17 @@ end_per_testcase(T, Config) T =:= client_id_from_cert_san_email; T =:= client_id_from_cert_dn -> SetupProcess = ?config(mock_setup_process, Config), - SetupProcess ! stop; + SetupProcess ! stop, + close_all_connections(Config); end_per_testcase(Testcase, Config) -> + close_all_connections(Config), rabbit_ct_helpers:testcase_finished(Config, Testcase). +close_all_connections(Config) -> + rpc(Config, 0, rabbit_mqtt, close_local_client_connections, + [end_per_testcase]). + delete_cert_user(Config) -> User = ?config(temp_ssl_user, Config), {ok,_} = rabbit_ct_broker_helpers:rabbitmqctl(Config, 0, ["delete_user", User]). From 9c66d2d2831599271beb03f3a8b87da41dc727bb Mon Sep 17 00:00:00 2001 From: Diana Parra Corbacho Date: Thu, 21 Nov 2024 09:45:00 +0100 Subject: [PATCH 06/14] Tests: clustering_SUITE retry all GET queries --- .../test/clustering_SUITE.erl | 360 +++++++++++------- 1 file changed, 227 insertions(+), 133 deletions(-) diff --git a/deps/rabbitmq_management/test/clustering_SUITE.erl b/deps/rabbitmq_management/test/clustering_SUITE.erl index ed0ccc9eef9c..38d58e0d28ae 100644 --- a/deps/rabbitmq_management/test/clustering_SUITE.erl +++ b/deps/rabbitmq_management/test/clustering_SUITE.erl @@ -15,6 +15,7 @@ -include_lib("rabbitmq_ct_helpers/include/rabbit_assert.hrl"). -import(rabbit_ct_broker_helpers, [get_node_config/3, restart_node/2]). +-import(rabbit_ct_helpers, [eventually/3]). -import(rabbit_mgmt_test_util, [http_get/2, http_put/4, http_post/4, http_delete/3, http_delete/4]). -import(rabbit_misc, [pget/2]). @@ -120,7 +121,8 @@ end_per_testcase(Testcase, Config) -> list_cluster_nodes_test(Config) -> %% see rmq_nodes_count in init_per_suite - ?assertEqual(2, length(http_get(Config, "/nodes"))), + eventually(?_assertEqual(2, length(http_get(Config, "/nodes"))), + 1000, 30), passed. qq_replicas_add(Config) -> @@ -217,15 +219,27 @@ queue_on_other_node(Config) -> {ok, Chan2} = amqp_connection:open_channel(?config(conn, Config)), consume(Chan2, <<"some-queue">>), - force_stats(Config), - ?awaitMatch([_], maps:get(consumer_details, http_get(Config, "/queues/%2F/some-queue")), 60000), - - Res = http_get(Config, "/queues/%2F/some-queue"), - % assert some basic data is present - [Cons] = maps:get(consumer_details, Res), - #{} = maps:get(channel_details, Cons), % channel details proplist must not be empty - 0 = maps:get(prefetch_count, Cons), % check one of the augmented properties - <<"some-queue">> = maps:get(name, Res), + ?awaitMatch([_], + begin + force_stats(Config), + maps:get(consumer_details, http_get(Config, "/queues/%2F/some-queue")) + end, + 60000), + + ?awaitMatch({#{}, 0, <<"some-queue">>}, + begin + Res = http_get(Config, "/queues/%2F/some-queue"), + %% assert some basic data is present + case maps:get(consumer_details, Res, undefined) of + [Cons] -> + {maps:get(channel_details, Cons, undefined), % channel details proplist must not be empty + maps:get(prefetch_count, Cons, undefined), % check one of the augmented properties + maps:get(name, Res, undefined)}; + Any -> + {unexpected_consumer_details, Any} + end + end, + 60000), http_delete(Config, "/queues/%2F/some-queue", ?NO_CONTENT), @@ -253,21 +267,29 @@ queue_with_multiple_consumers(Config) -> amqp_channel:cast(Chan, #'basic.ack'{delivery_tag = T}) end, - force_stats(Config), + eventually(?_assertMatch( + {#{}, #{}, 0, 0, Q}, + begin + force_stats(Config), + Res = http_get(Config, "/queues/%2F/multi-consumer-queue1"), + %% assert some basic data is there + case maps:get(consumer_details, Res) of + [C1, C2] -> + %% channel details proplist must not be empty + {maps:get(channel_details, C1), + maps:get(channel_details, C2), + %% check one of the augmented properties + maps:get(prefetch_count, C1), + maps:get(prefetch_count, C2), + maps:get(name, Res)}; + Any -> + {unexpected_consumer_details, Any} + end + end), + 1000, 60), - Res = http_get(Config, "/queues/%2F/multi-consumer-queue1"), http_delete(Config, "/queues/%2F/multi-consumer-queue1", ?NO_CONTENT), - % assert some basic data is there - [C1, C2] = maps:get(consumer_details, Res), - % channel details proplist must not be empty - #{} = maps:get(channel_details, C1), - #{} = maps:get(channel_details, C2), - % check one of the augmented properties - 0 = maps:get(prefetch_count, C1), - 0 = maps:get(prefetch_count, C2), - Q = maps:get(name, Res), - amqp_channel:close(Chan), amqp_channel:close(Chan2), rabbit_ct_client_helpers:close_connection(Conn), @@ -283,14 +305,19 @@ queue_consumer_cancelled(Config) -> #'basic.cancel_ok'{} = amqp_channel:call(Chan, #'basic.cancel'{consumer_tag = Tag}), - force_stats(Config), - Res = http_get(Config, "/queues/%2F/some-queue"), + eventually(?_assertMatch( + {[], <<"some-queue">>}, + begin + force_stats(Config), + Res = http_get(Config, "/queues/%2F/some-queue"), + %% assert there are no consumer details + {maps:get(consumer_details, Res), + maps:get(name, Res)} + end), + 1000, 60), amqp_channel:close(Chan), - % assert there are no consumer details - [] = maps:get(consumer_details, Res), - <<"some-queue">> = maps:get(name, Res), http_delete(Config, "/queues/%2F/some-queue", ?NO_CONTENT), ok. @@ -347,12 +374,17 @@ queues_single(Config) -> http_put(Config, "/queues/%2F/some-queue", none, [?CREATED, ?NO_CONTENT]), _ = wait_for_queue(Config, "/queues/%2F/some-queue"), - force_stats(Config), - Res = http_get(Config, "/queues/%2F"), - http_delete(Config, "/queues/%2F/some-queue", ?NO_CONTENT), + eventually(?_assertMatch( + true, + begin + force_stats(Config), + Res = http_get(Config, "/queues/%2F"), + %% assert at least one queue is returned + length(Res) >= 1 + end), + 1000, 60), - % assert at least one queue is returned - ?assert(length(Res) >= 1), + http_delete(Config, "/queues/%2F/some-queue", ?NO_CONTENT), ok. @@ -363,16 +395,25 @@ queues_multiple(Config) -> _ = wait_for_queue(Config, "/queues/%2F/some-queue"), _ = wait_for_queue(Config, "/queues/%2F/some-other-queue"), - force_stats(Config), - - Res = http_get(Config, "/queues/%2F"), - [Q1, Q2 | _] = Res, + eventually(?_assertNot( + begin + force_stats(Config), + case http_get(Config, "/queues/%2F") of + [Q1, Q2 | _] -> + %% assert some basic data is present + ct:pal("Name q1 ~p q2 ~p", + [maps:get(name, Q1), + maps:get(name, Q2)]), + maps:get(name, Q1) =:= maps:get(name, Q2); + Any -> + {unexpected_queues, Any} + end + end), + 1000, 60), - % assert some basic data is present http_delete(Config, "/queues/%2F/some-queue", ?NO_CONTENT), http_delete(Config, "/queues/%2F/some-other-queue", ?NO_CONTENT), - false = (maps:get(name, Q1) =:= maps:get(name, Q2)), amqp_channel:close(Chan), ok. @@ -382,8 +423,13 @@ queues_removed(Config) -> force_stats(Config), N = length(http_get(Config, "/queues/%2F")), http_delete(Config, "/queues/%2F/some-queue", ?NO_CONTENT), - force_stats(Config), - ?assertEqual(N - 1, length(http_get(Config, "/queues/%2F"))), + eventually(?_assertEqual( + N - 1, + begin + force_stats(Config), + length(http_get(Config, "/queues/%2F")) + end), + 1000, 60), ok. channels_multiple_on_different_nodes(Config) -> @@ -396,11 +442,12 @@ channels_multiple_on_different_nodes(Config) -> {ok, Chan2} = amqp_connection:open_channel(Conn2), consume(Chan, <<"some-queue">>), - force_stats(Config), - % assert two channels are present ?awaitMatch([_,_], - http_get(Config, "/channels"), + begin + force_stats(Config), + http_get(Config, "/channels") + end, 30000), http_delete(Config, "/queues/%2F/some-queue", ?NO_CONTENT), @@ -423,10 +470,9 @@ channel_closed(Config) -> consume(Chan2, <<"some-queue">>), amqp_channel:close(Chan), - force_stats(Config), - rabbit_ct_helpers:await_condition( fun() -> + force_stats(Config), %% assert one channel is present length(http_get(Config, "/channels")) == 1 end, @@ -443,11 +489,15 @@ channel(Config) -> [{_, ChData}] = rabbit_ct_broker_helpers:rpc(Config, 0, ets, tab2list, [channel_created]), ChName = uri_string:recompose(#{path => binary_to_list(pget(name, ChData))}), - - force_stats(Config), - Res = http_get(Config, "/channels/" ++ ChName ), - % assert channel is non empty - #{} = Res, + + eventually(?_assertMatch( + #{}, + begin + force_stats(Config), + %% assert channel is non empty + http_get(Config, "/channels/" ++ ChName ) + end), + 1000, 60), amqp_channel:close(Chan), ok. @@ -463,14 +513,20 @@ channel_other_node(Config) -> consume(Chan, Q), publish(Chan, Q), - wait_for_collect_statistics_interval(), - force_stats(Config), - - Res = http_get(Config, "/channels/" ++ ChName ), - % assert channel is non empty - #{} = Res, - [#{}] = maps:get(deliveries, Res), - #{} = maps:get(connection_details, Res), + eventually(?_assertMatch( + {[#{}], #{}}, + begin + force_stats(Config), + case http_get(Config, "/channels/" ++ ChName) of + %% assert channel is non empty + #{} = Res -> + {maps:get(deliveries, Res), + maps:get(connection_details, Res)}; + Any -> + {unexpected_channels, Any} + end + end), + 1000, 60), http_delete(Config, "/queues/%2F/some-queue", ?NO_CONTENT), amqp_connection:close(Conn), @@ -487,14 +543,21 @@ channel_with_consumer_on_other_node(Config) -> consume(Chan, Q), publish(Chan, Q), - force_stats(Config), + eventually(?_assertMatch( + [#{}], + begin + force_stats(Config), + case http_get(Config, "/channels/" ++ ChName) of + %% assert channel is non empty + #{} = Res -> + maps:get(consumer_details, Res); + Any -> + {unexpected_channels, Any} + end + end), + 1000, 60), - Res = http_get(Config, "/channels/" ++ ChName), http_delete(Config, "/queues/%2F/some-queue", ?NO_CONTENT), - % assert channel is non empty - #{} = Res, - [#{}] = maps:get(consumer_details, Res), - amqp_channel:close(Chan), ok. @@ -508,15 +571,19 @@ channel_with_consumer_on_one_node(Config) -> ChName = get_channel_name(Config, 0), consume(Chan, Q), - force_stats(Config), + eventually(?_assertMatch( + [#{}], + begin + force_stats(Config), + Res = http_get(Config, "/channels/" ++ ChName), + %% assert channel is non empty + maps:get(consumer_details, Res) + end), + 1000, 60), - Res = http_get(Config, "/channels/" ++ ChName), amqp_channel:close(Chan), http_delete(Config, "/queues/%2F/some-queue", ?NO_CONTENT), - % assert channel is non empty - #{} = Res, - [#{}] = maps:get(consumer_details, Res), ok. consumers(Config) -> @@ -530,13 +597,20 @@ consumers(Config) -> consume(Chan, <<"some-queue">>), consume(Chan2, <<"some-queue">>), - force_stats(Config), - Res = http_get(Config, "/consumers"), - - % assert there are two non-empty consumer records - [#{} = C1, #{} = C2] = Res, - #{} = maps:get(channel_details, C1), - #{} = maps:get(channel_details, C2), + %% assert there are two non-empty consumer records + eventually(?_assertMatch([#{}, #{}], + begin + force_stats(Config), + http_get(Config, "/consumers") + end), + 1000, 30), + eventually(?_assertMatch([#{}, #{}], + begin + [C1, C2] = http_get(Config, "/consumers"), + [maps:get(channel_details, C1), + maps:get(channel_details, C2)] + end), + 1000, 30), http_delete(Config, "/queues/%2F/some-queue", ?NO_CONTENT), @@ -554,16 +628,19 @@ connections(Config) -> Conn2 = rabbit_ct_client_helpers:open_unmanaged_connection(Config, 0), {ok, _Chan2} = amqp_connection:open_channel(Conn2), - %% channel count needs a bit longer for 2nd chan - wait_for_collect_statistics_interval(), - force_stats(Config), - - Res = http_get(Config, "/connections"), - - % assert there are two non-empty connection records - [#{} = C1, #{} = C2] = Res, - 1 = maps:get(channels, C1), - 1 = maps:get(channels, C2), + %% assert there are two non-empty connection records + eventually(?_assertMatch([1, 1], + begin + force_stats(Config), + case http_get(Config, "/connections") of + [#{} = C1, #{} = C2] -> + [maps:get(channels, C1), + maps:get(channels, C2)]; + Any -> + {unexpected_connections, Any} + end + end), + 1000, 30), amqp_channel:close(Chan), rabbit_ct_client_helpers:close_connection(Conn2), @@ -583,11 +660,13 @@ exchanges(Config) -> consume(Chan, QName), publish_to(Chan, XName, <<"some-key">>), - force_stats(Config), - Res = http_get(Config, "/exchanges"), - [X] = [X || X <- Res, maps:get(name, X) =:= XName], - - ?assertEqual(<<"direct">>, maps:get(type, X)), + eventually(?_assertEqual([<<"direct">>], + begin + force_stats(Config), + Res = http_get(Config, "/exchanges"), + [maps:get(type, X) || X <- Res, maps:get(name, X) =:= XName] + end), + 1000, 30), amqp_channel:close(Chan), rabbit_ct_client_helpers:close_connection(Conn), @@ -607,11 +686,12 @@ exchange(Config) -> consume(Chan, QName), publish_to(Chan, XName, <<"some-key">>), - force_stats(Config), - force_stats(Config), - Res = http_get(Config, "/exchanges/%2F/some-other-exchange"), - - ?assertEqual(<<"direct">>, maps:get(type, Res)), + eventually(?_assertEqual(<<"direct">>, + begin + force_stats(Config), + maps:get(type, http_get(Config, "/exchanges/%2F/some-other-exchange")) + end), + 1000, 30), amqp_channel:close(Chan), rabbit_ct_client_helpers:close_connection(Conn), @@ -628,15 +708,21 @@ vhosts(Config) -> {ok, Chan2} = amqp_connection:open_channel(Conn2), publish(Chan2, <<"some-queue">>), - wait_for_collect_statistics_interval(), - force_stats(Config), - Res = http_get(Config, "/vhosts"), + eventually(?_assertMatch(#{}, + begin + force_stats(Config), + %% default vhost + case http_get(Config, "/vhosts") of + [#{} = Vhost] -> + %% assert vhost has some message stats + maps:get(message_stats, Vhost); + Any -> + {unexpected_vhosts, Any} + end + end), + 1000, 30), http_delete(Config, "/queues/%2F/some-queue", ?NO_CONTENT), - % default vhost - [#{} = Vhost] = Res, - % assert vhost has some message stats - #{} = maps:get(message_stats, Vhost), amqp_channel:close(Chan), amqp_channel:close(Chan2), @@ -653,16 +739,22 @@ nodes(Config) -> {ok, Chan2} = amqp_connection:open_channel(Conn), publish(Chan2, <<"some-queue">>), - wait_for_collect_statistics_interval(), - force_stats(Config), - Res = http_get(Config, "/nodes"), - http_delete(Config, "/queues/%2F/some-queue", ?NO_CONTENT), + eventually(?_assertMatch({true, true, [#{} | _], [#{} | _]}, + begin + force_stats(Config), + case http_get(Config, "/nodes") of + [#{} = N1 , #{} = N2] -> + {is_binary(maps:get(name, N1)), + is_binary(maps:get(name, N2)), + maps:get(cluster_links, N1), + maps:get(cluster_links, N2)}; + Any -> + {unexpected_nodes, Any} + end + end), + 1000, 30), - [#{} = N1 , #{} = N2] = Res, - ?assert(is_binary(maps:get(name, N1))), - ?assert(is_binary(maps:get(name, N2))), - [#{} | _] = maps:get(cluster_links, N1), - [#{} | _] = maps:get(cluster_links, N2), + http_delete(Config, "/queues/%2F/some-queue", ?NO_CONTENT), amqp_channel:close(Chan), amqp_channel:close(Chan2), @@ -682,27 +774,32 @@ overview(Config) -> {ok, Chan2} = amqp_connection:open_channel(Conn2), publish(Chan, <<"queue-n1">>), publish(Chan2, <<"queue-n2">>), - wait_for_collect_statistics_interval(), - force_stats(Config), % channel count needs a bit longer for 2nd chan - Res = http_get(Config, "/overview"), + + eventually(?_assertMatch( + {true, true, true, true, 2, 2, 0, 2, 0, 0}, + begin + force_stats(Config), % channel count needs a bit longer for 2nd chan + Res = http_get(Config, "/overview"), + %% assert there are two non-empty connection records + ObjTots = maps:get(object_totals, Res), + QT = maps:get(queue_totals, Res), + MS = maps:get(message_stats, Res), + ChurnRates = maps:get(churn_rates, Res), + {maps:get(connections, ObjTots) >= 2, + maps:get(channels, ObjTots) >= 2, + maps:get(messages_ready, QT) >= 2, + maps:get(publish, MS) >= 2, + maps:get(queue_declared, ChurnRates), + maps:get(queue_created, ChurnRates), + maps:get(queue_deleted, ChurnRates), + maps:get(channel_created, ChurnRates), + maps:get(channel_closed, ChurnRates), + maps:get(connection_closed, ChurnRates)} + end), + 1000, 60), http_delete(Config, "/queues/%2F/queue-n1", ?NO_CONTENT), http_delete(Config, "/queues/%2F/queue-n2", ?NO_CONTENT), - % assert there are two non-empty connection records - ObjTots = maps:get(object_totals, Res), - ?assert(maps:get(connections, ObjTots) >= 2), - ?assert(maps:get(channels, ObjTots) >= 2), - #{} = QT = maps:get(queue_totals, Res), - ?assert(maps:get(messages_ready, QT) >= 2), - MS = maps:get(message_stats, Res), - ?assert(maps:get(publish, MS) >= 2), - ChurnRates = maps:get(churn_rates, Res), - ?assertEqual(maps:get(queue_declared, ChurnRates), 2), - ?assertEqual(maps:get(queue_created, ChurnRates), 2), - ?assertEqual(maps:get(queue_deleted, ChurnRates), 0), - ?assertEqual(maps:get(channel_created, ChurnRates), 2), - ?assertEqual(maps:get(channel_closed, ChurnRates), 0), - ?assertEqual(maps:get(connection_closed, ChurnRates), 0), amqp_channel:close(Chan), amqp_channel:close(Chan2), @@ -878,6 +975,3 @@ listener_proto(Proto) when is_atom(Proto) -> %% rabbit:status/0 used this formatting before rabbitmq/rabbitmq-cli#340 listener_proto({Proto, _Port, _Interface}) -> Proto. - -wait_for_collect_statistics_interval() -> - timer:sleep(?STATS_INTERVAL * 2). From 894bdb6e9189e96bf60bba7b1fcd389f4fe55f56 Mon Sep 17 00:00:00 2001 From: Diana Parra Corbacho Date: Thu, 21 Nov 2024 12:43:23 +0100 Subject: [PATCH 07/14] Tests: amqp_client_SUITE delete all queues on end per testcase --- deps/rabbit/test/amqp_client_SUITE.erl | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/deps/rabbit/test/amqp_client_SUITE.erl b/deps/rabbit/test/amqp_client_SUITE.erl index 26e4e78f3da7..851db5e2c9a2 100644 --- a/deps/rabbit/test/amqp_client_SUITE.erl +++ b/deps/rabbit/test/amqp_client_SUITE.erl @@ -341,6 +341,7 @@ init_per_testcase(Testcase, Config) -> end_per_testcase(Testcase, Config) -> %% Assert that every testcase cleaned up. + rabbit_ct_broker_helpers:rpc(Config, 0, ?MODULE, delete_queues, []), eventually(?_assertEqual([], rpc(Config, rabbit_amqqueue, list, []))), %% Wait for sessions to terminate before starting the next test case. eventually(?_assertEqual([], rpc(Config, rabbit_amqp_session, list_local, []))), @@ -350,6 +351,10 @@ end_per_testcase(Testcase, Config) -> get_global_counters(Config))), rabbit_ct_helpers:testcase_finished(Config, Testcase). +delete_queues() -> + [rabbit_amqqueue:delete(Q, false, false, <<"dummy">>) + || Q <- rabbit_amqqueue:list()]. + reliable_send_receive_with_outcomes_classic_queue(Config) -> reliable_send_receive_with_outcomes(<<"classic">>, Config). From c1e60c16b8abce09d9f8003e49ec21f723efc372 Mon Sep 17 00:00:00 2001 From: Diana Parra Corbacho Date: Wed, 20 Nov 2024 08:21:43 +0100 Subject: [PATCH 08/14] Tests: SSL certificates Parallel/sharding groups often fail to create certificates in CI. Most likely it is related to the fact they use the same directory for certificates. This commit uses shard/node name and unique id for each SSL certificate --- deps/rabbitmq_ct_helpers/src/rabbit_ct_helpers.erl | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/deps/rabbitmq_ct_helpers/src/rabbit_ct_helpers.erl b/deps/rabbitmq_ct_helpers/src/rabbit_ct_helpers.erl index 801de565d125..ee109b9f9c56 100644 --- a/deps/rabbitmq_ct_helpers/src/rabbit_ct_helpers.erl +++ b/deps/rabbitmq_ct_helpers/src/rabbit_ct_helpers.erl @@ -584,9 +584,14 @@ ensure_rabbitmq_queues_cmd(Config) -> ensure_ssl_certs(Config) -> SrcDir = ?config(rabbitmq_ct_helpers_srcdir, Config), + UniqueDir = io_lib:format( + "~s2-~p", + [node(), erlang:unique_integer([positive,monotonic])]), CertsMakeDir = filename:join([SrcDir, "tools", "tls-certs"]), PrivDir = ?config(priv_dir, Config), - CertsDir = filename:join(PrivDir, "certs"), + CertsDir = filename:join([PrivDir, UniqueDir, "certs"]), + _ = filelib:ensure_dir(CertsDir), + _ = file:make_dir(CertsDir), CertsPwd = proplists:get_value(rmq_certspwd, Config, ?SSL_CERT_PASSWORD), Cmd = [ "PASSWORD=" ++ CertsPwd, From ec2ad376d5cd965979c81ed7e14a09091db9a55a Mon Sep 17 00:00:00 2001 From: Marcial Rosales Date: Thu, 21 Nov 2024 18:55:54 +0100 Subject: [PATCH 09/14] Store the certsDir of the group which initializes rabbitmq configuration --- deps/oauth2_client/test/system_SUITE.erl | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/deps/oauth2_client/test/system_SUITE.erl b/deps/oauth2_client/test/system_SUITE.erl index a0be9dd3976d..5010ae812b32 100644 --- a/deps/oauth2_client/test/system_SUITE.erl +++ b/deps/oauth2_client/test/system_SUITE.erl @@ -93,6 +93,7 @@ init_per_group(https, Config) -> CaCertFile = filename:join([CertsDir, "testca", "cacert.pem"]), WrongCaCertFile = filename:join([CertsDir, "server", "server.pem"]), [{group, https}, + {certsdir, CertsDir}, {oauth_provider_id, <<"uaa">>}, {oauth_provider, build_https_oauth_provider(<<"uaa">>, CaCertFile)}, {oauth_provider_with_issuer, keep_only_issuer_and_ssl_options( @@ -109,6 +110,7 @@ init_per_group(https_down, Config) -> CaCertFile = filename:join([CertsDir, "testca", "cacert.pem"]), [{issuer, build_issuer("https")}, + {certsdir, CertsDir}, {oauth_provider_id, <<"uaa">>}, {oauth_provider, build_https_oauth_provider(<<"uaa">>, CaCertFile)} | Config]; @@ -121,6 +123,7 @@ init_per_group(with_all_oauth_provider_settings, Config) -> CaCertFile = filename:join([CertsDir, "testca", "cacert.pem"]), [{with_all_oauth_provider_settings, true}, + {certsdir, CertsDir}, {oauth_provider_id, <<"uaa">>}, {oauth_provider, build_https_oauth_provider(<<"uaa">>, CaCertFile)} | Config]; @@ -130,6 +133,7 @@ init_per_group(without_all_oauth_providers_settings, Config) -> CaCertFile = filename:join([CertsDir, "testca", "cacert.pem"]), [{with_all_oauth_provider_settings, false}, + {certsdir, CertsDir}, {oauth_provider_id, <<"uaa">>}, {oauth_provider, keep_only_issuer_and_ssl_options( build_https_oauth_provider(<<"uaa">>, CaCertFile))} | Config]; @@ -244,8 +248,12 @@ init_per_testcase(TestCase, Config) -> case ?config(group, Config) of https -> +<<<<<<< HEAD ct:log("Start https with expectations ~p", [ListOfExpectations]), start_https_oauth_server(?AUTH_PORT, ?config(rmq_certsdir, Config), +======= + start_https_oauth_server(?AUTH_PORT, ?config(certsdir, Config), +>>>>>>> 9b1e762081 (Store the certsDir of the group which) ListOfExpectations); _ -> do_nothing From 3310f2398c5a6bbb86db8f6f8159d9dfd05fc455 Mon Sep 17 00:00:00 2001 From: Marcial Rosales Date: Fri, 22 Nov 2024 09:57:45 +0100 Subject: [PATCH 10/14] Fix issue --- selenium/test/basic-auth/ac-management.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/selenium/test/basic-auth/ac-management.js b/selenium/test/basic-auth/ac-management.js index d5282d386b82..5b4509f02939 100644 --- a/selenium/test/basic-auth/ac-management.js +++ b/selenium/test/basic-auth/ac-management.js @@ -59,7 +59,7 @@ describe('management user with vhosts permissions', function () { await overview.clickOnAdminTab() await admin.clickOnLimits() await limits.list_virtual_host_limits() - assert.rejects(limits.list_user_limits()) + assert.rejects(await limits.list_user_limits()) }) From 9f00857a027519723310f15b87a261d3e91a1f48 Mon Sep 17 00:00:00 2001 From: Marcial Rosales Date: Fri, 22 Nov 2024 10:34:39 +0100 Subject: [PATCH 11/14] Add extra logging To capture where the flake occurs --- selenium/test/basic-auth/ac-management.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/selenium/test/basic-auth/ac-management.js b/selenium/test/basic-auth/ac-management.js index 5b4509f02939..a07484d0f0c1 100644 --- a/selenium/test/basic-auth/ac-management.js +++ b/selenium/test/basic-auth/ac-management.js @@ -50,8 +50,11 @@ describe('management user with vhosts permissions', function () { assert.ok(!await overview.isPopupWarningDisplayed()) }) it('can access limited options in admin tab', async function () { + console.log("before clickOnAdminTab") await overview.clickOnAdminTab() + console.log("before waitForAdminTab") await overview.waitForAdminTab() + console.log("after waitForAdminTab") assert.ok(!await overview.isPopupWarningDisplayed()) }) From 251ef24d62257a3598fc3b7dfb6a9380295107e4 Mon Sep 17 00:00:00 2001 From: Diana Parra Corbacho Date: Mon, 25 Nov 2024 14:49:54 +0100 Subject: [PATCH 12/14] Tests: system_SUITE return configuration from run steps --- deps/oauth2_client/test/system_SUITE.erl | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/deps/oauth2_client/test/system_SUITE.erl b/deps/oauth2_client/test/system_SUITE.erl index 5010ae812b32..13c04344dd63 100644 --- a/deps/oauth2_client/test/system_SUITE.erl +++ b/deps/oauth2_client/test/system_SUITE.erl @@ -93,7 +93,6 @@ init_per_group(https, Config) -> CaCertFile = filename:join([CertsDir, "testca", "cacert.pem"]), WrongCaCertFile = filename:join([CertsDir, "server", "server.pem"]), [{group, https}, - {certsdir, CertsDir}, {oauth_provider_id, <<"uaa">>}, {oauth_provider, build_https_oauth_provider(<<"uaa">>, CaCertFile)}, {oauth_provider_with_issuer, keep_only_issuer_and_ssl_options( @@ -110,9 +109,8 @@ init_per_group(https_down, Config) -> CaCertFile = filename:join([CertsDir, "testca", "cacert.pem"]), [{issuer, build_issuer("https")}, - {certsdir, CertsDir}, {oauth_provider_id, <<"uaa">>}, - {oauth_provider, build_https_oauth_provider(<<"uaa">>, CaCertFile)} | Config]; + {oauth_provider, build_https_oauth_provider(<<"uaa">>, CaCertFile)} | Config0]; init_per_group(openid_configuration_with_path, Config) -> [{use_openid_configuration_with_path, true} | Config]; @@ -123,9 +121,8 @@ init_per_group(with_all_oauth_provider_settings, Config) -> CaCertFile = filename:join([CertsDir, "testca", "cacert.pem"]), [{with_all_oauth_provider_settings, true}, - {certsdir, CertsDir}, {oauth_provider_id, <<"uaa">>}, - {oauth_provider, build_https_oauth_provider(<<"uaa">>, CaCertFile)} | Config]; + {oauth_provider, build_https_oauth_provider(<<"uaa">>, CaCertFile)} | Config0]; init_per_group(without_all_oauth_providers_settings, Config) -> Config0 = rabbit_ct_helpers:run_setup_steps(Config), @@ -133,10 +130,9 @@ init_per_group(without_all_oauth_providers_settings, Config) -> CaCertFile = filename:join([CertsDir, "testca", "cacert.pem"]), [{with_all_oauth_provider_settings, false}, - {certsdir, CertsDir}, {oauth_provider_id, <<"uaa">>}, {oauth_provider, keep_only_issuer_and_ssl_options( - build_https_oauth_provider(<<"uaa">>, CaCertFile))} | Config]; + build_https_oauth_provider(<<"uaa">>, CaCertFile))} | Config0]; init_per_group(with_default_oauth_provider, Config) -> OAuthProvider = ?config(oauth_provider, Config), @@ -248,13 +244,12 @@ init_per_testcase(TestCase, Config) -> case ?config(group, Config) of https -> -<<<<<<< HEAD ct:log("Start https with expectations ~p", [ListOfExpectations]), start_https_oauth_server(?AUTH_PORT, ?config(rmq_certsdir, Config), -======= - start_https_oauth_server(?AUTH_PORT, ?config(certsdir, Config), ->>>>>>> 9b1e762081 (Store the certsDir of the group which) ListOfExpectations); + without_all_oauth_providers_settings -> + start_https_oauth_server(?AUTH_PORT, ?config(rmq_certsdir, Config), + ListOfExpectations); _ -> do_nothing end, @@ -270,6 +265,8 @@ end_per_testcase(_, Config) -> case ?config(group, Config) of https -> stop_https_auth_server(); + without_all_oauth_providers_settings -> + stop_https_auth_server(); _ -> do_nothing end, From 950a1a84561ba04c440a9fd71d0cae406535d738 Mon Sep 17 00:00:00 2001 From: Marcial Rosales Date: Tue, 26 Nov 2024 11:34:28 +0100 Subject: [PATCH 13/14] Use certs generated on first outer group https --- deps/oauth2_client/test/system_SUITE.erl | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/deps/oauth2_client/test/system_SUITE.erl b/deps/oauth2_client/test/system_SUITE.erl index 13c04344dd63..f833e4b53c75 100644 --- a/deps/oauth2_client/test/system_SUITE.erl +++ b/deps/oauth2_client/test/system_SUITE.erl @@ -90,9 +90,11 @@ init_per_group(https, Config) -> application:ensure_all_started(cowboy), Config0 = rabbit_ct_helpers:run_setup_steps(Config), CertsDir = ?config(rmq_certsdir, Config0), + ct:log("certsdir: ~p", [CertsDir]), CaCertFile = filename:join([CertsDir, "testca", "cacert.pem"]), WrongCaCertFile = filename:join([CertsDir, "server", "server.pem"]), [{group, https}, + {certsDir, CertsDir}, {oauth_provider_id, <<"uaa">>}, {oauth_provider, build_https_oauth_provider(<<"uaa">>, CaCertFile)}, {oauth_provider_with_issuer, keep_only_issuer_and_ssl_options( @@ -117,18 +119,18 @@ init_per_group(openid_configuration_with_path, Config) -> init_per_group(with_all_oauth_provider_settings, Config) -> Config0 = rabbit_ct_helpers:run_setup_steps(Config), - CertsDir = ?config(rmq_certsdir, Config0), + CertsDir = ?config(certsDir, Config0), CaCertFile = filename:join([CertsDir, "testca", "cacert.pem"]), - + ct:log("certsdir: ~p", [CertsDir]), [{with_all_oauth_provider_settings, true}, {oauth_provider_id, <<"uaa">>}, {oauth_provider, build_https_oauth_provider(<<"uaa">>, CaCertFile)} | Config0]; init_per_group(without_all_oauth_providers_settings, Config) -> Config0 = rabbit_ct_helpers:run_setup_steps(Config), - CertsDir = ?config(rmq_certsdir, Config0), + CertsDir = ?config(certsDir, Config0), CaCertFile = filename:join([CertsDir, "testca", "cacert.pem"]), - + ct:log("certsdir: ~p", [CertsDir]), [{with_all_oauth_provider_settings, false}, {oauth_provider_id, <<"uaa">>}, {oauth_provider, keep_only_issuer_and_ssl_options( @@ -244,8 +246,7 @@ init_per_testcase(TestCase, Config) -> case ?config(group, Config) of https -> - ct:log("Start https with expectations ~p", [ListOfExpectations]), - start_https_oauth_server(?AUTH_PORT, ?config(rmq_certsdir, Config), + start_https_oauth_server(?AUTH_PORT, ?config(certsDir, Config), ListOfExpectations); without_all_oauth_providers_settings -> start_https_oauth_server(?AUTH_PORT, ?config(rmq_certsdir, Config), @@ -623,8 +624,8 @@ start_https_oauth_server(Port, CertsDir, Expectations) when is_list(Expectations {'_', [{Path, oauth_http_mock, Expected} || #{request := #{path := Path}} = Expected <- Expectations ]} ]), - ct:log("start_https_oauth_server with expectation list : ~p -> dispatch: ~p", - [Expectations, Dispatch]), + ct:log("start_https_oauth_server with expectation : ~p -> dispatch: ~p . certsDir: ~p", + [Expectations, Dispatch, CertsDir]), {ok, _} = cowboy:start_tls( mock_http_auth_listener, [{port, Port}, @@ -635,8 +636,8 @@ start_https_oauth_server(Port, CertsDir, Expectations) when is_list(Expectations start_https_oauth_server(Port, CertsDir, #{request := #{path := Path}} = Expected) -> Dispatch = cowboy_router:compile([{'_', [{Path, oauth_http_mock, Expected}]}]), - ct:log("start_https_oauth_server with expectation : ~p -> dispatch: ~p", - [Expected, Dispatch]), + ct:log("start_https_oauth_server with expectation : ~p -> dispatch: ~p . certsDir: ~p", + [Expected, Dispatch, CertsDir]), {ok, _} = cowboy:start_tls( mock_http_auth_listener, [{port, Port}, From eb183678d468edd81c45c237b1d174d4eb4cb947 Mon Sep 17 00:00:00 2001 From: Marcial Rosales Date: Tue, 26 Nov 2024 11:37:27 +0100 Subject: [PATCH 14/14] Use certs generated on first outer group https --- deps/oauth2_client/test/system_SUITE.erl | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/deps/oauth2_client/test/system_SUITE.erl b/deps/oauth2_client/test/system_SUITE.erl index f833e4b53c75..ecc5aa733bef 100644 --- a/deps/oauth2_client/test/system_SUITE.erl +++ b/deps/oauth2_client/test/system_SUITE.erl @@ -247,10 +247,7 @@ init_per_testcase(TestCase, Config) -> case ?config(group, Config) of https -> start_https_oauth_server(?AUTH_PORT, ?config(certsDir, Config), - ListOfExpectations); - without_all_oauth_providers_settings -> - start_https_oauth_server(?AUTH_PORT, ?config(rmq_certsdir, Config), - ListOfExpectations); + ListOfExpectations); _ -> do_nothing end,