Feature flags: Support "auto-enable" feature flagssupport-auto-enable-for-feature-flags

A feature flag can be marked as "auto-enable" by setting `auto_enable` to
true in its properties.

An auto-enable feature flag is automatically enabled as soon as all
nodes in the cluster support it. This is achieved by trying to enable it
when RabbitMQ starts, when a plugin is enabled/disabled or when a node
joins/re-joins a cluster. If the feature flag can't be enabled, the
error is ignored.

An auto-enable feature flag also implicitly depends on
`feature_flags_v2`.

However, it should be used cautiously, especially if the feature flag
has a migration function, because it might be enabled at an
inappropriate time w.r.t. the user's workload.

3 files changed, 140 insertions, 15 deletions

diff --git a/deps/rabbit/src/rabbit.erl b/deps/rabbit/src/rabbit.erl
index 738883a18d..11ebad246b 100644
--- a/deps/rabbit/src/rabbit.erl
+++ b/deps/rabbit/src/rabbit.erl
@@ -871,6 +871,7 @@ start(normal, []) ->
         %% will be used. We start it now because we can't wait for boot steps
         %% to do this (feature flags are refreshed before boot steps run).
         ok = rabbit_sup:start_child(rabbit_ff_controller),
+        rabbit_feature_flags:enable_auto(),
 
         %% Compatibility with older RabbitMQ versions + required by
         %% rabbit_node_monitor:notify_node_up/0:
diff --git a/deps/rabbit/src/rabbit_feature_flags.erl b/deps/rabbit/src/rabbit_feature_flags.erl
index 8dec0b0996..a77d04b9cd 100644
--- a/deps/rabbit/src/rabbit_feature_flags.erl
+++ b/deps/rabbit/src/rabbit_feature_flags.erl
@@ -88,6 +88,7 @@
          enable/1,
          enable_all/0,
          enable_all/1,
+         enable_auto/0,
          disable/1,
          disable_all/0,
          is_supported/1,
@@ -156,6 +157,7 @@
 -type feature_props() :: #{desc => string(),
                            doc_url => string(),
                            stability => stability(),
+                           auto_enable => boolean(),
                            depends_on => [feature_name()],
                            migration_fun => migration_fun_name(),
                            callbacks =>
@@ -170,6 +172,10 @@
 %% <li>`doc_url': a URL pointing to more documentation about the feature
 %%   flag</li>
 %% <li>`stability': the level of stability</li>
