Skip to content

Commit 03620c9

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.
1 parent 415dc81 commit 03620c9

File tree

1 file changed

+15
-66
lines changed

1 file changed

+15
-66
lines changed

deps/rabbitmq_ct_helpers/src/rabbit_ct_broker_helpers.erl

Lines changed: 15 additions & 66 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
@@ -1057,60 +1044,22 @@ configured_metadata_store(Config) ->
10571044

10581045
configure_metadata_store(Config) ->
10591046
ct:log("Configuring metadata store..."),
1060-
Value = rabbit_ct_helpers:get_app_env(
1061-
Config, rabbit, forced_feature_flags_on_init, undefined),
10621047
MetadataStore = configured_metadata_store(Config),
10631048
Config1 = rabbit_ct_helpers:set_config(
10641049
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
1113-
end.
1050+
FeatureNames0 = case MetadataStore of
1051+
mnesia ->
1052+
ct:log("Enabling Mnesia metadata store"),
1053+
?REQUIRED_FEATURE_FLAGS;
1054+
khepri ->
1055+
ct:log("Enabling Khepri metadata store"),
1056+
[khepri_db | ?REQUIRED_FEATURE_FLAGS]
1057+
end,
1058+
OtherFeatureNames = rabbit_ct_helpers:get_app_env(
1059+
Config, rabbit, forced_feature_flags_on_init, []),
1060+
FeatureNames1 = lists:usort(FeatureNames0 ++ OtherFeatureNames),
1061+
rabbit_ct_helpers:merge_app_env(
1062+
Config1, {rabbit, [{forced_feature_flags_on_init, FeatureNames1}]}).
11141063

11151064
%% Waits until the metadata store replica on Node is up to date with the leader.
11161065
await_metadata_store_consistent(Config, Node) ->

0 commit comments

Comments
 (0)