Skip to content

Commit 57ed962

Browse files
committed
rabbitmq_ct_helpers: Fix how we set $RABBITMQ_FEATURE_FLAGS in tests
[Why] In order to make `khepri_db` the default in the future, the handling of `$RABBITMQ_FEATURE_FLAGS` had to be adapted to be able to *disable* Khepri instead. Unfortunately I broke the behavior with stable feature flags that are only available in the primary umbrella. In this case, they were automatically enabled and thus, clustering with an old umbrella that did not have these feature flags failed with `incompatible_feature_flags`. [How] The solution is to always use an absolute list of feature flags, not the new relative list. V2: Allow a testsuite to skip the configuration of the metadata store. This is needed for the feature_flags_SUITE testsuite because it tests the default behavior and the configuration of the metadata store changes that behavior. While here, fix a ct log message where variables were swapped compared to the format strieg expectation. V3: Enable `rabbitmq_4.0.0` feature flag in rabbit_mgmt_http_SUITE. This testsuite apparently requires it and if it's not enabled, it fails.
1 parent 415dc81 commit 57ed962

File tree

3 files changed

+66
-84
lines changed

3 files changed

+66
-84
lines changed

deps/rabbit/test/feature_flags_SUITE.erl

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,9 @@ groups() ->
119119

120120
init_per_suite(Config) ->
121121
rabbit_ct_helpers:log_environment(),
122-
rabbit_ct_helpers:run_setup_steps(Config, [
122+
Config1 = rabbit_ct_helpers:set_config(
123+
Config, {skip_metadata_store_configuration, true}),
124+
rabbit_ct_helpers:run_setup_steps(Config1, [
123125
fun rabbit_ct_broker_helpers:configure_dist_proxy/1
124126
]).
125127

deps/rabbitmq_ct_helpers/src/rabbit_ct_broker_helpers.erl

Lines changed: 52 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -753,17 +753,6 @@ do_start_rabbitmq_node(Config, NodeConfig, I) ->
753753
false -> ExtraArgs3;
754754
_ -> ["NOBUILD=1" | ExtraArgs3]
755755
end,
756-
%% TODO: When we start to do mixed-version testing against 4.1.x as the
757-
%% secondary umbrella, we will need to stop setting
758-
%% `$RABBITMQ_FEATURE_FLAGS'.
759-
MetadataStore = rabbit_ct_helpers:get_config(Config, metadata_store),
760-
SecFeatureFlags0 = case MetadataStore of
761-
mnesia -> ?REQUIRED_FEATURE_FLAGS;
762-
khepri -> [khepri_db | ?REQUIRED_FEATURE_FLAGS]
763-
end,
764-
SecFeatureFlags = string:join(
765-
[atom_to_list(F) || F <- SecFeatureFlags0],
766-
","),
767756
ExtraArgs = case UseSecondaryUmbrella of
768757
true ->
769758
DepsDir = ?config(erlang_mk_depsdir, Config),
@@ -793,8 +782,7 @@ do_start_rabbitmq_node(Config, NodeConfig, I) ->
793782
{"RABBITMQ_SCRIPTS_DIR=~ts", [SecScriptsDir]},
794783
{"RABBITMQ_SERVER=~ts/rabbitmq-server", [SecScriptsDir]},
795784
{"RABBITMQCTL=~ts/rabbitmqctl", [SecScriptsDir]},
796-
{"RABBITMQ_PLUGINS=~ts/rabbitmq-plugins", [SecScriptsDir]},
797-
{"RABBITMQ_FEATURE_FLAGS=~ts", [SecFeatureFlags]}
785+
{"RABBITMQ_PLUGINS=~ts/rabbitmq-plugins", [SecScriptsDir]}
798786
| ExtraArgs4];
799787
false ->
800788
case UseSecondaryDist of
@@ -815,8 +803,7 @@ do_start_rabbitmq_node(Config, NodeConfig, I) ->
815803
{"CLI_ESCRIPTS_DIR=~ts/escript", [SecondaryDist]},
816804
{"RABBITMQ_SCRIPTS_DIR=~ts/sbin", [SecondaryDist]},
817805
{"RABBITMQ_SERVER=~ts/sbin/rabbitmq-server", [SecondaryDist]},
818-
{"RABBITMQ_ENABLED_PLUGINS=~ts", [SecondaryEnabledPlugins]},
819-
{"RABBITMQ_FEATURE_FLAGS=~ts", [SecFeatureFlags]}
806+
{"RABBITMQ_ENABLED_PLUGINS=~ts", [SecondaryEnabledPlugins]}
820807
| ExtraArgs4];
821808
false ->
822809
ExtraArgs4
@@ -915,19 +902,27 @@ query_node(Config, NodeConfig) ->
915902
rabbit_ct_helpers:set_config(NodeConfig, Vars).
916903

917904
uses_expected_metadata_store(Config, NodeConfig) ->
918-
%% We want to verify if the active metadata store matches the expected one.
919-
Nodename = ?config(nodename, NodeConfig),
920-
ExpectedMetadataStore = rabbit_ct_helpers:get_config(
921-
Config, metadata_store),
922-
IsKhepriEnabled = rpc(Config, Nodename, rabbit_khepri, is_enabled, []),
923-
UsedMetadataStore = case IsKhepriEnabled of
924-
true -> khepri;
925-
false -> mnesia
926-
end,
927-
ct:pal(
928-
"Metadata store on ~s: expected=~s, used=~s",
929-
[Nodename, UsedMetadataStore, ExpectedMetadataStore]),
930-
{ExpectedMetadataStore, UsedMetadataStore}.
905+
case skip_metadata_store_configuration(Config) of
906+
true ->
907+
{undefined, undefined};
908+
false ->
909+
%% We want to verify if the active metadata store matches the
910+
%% expected one.
911+
Nodename = ?config(nodename, NodeConfig),
912+
ExpectedMetadataStore = rabbit_ct_helpers:get_config(
913+
Config, metadata_store),
914+
IsKhepriEnabled = rpc(
915+
Config, Nodename,
916+
rabbit_khepri, is_enabled, []),
917+
UsedMetadataStore = case IsKhepriEnabled of
918+
true -> khepri;
919+
false -> mnesia
920+
end,
921+
ct:pal(
922+
"Metadata store on ~s: expected=~s, used=~s",
923+
[Nodename, ExpectedMetadataStore, UsedMetadataStore]),
924+
{ExpectedMetadataStore, UsedMetadataStore}
925+
end.
931926

932927
maybe_cluster_nodes(Config) ->
933928
Clustered0 = rabbit_ct_helpers:get_config(Config, rmq_nodes_clustered),
@@ -1056,62 +1051,37 @@ configured_metadata_store(Config) ->
10561051
end.
10571052

10581053
configure_metadata_store(Config) ->
1059-
ct:log("Configuring metadata store..."),
1060-
Value = rabbit_ct_helpers:get_app_env(
1061-
Config, rabbit, forced_feature_flags_on_init, undefined),
1062-
MetadataStore = configured_metadata_store(Config),
1063-
Config1 = rabbit_ct_helpers:set_config(
1064-
Config, {metadata_store, MetadataStore}),
1065-
%% To enabled or disable `khepri_db', we use the relative forced feature
1066-
%% flags mechanism. This allows us to select the state of Khepri without
1067-
%% having to worry about other feature flags.
1068-
%%
1069-
%% However, RabbitMQ 4.0.x and older don't support it. See the
1070-
%% `uses_expected_metadata_store/2' check to see how Khepri is enabled in
1071-
%% this case.
1072-
%%
1073-
%% Note that this setting will be ignored by the secondary umbrella because
1074-
%% we set `$RABBITMQ_FEATURE_FLAGS' explisitly. In this case, we handle the
1075-
%% `khepri_db' feature flag when we compute the value of that variable.
1076-
%%
1077-
%% TODO: When we start to do mixed-version testing against 4.1.x as the
1078-
%% secondary umbrella, we will need to stop setting
1079-
%% `$RABBITMQ_FEATURE_FLAGS'.
1080-
case MetadataStore of
1081-
khepri ->
1082-
ct:log("Enabling Khepri metadata store"),
1083-
case Value of
1084-
undefined ->
1085-
rabbit_ct_helpers:merge_app_env(
1086-
Config1,
1087-
{rabbit,
1088-
[{forced_feature_flags_on_init,
1089-
{rel, [khepri_db], []}}]});
1090-
_ ->
1091-
rabbit_ct_helpers:merge_app_env(
1092-
Config1,
1093-
{rabbit,
1094-
[{forced_feature_flags_on_init,
1095-
[khepri_db | Value]}]})
1096-
end;
1097-
mnesia ->
1098-
ct:log("Enabling Mnesia metadata store"),
1099-
case Value of
1100-
undefined ->
1101-
rabbit_ct_helpers:merge_app_env(
1102-
Config1,
1103-
{rabbit,
1104-
[{forced_feature_flags_on_init,
1105-
{rel, [], [khepri_db]}}]});
1106-
_ ->
1107-
rabbit_ct_helpers:merge_app_env(
1108-
Config1,
1109-
{rabbit,
1110-
[{forced_feature_flags_on_init,
1111-
Value -- [khepri_db]}]})
1112-
end
1054+
case skip_metadata_store_configuration(Config) of
1055+
true ->
1056+
ct:log("Skipping metadata store configuration as requested"),
1057+
Config;
1058+
false ->
1059+
ct:log("Configuring metadata store..."),
1060+
MetadataStore = configured_metadata_store(Config),
1061+
Config1 = rabbit_ct_helpers:set_config(
1062+
Config, {metadata_store, MetadataStore}),
1063+
FeatureNames0 = case MetadataStore of
1064+
mnesia ->
1065+
ct:log("Enabling Mnesia metadata store"),
1066+
?REQUIRED_FEATURE_FLAGS;
1067+
khepri ->
1068+
ct:log("Enabling Khepri metadata store"),
1069+
[khepri_db | ?REQUIRED_FEATURE_FLAGS]
1070+
end,
1071+
OtherFeatureNames = rabbit_ct_helpers:get_app_env(
1072+
Config,
1073+
rabbit, forced_feature_flags_on_init, []),
1074+
FeatureNames1 = lists:usort(FeatureNames0 ++ OtherFeatureNames),
1075+
rabbit_ct_helpers:merge_app_env(
1076+
Config1,
1077+
{rabbit, [{forced_feature_flags_on_init, FeatureNames1}]})
11131078
end.
11141079

1080+
skip_metadata_store_configuration(Config) ->
1081+
Skip = rabbit_ct_helpers:get_config(
1082+
Config, skip_metadata_store_configuration),
1083+
Skip =:= true.
1084+
11151085
%% Waits until the metadata store replica on Node is up to date with the leader.
11161086
await_metadata_store_consistent(Config, Node) ->
11171087
case configured_metadata_store(Config) of

deps/rabbitmq_management/test/rabbit_mgmt_http_SUITE.erl

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,17 @@ start_broker(Config) ->
244244
Setup0 = rabbit_ct_broker_helpers:setup_steps(),
245245
Setup1 = rabbit_ct_client_helpers:setup_steps(),
246246
Steps = Setup0 ++ Setup1,
247-
rabbit_ct_helpers:run_setup_steps(Config, Steps).
247+
case rabbit_ct_helpers:run_setup_steps(Config, Steps) of
248+
{skip, _} = Skip ->
249+
Skip;
250+
Config1 ->
251+
Ret = rabbit_ct_broker_helpers:enable_feature_flag(
252+
Config1, 'rabbitmq_4.0.0'),
253+
case Ret of
254+
ok -> Config1;
255+
_ -> Ret
256+
end
257+
end.
248258

249259
finish_init(Group, Config) ->
250260
rabbit_ct_helpers:log_environment(),

0 commit comments

Comments
 (0)