+%% <li>`auto_enable': a boolean to indicate if the feature flag should be
+%%   auto-enabled when RabbitMQ starts, a plugin is enabled/disabled or a node
+%%   joins/leaves the cluster. This property is ignored for experimental
+%%   feature flags.</li>
 %% <li>`depends_on': a list of feature flags name which must be enabled
 %%   before this one</li>
 %% <li>`migration_fun': a migration function specified by its module and
@@ -195,6 +201,7 @@
 -type feature_props_extended() :: #{desc => string(),
                                     doc_url => string(),
                                     stability => stability(),
+                                    auto_enable => boolean(),
                                     migration_fun => migration_fun_name(),
                                     callbacks =>
                                     #{callback_name() => migration_fun_name()},
@@ -459,8 +466,11 @@ sort_feature_flags_v2_first(FeatureNames) ->
           (A, B)                -> A =< B
       end, FeatureNames).
 
+requires_feature_flags_v2(feature_flags_v2) ->
+    false;
 requires_feature_flags_v2(FeatureName) ->
-    uses_callbacks(FeatureName).
+    uses_callbacks(FeatureName) orelse
+    is_marked_auto_enable(FeatureName).
 
 uses_callbacks(FeatureName) when is_atom(FeatureName) ->
     case rabbit_ff_registry:get(FeatureName) of
@@ -470,6 +480,14 @@ uses_callbacks(FeatureName) when is_atom(FeatureName) ->
 uses_callbacks(FeatureProps) when is_map(FeatureProps) ->
     maps:is_key(callbacks, FeatureProps).
 
+is_marked_auto_enable(FeatureName) when is_atom(FeatureName) ->
+    case rabbit_ff_registry:get(FeatureName) of
+        undefined    -> false;
+        FeatureProps -> is_marked_auto_enable(FeatureProps)
+    end;
+is_marked_auto_enable(FeatureProps) when is_map(FeatureProps) ->
+    maps:is_key(auto_enable, FeatureProps).
+
 enable_v1(FeatureName) ->
     rabbit_log_feature_flags:debug(
       "Feature flags: `~s`: REQUEST TO ENABLE",
@@ -531,6 +549,42 @@ enable_all(Stability)
   when Stability =:= stable orelse Stability =:= experimental ->
     enable(maps:keys(list(all, Stability))).
 
+-spec enable_auto() -> ok.
+%% @doc
+%% Tries to enables all feature flags marked with `auto_enable`.
+%%
+%% All auto-enable feature flags are tried even if one of them fails. Errors
+%% are ignored by this function.
+%%
+%% @returns `ok'.
+
+enable_auto() ->
+    DisabledFeatureFlags = list(disabled, stable),
+    AutoEnableFeatureNames0 = maps:fold(
+                                fun(FeatureName, FeatureProps, Acc) ->
+                                        case is_auto_enable(FeatureProps) of
+                                            true  -> [FeatureName | Acc];
+                                            false -> Acc
+                                        end
+                                end, [], DisabledFeatureFlags),
+    case AutoEnableFeatureNames0 of
+        [] ->
+            ok;
+        _ ->
+            AutoEnableFeatureNames = lists:sort(AutoEnableFeatureNames0),
+            rabbit_log_feature_flags:info(
+              lists:flatten(
+                ["Feature flags: trying to auto-enable the following feature "
+                 "flags:" |
+                 ["~nFeature flags:   - ~s" || _ <- AutoEnableFeatureNames]]),
+              AutoEnableFeatureNames),
+            _ = lists:foreach(fun enable/1, AutoEnableFeatureNames)
+    end,
+    ok.
+
+is_auto_enable(#{auto_enable := AutoEnable}) -> AutoEnable;
+is_auto_enable(_)                            -> false.
+
 -spec disable(feature_name() | [feature_name()]) -> ok | {error, any()}.
 %% @doc
 %% Disables the specified feature flag or set of feature flags.
@@ -1022,9 +1076,10 @@ assert_feature_flag_is_valid(FeatureName, FeatureProps) ->
         ?assert(is_atom(FeatureName)),
         ?assert(is_map(FeatureProps)),
         Stability = get_stability(FeatureProps),
-        ?assert(Stability =:= stable orelse
-                Stability =:= experimental orelse
-                Stability =:= required),
+        ?assert(Stability =:= required orelse
+                Stability =:= stable orelse
+                Stability =:= experimental),
+        ?assert(is_boolean(is_auto_enable(FeatureProps))),
         case FeatureProps of
             #{migration_fun := _} when Stability =:= required ->
                 rabbit_log_feature_flags:error(
@@ -1825,10 +1880,12 @@ sync_feature_flags_with_cluster(Nodes, NodeIsVirgin) ->
 %% @private
 
 sync_feature_flags_with_cluster(Nodes, NodeIsVirgin, Timeout) ->
-    case is_enabled(feature_flags_v2) of
-        true  -> rabbit_ff_controller:sync_cluster();
-        false -> sync_cluster_v1(Nodes, NodeIsVirgin, Timeout)
-    end.
+    Ret = case is_enabled(feature_flags_v2) of
+              true  -> rabbit_ff_controller:sync_cluster();
+              false -> sync_cluster_v1(Nodes, NodeIsVirgin, Timeout)
+          end,
+    enable_auto(),
+    Ret.
 
 sync_cluster_v1([], NodeIsVirgin, _) ->
     case NodeIsVirgin of
@@ -2177,10 +2234,12 @@ enabled_feature_flags_to_feature_states(FeatureNames) ->
     ok | {error, any()} | no_return().
 
 refresh_feature_flags_after_app_load(Apps) ->
-    case is_enabled(feature_flags_v2) of
-        true  -> rabbit_ff_controller:refresh_after_app_load();
-        false -> refresh_feature_flags_after_app_load_v1(Apps)
-    end.
+    Ret = case is_enabled(feature_flags_v2) of
+              true  -> rabbit_ff_controller:refresh_after_app_load();
+              false -> refresh_feature_flags_after_app_load_v1(Apps)
+          end,
+    enable_auto(),
+    Ret.
 
 refresh_feature_flags_after_app_load_v1([]) ->
     ok;
diff --git a/deps/rabbit/test/feature_flags_SUITE.erl b/deps/rabbit/test/feature_flags_SUITE.erl
index 78fc0697cd..bd1bd34fbb 100644
--- a/deps/rabbit/test/feature_flags_SUITE.erl
+++ b/deps/rabbit/test/feature_flags_SUITE.erl
@@ -38,7 +38,9 @@
          clustering_ok_with_new_ff_enabled_from_plugin_on_some_nodes/1,
          clustering_ok_with_supported_required_ff/1,
          activating_plugin_with_new_ff_disabled/1,
-         activating_plugin_with_new_ff_enabled/1
+         activating_plugin_with_new_ff_enabled/1,
+
+         auto_enable_works_on_startup/1
         ]).
 
 suite() ->
