Feature flags: Sync enabled feature flags differently on virgin node

deps/rabbit/src/rabbit_feature_flags.erl

@@ -1872,7 +1872,59 @@ sync_cluster_v1([], NodeIsVirgin, _) ->
               "current state"),
             ok
     end;
-sync_cluster_v1(Nodes, _, Timeout) ->
+sync_cluster_v1(Nodes, true = _NodeIsVirgin, Timeout) ->
+    [Node | _] = Nodes,
+    rabbit_log_feature_flags:debug(
+      "Feature flags: marking feature flags required remotely as enabled "
+      "locally because this node is virgin; using ~s as the remote node",
+     [Node]),
+    ?assertNotEqual(node(), Node),
+    Ret1 = rpc:call(Node, ?MODULE, list, [enabled], Timeout),
+    case Ret1 of
+        FeatureFlags when is_map(FeatureFlags) ->
+            %% We only consider required feature flags (for which there is no
+            %% migration code on nodes where it is required). Remotely enabled
+            %% feature flags will be handled by the regular sync.
+            RequiredFeatureFlags =
+            maps:filter(
+              fun(_, FeatureProps) ->
+                      required =:= get_stability(FeatureProps)
+              end, FeatureFlags),
+            FeatureNames = lists:sort(maps:keys(RequiredFeatureFlags)),
+            rabbit_log_feature_flags:error(
+              "Feature flags: list of feature flags to automatically mark as "
+              "enabled on this virgin node: ~0p",
+              [FeatureNames]),
+
+            %% We mark them as enabled directly (no migration code is executed)
+            %% because this node is virgin (i.e. no files on disk) and will
+            %% inherit the database from remote nodes.
+            Ret2 =
+            lists:foldl(
+              fun
+                  (FeatureName, ok) ->
+                      case is_supported_locally(FeatureName) of
+                          true  -> mark_as_enabled_locally(FeatureName, true);
+                          false -> ok
+                      end;
+                  (_, Acc) ->
+                      Acc
+              end, ok, FeatureNames),
+
+            %% We continue with the regular sync process. It will handle other
+            %% enabled feature flags and unsupported ones too.
+            case Ret2 of
+                ok -> sync_feature_flags_with_cluster(Nodes, false, Timeout);
+                _  -> Ret2
+            end;
+        {badrpc, _} = Reason ->
+            rabbit_log_feature_flags:error(
+              "Feature flags: failed to query remotely enabled feature flags "
+              "on node ~s: ~p",
+              [Node, Reason]),
+            {error, Reason}
+    end;
+sync_cluster_v1(Nodes, _NodeIsVirgin, Timeout) ->
     case sync_feature_flags_v2_first(Nodes, Timeout) of
         true ->
             ?LOG_DEBUG(

deps/rabbit/src/rabbit_ff_controller.erl

@@ -577,7 +577,7 @@ enable_with_registry_locked(
                [FeatureName],
                #{domain => ?RMQLOG_DOMAIN_FEAT_FLAGS}),
 
-            case update_feature_state_and_enable(Inventory, FeatureName) of
+            case check_required_and_enable(Inventory, FeatureName) of
                 {ok, _Inventory} = Ok ->
                     ?LOG_NOTICE(
                        "Feature flags: `~ts` enabled",
@@ -593,6 +593,71 @@ enable_with_registry_locked(
             end
     end.
 
+-spec check_required_and_enable(Inventory, FeatureName) -> Ret when
+      Inventory :: rabbit_feature_flags:cluster_inventory(),
+      FeatureName :: rabbit_feature_flags:feature_name(),
+      Ret :: {ok, Inventory} | {error, Reason},
+      Reason :: term().
+
+check_required_and_enable(
+  #{feature_flags := FeatureFlags,
+    states_per_node := _} = Inventory,
+  FeatureName) ->
+    %% Required feature flags vs. virgin nodes.
+    FeatureProps = maps:get(FeatureName, FeatureFlags),
+    Stability = rabbit_feature_flags:get_stability(FeatureProps),
+    NodesWhereDisabled = list_nodes_where_feature_flag_is_disabled(
+                           Inventory, FeatureName),
+
+    MarkDirectly = case Stability of
+                       required ->
+                           ?LOG_DEBUG(
+                              "Feature flags: `~s`: the feature flag is "
+                              "required on some nodes; list virgin nodes "
+                              "to determine if the feature flag can simply "
+                              "be marked as enabled",
+                              [FeatureName],
+                              #{domain => ?RMQLOG_DOMAIN_FEAT_FLAGS}),
+                           VirginNodesWhereDisabled =
+                           lists:filter(
+                             fun(Node) ->
+                                     case is_virgin_node(Node) of
+                                         IsVirgin when is_boolean(IsVirgin) ->
+                                             IsVirgin;
+                                         undefined ->
+                                             false
+                                     end
+                             end, NodesWhereDisabled),
+                           VirginNodesWhereDisabled =:= NodesWhereDisabled;
+                       _ ->
+                           false
+                   end,
+
+    case MarkDirectly of
+        false ->
+            case Stability of
+                required ->
+                    ?LOG_DEBUG(
+                       "Feature flags: `~s`: some nodes where the feature "
+                       "flag is disabled are not virgin, we need to perform "
+                       "a regular sync",
+                       [FeatureName],
+                       #{domain => ?RMQLOG_DOMAIN_FEAT_FLAGS});
+                _ ->
+                    ok
+            end,
+            update_feature_state_and_enable(Inventory, FeatureName);
+        true ->
+            ?LOG_DEBUG(
+               "Feature flags: `~s`: all nodes where the feature flag is "
+               "disabled are virgin, we can directly mark it as enabled "
+               "there",
+               [FeatureName],
+               #{domain => ?RMQLOG_DOMAIN_FEAT_FLAGS}),
+            mark_as_enabled_on_nodes(
+              NodesWhereDisabled, Inventory, FeatureName, true)
+    end.
+
 -spec update_feature_state_and_enable(Inventory, FeatureName) -> Ret when
       Inventory :: rabbit_feature_flags:cluster_inventory(),
       FeatureName :: rabbit_feature_flags:feature_name(),
@@ -646,6 +711,14 @@ update_feature_state_and_enable(
             Error
     end.
 
+is_virgin_node(Node) ->
+    case rpc_call(Node, rabbit_mnesia, is_virgin_node, [], ?TIMEOUT) of
+        IsVirgin when is_boolean(IsVirgin) ->
+            IsVirgin;
+        {error, _} ->
+            undefined
+    end.
+
 -spec do_enable(Inventory, FeatureName, Nodes) -> Ret when
       Inventory :: rabbit_feature_flags:cluster_inventory(),
       FeatureName :: rabbit_feature_flags:feature_name(),
@@ -740,7 +813,7 @@ collect_inventory_on_nodes(Nodes, Timeout) ->
            {ok, #{feature_flags := FeatureFlags,
                   applications_per_node := ScannedAppsPerNode,
                   states_per_node := StatesPerNode} = Inventory}) ->
-            FeatureFlags2 = maps:merge(FeatureFlags, FeatureFlags1),
+            FeatureFlags2 = merge_feature_flags(FeatureFlags, FeatureFlags1),
             ScannedAppsPerNode1 = ScannedAppsPerNode#{Node => ScannedApps},
             StatesPerNode1 = StatesPerNode#{Node => FeatureStates},
             Inventory1 = Inventory#{
@@ -756,6 +829,32 @@ collect_inventory_on_nodes(Nodes, Timeout) ->
               Error
       end, {ok, Inventory0}, Rets).
 
+merge_feature_flags(FeatureFlagsA, FeatureFlagsB) ->
+    FeatureFlags = maps:merge(FeatureFlagsA, FeatureFlagsB),
+    maps:map(
+      fun(FeatureName, FeatureProps) ->
+              FeaturePropsA = maps:get(FeatureName, FeatureFlagsA, #{}),
+              FeaturePropsB = maps:get(FeatureName, FeatureFlagsB, #{}),
+
+              %% There is a rank between stability levels. If a feature flag
+              %% is required somewhere, it is required globally. Otherwise if
+              %% it is stable somewhere, it is stable globally.
+              StabilityA = rabbit_feature_flags:get_stability(FeaturePropsA),
+              StabilityB = rabbit_feature_flags:get_stability(FeaturePropsB),
+              Stability = case {StabilityA, StabilityB} of
+                              {required, _} -> required;
+                              {_, required} -> required;
+                              {stable, _}   -> stable;
+                              {_, stable}   -> stable;
+                              _             -> experimental
+                          end,
+
+              FeatureProps1 = FeatureProps#{stability => Stability},
+              FeatureProps2 = maps:remove(migration_fun, FeatureProps1),
+              FeatureProps3 = maps:remove(callbacks, FeatureProps2),
+              FeatureProps3
+      end, FeatureFlags).
+
 -spec list_feature_flags_enabled_somewhere(Inventory, HandleStateChanging) ->
     Ret when
       Inventory :: rabbit_feature_flags:cluster_inventory(),

deps/rabbit/src/rabbit_mnesia.erl

@@ -25,6 +25,7 @@
          is_registered_process_alive/1,
          cluster_nodes/1,
          node_type/0,
+         is_virgin_node/0,
          dir/0,
          cluster_status_from_mnesia/0,