From 9aead56e29e5d660623f6a22a9765e7f8c83965b Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Fri, 26 Jun 2020 04:01:10 +0300 Subject: [PATCH 1/2] Export definitions on boot after all plugins have been enabled This way definitions that depend on a plugin, such as federation upstreams or exchanges of custom types, can be imported successfully. Closes #2384 --- src/rabbit.erl | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/rabbit.erl b/src/rabbit.erl index 83f9e4a02a3b..934e527ae245 100644 --- a/src/rabbit.erl +++ b/src/rabbit.erl @@ -104,17 +104,7 @@ -rabbit_boot_step({definition_import_worker_pool, [{description, "dedicated worker pool for definition import"}, {mfa, {rabbit_definitions, boot, []}}, - {requires, external_infrastructure}, - {enables, load_core_definitions}]}). - -%% We want to A) make sure we apply definitions before the node begins serving -%% traffic and B) in fact do it before empty_db_check (so the defaults will not -%% get created if we don't need 'em). --rabbit_boot_step({load_core_definitions, - [{description, "imports definitions"}, - {mfa, {rabbit_definitions, maybe_load_definitions, []}}, - {requires, [recovery, definition_import_worker_pool]}, - {enables, empty_db_check}]}). + {requires, external_infrastructure}]}). -rabbit_boot_step({external_infrastructure, [{description, "external infrastructure ready"}]}). @@ -935,7 +925,10 @@ do_run_postlaunch_phase() -> ok = rabbit_lager:broker_is_started(), ok = log_broker_started( - rabbit_plugins:strictly_plugins(rabbit_plugins:active())) + rabbit_plugins:strictly_plugins(rabbit_plugins:active())), + %% export definitions after all plugins have been enabled, + %% see rabbitmq/rabbitmq-server#2384 + rabbit_definitions:maybe_load_definitions() catch throw:{error, _} = Error -> rabbit_prelaunch_errors:log_error(Error), From 0523b4433e3fd5f736788abbe1b326ac8714808e Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Fri, 26 Jun 2020 04:18:39 +0300 Subject: [PATCH 2/2] Avoid seeding default data if there are configured definitions to load on boot This is a behavior we use to provide because definitions were imported before the empty DB boot step. Now when the definitions are imported in the post-launch phase, we'd always end up with default data seeded in addition to definitions being imported. References #2384 --- src/rabbit.erl | 11 ++++++++--- src/rabbit_definitions.erl | 25 +++++++++++++++++++------ 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/src/rabbit.erl b/src/rabbit.erl index 934e527ae245..623a91736683 100644 --- a/src/rabbit.erl +++ b/src/rabbit.erl @@ -981,9 +981,14 @@ recover() -> -spec maybe_insert_default_data() -> 'ok'. maybe_insert_default_data() -> - case rabbit_table:needs_default_data() of - true -> insert_default_data(); - false -> ok + NoDefsToImport = not rabbit_definitions:has_configured_definitions_to_load(), + case rabbit_table:needs_default_data() andalso NoDefsToImport of + true -> + rabbit_log:info("Will seed default virtual host and user..."), + insert_default_data(); + false -> + rabbit_log:info("Will not seed default virtual host and user: have definitions to load..."), + ok end. insert_default_data() -> diff --git a/src/rabbit_definitions.erl b/src/rabbit_definitions.erl index 109d6288271d..4f90861fdf9c 100644 --- a/src/rabbit_definitions.erl +++ b/src/rabbit_definitions.erl @@ -19,7 +19,8 @@ -export([boot/0]). %% automatic import on boot --export([maybe_load_definitions/0, maybe_load_definitions/2, maybe_load_definitions_from/2]). +-export([maybe_load_definitions/0, maybe_load_definitions/2, maybe_load_definitions_from/2, + has_configured_definitions_to_load/0]). %% import -export([import_raw/1, import_raw/2, import_parsed/1, import_parsed/2, apply_defs/2, apply_defs/3, apply_defs/4, apply_defs/5]). @@ -66,7 +67,6 @@ boot() -> rabbit_sup:start_supervisor_child(definition_import_pool_sup, worker_pool_sup, [PoolSize, ?IMPORT_WORK_POOL]). maybe_load_definitions() -> - rabbit_log:debug("Will import definitions file from load_definitions"), %% Note that management.load_definitions is handled in the plugin for backwards compatibility. %% This executes the "core" version of load_definitions. maybe_load_definitions(rabbit, load_definitions). @@ -138,11 +138,24 @@ all_definitions() -> %% Implementation %% +-spec has_configured_definitions_to_load() -> boolean(). +has_configured_definitions_to_load() -> + case application:get_env(rabbit, load_definitions) of + undefined -> false; + {ok, none} -> false; + {ok, _Path} -> true + end. + maybe_load_definitions(App, Key) -> case application:get_env(App, Key) of - undefined -> ok; - {ok, none} -> ok; + undefined -> + rabbit_log:debug("No definition file configured to import via load_definitions"), + ok; + {ok, none} -> + rabbit_log:debug("No definition file configured to import via load_definitions"), + ok; {ok, FileOrDir} -> + rabbit_log:debug("Will import definitions file from load_definitions"), IsDir = filelib:is_dir(FileOrDir), maybe_load_definitions_from(IsDir, FileOrDir) end. @@ -172,10 +185,10 @@ load_definitions_from_filenames([File|Rest]) -> load_definitions_from_file(File) -> case file:read_file(File) of {ok, Body} -> - rabbit_log:info("Applying definitions from ~s", [File]), + rabbit_log:info("Applying definitions from file at '~s'", [File]), import_raw(Body); {error, E} -> - rabbit_log:error("Could not read definitions from ~s, Error: ~p", [File, E]), + rabbit_log:error("Could not read definitions from file at '~s', error: ~p", [File, E]), {error, {could_not_read_defs, {File, E}}} end.