@@ -58,7 +60,8 @@ groups() ->
       [
        enable_feature_flag_in_a_healthy_situation,
        enable_unsupported_feature_flag_in_a_healthy_situation,
-       required_feature_flag_enabled_by_default
+       required_feature_flag_enabled_by_default,
+       auto_enable_works_on_startup
       ]},
      {enabling_in_cluster, [],
       [
@@ -66,7 +69,8 @@ groups() ->
        enable_unsupported_feature_flag_in_a_healthy_situation,
        enable_feature_flag_with_a_network_partition,
        mark_feature_flag_as_enabled_with_a_network_partition,
-       required_feature_flag_enabled_by_default
+       required_feature_flag_enabled_by_default,
+       auto_enable_works_on_startup
       ]},
      {clustering, [],
       [
@@ -1148,6 +1152,67 @@ activating_plugin_with_new_ff_enabled(Config) ->
     end,
     ok.
 
+auto_enable_works_on_startup(Config) ->
+    Nodes = rabbit_ct_broker_helpers:get_node_configs(Config, nodename),
+    NewFeatureFlags = #{brewery =>
+                        #{desc => "Brewery in RabbitMQ",
+                          provided_by => rabbit,
+                          stability => stable,
+                          auto_enable => true}},
+
+    FFSubsysOk = is_feature_flag_subsystem_available(Config),
+
+    log_feature_flags_of_all_nodes(Config),
+    case FFSubsysOk of
+        true ->
+            ?assertEqual([false || _ <- Nodes],
+                         is_feature_flag_supported(Config, brewery));
+        false ->
+            ok
+    end,
+
+    inject_ff_on_nodes(Config, Nodes, NewFeatureFlags),
+
+    log_feature_flags_of_all_nodes(Config),
+    case FFSubsysOk of
+        true ->
+            ?assertEqual([true || _ <- Nodes],
+                         is_feature_flag_supported(Config, brewery)),
+            ?assertEqual([false || _ <- Nodes],
+                         is_feature_flag_enabled(Config, brewery));
+        false ->
+            ok
+    end,
+
+    ?assertEqual(
+       ok,
+       rabbit_ct_broker_helpers:rpc(Config, hd(Nodes), rabbit, stop, [])),
+    ?assertEqual(
+       ok,
+       rabbit_ct_broker_helpers:rpc(Config, hd(Nodes), rabbit, start, [])),
+
+    log_feature_flags_of_all_nodes(Config),
+    %% When testing in a mixed-version cluster, auto-enable will be inactive
+    %% if this is a cluster and not all of them support `feature_flags_v2'.
+    SingleNode = erlang:length(Nodes) =:= 1,
+    SupportFFv2 = lists:all(
+                    fun(SupportFFv2) -> SupportFFv2 end,
+                    rabbit_ct_broker_helpers:rpc(
+                      Config, Nodes,
+                      rabbit_feature_flags, is_supported_locally,
+                      [feature_flags_v2])),
+    ExpectEnabled = SingleNode orelse SupportFFv2,
+    case FFSubsysOk of
+        true ->
+            ?assertEqual([true || _ <- Nodes],
+                         is_feature_flag_supported(Config, brewery)),
+            ?assertEqual([ExpectEnabled || _ <- Nodes],
+                         is_feature_flag_enabled(Config, brewery));
+        false ->
+            ok
+    end,
+    ok.
+
 %% -------------------------------------------------------------------
 %% Internal helpers.
 %% -------------------------------------------------------------------