Merge pull request #12966 from rabbitmq/take-feature-flag-callback-definition-from-correct-node

rabbit_feature_flags: Take callback definition from correct node

Author: dumbbell

deps/rabbit/src/rabbit_ff_controller.erl

@@ -1840,8 +1840,13 @@ enable_dependencies1(
                 rabbit_deprecated_features:is_feature_used_callback_ret() |
                 run_callback_error()}.
 
-run_callback(Nodes, FeatureName, Command, Extra, Timeout) ->
-    FeatureProps = rabbit_ff_registry_wrapper:get(FeatureName),
+run_callback([FirstNode | _] = Nodes, FeatureName, Command, Extra, Timeout) ->
+    %% We need to take the callback definition from a node in the `Nodes' list
+    %% because the local node may not know this feature flag and thus does not
+    %% have the callback definition.
+    FeatureProps = erpc:call(
+                     FirstNode,
+                     rabbit_ff_registry_wrapper, get, [FeatureName]),
     Callbacks = maps:get(callbacks, FeatureProps, #{}),
     case Callbacks of
         #{Command := {CallbackMod, CallbackFun}}

deps/rabbit/test/feature_flags_SUITE.erl

@@ -45,7 +45,8 @@
          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,
+         enable_plugin_feature_flag_after_deactivating_plugin/1
         ]).
 
 suite() ->
@@ -95,7 +96,8 @@ groups() ->
      {activating_plugin, [],
       [
        activating_plugin_with_new_ff_disabled,
-       activating_plugin_with_new_ff_enabled
+       activating_plugin_with_new_ff_enabled,
+       enable_plugin_feature_flag_after_deactivating_plugin
       ]}
     ],
 
@@ -1260,6 +1262,36 @@ activating_plugin_with_new_ff_enabled(Config) ->
     end,
     ok.
 
+enable_plugin_feature_flag_after_deactivating_plugin(Config) ->
+    FFSubsysOk = is_feature_flag_subsystem_available(Config),
+
+    log_feature_flags_of_all_nodes(Config),
+    case FFSubsysOk of
+        true ->
+            ?assertEqual([false, false],
+                         is_feature_flag_supported(Config, plugin_ff)),
+            ?assertEqual([false, false],
+                         is_feature_flag_enabled(Config, plugin_ff));
+        false ->
+            ok
+    end,
+
+    rabbit_ct_broker_helpers:enable_plugin(Config, 1, "my_plugin"),
+    rabbit_ct_broker_helpers:disable_plugin(Config, 1, "my_plugin"),
+
+    log_feature_flags_of_all_nodes(Config),
+    case FFSubsysOk of
+        true ->
+            enable_feature_flag_on(Config, 0, plugin_ff),
+            ?assertEqual([true, true],
+                         is_feature_flag_supported(Config, plugin_ff)),
+            ?assertEqual([false, true],
+                         is_feature_flag_enabled(Config, plugin_ff));
+        false ->
+            ok
+    end,
+    ok.
+
 %% -------------------------------------------------------------------
 %% Internal helpers.
 %% -------------------------------------------------------------------