Merge pull request #13077 from rabbitmq/fix-how-RABBITMQ_FEATURE_FLAGS-is-set-in-tests

rabbitmq_ct_helpers: Fix how we set `$RABBITMQ_FEATURE_FLAGS` in tests

Author: dumbbell

deps/rabbit/test/feature_flags_SUITE.erl

@@ -119,7 +119,9 @@ groups() ->
 
 init_per_suite(Config) ->
     rabbit_ct_helpers:log_environment(),
-    rabbit_ct_helpers:run_setup_steps(Config, [
+    Config1 = rabbit_ct_helpers:set_config(
+                Config, {skip_metadata_store_configuration, true}),
+    rabbit_ct_helpers:run_setup_steps(Config1, [
       fun rabbit_ct_broker_helpers:configure_dist_proxy/1
     ]).
 

deps/rabbitmq_ct_helpers/src/rabbit_ct_broker_helpers.erl

@@ -753,17 +753,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),
@@ -793,8 +782,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
@@ -815,8 +803,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
@@ -915,19 +902,27 @@ query_node(Config, NodeConfig) ->
     rabbit_ct_helpers:set_config(NodeConfig, Vars).
 
 uses_expected_metadata_store(Config, NodeConfig) ->
-    %% We want to verify if the active metadata store matches the expected one.
-    Nodename = ?config(nodename, NodeConfig),
-    ExpectedMetadataStore = rabbit_ct_helpers:get_config(
-                              Config, metadata_store),
-    IsKhepriEnabled = rpc(Config, Nodename, rabbit_khepri, is_enabled, []),
-    UsedMetadataStore = case IsKhepriEnabled of
-                            true  -> khepri;
-                            false -> mnesia
-                        end,
-    ct:pal(
-      "Metadata store on ~s: expected=~s, used=~s",
-      [Nodename, UsedMetadataStore, ExpectedMetadataStore]),
-    {ExpectedMetadataStore, UsedMetadataStore}.
+    case skip_metadata_store_configuration(Config) of
+        true ->
+            {undefined, undefined};
+        false ->
+            %% We want to verify if the active metadata store matches the
+            %% expected one.
+            Nodename = ?config(nodename, NodeConfig),
+            ExpectedMetadataStore = rabbit_ct_helpers:get_config(
+                                      Config, metadata_store),
+            IsKhepriEnabled = rpc(
+                                Config, Nodename,
+                                rabbit_khepri, is_enabled, []),
+            UsedMetadataStore = case IsKhepriEnabled of
+                                    true  -> khepri;
+                                    false -> mnesia
+                                end,
+            ct:pal(
+              "Metadata store on ~s: expected=~s, used=~s",
+              [Nodename, ExpectedMetadataStore, UsedMetadataStore]),
+            {ExpectedMetadataStore, UsedMetadataStore}
+    end.
 
 maybe_cluster_nodes(Config) ->
     Clustered0 = rabbit_ct_helpers:get_config(Config, rmq_nodes_clustered),
@@ -1056,62 +1051,37 @@ configured_metadata_store(Config) ->
     end.
 
 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
+    case skip_metadata_store_configuration(Config) of
+        true ->
+            ct:log("Skipping metadata store configuration as requested"),
+            Config;
+        false ->
+            ct:log("Configuring metadata store..."),
+            MetadataStore = configured_metadata_store(Config),
+            Config1 = rabbit_ct_helpers:set_config(
+                        Config, {metadata_store, MetadataStore}),
+            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}]})
     end.
 
+skip_metadata_store_configuration(Config) ->
+    Skip = rabbit_ct_helpers:get_config(
+             Config, skip_metadata_store_configuration),
+    Skip =:= true.
+
 %% Waits until the metadata store replica on Node is up to date with the leader.
 await_metadata_store_consistent(Config, Node) ->
     case configured_metadata_store(Config) of

deps/rabbitmq_management/test/rabbit_mgmt_http_SUITE.erl

@@ -244,7 +244,17 @@ start_broker(Config) ->
     Setup0 = rabbit_ct_broker_helpers:setup_steps(),
     Setup1 = rabbit_ct_client_helpers:setup_steps(),
     Steps = Setup0 ++ Setup1,
-    rabbit_ct_helpers:run_setup_steps(Config, Steps).
+    case rabbit_ct_helpers:run_setup_steps(Config, Steps) of
+        {skip, _} = Skip ->
+            Skip;
+        Config1 ->
+            Ret = rabbit_ct_broker_helpers:enable_feature_flag(
+                    Config1, 'rabbitmq_4.0.0'),
+            case Ret of
+                ok -> Config1;
+                _  -> Ret
+            end
+    end.
 
 finish_init(Group, Config) ->
     rabbit_ct_helpers:log_environment(),