diff --git a/Makefile b/Makefile index 46da1152..c5cc85e3 100644 --- a/Makefile +++ b/Makefile @@ -45,6 +45,7 @@ integration_test: ./integration_test/test_amoc_cluster.sh ./integration_test/test_distribute_scenario.sh ./integration_test/test_run_scenario.sh + ./integration_test/test_update_settings.sh ./integration_test/test_add_new_node.sh rerun_integration_test: @@ -53,6 +54,7 @@ rerun_integration_test: ./integration_test/test_amoc_cluster.sh ./integration_test/test_distribute_scenario.sh ./integration_test/test_run_scenario.sh + ./integration_test/test_update_settings.sh ./integration_test/test_add_new_node.sh dialyzer: diff --git a/integration_test/build_docker_image.sh b/integration_test/build_docker_image.sh index 3905026a..5ea16c62 100755 --- a/integration_test/build_docker_image.sh +++ b/integration_test/build_docker_image.sh @@ -4,7 +4,7 @@ source "$(dirname "$0")/helper.sh" enable_strict_mode cd "$git_root" -otp_vsn="${OTP_RELEASE:-25.3}" +otp_vsn="${OTP_RELEASE:-28}" echo "ERLANG/OTP '${otp_vsn}'" docker build \ diff --git a/integration_test/extra_code_paths/path1/dummy_helper.erl b/integration_test/extra_code_paths/path1/dummy_helper.erl index c0c34a47..51c22702 100644 --- a/integration_test/extra_code_paths/path1/dummy_helper.erl +++ b/integration_test/extra_code_paths/path1/dummy_helper.erl @@ -33,9 +33,8 @@ test_amoc_dist() -> ?assertEqual(N2, N1 + 20, ?comment(Users2)), %% remove 10 users remove_and_wait(Master, 10), - {N3, Max3, Nodes3, _Ids3, Users3} = get_users_info(Slaves), + {N3, _Max3, Nodes3, _Ids3, Users3} = get_users_info(Slaves), ?assertEqual(N2 - 10, N3, ?comment(Users3)), - ?assertEqual(Max2, Max3, ?comment(Users3)), ?assertEqual(Nodes2, Nodes3, ?comment(Users3)), %% try to remove N3 users Ret = remove_and_wait(Master, N3), @@ -67,6 +66,7 @@ get_users_info(SlaveNodes) -> Nodes = lists:usort([Node || {Node, Users} <- Distrib, [] =/= Users]), N = length(Ids), MaxId = lists:max(Ids), + %% logger:error(#{n => N, max_id => MaxId, nodes => Nodes, ids => Ids, distrib => Distrib}), {N, MaxId, Nodes, Ids, Distrib}. add_and_wait(Master, Num) -> diff --git a/integration_test/extra_code_paths/path2/dummy_scenario.erl b/integration_test/extra_code_paths/path2/dummy_scenario.erl index b86218ec..ecf46868 100644 --- a/integration_test/extra_code_paths/path2/dummy_scenario.erl +++ b/integration_test/extra_code_paths/path2/dummy_scenario.erl @@ -29,7 +29,15 @@ #{name => var6, description => "description6"}, #{name => nodes, description => "this variable is set for docker " "container via AMOC_NODES env"}, - #{name => test, description => "this one to be set via REST API"}]). + #{name => test1, description => "this variable is to be set via initial settings"}, + #{name => test2, update => none, description => "this one to be updated"}]). + + +-required_variable([ + #{name => global_test1, scope => global, + description => "this variable is to be set via initial settings"}, + #{name => global_test2, scope => global, update => none, + description => "this one to be updated"}]). %% parameter verification method -export([test_verification_function/1]). @@ -63,8 +71,9 @@ init() -> %% results in exception. {invalid_setting, undeclared_variable} = (catch amoc_config:get(undeclared_variable)), - %% this variable is set via REST API - <<"test_value">> = amoc_config:get(test), + %% this variable is set via settings + <<"test_value2">> = amoc_config:get(test1), + %% dummy_var variable is defined in the dummy_helper module. %% if dummy_helper is not propagated, then this call crashes default_value = amoc_config:get(dummy_var), diff --git a/integration_test/test_add_new_node.sh b/integration_test/test_add_new_node.sh index fa38b99c..52989fa1 100755 --- a/integration_test/test_add_new_node.sh +++ b/integration_test/test_add_new_node.sh @@ -6,6 +6,12 @@ enable_strict_mode docker_compose up --wait --wait-timeout 100 amoc-worker-3 amoc_eval amoc-worker-3 "amoc_controller:get_status()." | contains_all dummy_scenario running -amoc_eval amoc-worker-3 "binary_to_list(amoc_config:get(test))." | contains_all "test_value" + + +amoc_eval amoc-worker-3 "binary_to_list(amoc_config:get(global_test1))." | contains_all "test_value1" +amoc_eval amoc-worker-3 "binary_to_list(amoc_config:get(global_test2))." | contains_all "test_value3" +amoc_eval amoc-worker-3 "binary_to_list(amoc_config:get(test1))." | contains_all "test_value2" +amoc_eval amoc-worker-3 "atom_to_list(amoc_config:get(test2))." | contains_all "undefined" + amoc_eval amoc-worker-3 "dummy_helper:test_amoc_dist()." | contains_all 'amoc_dist_works_as_expected' echo "amoc_dist_works_as_expected" diff --git a/integration_test/test_run_scenario.sh b/integration_test/test_run_scenario.sh index bd2caf90..f0f069c6 100755 --- a/integration_test/test_run_scenario.sh +++ b/integration_test/test_run_scenario.sh @@ -3,11 +3,12 @@ source "$(dirname "$0")/helper.sh" enable_strict_mode -############################# -## amoc REST API functions ## -############################# +#################### +## amoc functions ## +#################### run_scenario() { - amoc_eval "$1" "amoc_dist:do(${2}, ${3}, [{test, <<\"test_value\">>}])." + amoc_eval "$1" "amoc_dist:do(${2}, ${3}, [{global_test1, <<\"test_value1\">>}, + {test1, <<\"test_value2\">>}])." } result="$(run_scenario amoc-master dummy_scenario 10)" diff --git a/integration_test/test_update_settings.sh b/integration_test/test_update_settings.sh new file mode 100755 index 00000000..24999fd1 --- /dev/null +++ b/integration_test/test_update_settings.sh @@ -0,0 +1,49 @@ +#!/bin/bash + +source "$(dirname "$0")/helper.sh" +enable_strict_mode + +#################### +## amoc functions ## +#################### +update_settings() { + amoc_eval "$1" "amoc_dist:update_settings(${2}, ${3})." +} + +amoc_eval amoc-worker-1 "binary_to_list(amoc_config:get(global_test1))." | contains_all "test_value1" +amoc_eval amoc-worker-1 "atom_to_list(amoc_config:get(global_test2))." | contains_all "undefined" +amoc_eval amoc-worker-1 "binary_to_list(amoc_config:get(test1))." | contains_all "test_value2" +amoc_eval amoc-worker-1 "atom_to_list(amoc_config:get(test2))." | contains_all "undefined" + +amoc_eval amoc-worker-2 "binary_to_list(amoc_config:get(global_test1))." | contains_all "test_value1" +amoc_eval amoc-worker-2 "atom_to_list(amoc_config:get(global_test2))." | contains_all "undefined" +amoc_eval amoc-worker-2 "binary_to_list(amoc_config:get(test1))." | contains_all "test_value2" +amoc_eval amoc-worker-2 "atom_to_list(amoc_config:get(test2))." | contains_all "undefined" + +amoc_eval amoc-master "binary_to_list(amoc_config:get(global_test1))." | contains_all "test_value1" +amoc_eval amoc-master "atom_to_list(amoc_config:get(global_test2))." | contains_all "undefined" +amoc_eval amoc-master "binary_to_list(amoc_config:get(test1))." | contains_all "test_value2" +amoc_eval amoc-master "atom_to_list(amoc_config:get(test2))." | contains_all "undefined" + +update_settings amoc-master "[{global_test2, <<\"test_value3\">>}, {test2, <<\"test_value4\">>}]" "[node()]" + +amoc_eval amoc-master "binary_to_list(amoc_config:get(global_test2))." | contains_all "test_value3" +amoc_eval amoc-master "binary_to_list(amoc_config:get(test2))." | contains_all "test_value4" + +amoc_eval amoc-worker-1 "binary_to_list(amoc_config:get(global_test2))." | contains_all "test_value3" +amoc_eval amoc-worker-1 "atom_to_list(amoc_config:get(test2))." | contains_all "undefined" + +amoc_eval amoc-worker-2 "binary_to_list(amoc_config:get(global_test2))." | contains_all "test_value3" +amoc_eval amoc-worker-2 "atom_to_list(amoc_config:get(test2))." | contains_all "undefined" + +update_settings amoc-master "[{test2, <<\"test_value5\">>}]" "nodes()" + +amoc_eval amoc-worker-1 "binary_to_list(amoc_config:get(test2))." | contains_all "test_value5" +amoc_eval amoc-worker-2 "binary_to_list(amoc_config:get(test2))." | contains_all "test_value5" + +update_settings amoc-master "[{global_test2, <<\"test_value6\">>}]" "nodes()" | \ + contains_all "changing_global_parameters_on_a_slave_node" "error" + +amoc_eval amoc-master "binary_to_list(amoc_config:get(global_test2))." | contains_all "test_value3" +amoc_eval amoc-worker-1 "binary_to_list(amoc_config:get(global_test2))." | contains_all "test_value3" +amoc_eval amoc-worker-2 "binary_to_list(amoc_config:get(global_test2))." | contains_all "test_value3" diff --git a/src/amoc_controller.erl b/src/amoc_controller.erl index 1ab10eef..6aa1f2a0 100644 --- a/src/amoc_controller.erl +++ b/src/amoc_controller.erl @@ -11,7 +11,7 @@ -define(DEFAULT_USER_RATE, 1200). -required_variable(#{name => user_rate, default_value => ?DEFAULT_USER_RATE, - verification => {?MODULE, verify_user_rate, 1}, + verification => {?MODULE, verify_user_rate, 1}, scope => global, description => "Throttle rate for the Scenario:start/1,2 callback", update => {?MODULE, update_user_rate, 2}}). @@ -63,10 +63,11 @@ start_scenario/2, stop_scenario/0, update_settings/1, + propagate_config/2, add_users/2, remove_users/2, get_status/0, - disable/0]). + disable/2]). %% ------------------------------------------------------------------ %% Parameters verification functions @@ -107,6 +108,10 @@ stop_scenario() -> update_settings(Settings) -> gen_server:call(?SERVER, {update_settings, Settings}). +-spec propagate_config(node(), amoc_config:config()) -> ok. +propagate_config(Node, Config) -> + gen_server:cast({?SERVER, Node}, {propagate_config, Config}). + -spec add_users(amoc_scenario:user_id(), amoc_scenario:user_id()) -> ok | {error, term()}. add_users(StartId, EndId) -> @@ -123,9 +128,9 @@ get_status() -> {ok, Status} = gen_server:call(?SERVER, get_status), Status. --spec disable() -> ok | {error, term()}. -disable() -> - gen_server:call(?SERVER, disable). +-spec disable(amoc:scenario(), amoc_config:settings()) -> ok | {error, term()}. +disable(Scenario, Settings) -> + gen_server:call(?SERVER, {disable, Scenario, Settings}). %% @private -spec verify_user_rate(any()) -> boolean(). @@ -178,8 +183,8 @@ handle_call({remove, Count, ForceRemove}, _From, State) -> handle_call(get_status, _From, State) -> RetValue = handle_status(State), {reply, {ok, RetValue}, State}; -handle_call(disable, _From, State) -> - {RetValue, NewState} = handle_disable(State), +handle_call({disable, Scenario, Settings}, _From, State) -> + {RetValue, NewState} = handle_disable(Scenario, Settings, State), {reply, RetValue, NewState}; handle_call(_Request, _From, State) -> {reply, {error, not_implemented}, State}. @@ -189,6 +194,9 @@ handle_call(_Request, _From, State) -> handle_cast(zero_users_running, State) -> NewSate = handle_zero_users_running(State), {noreply, NewSate}; +handle_cast({propagate_config, Config}, State) -> + handle_propagate_config(Config), + {noreply, State}; handle_cast(_Msg, State) -> {noreply, State}. @@ -236,7 +244,8 @@ handle_stop_scenario(#state{status = Status} = State) -> {{error, {invalid_status, Status}}, State}. -spec handle_update_settings(amoc_config:settings(), state()) -> handle_call_res(). -handle_update_settings(Settings, #state{status = running}) -> +handle_update_settings(Settings, #state{status = Status}) when Status =:= disabled; + Status =:= running -> case amoc_config_scenario:update_settings(Settings) of ok -> ok; {error, Type, Reason} -> {error, {Type, Reason}} @@ -244,6 +253,10 @@ handle_update_settings(Settings, #state{status = running}) -> handle_update_settings(_Settings, #state{status = Status}) -> {error, {invalid_status, Status}}. +-spec handle_propagate_config(amoc_config:config()) -> ok. +handle_propagate_config(Config) -> + amoc_config_utils:store_scenario_config(Config). + -spec handle_add(amoc_scenario:user_id(), amoc_scenario:user_id(), state()) -> {handle_call_res(), state()}. handle_add(StartId, EndId, #state{status = running, @@ -281,10 +294,14 @@ handle_status(#state{status = finished, scenario = Scenario}) -> handle_status(#state{status = Status}) -> Status. %% idle, disabled or {error, Reason}. --spec handle_disable(state()) -> {handle_call_res(), state()}. -handle_disable(#state{status = idle} = State) -> - {ok, State#state{status = disabled}}; -handle_disable(#state{status = Status} = State) -> +-spec handle_disable(amoc:scenario(), amoc_config:settings(), state()) -> + {handle_call_res(), state()}. +handle_disable(Scenario, Settings, #state{status = idle} = State) -> + case amoc_config_scenario:parse_scenario_settings(Scenario, Settings) of + ok -> {ok, State#state{status = disabled}}; + {error, Type, Reason} -> {error, {Type, Reason}} + end; +handle_disable(_Scenario, _Settings, #state{status = Status} = State) -> {{error, {invalid_status, Status}}, State}. %% ------------------------------------------------------------------ diff --git a/src/config/amoc_config.erl b/src/config/amoc_config.erl index 0f1792ec..402cca7d 100644 --- a/src/config/amoc_config.erl +++ b/src/config/amoc_config.erl @@ -5,8 +5,9 @@ -include("amoc_config.hrl"). +-type config() :: module_configuration(). -export([get/1, get/2]). --export_type([name/0, value/0, settings/0, maybe_module_config/0]). +-export_type([name/0, value/0, settings/0, config/0, maybe_module_config/0]). %% ------------------------------------------------------------------ %% API diff --git a/src/config/amoc_config.hrl b/src/config/amoc_config.hrl index d966d10f..4b1027c9 100644 --- a/src/config/amoc_config.hrl +++ b/src/config/amoc_config.hrl @@ -34,10 +34,13 @@ -type maybe_verification_fun() :: verification_fun() | fun((_)-> any()). -type maybe_update_fun() :: update_fun() | fun((_,_)-> any()). +-type scope() :: local | global. + -record(module_parameter, {name :: name(), mod :: module(), value :: value(), description :: unicode:chardata(), + scope = local :: scope(), verification_fn :: maybe_verification_fun(), update_fn = read_only :: maybe_update_fun() | read_only}). diff --git a/src/config/amoc_config_attributes.erl b/src/config/amoc_config_attributes.erl index 7a39196c..cd7b65c3 100644 --- a/src/config/amoc_config_attributes.erl +++ b/src/config/amoc_config_attributes.erl @@ -61,6 +61,7 @@ process_var_attr(Module, Attr) -> {fun check_default_value/1, []}, {fun check_verification_method/1, []}, {fun check_update_method/1, []}, + {fun check_scope/1, []}, {fun make_module_parameter/2, [Module]}], case amoc_config_utils:pipeline(PipelineActions, {ok, Attr}) of {error, Reason} -> {error, add_original_attribute(Reason, Attr)}; @@ -109,18 +110,35 @@ check_update_method(Attr) -> {ok, Attr#{update => UpdateFn}} end. +-spec check_scope(#{any() => any()}) -> + {ok, #{scope := scope(), any() => any()}} | {error, reason()}. +check_scope(Attr) -> + Scope = maps:get(scope, Attr, local), + case lists:member(Scope, [local, global]) of + true -> + {ok, Attr#{scope => Scope}}; + false -> + {error, invalid_scope} + end. + -spec make_module_parameter(#{name := name(), description := unicode:chardata(), default_value := value(), verification := maybe_verification_fun(), update := maybe_update_fun(), + scope := scope(), any() => any()}, module()) -> {ok, module_parameter()}. -make_module_parameter(#{name := Name, description := Description, default_value := Value, - verification := VerificationFn, update := UpdateFn}, Module) -> +make_module_parameter(#{name := Name, + description := Description, + default_value := Value, + verification := VerificationFn, + update := UpdateFn, + scope := Scope}, + Module) -> {ok, #module_parameter{name = Name, mod = Module, description = Description, value = Value, - verification_fn = VerificationFn, update_fn = UpdateFn}}. + scope = Scope, verification_fn = VerificationFn, update_fn = UpdateFn}}. -spec verification_fn(maybe_verification_method()) -> maybe_verification_fun() | not_exported | invalid_method. diff --git a/src/config/amoc_config_scenario.erl b/src/config/amoc_config_scenario.erl index d82b9b27..9ff48d80 100644 --- a/src/config/amoc_config_scenario.erl +++ b/src/config/amoc_config_scenario.erl @@ -34,12 +34,17 @@ parse_scenario_settings(Module, Settings) when is_atom(Module) -> -spec update_settings(settings()) -> ok | error(). update_settings(Settings) -> + IsMasterNode = amoc_cluster:master_node() =:= node(), PipelineActions = [ {fun get_existing_configuration/0, []}, {fun verify_settings/2, [Settings]}, - {fun filter_configuration/2, [Settings]}, + {fun filter_only_updated_parameters/2, [Settings]}, + {fun verify_read_only/1, []}, + {fun verify_scope/2, [IsMasterNode]}, + {fun filter_only_changed_params/2, [Settings]}, {fun amoc_config_verification:process_scenario_config/2, [Settings]}, - {fun store_scenario_config_and_run_update_functions/1, []}], + {fun store_scenario_config_and_run_update_functions/1, []}, + {fun maybe_propagate_global_parameters/2, [IsMasterNode]}], case amoc_config_utils:pipeline(PipelineActions, ok) of ok -> ok; {error, _, _} = Error -> Error; @@ -123,26 +128,55 @@ verify_settings(UndefinedParameters, Config, [{Name, _} | T]) -> get_existing_configuration() -> {ok, ets:tab2list(amoc_config)}. -filter_configuration(Config, Settings) -> +filter_only_updated_parameters(Config, Settings) -> Keys = proplists:get_keys(Settings), KeyPos = #module_parameter.name, FilteredConfig = [lists:keyfind(Name, KeyPos, Config) || Name <- Keys], + {ok, FilteredConfig}. + +verify_scope(Config, true) -> + {ok, Config}; +verify_scope(Config, false) -> + case [{N, M} || #module_parameter{name = N, mod = M, + scope = global} <- Config] of + [] -> {ok, Config}; + GlobalParameters -> + {error, changing_global_parameters_on_a_slave_node, GlobalParameters} + end. + +verify_read_only(Config) -> case [{N, M} || #module_parameter{name = N, mod = M, - update_fn = read_only} <- FilteredConfig] of - [] -> - %% filter out unchanged parameters - ChangedParameters = - [P || #module_parameter{name = N, value = V} = P <- FilteredConfig, - V =/= proplists:get_value(N, Settings)], - {ok, ChangedParameters}; + update_fn = read_only} <- Config] of + [] -> {ok, Config}; ReadOnlyParameters -> {error, readonly_parameters, ReadOnlyParameters} end. +filter_only_changed_params(Config, Settings) -> + %% filter out unchanged parameters + ChangedParameters = + [P || #module_parameter{name = N, value = V} = P <- Config, + V =/= proplists:get_value(N, Settings)], + {ok, ChangedParameters}. + store_scenario_config_and_run_update_functions(Config) -> amoc_config_utils:store_scenario_config(Config), [spawn(fun() -> apply(Fn, [Name, Value]) end) || #module_parameter{name = Name, value = Value, update_fn = Fn} <- Config], + {ok, Config}. + +maybe_propagate_global_parameters(Config, true) -> + case [P || #module_parameter{scope = global} = P <- Config] of + [] -> ok; + GlobalParameters -> + propagate_global_parameters(GlobalParameters) + end; +%% global parameters cannot be changed on a slave node +maybe_propagate_global_parameters(_Config, false) -> ok. + +propagate_global_parameters(GlobalParameters) -> + SlaveNodes = amoc_cluster:slave_nodes(), + [amoc_controller:propagate_config(Node, GlobalParameters) || Node <- SlaveNodes], ok. convert_to_config_map(Config) -> diff --git a/src/config/amoc_config_verification.erl b/src/config/amoc_config_verification.erl index 7e86cbb4..018358c1 100644 --- a/src/config/amoc_config_verification.erl +++ b/src/config/amoc_config_verification.erl @@ -19,9 +19,25 @@ process_scenario_config(Config, Settings) -> -spec get_value_and_verify(module_parameter(), settings()) -> {ok, module_parameter()} | {error, reason()}. -get_value_and_verify(#module_parameter{name = Name, value = Default, - verification_fn = VerificationFn} = Param, +get_value_and_verify(#module_parameter{name = Name, value = Default, scope = Scope} = Param, Settings) -> + MasterNode = amoc_cluster:master_node(), + case {Scope, node()} of + {local, _} -> + get_local_value_and_verify(Param, Settings); + {global, MasterNode} -> + get_local_value_and_verify(Param, Settings); + {global, _} -> + %% for a global parameters, fetch values from the master + NewValue = erpc:call(MasterNode, amoc_config, get, [Name, Default]), + {ok, Param#module_parameter{value = NewValue}} + end. + +-spec get_local_value_and_verify(module_parameter(), settings()) -> + {ok, module_parameter()} | {error, reason()}. +get_local_value_and_verify(#module_parameter{name = Name, value = Default, + verification_fn = VerificationFn} = Param, + Settings) -> DefaultValue = amoc_config_env:get(Name, Default), Value = proplists:get_value(Name, Settings, DefaultValue), case verify(VerificationFn, Value) of diff --git a/src/dist/amoc_dist.erl b/src/dist/amoc_dist.erl index 73b0c12e..cf0d307c 100644 --- a/src/dist/amoc_dist.erl +++ b/src/dist/amoc_dist.erl @@ -68,7 +68,7 @@ remove(Count, ForceRemove, Nodes) when is_integer(Count), Count > 0 -> %% @see update_settings/2 -spec update_settings(amoc_config:settings()) -> {ok, any()} | {error, any()}. update_settings(Settings) -> - Ret = update_settings(Settings, amoc_cluster:slave_nodes()), + Ret = update_settings(Settings, amoc_cluster:all_nodes()), case Ret of {ok, _} -> set_param(settings, Settings); {error, _} -> ok @@ -119,9 +119,9 @@ set_state(State) -> -spec check_nodes([node()]) -> ok | {error, any()}. check_nodes(Nodes) -> - SlaveNodes = amoc_cluster:slave_nodes(), + AllNodes = amoc_cluster:all_nodes(), MasterNode = amoc_cluster:master_node(), - case {Nodes -- SlaveNodes, Nodes, node()} of + case {Nodes -- AllNodes, Nodes, node()} of {[], [_ | _], MasterNode} -> %% running on the master node with the proper non-empty Nodes list ok; @@ -129,7 +129,7 @@ check_nodes(Nodes) -> %% Nodes list is empty {error, empty_nodes_list}; {BadNodes, _, MasterNode} -> - %% non-slave nodes in the list + %% non-cluster nodes in the list {error, {bad_nodes, BadNodes}}; {_, _, _} -> {error, not_a_master} @@ -137,7 +137,7 @@ check_nodes(Nodes) -> -spec prepare_cluster(amoc:scenario(), amoc_config:settings()) -> {ok, any()} | {error, any()}. prepare_cluster(Scenario, Settings) -> - case setup_master_node() of + case setup_master_node(Scenario, Settings) of ok -> set_param(scenario, Scenario), set_param(settings, Settings), @@ -147,11 +147,11 @@ prepare_cluster(Scenario, Settings) -> Error -> Error end. --spec setup_master_node() -> ok | {error, any()}. -setup_master_node() -> +-spec setup_master_node(amoc:scenario(), amoc_config:settings()) -> ok | {error, any()}. +setup_master_node(Scenario, Settings) -> case {amoc_cluster:set_master_node(node()), get_param(scenario)} of {ok, undefined} -> - amoc_controller:disable(); + amoc_controller:disable(Scenario, Settings); {ok, AnotherScenario} -> {error, {scenario_is_running, AnotherScenario}}; {Error, _} -> Error diff --git a/src/throttle/amoc_throttle_controller.erl b/src/throttle/amoc_throttle_controller.erl index ae8a00eb..b24757c4 100644 --- a/src/throttle/amoc_throttle_controller.erl +++ b/src/throttle/amoc_throttle_controller.erl @@ -256,17 +256,20 @@ do_gradual_change_rate( -spec continue_plan(name(), state(), throttle_info(), change_rate_plan()) -> state(). continue_plan(Name, State, Info, #change_rate_plan{rates = [Rate]} = Plan) -> - Interval = Info#throttle_info.interval, - TRef = Plan#change_rate_plan.timer, - Info1 = do_change_rate(Name, Rate, Interval, Info), - {ok, cancel} = timer:cancel(TRef), - consume_all_timer_ticks({change_plan, Name}), + Info1 = do_change_rate(Name, Rate, Info#throttle_info.interval, Info), + stop_change_plan(Name, Plan), State#{Name => Info1#throttle_info{change_plan = undefined}}; continue_plan(Name, State, Info, #change_rate_plan{rates = [Rate | Rates]} = Plan) -> Info1 = do_change_rate(Name, Rate, Info#throttle_info.interval, Info), NewPlan = Plan#change_rate_plan{rates = Rates}, State#{Name => Info1#throttle_info{change_plan = NewPlan}}. +-spec stop_change_plan(name(), undefined | change_rate_plan()) -> ok. +stop_change_plan(_Name, undefined) -> ok; +stop_change_plan(Name, #change_rate_plan{timer = TRef}) -> + {ok, cancel} = timer:cancel(TRef), + consume_all_timer_ticks({change_plan, Name}). + -spec consume_all_timer_ticks(any()) -> ok. consume_all_timer_ticks(Msg) -> receive @@ -274,8 +277,9 @@ consume_all_timer_ticks(Msg) -> after 0 -> ok end. -do_run_op(stop, Name, #throttle_info{pool_sup = PoolSup}, State) -> +do_run_op(stop, Name, #throttle_info{pool_sup = PoolSup, change_plan = ChangePlan}, State) -> ok = amoc_throttle_pooler:stop_pool(PoolSup), + stop_change_plan(Name, ChangePlan), {reply, ok, maps:remove(Name, State)}; do_run_op(pause, Name, #throttle_info{pool_config = PoolConfig, active = true} = Info, State) -> Fun = fun(_, #{pid := Pid}) -> diff --git a/test/amoc_config_scenario_SUITE.erl b/test/amoc_config_scenario_SUITE.erl index fdb10744..2da457fe 100644 --- a/test/amoc_config_scenario_SUITE.erl +++ b/test/amoc_config_scenario_SUITE.erl @@ -14,6 +14,7 @@ %% execution of verification functions is done synchronously -define(VERIFY_TIMEOUT, 0). +-define(SLAVE_NODES, ['slave1@host', 'slave2@host']). %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% %% these attributes are required for the testing purposes %% %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% @@ -35,7 +36,8 @@ default_value => val1, verification => fun ?MOCK_MOD:verify_fun/1}, #{name => config_scenario_var2, description => "config_scenario_var2", default_value => val2, verification => [new_val2, val2], - update => {?MOCK_MOD, update_mfa, 2}}]). + update => {?MOCK_MOD, update_mfa, 2}, + scope => global}]). %% config_scenario_var3 is not declared with -required_variable(...), but it's fine. -override_variable(#{name => config_scenario_var3, description => "config_scenario_var3", default_value => def3}). @@ -54,6 +56,8 @@ all() -> invalid_settings, invalid_value, update_settings, + update_settings_global_on_slave, + update_settings_propagate_global, update_just_one_parameter, update_parameters_with_the_same_values, update_settings_readonly, @@ -69,16 +73,27 @@ init_per_suite(Config) -> meck:expect(?MOCK_MOD, verify_mfa, ['_'], true), meck:expect(?MOCK_MOD, update_fun, ['_', '_'], ok), meck:expect(?MOCK_MOD, verify_fun, ['_'], true), + meck:new(amoc_cluster, [non_strict, no_link]), + meck:expect(amoc_cluster, master_node, fun() -> node() end), + meck:expect(amoc_cluster, slave_nodes, fun() -> [] end), + meck:new(amoc_controller, [non_strict, no_link, passthrough]), + meck:expect(amoc_controller, propagate_config, fun(_, _) -> ok end), Config. end_per_suite(Config) -> meck:unload(?MOCK_MOD), + meck:unload(amoc_cluster), + meck:unload(amoc_controller), Config. init_per_testcase(TC, Config) when TC =:= crash_during_scenario_settings_parsing; TC =:= invalid_module_attributes -> meck:new(amoc_config_attributes, [no_link]), Config; +init_per_testcase(update_settings_propagate_global, Config) -> + %% set some slave nodes + meck:expect(amoc_cluster, slave_nodes, fun() -> ?SLAVE_NODES end), + Config; init_per_testcase(_, Config) -> Config. @@ -87,6 +102,16 @@ end_per_testcase(TC, Config) when TC =:= crash_during_scenario_settings_parsing; meck:unload(amoc_config_attributes), meck:reset(?MOCK_MOD), Config; +end_per_testcase(update_settings_global_on_slave, Config) -> + %% restore master_node expectation for subsequent tests + meck:expect(amoc_cluster, master_node, fun() -> node() end), + meck:reset(?MOCK_MOD), + Config; +end_per_testcase(update_settings_propagate_global, Config) -> + %% restore slave_nodes expectation for subsequent tests + meck:expect(amoc_cluster, slave_nodes, fun() -> [] end), + meck:reset(?MOCK_MOD), + Config; end_per_testcase(_, Config) -> meck:reset(?MOCK_MOD), Config. @@ -138,9 +163,33 @@ update_settings(_) -> meck:wait(?MOCK_MOD, update_mfa, [config_scenario_var2, new_val2], ?UPDATE_TIMEOUT), ?assertEqual(3, length(meck:history(?MOCK_MOD))). -update_just_one_parameter(_) -> +%% Test: changing global parameter on a non-master node must fail +update_settings_global_on_slave(_) -> set_initial_configuration(), + %% make this node a slave + meck:expect(amoc_cluster, master_node, fun() -> 'other@host' end), + Ret = amoc_config_scenario:update_settings([{config_scenario_var2, new_val2}]), + ?assertMatch({error, changing_global_parameters_on_a_slave_node, + [{config_scenario_var2, amoc_config_scenario_SUITE}]}, + Ret), + ok. +%% Test: when master, global parameters are propagated to slave nodes +update_settings_propagate_global(_) -> + set_initial_configuration(), + meck:reset(amoc_controller), + ?assertEqual(ok, amoc_config_scenario:update_settings([{config_scenario_var2, new_val2}])), + %% give spawned processes a moment to run + timer:sleep(100), + CallHistory = meck:history(amoc_controller), + ChangedParameters = ets:lookup(amoc_config, config_scenario_var2), + ExpectedArgs = [[N, ChangedParameters] || N <- ?SLAVE_NODES], + PropagateParams = [Args || {_Pid, {_Mod, Fun, Args}, _Ret} <- CallHistory, + Fun =:= propagate_config], + is_equal_list(ExpectedArgs, PropagateParams). + +update_just_one_parameter(_) -> + set_initial_configuration(), %% update only 1 parameter and check that update %% function is call for that parameter ?assertEqual(ok, amoc_config_scenario:update_settings([{config_scenario_var2, new_val2}])), @@ -231,8 +280,6 @@ update_settings_invalid_value(_) -> update_settings_undef_param(_) -> set_initial_configuration(), Table = ets:tab2list(amoc_config), - %% reset initial verification calls. - meck:reset(?MOCK_MOD), %% adding undefined parameter in settings ScenarioSettings3 = [{config_scenario_var3, new_val3}, {invalid_var2, val2}], @@ -323,4 +370,5 @@ set_initial_configuration() -> %% reset initial verification calls. assert_no_update_calls(), ?assertEqual(2, length(meck:history(?MOCK_MOD))), + %% reset initial verification calls. meck:reset(?MOCK_MOD). diff --git a/test/amoc_config_verification_SUITE.erl b/test/amoc_config_verification_SUITE.erl index f155d7ed..7e77774b 100644 --- a/test/amoc_config_verification_SUITE.erl +++ b/test/amoc_config_verification_SUITE.erl @@ -18,6 +18,15 @@ all() -> process_scenario_config_returns_error_for_invalid_values, process_scenario_config_returns_preprocessed_value]. +init_per_suite(Config) -> + meck:new(amoc_cluster, [non_strict, no_link]), + meck:expect(amoc_cluster, master_node, fun() -> node() end), + Config. + +end_per_suite(Config) -> + meck:unload(amoc_cluster), + Config. + process_scenario_config_uses_default_values(_) -> ScenarioConfig = correct_scenario_config(), given_scenario_parameters_not_set(ScenarioConfig), diff --git a/test/controller_SUITE.erl b/test/controller_SUITE.erl index fd0338ca..179749b9 100644 --- a/test/controller_SUITE.erl +++ b/test/controller_SUITE.erl @@ -117,12 +117,12 @@ no_scenario_running_status_is_idle(_) -> ?assertMatch(idle, Status). disable_controller_returns_status_disable(_) -> - amoc_controller:disable(), + amoc_controller:disable(testing_scenario, test_helpers:all_vars()), Status = amoc_controller:get_status(), ?assertMatch(disabled, Status). disable_controller_then_cant_start_scenario(_) -> - amoc_controller:disable(), + amoc_controller:disable(testing_scenario, test_helpers:all_vars()), Ret = do_start_scenario(testing_scenario), ?assertMatch({error, {invalid_status, disabled}}, Ret). diff --git a/test/throttle_SUITE.erl b/test/throttle_SUITE.erl index 66c4ef94..72ff79ce 100644 --- a/test/throttle_SUITE.erl +++ b/test/throttle_SUITE.erl @@ -6,65 +6,70 @@ -compile([export_all, nowarn_export_all]). -define(DEFAULT_INTERVAL, 60000). %% one minute --define(RECV(Msg, Timeout), receive Msg -> ok after Timeout -> {error, not_received_yet} end). +-define(RECV(Msg, Timeout), receive Msg -> ok after (Timeout + 50) -> {error, not_received_yet} end). all() -> [ - {group, api}, - {group, properties} + {group, properties}, + {group, api} ]. groups() -> [ - {api, [parallel], - [ - start, - start_descriptive, - start_interarrival, - start_interarrival_zero, - start_interarrival_infinity, - start_rate_zero, - start_rate_infinity, - start_interval_zero, - low_rate_does_not_get_remapped, - low_interval_does_not_get_remapped, - start_and_stop, - change_rate, - interval_equal_zero_limits_parallelism, - change_rate_to_interval_zero_limits_parallelism, - change_rate_triggers_parallelism, - change_rate_gradually, - change_interarrival_gradually, - change_rate_gradually_verify_descriptions, - just_wait, - wait_for_process_to_die_sends_a_kill, - async_runner_dies_while_waiting_raises_exit, - async_runner_dies_when_throttler_dies, - pause_and_resume, - get_state - ]}, - {properties, [], - [ - change_rate_gradually_verify_descriptions_properties, - % Note that the smallest delay possible for a process is 1ms (receive operations), - % hence if we give for example 10 workers 1ms delays, we get 600_000 ticks per minute. - % and if we give for example 48 workers 1ms delays, we get 2_880_000 ticks per minute. - % That means, that is realistically the maximum rate we could possibly manage - % with a static pool of such number of workers. - pool_config_is_precise_for_rates_1, - pool_config_is_precise_for_rates_2, - pool_config_is_precise_for_rates_3, - pool_config_is_precise_for_rates_4, - pool_config_is_precise_for_rates_5, - pool_config_is_precise_for_rates_6, - pool_config_is_precise_for_rates_7, - pool_config_is_precise_for_rates_8, - pool_config_is_precise_for_rates_9, - pool_config_is_precise_for_rates_10 - ]} + {api, [parallel, {repeat, 3}], api_testcases()}, + {properties, [], properties_testcases()} + ]. + +api_testcases() -> + [ + start, + start_descriptive, + start_interarrival, + start_interarrival_zero, + start_interarrival_infinity, + start_rate_zero, + start_rate_infinity, + start_interval_zero, + low_rate_does_not_get_remapped, + low_interval_does_not_get_remapped, + start_and_stop, + change_rate, + interval_equal_zero_limits_parallelism, + change_rate_to_interval_zero_limits_parallelism, + change_rate_triggers_parallelism, + change_rate_gradually, + change_interarrival_gradually, + change_rate_gradually_verify_descriptions, + just_wait, + wait_for_process_to_die_sends_a_kill, + async_runner_dies_while_waiting_raises_exit, + async_runner_dies_when_throttler_dies, + pause_and_resume, + get_state + ]. + +properties_testcases() -> + [ + change_rate_gradually_verify_descriptions_properties, + % Note that the smallest delay possible for a process is 1ms (receive operations), + % hence if we give for example 10 workers 1ms delays, we get 600_000 ticks per minute. + % and if we give for example 48 workers 1ms delays, we get 2_880_000 ticks per minute. + % That means, that is realistically the maximum rate we could possibly manage + % with a static pool of such number of workers. + pool_config_is_precise_for_rates_1, + pool_config_is_precise_for_rates_2, + pool_config_is_precise_for_rates_3, + pool_config_is_precise_for_rates_4, + pool_config_is_precise_for_rates_5, + pool_config_is_precise_for_rates_6, + pool_config_is_precise_for_rates_7, + pool_config_is_precise_for_rates_8, + pool_config_is_precise_for_rates_9, + pool_config_is_precise_for_rates_10 ]. init_per_suite(Config) -> + meck:new(amoc_throttle_config, [passthrough, non_strict, no_link]), application:ensure_all_started(amoc), amoc_cluster:set_master_node(node()), TelemetryEvents = [[amoc, throttle, Event] || Event <- [init, rate, request, execute, process]], @@ -74,18 +79,19 @@ init_per_suite(Config) -> end_per_suite(_) -> application:stop(amoc), telemetry_helpers:stop(), + meck:unload(), ok. init_per_group(properties, Config) -> - meck:new(amoc_throttle_config, [passthrough, non_strict, no_link]), - meck:expect(amoc_throttle_config, no_of_processes, [], 100), + ok = meck:expect(amoc_throttle_config, no_of_processes, [], 100), Config; init_per_group(_, Config) -> Config. end_per_group(properties, _Config) -> - meck:unload(amoc_throttle_config); + ok = meck:delete(amoc_throttle_config, no_of_processes, 0); end_per_group(_, _Config) -> + [amoc_throttle:stop(TC) || TC <- api_testcases() -- [async_runner_dies_when_throttler_dies]], ok. init_per_testcase(_, Config) ->