Skip to content

Commit

Permalink
rabbitmq_ct_helpers: Fix how we set $RABBITMQ_FEATURE_FLAGS in tests
Browse files Browse the repository at this point in the history
[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.
  • Loading branch information
dumbbell authored and ansd committed Jan 15, 2025
1 parent e69a4c8 commit 4ac4a64
Showing 1 changed file with 15 additions and 66 deletions.
81 changes: 15 additions & 66 deletions deps/rabbitmq_ct_helpers/src/rabbit_ct_broker_helpers.erl
Original file line number Diff line number Diff line change
Expand Up @@ -762,17 +762,6 @@ do_start_rabbitmq_node(Config, NodeConfig, I) ->
false -> ExtraArgs3;
_ -> ["NOBUILD=1" | ExtraArgs3]
end,
%% TODO: When we start to do mixed-version testing against 4.1.x as the
%% secondary umbrella, we will need to stop setting
%% `$RABBITMQ_FEATURE_FLAGS'.
MetadataStore = rabbit_ct_helpers:get_config(Config, metadata_store),
SecFeatureFlags0 = case MetadataStore of
mnesia -> ?REQUIRED_FEATURE_FLAGS;
khepri -> [khepri_db | ?REQUIRED_FEATURE_FLAGS]
end,
SecFeatureFlags = string:join(
[atom_to_list(F) || F <- SecFeatureFlags0],
","),
ExtraArgs = case UseSecondaryUmbrella of
true ->
DepsDir = ?config(erlang_mk_depsdir, Config),
Expand Down Expand Up @@ -802,8 +791,7 @@ do_start_rabbitmq_node(Config, NodeConfig, I) ->
{"RABBITMQ_SCRIPTS_DIR=~ts", [SecScriptsDir]},
{"RABBITMQ_SERVER=~ts/rabbitmq-server", [SecScriptsDir]},
{"RABBITMQCTL=~ts/rabbitmqctl", [SecScriptsDir]},
{"RABBITMQ_PLUGINS=~ts/rabbitmq-plugins", [SecScriptsDir]},
{"RABBITMQ_FEATURE_FLAGS=~ts", [SecFeatureFlags]}
{"RABBITMQ_PLUGINS=~ts/rabbitmq-plugins", [SecScriptsDir]}
| ExtraArgs4];
false ->
case UseSecondaryDist of
Expand All @@ -824,8 +812,7 @@ do_start_rabbitmq_node(Config, NodeConfig, I) ->
{"CLI_ESCRIPTS_DIR=~ts/escript", [SecondaryDist]},
{"RABBITMQ_SCRIPTS_DIR=~ts/sbin", [SecondaryDist]},
{"RABBITMQ_SERVER=~ts/sbin/rabbitmq-server", [SecondaryDist]},
{"RABBITMQ_ENABLED_PLUGINS=~ts", [SecondaryEnabledPlugins]},
{"RABBITMQ_FEATURE_FLAGS=~ts", [SecFeatureFlags]}
{"RABBITMQ_ENABLED_PLUGINS=~ts", [SecondaryEnabledPlugins]}
| ExtraArgs4];
false ->
ExtraArgs4
Expand Down Expand Up @@ -1066,60 +1053,22 @@ configured_metadata_store(Config) ->

configure_metadata_store(Config) ->
ct:log("Configuring metadata store..."),
Value = rabbit_ct_helpers:get_app_env(
Config, rabbit, forced_feature_flags_on_init, undefined),
MetadataStore = configured_metadata_store(Config),
Config1 = rabbit_ct_helpers:set_config(
Config, {metadata_store, MetadataStore}),
%% To enabled or disable `khepri_db', we use the relative forced feature
%% flags mechanism. This allows us to select the state of Khepri without
%% having to worry about other feature flags.
%%
%% However, RabbitMQ 4.0.x and older don't support it. See the
%% `uses_expected_metadata_store/2' check to see how Khepri is enabled in
%% this case.
%%
%% Note that this setting will be ignored by the secondary umbrella because
%% we set `$RABBITMQ_FEATURE_FLAGS' explisitly. In this case, we handle the
%% `khepri_db' feature flag when we compute the value of that variable.
%%
%% TODO: When we start to do mixed-version testing against 4.1.x as the
%% secondary umbrella, we will need to stop setting
%% `$RABBITMQ_FEATURE_FLAGS'.
case MetadataStore of
khepri ->
ct:log("Enabling Khepri metadata store"),
case Value of
undefined ->
rabbit_ct_helpers:merge_app_env(
Config1,
{rabbit,
[{forced_feature_flags_on_init,
{rel, [khepri_db], []}}]});
_ ->
rabbit_ct_helpers:merge_app_env(
Config1,
{rabbit,
[{forced_feature_flags_on_init,
[khepri_db | Value]}]})
end;
mnesia ->
ct:log("Enabling Mnesia metadata store"),
case Value of
undefined ->
rabbit_ct_helpers:merge_app_env(
Config1,
{rabbit,
[{forced_feature_flags_on_init,
{rel, [], [khepri_db]}}]});
_ ->
rabbit_ct_helpers:merge_app_env(
Config1,
{rabbit,
[{forced_feature_flags_on_init,
Value -- [khepri_db]}]})
end
end.
FeatureNames0 = case MetadataStore of
mnesia ->
ct:log("Enabling Mnesia metadata store"),
?REQUIRED_FEATURE_FLAGS;
khepri ->
ct:log("Enabling Khepri metadata store"),
[khepri_db | ?REQUIRED_FEATURE_FLAGS]
end,
OtherFeatureNames = rabbit_ct_helpers:get_app_env(
Config, rabbit, forced_feature_flags_on_init, []),
FeatureNames1 = lists:usort(FeatureNames0 ++ OtherFeatureNames),
rabbit_ct_helpers:merge_app_env(
Config1, {rabbit, [{forced_feature_flags_on_init, FeatureNames1}]}).

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

0 comments on commit 4ac4a64

Please sign in to comment.