From 40b73fb199c8399cc156545022f02a7c82ca727b Mon Sep 17 00:00:00 2001 From: EMQ-YangM Date: Tue, 18 Jan 2022 13:52:24 +0800 Subject: [PATCH 1/6] fix(emqx_resource_api): remove unused module --- apps/emqx_resource/src/emqx_resource_api.erl | 23 -------------------- 1 file changed, 23 deletions(-) delete mode 100644 apps/emqx_resource/src/emqx_resource_api.erl diff --git a/apps/emqx_resource/src/emqx_resource_api.erl b/apps/emqx_resource/src/emqx_resource_api.erl deleted file mode 100644 index 0c1c42d26..000000000 --- a/apps/emqx_resource/src/emqx_resource_api.erl +++ /dev/null @@ -1,23 +0,0 @@ -%%-------------------------------------------------------------------- -%% Copyright (c) 2020-2022 EMQ Technologies Co., Ltd. All Rights Reserved. -%% -%% Licensed under the Apache License, Version 2.0 (the "License"); -%% you may not use this file except in compliance with the License. -%% You may obtain a copy of the License at -%% -%% http://www.apache.org/licenses/LICENSE-2.0 -%% -%% Unless required by applicable law or agreed to in writing, software -%% distributed under the License is distributed on an "AS IS" BASIS, -%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -%% See the License for the specific language governing permissions and -%% limitations under the License. -%%-------------------------------------------------------------------- --module(emqx_resource_api). - --export([stringify/1]). - -stringify(Bin) when is_binary(Bin) -> Bin; -stringify(Str) when is_list(Str) -> list_to_binary(Str); -stringify(Reason) -> - iolist_to_binary(io_lib:format("~p", [Reason])). From 11f76db67df1e13100a1450fd80ce40e2aae5755 Mon Sep 17 00:00:00 2001 From: EMQ-YangM Date: Wed, 19 Jan 2022 13:45:15 +0800 Subject: [PATCH 2/6] fix(emqx_resource_validator): remove unused code --- apps/emqx_resource/src/emqx_resource_validator.erl | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/apps/emqx_resource/src/emqx_resource_validator.erl b/apps/emqx_resource/src/emqx_resource_validator.erl index 05c98eeb2..4d745d1e3 100644 --- a/apps/emqx_resource/src/emqx_resource_validator.erl +++ b/apps/emqx_resource/src/emqx_resource_validator.erl @@ -18,8 +18,6 @@ -export([ min/2 , max/2 - , equals/2 - , enum/1 , not_empty/1 ]). @@ -29,15 +27,6 @@ max(Type, Max) -> min(Type, Min) -> limit(Type, '>=', Min). -equals(Type, Expected) -> - limit(Type, '==', Expected). - -enum(Items) -> - fun(Value) -> - return(lists:member(Value, Items), - err_limit({enum, {is_member_of, Items}, {got, Value}})) - end. - not_empty(ErrMsg) -> fun(<<>>) -> {error, ErrMsg}; (_) -> ok From 127384a9aeb66e7784029d2b6418aeffc5712225 Mon Sep 17 00:00:00 2001 From: EMQ-YangM Date: Wed, 19 Jan 2022 15:03:32 +0800 Subject: [PATCH 3/6] test(emqx_resource_SUITE): add more test --- .../test/emqx_resource_SUITE.erl | 100 +++++++++++++++++- .../emqx_resource/test/emqx_test_resource.erl | 3 + 2 files changed, 98 insertions(+), 5 deletions(-) diff --git a/apps/emqx_resource/test/emqx_resource_SUITE.erl b/apps/emqx_resource/test/emqx_resource_SUITE.erl index 6559f769c..d632523c5 100644 --- a/apps/emqx_resource/test/emqx_resource_SUITE.erl +++ b/apps/emqx_resource/test/emqx_resource_SUITE.erl @@ -35,12 +35,12 @@ init_per_testcase(_, Config) -> init_per_suite(Config) -> code:ensure_loaded(?TEST_RESOURCE), - ok = emqx_common_test_helpers:start_apps([]), + ok = emqx_common_test_helpers:start_apps([emqx_conf]), {ok, _} = application:ensure_all_started(emqx_resource), Config. end_per_suite(_Config) -> - ok = emqx_common_test_helpers:stop_apps([emqx_resource]). + ok = emqx_common_test_helpers:stop_apps([emqx_resource, emqx_conf]). %%------------------------------------------------------------------------------ %% Tests @@ -62,15 +62,53 @@ t_create_remove(_) -> ?TEST_RESOURCE, #{unknown => test_resource}), + {ok, _} = emqx_resource:create( + ?ID, + ?TEST_RESOURCE, + #{name => test_resource}), + + emqx_resource:recreate( + ?ID, + ?TEST_RESOURCE, + #{name => test_resource}, + #{}), + #{pid := Pid} = emqx_resource:query(?ID, get_state), + + ?assert(is_process_alive(Pid)), + + ok = emqx_resource:remove(?ID), + {error, _} = emqx_resource:remove(?ID), + + ?assertNot(is_process_alive(Pid)). + +t_create_remove_local(_) -> + {error, _} = emqx_resource:check_and_create_local( + ?ID, + ?TEST_RESOURCE, + #{unknown => test_resource}), + {ok, _} = emqx_resource:create_local( ?ID, ?TEST_RESOURCE, #{name => test_resource}), + emqx_resource:recreate_local( + ?ID, + ?TEST_RESOURCE, + #{name => test_resource}, + #{}), #{pid := Pid} = emqx_resource:query(?ID, get_state), ?assert(is_process_alive(Pid)), + emqx_resource:set_resource_status_stoped(?ID), + + emqx_resource:recreate_local( + ?ID, + ?TEST_RESOURCE, + #{name => test_resource}, + #{}), + ok = emqx_resource:remove_local(?ID), {error, _} = emqx_resource:remove_local(?ID), @@ -88,6 +126,8 @@ t_query(_) -> #{pid := _} = emqx_resource:query(?ID, get_state), #{pid := _} = emqx_resource:query(?ID, get_state, {[{Success, []}], [{Failure, []}]}), + #{pid := _} = emqx_resource:query(?ID, get_state, undefined), + #{pid := _} = emqx_resource:query(?ID, get_state_failed, undefined), receive Message -> ?assertEqual(success, Message) @@ -104,10 +144,14 @@ t_healthy(_) -> {ok, _} = emqx_resource:create_local( ?ID, ?TEST_RESOURCE, - #{name => <<"test_resource">>}, #{async_create => true}), + #{name => <<"test_resource">>}, + #{async_create => true}), timer:sleep(300), + emqx_resource_health_check:create_checker(?ID, 15000), #{pid := Pid} = emqx_resource:query(?ID, get_state), + timer:sleep(300), + emqx_resource:set_resource_status_stoped(?ID), ok = emqx_resource:health_check(?ID), @@ -128,15 +172,55 @@ t_healthy(_) -> ok = emqx_resource:remove_local(?ID). t_stop_start(_) -> + {error, _} = emqx_resource:check_and_create( + ?ID, + ?TEST_RESOURCE, + #{unknown => test_resource}), + + {ok, _} = emqx_resource:check_and_create( + ?ID, + ?TEST_RESOURCE, + #{<<"name">> => <<"test_resource">>}), + + {ok, _} = emqx_resource:check_and_recreate( + ?ID, + ?TEST_RESOURCE, + #{<<"name">> => <<"test_resource">>}, + #{}), + + #{pid := Pid0} = emqx_resource:query(?ID, get_state), + + ?assert(is_process_alive(Pid0)), + + ok = emqx_resource:stop(?ID), + + ?assertNot(is_process_alive(Pid0)), + + ?assertMatch({error, {emqx_resource, #{reason := stopped}}}, + emqx_resource:query(?ID, get_state)), + + ok = emqx_resource:restart(?ID), + + #{pid := Pid1} = emqx_resource:query(?ID, get_state), + + ?assert(is_process_alive(Pid1)). + +t_stop_start_local(_) -> {error, _} = emqx_resource:check_and_create_local( ?ID, ?TEST_RESOURCE, #{unknown => test_resource}), - {ok, _} = emqx_resource:create_local( + {ok, _} = emqx_resource:check_and_create_local( ?ID, ?TEST_RESOURCE, - #{name => test_resource}), + #{<<"name">> => <<"test_resource">>}), + + {ok, _} = emqx_resource:check_and_recreate_local( + ?ID, + ?TEST_RESOURCE, + #{<<"name">> => <<"test_resource">>}, + #{}), #{pid := Pid0} = emqx_resource:query(?ID, get_state), @@ -184,6 +268,12 @@ t_create_dry_run_local(_) -> ?assertEqual(undefined, whereis(test_resource)). +t_test_func(_) -> + ?assertEqual(ok, erlang:apply(emqx_resource_validator:not_empty("not_empty"), [<<"someval">>])), + ?assertEqual(ok, erlang:apply(emqx_resource_validator:min(int, 3), [4])), + ?assertEqual(ok, erlang:apply(emqx_resource_validator:max(array, 10), [[a,b,c,d]])), + ?assertEqual(ok, erlang:apply(emqx_resource_validator:max(string, 10), ["less10"])). + %%------------------------------------------------------------------------------ %% Helpers %%------------------------------------------------------------------------------ diff --git a/apps/emqx_resource/test/emqx_test_resource.erl b/apps/emqx_resource/test/emqx_test_resource.erl index 1d91d9c22..0d1a3b90d 100644 --- a/apps/emqx_resource/test/emqx_test_resource.erl +++ b/apps/emqx_resource/test/emqx_test_resource.erl @@ -55,6 +55,9 @@ on_stop(_InstId, #{pid := Pid}) -> on_query(_InstId, get_state, AfterQuery, State) -> emqx_resource:query_success(AfterQuery), + State; +on_query(_InstId, get_state_failed, AfterQuery, State) -> + emqx_resource:query_failed(AfterQuery), State. on_health_check(_InstId, State = #{pid := Pid}) -> From cb9f14f6582c3c301a4a2813c7d84f0f6ad59bb0 Mon Sep 17 00:00:00 2001 From: EMQ-YangM Date: Tue, 25 Jan 2022 14:54:40 +0800 Subject: [PATCH 4/6] feat(emqx_resource_health_check): add timeout params to health_check_timeout_checker --- .../src/emqx_resource_health_check.erl | 36 +++++++++---------- .../src/emqx_resource_instance.erl | 3 +- .../test/emqx_resource_SUITE.erl | 2 +- 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/apps/emqx_resource/src/emqx_resource_health_check.erl b/apps/emqx_resource/src/emqx_resource_health_check.erl index f50b53e63..da5492a33 100644 --- a/apps/emqx_resource/src/emqx_resource_health_check.erl +++ b/apps/emqx_resource/src/emqx_resource_health_check.erl @@ -15,38 +15,38 @@ %%-------------------------------------------------------------------- -module(emqx_resource_health_check). --export([ start_link/2 - , create_checker/2 +-export([ start_link/3 + , create_checker/3 , delete_checker/1 ]). --export([ start_health_check/2 - , health_check_timeout_checker/3 +-export([ start_health_check/3 + , health_check_timeout_checker/4 ]). -define(SUP, emqx_resource_health_check_sup). -define(ID(NAME), {resource_health_check, NAME}). -child_spec(Name, Sleep) -> +child_spec(Name, Sleep, Timeout) -> #{id => ?ID(Name), - start => {?MODULE, start_link, [Name, Sleep]}, + start => {?MODULE, start_link, [Name, Sleep, Timeout]}, restart => transient, shutdown => 5000, type => worker, modules => [?MODULE]}. -start_link(Name, Sleep) -> - Pid = proc_lib:spawn_link(?MODULE, start_health_check, [Name, Sleep]), +start_link(Name, Sleep, Timeout) -> + Pid = proc_lib:spawn_link(?MODULE, start_health_check, [Name, Sleep, Timeout]), {ok, Pid}. -create_checker(Name, Sleep) -> - create_checker(Name, Sleep, false). +create_checker(Name, Sleep, Timeout) -> + create_checker(Name, Sleep, false, Timeout). -create_checker(Name, Sleep, Retry) -> - case supervisor:start_child(?SUP, child_spec(Name, Sleep)) of +create_checker(Name, Sleep, Retry, Timeout) -> + case supervisor:start_child(?SUP, child_spec(Name, Sleep, Timeout)) of {ok, _} -> ok; {error, already_present} -> ok; {error, {already_started, _}} when Retry == false -> ok = delete_checker(Name), - create_checker(Name, Sleep, true); + create_checker(Name, Sleep, true, Timeout); Error -> Error end. @@ -56,9 +56,9 @@ delete_checker(Name) -> Error -> Error end. -start_health_check(Name, Sleep) -> +start_health_check(Name, Sleep, Timeout) -> Pid = self(), - _ = proc_lib:spawn_link(?MODULE, health_check_timeout_checker, [Pid, Name, Sleep]), + _ = proc_lib:spawn_link(?MODULE, health_check_timeout_checker, [Pid, Name, Sleep, Timeout]), health_check(Name). health_check(Name) -> @@ -75,12 +75,12 @@ health_check(Name) -> end, health_check(Name). -health_check_timeout_checker(Pid, Name, SleepTime) -> +health_check_timeout_checker(Pid, Name, SleepTime, Timeout) -> SelfPid = self(), Pid ! {SelfPid, begin_health_check}, receive health_check_finish -> timer:sleep(SleepTime) - after 10000 -> + after Timeout -> emqx_alarm:activate(Name, #{name => Name}, <>), emqx_resource:set_resource_status_stoped(Name), @@ -88,4 +88,4 @@ health_check_timeout_checker(Pid, Name, SleepTime) -> health_check_finish -> timer:sleep(SleepTime) end end, - health_check_timeout_checker(Pid, Name, SleepTime). + health_check_timeout_checker(Pid, Name, SleepTime, Timeout). diff --git a/apps/emqx_resource/src/emqx_resource_instance.erl b/apps/emqx_resource/src/emqx_resource_instance.erl index 6308a5d60..0cdeeae50 100644 --- a/apps/emqx_resource/src/emqx_resource_instance.erl +++ b/apps/emqx_resource/src/emqx_resource_instance.erl @@ -249,7 +249,8 @@ start_and_check(InstId, ResourceType, Config, Opts, Data) -> case maps:get(async_create, Opts, false) of false -> do_health_check(Data2); true -> emqx_resource_health_check:create_checker(InstId, - maps:get(health_check_interval, Opts, 15000)) + maps:get(health_check_interval, Opts, 15000), + maps:get(health_check_timeout, Opts, 10000)) end; {error, Reason} -> ets:insert(emqx_resource_instance, {InstId, Data#{status => stopped}}), diff --git a/apps/emqx_resource/test/emqx_resource_SUITE.erl b/apps/emqx_resource/test/emqx_resource_SUITE.erl index d632523c5..bea8b350f 100644 --- a/apps/emqx_resource/test/emqx_resource_SUITE.erl +++ b/apps/emqx_resource/test/emqx_resource_SUITE.erl @@ -148,7 +148,7 @@ t_healthy(_) -> #{async_create => true}), timer:sleep(300), - emqx_resource_health_check:create_checker(?ID, 15000), + emqx_resource_health_check:create_checker(?ID, 15000, 10000), #{pid := Pid} = emqx_resource:query(?ID, get_state), timer:sleep(300), emqx_resource:set_resource_status_stoped(?ID), From d312f315acfd1d9d5214da5106ce2634f6581287 Mon Sep 17 00:00:00 2001 From: EMQ-YangM Date: Tue, 25 Jan 2022 15:07:15 +0800 Subject: [PATCH 5/6] test(emqx_resource_health_check): add more test to health_check_timeout_checker --- apps/emqx_resource/test/emqx_resource_SUITE.erl | 10 ++++++++++ apps/emqx_resource/test/emqx_test_resource.erl | 1 + 2 files changed, 11 insertions(+) diff --git a/apps/emqx_resource/test/emqx_resource_SUITE.erl b/apps/emqx_resource/test/emqx_resource_SUITE.erl index bea8b350f..3a352d252 100644 --- a/apps/emqx_resource/test/emqx_resource_SUITE.erl +++ b/apps/emqx_resource/test/emqx_resource_SUITE.erl @@ -140,6 +140,16 @@ t_query(_) -> ok = emqx_resource:remove_local(?ID). +t_healthy_timeout(_) -> + {ok, _} = emqx_resource:create_local( + ?ID, + ?TEST_RESOURCE, + #{name => <<"test_resource">>}, + #{async_create => true, health_check_timeout => 200}), + timer:sleep(500), + + ok = emqx_resource:remove_local(?ID). + t_healthy(_) -> {ok, _} = emqx_resource:create_local( ?ID, diff --git a/apps/emqx_resource/test/emqx_test_resource.erl b/apps/emqx_resource/test/emqx_test_resource.erl index 0d1a3b90d..f185ded5b 100644 --- a/apps/emqx_resource/test/emqx_test_resource.erl +++ b/apps/emqx_resource/test/emqx_test_resource.erl @@ -61,6 +61,7 @@ on_query(_InstId, get_state_failed, AfterQuery, State) -> State. on_health_check(_InstId, State = #{pid := Pid}) -> + timer:sleep(300), case is_process_alive(Pid) of true -> {ok, State}; false -> {error, dead, State} From 8cfbdc2730d930835d40183e125449f5f120b824 Mon Sep 17 00:00:00 2001 From: EMQ-YangM Date: Tue, 25 Jan 2022 17:59:29 +0800 Subject: [PATCH 6/6] test(emqx_resource): improve emqx_resource test coverage to 80% --- .../emqx_resource/test/emqx_resource_SUITE.erl | 15 ++++++++++++++- apps/emqx_resource/test/emqx_test_resource.erl | 18 ++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/apps/emqx_resource/test/emqx_resource_SUITE.erl b/apps/emqx_resource/test/emqx_resource_SUITE.erl index 3a352d252..41945f295 100644 --- a/apps/emqx_resource/test/emqx_resource_SUITE.erl +++ b/apps/emqx_resource/test/emqx_resource_SUITE.erl @@ -156,7 +156,7 @@ t_healthy(_) -> ?TEST_RESOURCE, #{name => <<"test_resource">>}, #{async_create => true}), - timer:sleep(300), + timer:sleep(400), emqx_resource_health_check:create_checker(?ID, 15000, 10000), #{pid := Pid} = emqx_resource:query(?ID, get_state), @@ -278,6 +278,19 @@ t_create_dry_run_local(_) -> ?assertEqual(undefined, whereis(test_resource)). +t_create_dry_run_local_failed(_) -> + {Res, _} = emqx_resource:create_dry_run_local(?TEST_RESOURCE, + #{cteate_error => true}), + ?assertEqual(error, Res), + + {Res, _} = emqx_resource:create_dry_run_local(?TEST_RESOURCE, + #{name => test_resource, health_check_error => true}), + ?assertEqual(error, Res), + + {Res, _} = emqx_resource:create_dry_run_local(?TEST_RESOURCE, + #{name => test_resource, stop_error => true}), + ?assertEqual(error, Res). + t_test_func(_) -> ?assertEqual(ok, erlang:apply(emqx_resource_validator:not_empty("not_empty"), [<<"someval">>])), ?assertEqual(ok, erlang:apply(emqx_resource_validator:min(int, 3), [4])), diff --git a/apps/emqx_resource/test/emqx_test_resource.erl b/apps/emqx_resource/test/emqx_test_resource.erl index f185ded5b..1849e329b 100644 --- a/apps/emqx_resource/test/emqx_test_resource.erl +++ b/apps/emqx_resource/test/emqx_test_resource.erl @@ -43,12 +43,28 @@ register(nullable) -> false; register(default) -> false; register(_) -> undefined. +on_start(_InstId, #{create_error := true}) -> + error("some error"); +on_start(InstId, #{name := Name, stop_error := true} = Opts) -> + Register = maps:get(register, Opts, false), + {ok, #{name => Name, + id => InstId, + stop_error => true, + pid => spawn_dummy_process(Name, Register)}}; +on_start(InstId, #{name := Name, health_check_error := true} = Opts) -> + Register = maps:get(register, Opts, false), + {ok, #{name => Name, + id => InstId, + health_check_error => true, + pid => spawn_dummy_process(Name, Register)}}; on_start(InstId, #{name := Name} = Opts) -> Register = maps:get(register, Opts, false), {ok, #{name => Name, id => InstId, pid => spawn_dummy_process(Name, Register)}}. +on_stop(_InstId, #{stop_error := true}) -> + {error, stop_error}; on_stop(_InstId, #{pid := Pid}) -> erlang:exit(Pid, shutdown), ok. @@ -60,6 +76,8 @@ on_query(_InstId, get_state_failed, AfterQuery, State) -> emqx_resource:query_failed(AfterQuery), State. +on_health_check(_InstId, State = #{health_check_error := true}) -> + {error, dead, State}; on_health_check(_InstId, State = #{pid := Pid}) -> timer:sleep(300), case is_process_alive(Pid) of