From ca88bdef12f1ecb2a28c5ff2aef51c01e264ddee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Fri, 1 Apr 2022 13:49:15 +0200 Subject: [PATCH 01/10] Process services in mongoose_config_parser Unify service and module processing: - Resolve deps - Unfold opts (this is temporary until only maps are used) Also: move 'build_state' to a more suitable place. --- src/config/mongoose_config_parser.erl | 44 ++++++++++++++++------ src/config/mongoose_config_parser_toml.erl | 13 +------ src/mongoose_modules.erl | 8 +--- 3 files changed, 35 insertions(+), 30 deletions(-) diff --git a/src/config/mongoose_config_parser.erl b/src/config/mongoose_config_parser.erl index ad698974cf1..608066e217c 100644 --- a/src/config/mongoose_config_parser.erl +++ b/src/config/mongoose_config_parser.erl @@ -7,15 +7,10 @@ -export([parse_file/1]). %% state API --export([new_state/0, - set_opts/2, - set_hosts/2, - set_host_types/2, - get_opts/1]). +-export([build_state/3, get_opts/1]). %% config post-processing --export([unfold_globals/1, - post_process_modules/1]). +-export([unfold_opts/1]). -callback parse_file(FileName :: string()) -> state(). @@ -48,6 +43,17 @@ parser_module(".toml") -> mongoose_config_parser_toml. %% State API +-spec build_state([jid:server()], [jid:server()], opts()) -> state(). +build_state(Hosts, HostTypes, Opts) -> + lists:foldl(fun(F, StateIn) -> F(StateIn) end, + new_state(), + [fun(S) -> set_hosts(Hosts, S) end, + fun(S) -> set_host_types(HostTypes, S) end, + fun(S) -> set_opts(Opts, S) end, + fun unfold_globals/1, + fun post_process_services/1, + fun post_process_modules/1]). + -spec new_state() -> state(). new_state() -> #state{}. @@ -125,19 +131,33 @@ keep_global_value(acl) -> true; keep_global_value(access) -> true; keep_global_value(_) -> false. +-spec post_process_services(state()) -> state(). +post_process_services(State = #state{opts = Opts}) -> + Opts1 = lists:map(fun post_process_services_opt/1, Opts), + State#state{opts = Opts1}. + +post_process_services_opt({services, Services}) -> + ServicesWithDeps = mongoose_service_deps:resolve_deps(Services), + {services, unfold_all_opts(ServicesWithDeps)}; +post_process_services_opt(Other) -> + Other. + -spec post_process_modules(state()) -> state(). post_process_modules(State = #state{opts = Opts}) -> - Opts2 = lists:map(fun post_process_modules_opt/1, Opts), - State#state{opts = Opts2}. + Opts1 = lists:map(fun post_process_modules_opt/1, Opts), + State#state{opts = Opts1}. post_process_modules_opt({{modules, HostType}, Modules}) -> ModulesWithDeps = gen_mod_deps:resolve_deps(HostType, Modules), - {{modules, HostType}, unfold_opts(ModulesWithDeps)}; + {{modules, HostType}, unfold_all_opts(ModulesWithDeps)}; post_process_modules_opt(Other) -> Other. -unfold_opts(Modules) -> - maps:map(fun(_Mod, Opts) -> mongoose_modules:unfold_opts(Opts) end, Modules). +unfold_all_opts(Modules) -> + maps:map(fun(_Mod, Opts) -> unfold_opts(Opts) end, Modules). + +unfold_opts(Opts) when is_map(Opts) -> Opts; +unfold_opts(Opts) -> proplists:unfold(Opts). %% local functions diff --git a/src/config/mongoose_config_parser_toml.erl b/src/config/mongoose_config_parser_toml.erl index dbf61f7a407..544a889b5a6 100644 --- a/src/config/mongoose_config_parser_toml.erl +++ b/src/config/mongoose_config_parser_toml.erl @@ -69,7 +69,7 @@ process(Content) -> HostTypes = proplists:get_value(host_types, Config, []), case extract_errors(Config) of [] -> - build_state(Hosts, HostTypes, Config); + mongoose_config_parser:build_state(Hosts, HostTypes, Config); Errors -> error(config_error(Errors)) end. @@ -305,17 +305,6 @@ item_key(_, _) -> item. %% Processing of the parsed options --spec build_state([jid:server()], [jid:server()], [top_level_config()]) -> - mongoose_config_parser:state(). -build_state(Hosts, HostTypes, Opts) -> - lists:foldl(fun(F, StateIn) -> F(StateIn) end, - mongoose_config_parser:new_state(), - [fun(S) -> mongoose_config_parser:set_hosts(Hosts, S) end, - fun(S) -> mongoose_config_parser:set_host_types(HostTypes, S) end, - fun(S) -> mongoose_config_parser:set_opts(Opts, S) end, - fun mongoose_config_parser:unfold_globals/1, - fun mongoose_config_parser:post_process_modules/1]). - %% Any nested config_part() may be a config_error() - this function extracts them all recursively -spec extract_errors([config()]) -> [config_error()]. extract_errors(Config) -> diff --git a/src/mongoose_modules.erl b/src/mongoose_modules.erl index c928b099924..be1a4a7a3e6 100644 --- a/src/mongoose_modules.erl +++ b/src/mongoose_modules.erl @@ -3,7 +3,7 @@ -include("mongoose.hrl"). %% API --export([start/0, stop/0, unfold_opts/1]). +-export([start/0, stop/0]). %% Module management utilities for tests -export([replace_modules/3, ensure_stopped/2, ensure_started/3]). @@ -68,7 +68,7 @@ ensure_stopped(HostType, Module) -> -spec ensure_started(mongooseim:host_type(), module(), module_opts()) -> already_started | {started, term()} | {restarted, module_opts(), term()}. ensure_started(HostType, Module, RawOpts) -> - Opts = unfold_opts(RawOpts), + Opts = mongoose_config_parser:unfold_opts(RawOpts), Modules = get_modules(HostType), case maps:find(Module, Modules) of error -> @@ -82,10 +82,6 @@ ensure_started(HostType, Module, RawOpts) -> {restarted, PrevOpts, Result} end. --spec unfold_opts(gen_mod:module_opts()) -> gen_mod:module_opts(). -unfold_opts(Opts) when is_map(Opts) -> Opts; -unfold_opts(Opts) -> proplists:unfold(Opts). - %% Helpers -spec start_module(mongooseim:host_type(), module(), module_opts(), module_map()) -> {ok, term()}. From 6db24a8d4459045128ffa8f69f8d82da552ebb56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Fri, 1 Apr 2022 13:52:23 +0200 Subject: [PATCH 02/10] Put services in a map that is always included --- src/config/mongoose_config_spec.erl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/config/mongoose_config_spec.erl b/src/config/mongoose_config_spec.erl index 9ba12e67fb3..db8e165d09c 100644 --- a/src/config/mongoose_config_spec.erl +++ b/src/config/mongoose_config_spec.erl @@ -779,7 +779,9 @@ services() -> || Service <- configurable_services()], #section{ items = maps:from_list(Services), - wrap = global_config + format_items = map, + wrap = global_config, + include = always }. configurable_services() -> From 94fb8646c4ee2bd51fe06973ea879a6df0434f8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Fri, 1 Apr 2022 13:57:19 +0200 Subject: [PATCH 03/10] Rework service configuration and management Unify the way services and modules are management Follow the solution for modules: - Store the config in a map and do not copy the config to ETS. This prevents inconsistency between the two and speeds up access. - Delegate dependency management to a separate module. - Start and stop all services in the dependency-related order. - Provide API for ensuring a service is stopped/started in tests, updating the configuration accordingly. - Drop the check if MongooseIM is running, because it does not seem to be a real risk. Anyway, service management code is not special and should not take the responsibility of stopping the whole node. --- src/mongoose_service.erl | 240 +++++++++++++++++---------------------- 1 file changed, 105 insertions(+), 135 deletions(-) diff --git a/src/mongoose_service.erl b/src/mongoose_service.erl index 45ad54beb3a..6c90728b85c 100644 --- a/src/mongoose_service.erl +++ b/src/mongoose_service.erl @@ -18,195 +18,165 @@ -include("mongoose.hrl"). --export([start/0, stop/0, - start_service/2, - stop_service/1, +%% API +-export([start/0, + stop/0, config_spec/1, + get_deps/1, is_loaded/1, assert_loaded/1, - ensure_loaded/1, - ensure_loaded/2, - purge_service/1, - get_service_opts/1, - loaded_services_with_opts/0 - ]). + get_service_opts/1]). --export([check_deps/1]). % for testing +%% Shell utilities +-export([loaded_services_with_opts/0]). --ignore_xref([behaviour_info/1, check_deps/1, ensure_loaded/1, purge_service/1, - get_service_opts/1, start_service/2, stop/0]). +%% Service management utilities for tests +-export([ensure_stopped/1, ensure_started/2]). -%%Question marks: -%%do we need the 'keep config' facility? -%%does anybody use the 'wait' option in stopping gen_mod? +-ignore_xref([loaded_services_with_opts/0, ensure_stopped/1, ensure_started/2]). --define(ETAB, mongoose_services). +-type service() :: module(). +-type opt_key() :: atom(). +-type opt_value() :: mongoose_config:value(). +-type options() :: [{opt_key(), opt_value()}] % deprecated, will be removed + | #{opt_key() => opt_value()}. % recommended +-type start_result() :: any(). +-type service_list() :: [{service(), options()}]. +-type service_map() :: #{service() => options()}. --type service() :: atom(). --type options() :: [term()]. +-export_type([service/0, service_list/0, service_map/0, options/0]). --export_type([service/0]). - --callback start(Opts :: list()) -> any(). +-callback start(options()) -> start_result(). -callback stop() -> any(). -callback config_spec() -> mongoose_config_spec:config_section(). -%%optional: -%%-callback deps() -> [service()]. +-callback deps() -> [service()]. + +-optional_callbacks([deps/0]). -spec start() -> ok. start() -> - ets:new(?ETAB, [named_table, public, {read_concurrency, true}]), + [start_service(Service, Opts) || {Service, Opts} <- sorted_services()], ok. -spec stop() -> ok. -stop() -> catch ets:delete(?ETAB), ok. - --spec start_service(service(), options() | undefined) -> ok | {error, already_started}. -start_service(Service, undefined) -> - error({service_not_configured, Service}); -start_service(Service, Opts) when is_list(Opts) -> - case is_loaded(Service) of - true -> {error, already_started}; - false -> run_start_service(Service, Opts) - end. - --spec stop_service(service()) -> ok | {error, not_running}. -stop_service(Service) -> - case is_loaded(Service) of - false -> {error, not_running}; - true -> run_stop_service(Service) - end. +stop() -> + [stop_service(Service) || {Service, _Opts} <- lists:reverse(sorted_services())], + ok. -spec config_spec(service()) -> mongoose_config_spec:config_section(). config_spec(Service) -> Service:config_spec(). --spec ensure_loaded(service()) -> ok. -ensure_loaded(Service) -> - Options = mongoose_config:get_opt(services, []), - start_service(Service, proplists:get_value(Service, Options)), - ok. - --spec ensure_loaded(service(), options()) -> ok. -ensure_loaded(Service, Opts) -> - start_service(Service, Opts), - ok. - --spec assert_loaded(service()) -> ok. -assert_loaded(Service) -> - case is_loaded(Service) of - true -> - ok; - false -> - error({service_not_loaded, Service}) +-spec get_deps(service()) -> [service()]. +get_deps(Service) -> + case backend_module:is_exported(Service, deps, 0) of + true -> Service:deps(); + false -> [] end. --spec is_loaded(service()) -> boolean(). -is_loaded(Service) -> - ets:member(?ETAB, Service). - --spec get_service_opts(service()) -> options(). -get_service_opts(Service) -> - case ets:lookup(?ETAB, Service) of - [] -> []; - [{Service, Opts}] -> Opts +-spec sorted_services() -> service_list(). +sorted_services() -> + mongoose_service_deps:sort_deps(loaded_services_with_opts()). + +-spec set_services(service_map()) -> ok. +set_services(Services) -> + mongoose_config:set_opt(services, Services). + +-spec ensure_stopped(service()) -> {stopped, options()} | already_stopped. +ensure_stopped(Service) -> + case loaded_services_with_opts() of + #{Service := Opts} = Services -> + stop_service(Service, Services), + {stopped, Opts}; + _Services -> + already_stopped end. --spec loaded_services_with_opts() -> [{service(), options()}]. -loaded_services_with_opts() -> - ets:tab2list(?ETAB). +-spec ensure_started(service(), options()) -> + {started, start_result()} | {restarted, options(), start_result()} | already_started. +ensure_started(Service, Opts) -> + case loaded_services_with_opts() of + #{Service := Opts} -> + already_started; + #{Service := PrevOpts} = Services -> + stop_service(Service, Services), + {ok, Result} = start_service(Service, Opts, Services), + {restarted, PrevOpts, Result}; + Services -> + {ok, Result} = start_service(Service, Opts, Services), + {started, Result} + end. -%% @doc to be used as an emergency feature if serviced crashed while stopping and is not -%% running but still lingers in the services tab --spec purge_service(service()) -> ok. -purge_service(Service) -> - ets:delete(?ETAB, Service), - ok. +-spec start_service(service(), options(), service_map()) -> {ok, start_result()}. +start_service(Service, Opts, Services) -> + set_services(Services#{Service => Opts}), + try + start_service(Service, Opts) + catch + C:R:S -> + set_services(Services), + erlang:raise(C, R, S) + end. -%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% +-spec stop_service(service(), service_map()) -> ok. +stop_service(Service, Services) -> + stop_service(Service), + set_services(maps:remove(Service, Services)). -run_start_service(Service, Opts0) -> - start_deps(Service), - Opts = proplists:unfold(Opts0), - ets:insert(?ETAB, {Service, Opts}), +start_service(Service, Opts) -> + assert_loaded(Service), try Res = Service:start(Opts), - ?LOG_INFO(#{what => service_startup_started, service => Service, + ?LOG_INFO(#{what => service_started, service => Service, text => <<"Started MongooseIM service">>}), case Res of {ok, _} -> Res; _ -> {ok, Res} end - catch - Class:Reason:Stacktrace -> - ets:delete(?ETAB, Service), + catch Class:Reason:Stacktrace -> ?LOG_CRITICAL(#{what => service_startup_failed, text => <<"Failed to start MongooseIM service">>, service => Service, opts => Opts, class => Class, reason => Reason, stacktrace => Stacktrace}), - case is_app_running(mongooseim) of - true -> - erlang:raise(Class, Reason, Stacktrace); - false -> - ?LOG_CRITICAL(#{what => stopping_mongooseim, - text => <<"mongooseim initialization was aborted " - "because a service start failed.">>, - service => Service, opts => Opts, - class => Class, reason => Reason, stacktrace => Stacktrace}), - timer:sleep(3000), - ErrorText = io_lib:format("Stopping MongooseIM because of bad service~n" - "service=~p ~nreason=~0p ~nstactrace=~0p", - [Service, Reason, Stacktrace]), - erlang:halt(string:substr(lists:flatten(ErrorText), 1, 199)) - end + erlang:raise(Class, Reason, Stacktrace) end. -run_stop_service(Service) -> +stop_service(Service) -> + assert_loaded(Service), try Service:stop() of _ -> ?LOG_INFO(#{what => service_stopped, service => Service, text => <<"Stopped MongooseIM service">>}), - ets:delete(?ETAB, Service), ok catch Class:Reason:Stacktrace -> - ets:delete(?ETAB, Service), ?LOG_ERROR(#{what => service_stop_failed, service => Service, text => <<"Failed to stop MongooseIM service">>, - class => Class, reason => Reason, stacktrace => Stacktrace}) + class => Class, reason => Reason, stacktrace => Stacktrace}), + erlang:raise(Class, Reason, Stacktrace) end. --spec is_app_running(_) -> boolean(). -is_app_running(AppName) -> - %% Use a high timeout to prevent a false positive in a high load system - Timeout = 15000, - lists:keymember(AppName, 1, application:which_applications(Timeout)). - --spec start_deps(service()) -> ok. -start_deps(Service) -> - check_deps(Service), % make sure there are no circular deps - lists:map(fun ensure_loaded/1, get_deps(Service)), - ok. - -check_deps(Service) -> - check_deps(Service, []). - -check_deps(Service, Stack) -> - case lists:member(Service, Stack) of +-spec assert_loaded(service()) -> ok. +assert_loaded(Service) -> + case is_loaded(Service) of true -> - error({circular_deps_detected, Service}); + ok; false -> - lists:foreach(fun(Serv) -> check_deps(Serv, [Service | Stack]) end, - get_deps(Service)) + error(#{what => service_not_loaded, + text => <<"Service missing from mongoose_config">>, + service => Service}) end. --spec get_deps(service()) -> [service()]. -get_deps(Service) -> - %% the module has to be loaded, - %% otherwise the erlang:function_exported/3 returns false - code:ensure_loaded(Service), - case erlang:function_exported(Service, deps, 0) of - true -> - Service:deps(); - _ -> - [] +-spec is_loaded(service()) -> boolean(). +is_loaded(Service) -> + case mongoose_config:lookup_opt([services, Service]) of + {ok, _Opts} -> true; + {error, not_found} -> false end. + +-spec get_service_opts(service()) -> options(). +get_service_opts(Service) -> + mongoose_config:get_opt([services, Service], []). + +-spec loaded_services_with_opts() -> service_map(). +loaded_services_with_opts() -> + mongoose_config:get_opt(services). From 5d6163e7202b914a50c661566a661fe14af96c23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Fri, 1 Apr 2022 14:04:55 +0200 Subject: [PATCH 04/10] Create a module responsible for service dependency management Follow the logic that is already present for modules. There are two main steps: 1. Dependency resolution. 2. Service sorting according to the dependency graph. Only maps are supported, but it is not a problem, because no services have deps anyway and all of them are going to use maps very soon. --- src/mongoose_service_deps.erl | 61 +++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 src/mongoose_service_deps.erl diff --git a/src/mongoose_service_deps.erl b/src/mongoose_service_deps.erl new file mode 100644 index 00000000000..8d2de494f85 --- /dev/null +++ b/src/mongoose_service_deps.erl @@ -0,0 +1,61 @@ +%% @doc Service dependency management + +-module(mongoose_service_deps). + +-export([resolve_deps/1, sort_deps/1]). + +-type service_list() :: mongoose_service:service_list(). +-type service_map() :: mongoose_service:service_map(). + +%% @doc Add dependencies to the service map +-spec resolve_deps(service_map()) -> service_map(). +resolve_deps(Services) when is_map(Services) -> + resolve_deps(maps:to_list(Services), #{}). + +-spec resolve_deps(service_list(), service_map()) -> service_map(). +resolve_deps([], Acc) -> + Acc; +resolve_deps([{Service, Opts} | Rest], Acc) -> + {DepsToResolve, NewAcc} = + case Acc of + #{Service := PreviousOpts} -> + {[], Acc#{Service => merge_opts(PreviousOpts, Opts)}}; + #{} -> + Deps = [{Dep, #{}} || Dep <- mongoose_service:get_deps(Service)], + {Deps, Acc#{Service => Opts}} + end, + resolve_deps(DepsToResolve ++ Rest, NewAcc). + +%% Deps always have no options, so one argument has to be an empty map +merge_opts(#{}, Opts) -> Opts; +merge_opts(Opts, #{}) -> Opts. + +%% @doc Sort services (which already include their dependencies) according to the dependency graph +-spec sort_deps(service_map()) -> service_list(). +sort_deps(Services) when is_map(Services) -> + DepsGraph = digraph:new([acyclic, private]), + try + [add_service_with_deps(Service, DepsGraph) || Service <- maps:keys(Services)], + [{Service, maps:get(Service, Services)} || Service <- digraph_utils:topsort(DepsGraph)] + after + digraph:delete(DepsGraph) + end. + +add_service_with_deps(Service, DepsGraph) -> + digraph:add_vertex(DepsGraph, Service), + lists:foreach(fun(DepService) -> add_service_dep(Service, DepService, DepsGraph) end, + mongoose_service:get_deps(Service)). + +add_service_dep(Service, DepService, DepsGraph) -> + digraph:add_vertex(DepsGraph, DepService), + case digraph:add_edge(DepsGraph, DepService, Service) of + {error, Error} -> + {bad_edge, CyclePath} = Error, + error(#{what => resolving_dependencies_aborted, + text => <<"Service dependency cycle found">>, + service => Service, + dep_service => DepService, + cycle_path => CyclePath}); + _Edge -> + ok + end. From aa7b94ba32e079a051bbd13646efa621085322e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Fri, 1 Apr 2022 13:56:19 +0200 Subject: [PATCH 05/10] Start and stop services using the new API --- src/ejabberd_app.erl | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/src/ejabberd_app.erl b/src/ejabberd_app.erl index 9e1b2bf97b3..44dca969625 100644 --- a/src/ejabberd_app.erl +++ b/src/ejabberd_app.erl @@ -51,7 +51,6 @@ start(normal, _Args) -> ejabberd_ctl:init(), ejabberd_commands:init(), mongoose_commands:init(), - mongoose_service:start(), mongoose_config:start(), mongoose_router:start(), mongoose_logs:set_global_loglevel(mongoose_config:get_opt(loglevel)), @@ -65,7 +64,7 @@ start(normal, _Args) -> ejabberd_sm:start(), ejabberd_auth:start(), mongoose_cluster_id:start(), - start_services(), + mongoose_service:start(), mongoose_modules:start(), service_mongoose_system_metrics:verify_if_configured(), mongoose_metrics:init(), @@ -84,7 +83,7 @@ prep_stop(State) -> mongoose_deprecations:stop(), ejabberd_listener:stop_listeners(), mongoose_modules:stop(), - stop_services(), + mongoose_service:stop(), broadcast_c2s_shutdown(), mongoose_wpool:stop(), mongoose_metrics:remove_all_metrics(), @@ -114,20 +113,6 @@ db_init() -> end, mnesia:wait_for_tables(mnesia:system_info(local_tables), infinity). --spec start_services() -> ok. -start_services() -> - lists:foreach( - fun({Service, Opts}) -> mongoose_service:ensure_loaded(Service, Opts) end, - mongoose_config:get_opt(services, []) - ). - --spec stop_services() -> ok. -stop_services() -> - lists:foreach( - fun({Service, _Options}) -> mongoose_service:stop_service(Service) end, - mongoose_service:loaded_services_with_opts() - ). - -spec broadcast_c2s_shutdown() -> 'ok'. broadcast_c2s_shutdown() -> Children = supervisor:which_children(ejabberd_c2s_sup), From 3e05a483f8e68117731857c307947557f608827a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Fri, 1 Apr 2022 14:14:21 +0200 Subject: [PATCH 06/10] Use the new mongoose_service API in system metrics --- src/system_metrics/service_mongoose_system_metrics.erl | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/system_metrics/service_mongoose_system_metrics.erl b/src/system_metrics/service_mongoose_system_metrics.erl index 3019e550572..a381783e52b 100644 --- a/src/system_metrics/service_mongoose_system_metrics.erl +++ b/src/system_metrics/service_mongoose_system_metrics.erl @@ -45,8 +45,7 @@ -spec verify_if_configured() -> ok | ignore. verify_if_configured() -> - Services = mongoose_config:get_opt(services, []), - case proplists:is_defined(?MODULE, Services) of + case mongoose_service:is_loaded(?MODULE) of false -> %% Technically, notice level. %% Though make it louder, in case people set minimum level as warning. From 55822ec0b9e3a7bde2014f97477dcf41efdde7cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Fri, 1 Apr 2022 14:15:05 +0200 Subject: [PATCH 07/10] Update config parser tests with services in maps - Services are always present - Service opts are unfolded Also: remove unused function --- test/common/config_parser_helper.erl | 27 ++++++++++++++++----------- test/config_parser_SUITE.erl | 18 ++++-------------- test/mongoose_config_SUITE.erl | 1 + 3 files changed, 21 insertions(+), 25 deletions(-) diff --git a/test/common/config_parser_helper.erl b/test/common/config_parser_helper.erl index 838585eee46..62bb721954b 100644 --- a/test/common/config_parser_helper.erl +++ b/test/common/config_parser_helper.erl @@ -21,6 +21,7 @@ options("host_types") -> {rdbms_server_type, generic}, {registration_timeout, 600}, {routing_modules, mongoose_router:default_routing_modules()}, + {services, #{}}, {sm_backend, {mnesia, []}}, {{s2s, <<"another host type">>}, default_s2s()}, {{s2s, <<"localhost">>}, default_s2s()}, @@ -79,11 +80,10 @@ options("miscellaneous") -> {routing_modules, [mongoose_router_global, mongoose_router_localdomain]}, {services, - [{service_mongoose_system_metrics, - [{initial_report, 300000}, - {periodic_report, 10800000}, - report, - {tracking_id, "UA-123456789"}]}]}, + #{service_mongoose_system_metrics => [{initial_report, 300000}, + {periodic_report, 10800000}, + {report, true}, + {tracking_id, "UA-123456789"}]}}, {{s2s, <<"anonymous.localhost">>}, default_s2s()}, {{s2s, <<"localhost">>}, default_s2s()}, {sm_backend, {mnesia, []}}, @@ -108,6 +108,7 @@ options("modules") -> {rdbms_server_type, generic}, {registration_timeout, 600}, {routing_modules, mongoose_router:default_routing_modules()}, + {services, #{}}, {{s2s, <<"dummy_host">>}, default_s2s()}, {{s2s, <<"localhost">>}, default_s2s()}, {sm_backend, {mnesia, []}}, @@ -244,12 +245,14 @@ options("mongooseim-pgsql") -> {registration_timeout, infinity}, {routing_modules, mongoose_router:default_routing_modules()}, {services, - [{service_admin_extra, - [{submods, - [node, accounts, sessions, vcard, gdpr, upload, roster, last, private, - stanza, stats]}]}, - {service_mongoose_system_metrics, - [{initial_report, 300000}, {periodic_report, 10800000}]}]}, + #{service_admin_extra => + [{submods, + [node, accounts, sessions, vcard, gdpr, upload, roster, last, private, + stanza, stats]}], + service_mongoose_system_metrics => + [{initial_report, 300000}, + {periodic_report, 10800000}] + }}, {sm_backend, {mnesia, []}}, {{auth, <<"anonymous.localhost">>}, (default_auth())#{anonymous => #{allow_multiple_connections => true, @@ -352,6 +355,7 @@ options("outgoing_pools") -> {rdbms_server_type, generic}, {registration_timeout, 600}, {routing_modules, mongoose_router:default_routing_modules()}, + {services, #{}}, {{s2s, <<"anonymous.localhost">>}, default_s2s()}, {{s2s, <<"localhost">>}, default_s2s()}, {{s2s, <<"localhost.bis">>}, default_s2s()}, @@ -378,6 +382,7 @@ options("s2s_only") -> {rdbms_server_type, generic}, {registration_timeout, 600}, {routing_modules, mongoose_router:default_routing_modules()}, + {services, #{}}, {sm_backend, {mnesia, []}}, {{auth, <<"dummy_host">>}, default_auth()}, {{auth, <<"localhost">>}, default_auth()}, diff --git a/test/config_parser_SUITE.erl b/test/config_parser_SUITE.erl index 4ddb3d7c930..2eac2756bce 100644 --- a/test/config_parser_SUITE.erl +++ b/test/config_parser_SUITE.erl @@ -3110,7 +3110,9 @@ service_mongoose_system_metrics(_Config) -> T(#{<<"periodic_report">> => 5000})), ?cfg(servopts(M, [{tracking_id, "UA-123456789"}]), T(#{<<"tracking_id">> => <<"UA-123456789">>})), - ?cfg(servopts(M, [no_report]), + ?cfg(servopts(M, [{report, true}]), + T(#{<<"report">> => true})), + ?cfg(servopts(M, [{no_report, true}]), T(#{<<"report">> => false})), %% error cases ?err(T(#{<<"initial_report">> => <<"forever">>})), @@ -3157,7 +3159,7 @@ modopts(Mod, Opts) -> [{[modules, Mod], Opts}]. servopts(Service, Opts) -> - [{services, [{Service, Opts}]}]. + [{[services, Service], Opts}]. %% helpers for 'listen' tests @@ -3276,18 +3278,6 @@ get_config_value([TopKey | Rest], Config) -> {_, TopValue} -> lists:foldl(fun maps:get/2, TopValue, Rest) end. --spec assert_error([mongoose_config_parser_toml:config()]) -> - [mongoose_config_parser_toml:config_error()]. -assert_error(Config) -> - case extract_errors(Config) of - [] -> - ct:fail({"Expected errors but found none", Config}); - Errors -> - [?assertMatch(#{class := error, - what := toml_processing_failed}, Error) || Error <- Errors], - Errors - end. - %% helpers for file tests test_config_file(Config, File) -> diff --git a/test/mongoose_config_SUITE.erl b/test/mongoose_config_SUITE.erl index 38afc2076df..926e8bd25bc 100644 --- a/test/mongoose_config_SUITE.erl +++ b/test/mongoose_config_SUITE.erl @@ -184,6 +184,7 @@ minimal_config_opts() -> {rdbms_server_type, generic}, {registration_timeout, 600}, {routing_modules, mongoose_router:default_routing_modules()}, + {services, #{}}, {sm_backend, {mnesia, []}}, {{auth, <<"localhost">>}, config_parser_helper:default_auth()}, {{modules, <<"localhost">>}, #{}}, From 48d30580ea89ce0252779b55e11c75189cd216c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Fri, 1 Apr 2022 14:17:34 +0200 Subject: [PATCH 08/10] Split and update unit tests for services Follow the module tests - Simplify service start/stop tests - Check a subset of the deps cases for module corresponding to the features that are present for services --- test/mongoose_service_SUITE.erl | 276 +++++++++----------------------- test/service_deps_SUITE.erl | 61 +++++++ 2 files changed, 136 insertions(+), 201 deletions(-) create mode 100644 test/service_deps_SUITE.erl diff --git a/test/mongoose_service_SUITE.erl b/test/mongoose_service_SUITE.erl index e5b0c933f01..896aa1217bb 100644 --- a/test/mongoose_service_SUITE.erl +++ b/test/mongoose_service_SUITE.erl @@ -1,217 +1,91 @@ -%%%------------------------------------------------------------------- -%%% @doc -%%% -%%% @end -%%%------------------------------------------------------------------- -module(mongoose_service_SUITE). + -compile([export_all, nowarn_export_all]). --include_lib("common_test/include/ct.hrl"). --include_lib("eunit/include/eunit.hrl"). +-include_lib("eunit/include/eunit.hrl"). all() -> - [ - starts_service, - ensure, - verify_deps, - start_deps, - module_deps, - misconfigured - ]. - -services() -> [service_a, service_b, service_c, service_d, service_e, service_f, service_g, - service_h, service_i, service_j, service_k, service_l]. - -dep_services() -> [service_c, service_d, service_e, service_f, service_g, service_h]. - -%% c -%% / \ -%% d e -%% / \ / \ -%% f g h -%% ^ | -%% | | -%% ------- - -%% i -%% / \ -%% j k <-- -%% \ | -%% l-- - -service_deps() -> - [ - {service_c, [service_d, service_e]}, - {service_d, [service_f, service_g]}, - {service_e, [service_g, service_h]}, - {service_h, [service_f]}, - % and now for something completely cicrular - {service_i, [service_j, service_k]}, - {service_k, [service_l]}, - {service_l, [service_k]} - ]. - -init_per_testcase(misconfigured, C) -> - mongoose_service:start(), - mongoose_config:set_opt(services, [{service_a, []}]), - meck:new(service_a, [non_strict]), - meck:expect(service_a, deps, fun() -> [service_b] end), - C; - -init_per_testcase(module_deps, C) -> - init_per_testcase(generic, C), - mongoose_config:set_opt(hosts, [<<"localhost">>]), - mongoose_config:set_opt(host_types, []), - mongoose_config:set_opt({modules, <<"localhost">>}, #{module_a => []}), - meck:new(module_a, [non_strict]), - meck:expect(module_a, deps, fun(_, _) -> [{service, service_d}, {service, service_h}] end), - meck:expect(module_a, start, fun(_, _) -> ok end), - meck:expect(module_a, stop, fun(_) -> ok end), - C; -init_per_testcase(_, C) -> - mongoose_service:start(), - ets:new(testservice, [named_table]), - meck:new(services(), [non_strict]), - lists:map(fun(S) -> - meck:expect(S, start, fun(_Opts) -> increment(S), done end) - end, services()), - lists:map(fun(S) -> - meck:expect(S, stop, fun() -> decrement(S) end) - end, services()), - mongoose_config:set_opt(services, [{Serv, []} || Serv <- services()]), - lists:map(fun(Serv) -> - meck:expect(Serv, deps, fun() -> proplists:get_value(Serv, service_deps()) end) - end, - proplists:get_keys(service_deps())), - C. + [starts_and_stops_services, + ensures_service, + reverts_config_when_service_fails_to_start, + does_not_change_config_when_service_fails_to_stop]. -end_per_testcase(misconfigured, C) -> - mongoose_config:unset_opt(services), - meck:unload(service_a), - C; - -end_per_testcase(module_deps, C) -> - mongoose_config:unset_opt(hosts), - mongoose_config:unset_opt(host_types), - mongoose_config:unset_opt({modules, <<"localhost">>}), - meck:unload(module_a), - end_per_testcase(generic, C); -end_per_testcase(_, C) -> - mongoose_config:unset_opt(services), - meck:unload(services()), - lists:foreach(fun mongoose_service:stop_service/1, services()), - mongoose_service:stop(), +init_per_suite(C) -> C. -%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% -%% -%% tests -%% -%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% - -starts_service(_C) -> - {ok, done} = mongoose_service:start_service(service_a, []), - {error, already_started} = mongoose_service:start_service(service_a, []), - ?assertEqual(1, read(service_a)), - {ok, done} = mongoose_service:start_service(service_b, []), - {error, already_started} = mongoose_service:start_service(service_b, []), - ?assertEqual(1, read(service_b)), - ok = mongoose_service:stop_service(service_a), - ?assertEqual(0, read(service_a)), - {error, not_running} = mongoose_service:stop_service(service_a), - ?assertEqual(0, read(service_a)), - ok = mongoose_service:stop_service(service_b), - ?assertEqual(0, read(service_b)), - {error, not_running} = mongoose_service:stop_service(service_b), - ?assertEqual(0, read(service_b)), +end_per_suite(_C) -> ok. -ensure(_C) -> - ok = mongoose_service:ensure_loaded(service_a), - ok = mongoose_service:ensure_loaded(service_a), - ok = mongoose_service:ensure_loaded(service_a), - ?assertEqual(1, read(service_a)), - ok = mongoose_service:stop_service(service_a), - ?assertEqual(0, read(service_a)), - ok. +init_per_testcase(_TC, C) -> + [mock_service(Service) || Service <- test_services()], + C. -verify_deps(_) -> - mongoose_service:check_deps(service_c), - mongoose_service:check_deps(service_d), - mongoose_service:check_deps(service_e), - mongoose_service:check_deps(service_f), - mongoose_service:check_deps(service_g), - mongoose_service:check_deps(service_g), - ?assertException(error, {circular_deps_detected, _}, mongoose_service:check_deps(service_i)), - mongoose_service:check_deps(service_j), - ?assertException(error, {circular_deps_detected, _}, mongoose_service:check_deps(service_k)), - ?assertException(error, {circular_deps_detected, _}, mongoose_service:check_deps(service_l)), - ok. +end_per_testcase(_, _C) -> + mongoose_config:unset_opt(services), + meck:unload(test_services()). -start_deps(_) -> - assert_loaded([]), - mongoose_service:ensure_loaded(service_f), - assert_loaded([service_f]), - mongoose_service:ensure_loaded(service_d), - assert_loaded([service_d, service_f, service_g]), - mongoose_service:ensure_loaded(service_e), - assert_loaded([service_d, service_e, service_f, service_g, service_h]), - mongoose_service:ensure_loaded(service_c), - assert_loaded(dep_services()), - lists:foreach(fun mongoose_service:stop_service/1, services()), - assert_loaded([]), - mongoose_service:ensure_loaded(service_c), - assert_loaded(dep_services()), - ok. +%% Test cases -module_deps(_) -> - assert_loaded([]), - meck:new(gen_mod, [passthrough]), - meck:expect(gen_mod, is_app_running, fun(_Mooo) -> true end), - ?assertError({service_not_loaded, _}, mongoose_modules:start()), - meck:unload(gen_mod), - mongoose_service:ensure_loaded(service_c), - mongoose_modules:start(), - ?assert(gen_mod:is_loaded(<<"localhost">>, module_a)), - ok. +starts_and_stops_services(_Config) -> + set_services(Services = #{service_a => #{}, service_b => [{opt, val}]}), + ok = mongoose_service:start(), + check_started(maps:to_list(Services)), -misconfigured(_) -> - ?assertError({service_not_configured, service_b}, mongoose_service:ensure_loaded(service_a)), - ok. + ok = mongoose_service:stop(), + check_stopped([service_a, service_b]). +ensures_service(_Config) -> + set_services(#{}), + ?assertEqual({started, start_result}, mongoose_service:ensure_started(service_a, #{})), + ?assertEqual(#{service_a => #{}}, get_services()), -%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% -%% -%% helpers -%% -%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% - -increment(S) -> - Ni = case ets:lookup(testservice, S) of - [{S, I}] -> I + 1; - [] -> 1 - end, - ets:insert(testservice, {S, Ni}). - -decrement(S) -> - Ni = case ets:lookup(testservice, S) of - [{S, I}] -> I - 1; - [] -> -1 - end, - ets:insert(testservice, {S, Ni}). - -read(S) -> - case ets:lookup(testservice, S) of - [{S, I}] -> I; - [] -> 0 - end. - -assert_loaded(Loaded) -> - _ = [{S, mongoose_service:is_loaded(S)} || S <- services()], - NotLoaded = sets:to_list( - sets:subtract( - sets:from_list(dep_services()), - sets:from_list(Loaded))), - ?assert(lists:all(fun mongoose_service:is_loaded/1, Loaded)), - ?assert(lists:all(fun(S) -> not mongoose_service:is_loaded(S) end, NotLoaded)), - ok. + ?assertEqual(already_started, mongoose_service:ensure_started(service_a, #{})), + ?assertEqual(#{service_a => #{}}, get_services()), + + ?assertEqual({restarted, #{}, start_result}, + mongoose_service:ensure_started(service_a, #{opt => val})), + ?assertEqual(#{service_a => #{opt => val}}, get_services()), + + ?assertEqual({stopped, #{opt => val}}, mongoose_service:ensure_stopped(service_a)), + ?assertEqual(#{}, get_services()), + + ?assertEqual(already_stopped, mongoose_service:ensure_stopped(service_a)), + ?assertEqual(#{}, get_services()). + +reverts_config_when_service_fails_to_start(_Config) -> + set_services(#{}), + meck:expect(service_a, start, fun(_) -> error(something_awful) end), + ?assertError(something_awful, mongoose_service:ensure_started(service_a, #{})), + ?assertEqual(#{}, get_services()). + +does_not_change_config_when_service_fails_to_stop(_Config) -> + set_services(#{service_a => #{}}), + meck:expect(service_a, stop, fun() -> error(something_awful) end), + ?assertError(something_awful, mongoose_service:ensure_stopped(service_a)), + ?assertEqual(#{service_a => #{}}, get_services()). + +%% Helpers + +set_services(Services) -> + mongoose_config:set_opt(services, Services). + +get_services() -> + mongoose_config:get_opt(services). + +check_started(ServicesWithOpts) -> + lists:foreach(fun({Service, Opts}) -> + ?assert(meck:called(Service, start, [Opts])) + end, ServicesWithOpts). + +check_stopped(Services) -> + lists:foreach(fun(Service) -> + ?assert(meck:called(Service, stop, [])) + end, Services). + +mock_service(Service) -> + meck:new(Service, [non_strict]), + meck:expect(Service, start, fun(_) -> start_result end), + meck:expect(Service, stop, fun() -> ok end). + +test_services() -> + [service_a, service_b, service_c, service_d]. diff --git a/test/service_deps_SUITE.erl b/test/service_deps_SUITE.erl new file mode 100644 index 00000000000..d87d96e8ac8 --- /dev/null +++ b/test/service_deps_SUITE.erl @@ -0,0 +1,61 @@ +-module(service_deps_SUITE). +-compile([export_all, nowarn_export_all]). + +-include_lib("eunit/include/eunit.hrl"). + +-import(mongoose_service_deps, [resolve_deps/1, sort_deps/1]). + +all() -> + [ + no_dependencies, + dependency, + dependency_chain, + dependency_dag, + fails_on_dependency_cycle, + preserves_dependency_opts + ]. + +init_per_testcase(_, C) -> + meck:new(test_services(), [non_strict]), + C. + +end_per_testcase(_, _C) -> + meck:unload(test_services()). + +%% Test cases + +no_dependencies(_Config) -> + Services = #{service_a => #{}, service_b => #{}}, + ?assertEqual(Services, resolve_deps(Services)). + +dependency(_Config) -> + set_deps(#{service_a => [service_b]}), + ?assertMatch([{service_b, _}, {service_a, _}], add_deps(#{service_a => #{}})). + +dependency_chain(_Config) -> + set_deps(#{service_a => [service_b], service_b => [service_c]}), + ?assertMatch([{service_c, _}, {service_b, _}, {service_a, _}], add_deps(#{service_a => #{}})). + +dependency_dag(_Config) -> + set_deps(#{service_a => [service_b, service_c], service_b => [service_c]}), + ?assertMatch([{service_c, _}, {service_b, _}, {service_a, _}], add_deps(#{service_a => #{}})). + +fails_on_dependency_cycle(_Config) -> + set_deps(#{service_a => [service_b], service_b => [service_c], service_c => [service_a]}), + ?assertError(#{what := resolving_dependencies_aborted}, add_deps(#{service_a => #{}})). + +preserves_dependency_opts(_Config) -> + set_deps(#{service_a => [service_b]}), + ?assertMatch([{service_b, #{opt1 := 1}}, {service_a, _}], + add_deps(#{service_a => #{}, service_b => #{opt1 => 1}})). + +%% Helpers + +add_deps(ServiceMap) -> + sort_deps(resolve_deps(ServiceMap)). + +set_deps(DepsMap) -> + maps:map(fun(Service, Deps) -> meck:expect(Service, deps, fun() -> Deps end) end, DepsMap). + +test_services() -> + [service_a, service_b, service_c]. From a9530c27fb515176de19fcb0a6130bac48f59d24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Fri, 1 Apr 2022 14:44:55 +0200 Subject: [PATCH 09/10] Check for service deps in gen_mod_SUITE This check is in gen_mod, so the test should be here as well. Also: update module tests to use maps. --- test/gen_mod_SUITE.erl | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/test/gen_mod_SUITE.erl b/test/gen_mod_SUITE.erl index 48ff01091ff..99665a3f23d 100644 --- a/test/gen_mod_SUITE.erl +++ b/test/gen_mod_SUITE.erl @@ -27,6 +27,7 @@ all() -> [start_and_stop, start_error, + start_with_service_deps, stop_error, loaded_modules, loaded_modules_with_opts, @@ -44,10 +45,10 @@ end_per_testcase(_, Config) -> Config. start_and_stop(_Config) -> - ?assertEqual({ok, ok}, gen_mod:start_module(host(a), a_module, [])), - ?assertError(#{what := module_not_loaded}, gen_mod:start_module(host(a), b_module, [{k, v}])), - ?assertError(#{what := module_not_loaded}, gen_mod:start_module(host(b), a_module, [])), - ?assertEqual({ok, ok}, gen_mod:start_module(host(b), b_module, [{k, v}])), + ?assertEqual({ok, ok}, gen_mod:start_module(host(a), a_module, #{})), + ?assertError(#{what := module_not_loaded}, gen_mod:start_module(host(a), b_module, #{k => v})), + ?assertError(#{what := module_not_loaded}, gen_mod:start_module(host(b), a_module, #{})), + ?assertEqual({ok, ok}, gen_mod:start_module(host(b), b_module, #{k => v})), ?assertEqual(ok, gen_mod:stop_module(host(a), a_module)), ?assertError(#{what := module_not_loaded}, gen_mod:stop_module(host(a), b_module)), ?assertError(#{what := module_not_loaded}, gen_mod:stop_module(host(b), a_module)), @@ -55,7 +56,13 @@ start_and_stop(_Config) -> start_error(_Config) -> meck:expect(a_module, start, fun(_, _) -> error(bad_weather) end), - ?assertError(bad_weather, gen_mod:start_module(host(a), a_module, [])). + ?assertError(bad_weather, gen_mod:start_module(host(a), a_module, #{})). + +start_with_service_deps(_Config) -> + meck:expect(a_module, deps, fun(_, _) -> [{service, a_service}] end), + ?assertError(#{what := service_not_loaded}, gen_mod:start_module(host(a), a_module, #{})), + mongoose_config:set_opt(services, #{a_service => #{}}), + ?assertEqual({ok, ok}, gen_mod:start_module(host(a), a_module, #{})). stop_error(_Config) -> meck:expect(a_module, stop, fun(_) -> error(bad_mood) end), @@ -67,8 +74,8 @@ loaded_modules(_Config) -> ?assertEqual([a_module, b_module], gen_mod:loaded_modules()). loaded_modules_with_opts(_Config) -> - MA = #{a_module => []}, - MB = #{b_module => [{k, v}]}, + MA = #{a_module => #{}}, + MB = #{b_module => #{k => v}}, ?assertEqual(MA, gen_mod:loaded_modules_with_opts(host(a))), ?assertEqual(MB, gen_mod:loaded_modules_with_opts(host(b))), ?assertEqual(#{host(a) => MA, host(b) => MB}, gen_mod:loaded_modules_with_opts()). @@ -78,8 +85,8 @@ hosts_with_module(_Config) -> ?assertEqual([host(b)], gen_mod:hosts_with_module(b_module)). hosts_and_opts_with_module(_Config) -> - ?assertEqual(#{host(a) => []}, gen_mod:hosts_and_opts_with_module(a_module)), - ?assertEqual(#{host(b) => [{k, v}]}, gen_mod:hosts_and_opts_with_module(b_module)). + ?assertEqual(#{host(a) => #{}}, gen_mod:hosts_and_opts_with_module(a_module)), + ?assertEqual(#{host(b) => #{k => v}}, gen_mod:hosts_and_opts_with_module(b_module)). host(a) -> <<"localhost">>; @@ -94,5 +101,6 @@ setup_meck(Module) -> opts() -> [{hosts, [host(a), host(b)]}, {host_types, []}, - {{modules, host(a)}, #{a_module => []}}, - {{modules, host(b)}, #{b_module => [{k, v}]}}]. + {services, #{}}, + {{modules, host(a)}, #{a_module => #{}}}, + {{modules, host(b)}, #{b_module => #{k => v}}}]. From 1a7a30d867e02c821b64f50327d978cc9b15b093 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Fri, 1 Apr 2022 13:48:09 +0200 Subject: [PATCH 10/10] Use the new mongoose_service API in big tests --- big_tests/tests/service_domain_db_SUITE.erl | 8 ++++---- .../service_mongoose_system_metrics_SUITE.erl | 20 +++++++------------ 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/big_tests/tests/service_domain_db_SUITE.erl b/big_tests/tests/service_domain_db_SUITE.erl index 72a730b8e7e..676642deba2 100644 --- a/big_tests/tests/service_domain_db_SUITE.erl +++ b/big_tests/tests/service_domain_db_SUITE.erl @@ -1054,11 +1054,11 @@ service_enabled(Node) -> sync_local(Node). service_enabled(Node, Opts) -> - rpc(Node, mongoose_service, start_service, [service_domain_db, Opts]), + rpc(Node, mongoose_service, ensure_started, [service_domain_db, Opts]), true = rpc(Node, service_domain_db, enabled, []). service_disabled(Node) -> - rpc(Node, mongoose_service, stop_service, [service_domain_db]), + rpc(Node, mongoose_service, ensure_stopped, [service_domain_db]), false = rpc(Node, service_domain_db, enabled, []). init_with(Node, Pairs, AllowedHostTypes) -> @@ -1159,12 +1159,12 @@ force_check_for_updates(Node) -> ok = rpc(Node, service_domain_db, force_check_for_updates, []). restore_conf(Node, #{loaded := Loaded, service_opts := ServiceOpts, core_opts := CoreOpts}) -> - rpc(Node, mongoose_service, stop_service, [service_domain_db]), + rpc(Node, mongoose_service, ensure_stopped, [service_domain_db]), [Pairs, AllowedHostTypes] = CoreOpts, init_with(Node, Pairs, AllowedHostTypes), case Loaded of true -> - rpc(Node, mongoose_service, start_service, [service_domain_db, ServiceOpts]); + rpc(Node, mongoose_service, ensure_started, [service_domain_db, ServiceOpts]); _ -> ok end. diff --git a/big_tests/tests/service_mongoose_system_metrics_SUITE.erl b/big_tests/tests/service_mongoose_system_metrics_SUITE.erl index 44b9b10e731..39c2cb93d12 100644 --- a/big_tests/tests/service_mongoose_system_metrics_SUITE.erl +++ b/big_tests/tests/service_mongoose_system_metrics_SUITE.erl @@ -87,7 +87,7 @@ init_per_suite(Config) -> end_per_suite(Config) -> http_helper:stop(), Args = [{initial_report, timer:seconds(20)}, {periodic_report, timer:minutes(5)}], - [start_system_metrics_module(Node, Args) || Node <- [mim(), mim2()]], + [start_system_metrics_service(Node, Args) || Node <- [mim(), mim2()]], escalus:end_per_suite(Config). %%-------------------------------------------------------------------- @@ -285,7 +285,6 @@ config_type_is_reported(_Config) -> just_removed_from_config_logs_question(_Config) -> disable_system_metrics(mim3()), - remove_service_from_config(service_mongoose_system_metrics), %% WHEN Result = distributed_helper:rpc( mim3(), service_mongoose_system_metrics, verify_if_configured, []), @@ -309,7 +308,7 @@ in_config_unmodified_logs_request_for_agreement(_Config) -> in_config_with_explicit_no_report_goes_off_silently(_Config) -> %% WHEN logger_ct_backend:capture(warning), - start_system_metrics_module(mim(), [no_report]), + start_system_metrics_service(mim(), [{no_report, true}]), logger_ct_backend:stop_capture(), %% THEN FilterFun = fun(warning, Msg) -> @@ -323,7 +322,7 @@ in_config_with_explicit_no_report_goes_off_silently(_Config) -> in_config_with_explicit_reporting_goes_on_silently(_Config) -> %% WHEN logger_ct_backend:capture(warning), - start_system_metrics_module(mim(), [report]), + start_system_metrics_service(mim(), [{report, true}]), logger_ct_backend:stop_capture(), %% THEN FilterFun = fun(warning, Msg) -> @@ -425,17 +424,17 @@ enable_system_metrics(Node) -> enable_system_metrics(Node, Timers) -> UrlArgs = [google_analytics_url, ?SERVER_URL], ok = mongoose_helper:successful_rpc(Node, mongoose_config, set_opt, UrlArgs), - start_system_metrics_module(Node, Timers). + start_system_metrics_service(Node, Timers). enable_system_metrics_with_configurable_tracking_id(Node) -> enable_system_metrics(Node, [{initial_report, 100}, {periodic_report, 100}, {tracking_id, ?TRACKING_ID_EXTRA}]). -start_system_metrics_module(Node, Args) -> +start_system_metrics_service(Node, Args) -> distributed_helper:rpc( - Node, mongoose_service, start_service, [service_mongoose_system_metrics, Args]). + Node, mongoose_service, ensure_started, [service_mongoose_system_metrics, Args]). disable_system_metrics(Node) -> - distributed_helper:rpc(Node, mongoose_service, stop_service, [service_mongoose_system_metrics]), + distributed_helper:rpc(Node, mongoose_service, ensure_stopped, [service_mongoose_system_metrics]), mongoose_helper:successful_rpc(Node, mongoose_config, unset_opt, [ google_analytics_url ]). delete_prev_client_id(Node) -> @@ -454,11 +453,6 @@ system_metrics_service_is_enabled(Node) -> system_metrics_service_is_disabled(Node) -> not system_metrics_service_is_enabled(Node). -remove_service_from_config(Service) -> - Services = distributed_helper:rpc(mim3(), mongoose_config, get_opt, [services]), - NewServices = proplists:delete(Service, Services), - distributed_helper:rpc(mim3(), mongoose_config, set_opt, [services, NewServices]). - events_are_reported_to_primary_tracking_id() -> events_are_reported_to_tracking_ids([primary_tracking_id()]).