From ea2c8db2d1f5dcb8ba6b58bbf4c76c65ea1fa2db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-S=C3=A9bastien=20P=C3=A9dron?= Date: Thu, 19 Dec 2024 12:13:06 +0100 Subject: [PATCH] rabbit_feature_flags: Add testcase after issue #12963 [Why] Up-to RabbitMQ 3.13.x, there was a case where if: 1. you enabled a plugin 2. you enabled its feature flags 3. you disabled the plugin 4. you restarted a node (or upgraded it) ... the node could crash on startup because it had a feature flag marked as enabled that it didn't know about: error:{badmatch,#{feature_flags => ... rabbit_ff_controller:-check_one_way_compatibility/2-fun-0-/3, line 514 lists:all_1/2, line 1520 rabbit_ff_controller:are_compatible/2, line 496 rabbit_ff_controller:check_node_compatibility_task1/4, line 437 rabbit_db_cluster:check_compatibility/1, line 376 This was "fixed" by the new way of keeping the registry in memory (#10988) because it introduces a slight change of behavior. Indeed, the old way walked through the `FeatureFlags` map and looked up the state in the `FeatureStates` map to create the `is_enabled/1` function. The new way just looks up the state in `FeatureStates`. [How] The new testcase succeeds on 4.0.x and `main`, but would fail on 3.13.x with the aforementionne crash. --- deps/rabbit/test/feature_flags_SUITE.erl | 53 +++++++++++++++++++++++- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/deps/rabbit/test/feature_flags_SUITE.erl b/deps/rabbit/test/feature_flags_SUITE.erl index d2fd415d3ff0..205b33ed2848 100644 --- a/deps/rabbit/test/feature_flags_SUITE.erl +++ b/deps/rabbit/test/feature_flags_SUITE.erl @@ -46,7 +46,8 @@ clustering_ok_with_supported_required_ff/1, activating_plugin_with_new_ff_disabled/1, activating_plugin_with_new_ff_enabled/1, - enable_plugin_feature_flag_after_deactivating_plugin/1 + enable_plugin_feature_flag_after_deactivating_plugin/1, + restart_node_with_unknown_enabled_feature_flag/1 ]). suite() -> @@ -97,7 +98,8 @@ groups() -> [ activating_plugin_with_new_ff_disabled, activating_plugin_with_new_ff_enabled, - enable_plugin_feature_flag_after_deactivating_plugin + enable_plugin_feature_flag_after_deactivating_plugin, + restart_node_with_unknown_enabled_feature_flag ]} ], @@ -1292,6 +1294,53 @@ enable_plugin_feature_flag_after_deactivating_plugin(Config) -> end, ok. +restart_node_with_unknown_enabled_feature_flag(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, 0, "my_plugin"), + rabbit_ct_broker_helpers:enable_plugin(Config, 1, "my_plugin"), + rabbit_ct_broker_helpers:disable_plugin(Config, 0, "my_plugin"), + rabbit_ct_broker_helpers:disable_plugin(Config, 1, "my_plugin"), + + enable_feature_flag_on(Config, 0, plugin_ff), + case FFSubsysOk of + true -> + enable_feature_flag_on(Config, 0, plugin_ff), + ?assertEqual([true, true], + is_feature_flag_supported(Config, plugin_ff)), + ?assertEqual([true, true], + is_feature_flag_enabled(Config, plugin_ff)); + false -> + ok + end, + + rabbit_ct_broker_helpers:restart_node(Config, 0), + rabbit_ct_broker_helpers:restart_node(Config, 1), + + log_feature_flags_of_all_nodes(Config), + case FFSubsysOk of + true -> + enable_feature_flag_on(Config, 0, plugin_ff), + ?assertEqual([false, false], + is_feature_flag_supported(Config, plugin_ff)), + ?assertEqual([false, false], + is_feature_flag_enabled(Config, plugin_ff)); + false -> + ok + end, + ok. + %% ------------------------------------------------------------------- %% Internal helpers. %% -------------------------------------------------------------------