From fb3bbd7ce7e6ecbc7c70f7d724cb041017a73937 Mon Sep 17 00:00:00 2001 From: Micael Karlberg Date: Fri, 24 May 2019 14:15:49 +0200 Subject: [snmp|test] Add more printouts to understand IPv6 issues OTP-15764 --- lib/snmp/test/snmp_test_mgr.erl | 17 ++++++++++++++--- lib/snmp/test/snmp_test_mgr_misc.erl | 3 ++- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/lib/snmp/test/snmp_test_mgr.erl b/lib/snmp/test/snmp_test_mgr.erl index 9190c07e6d..9d6be65088 100644 --- a/lib/snmp/test/snmp_test_mgr.erl +++ b/lib/snmp/test/snmp_test_mgr.erl @@ -247,10 +247,21 @@ init({Options, CallerPid}) -> IpFamily = get_value(ipfamily, Options, inet), print("[~w] ~p -> IpFamily: ~p~n", [?MODULE, self(), IpFamily]), AgIp = case snmp_misc:assq(agent, Options) of - {value, Tuple4} when is_tuple(Tuple4) andalso - (size(Tuple4) =:= 4) -> - Tuple4; + {value, Addr} when is_tuple(Addr) andalso + (size(Addr) =:= 4) andalso + (IpFamily =:= inet) -> + print("[~w] ~p -> Addr: ~p~n", + [?MODULE, self(), Addr]), + Addr; + {value, Addr} when is_tuple(Addr) andalso + (size(Addr) =:= 8) andalso + (IpFamily =:= inet6) -> + print("[~w] ~p -> Addr: ~p~n", + [?MODULE, self(), Addr]), + Addr; {value, Host} when is_list(Host) -> + print("[~w] ~p -> Host: ~p~n", + [?MODULE, self(), Host]), {ok, Ip} = snmp_misc:ip(Host, IpFamily), Ip end, diff --git a/lib/snmp/test/snmp_test_mgr_misc.erl b/lib/snmp/test/snmp_test_mgr_misc.erl index 315e3ebd9e..6608a88c00 100644 --- a/lib/snmp/test/snmp_test_mgr_misc.erl +++ b/lib/snmp/test/snmp_test_mgr_misc.erl @@ -576,6 +576,7 @@ vsn('version-2') -> v2c. udp_send(UdpId, AgentIp, UdpPort, B) -> + ?vlog("attempt send message (~w bytes) to ~p", [sz(B), {AgentIp, UdpPort}]), case (catch gen_udp:send(UdpId, AgentIp, UdpPort, B)) of {error,ErrorReason} -> error("failed (error) sending message to ~p:~p: " @@ -880,7 +881,7 @@ display_prop_hdr(S) -> %%---------------------------------------------------------------------- sz(L) when is_list(L) -> - length(lists:flatten(L)); + iolist_size(L); sz(B) when is_binary(B) -> size(B); sz(O) -> -- cgit v1.2.3 From 9261b5e9b56019fdba8426f1fb0aa1ef1dc2d18a Mon Sep 17 00:00:00 2001 From: Micael Karlberg Date: Tue, 28 May 2019 12:15:03 +0200 Subject: [snmp|test] Improve the test case start The function used by "all" the agent test cases to actually run the operations have been improved. There was previously very little monitoring of the result. Have added some (minor) checks, both before trying to running the test case, and "during". Such as, is the node we attempt to use actually alive. Then, when we spawn the test case runner process on the (remote) node, make it report back before trying to run the actuall test case (so we know that the spawn worked. Also added a monitor of the process, so that we will detect fatal errors. OTP-15764 --- lib/snmp/test/snmp_agent_test_lib.erl | 183 ++++++++++++++++++++++------------ lib/snmp/test/snmp_test_lib.hrl | 2 + 2 files changed, 119 insertions(+), 66 deletions(-) diff --git a/lib/snmp/test/snmp_agent_test_lib.erl b/lib/snmp/test/snmp_agent_test_lib.erl index 6defdadb5a..c1fcaf27d6 100644 --- a/lib/snmp/test/snmp_agent_test_lib.erl +++ b/lib/snmp/test/snmp_agent_test_lib.erl @@ -66,7 +66,7 @@ ]). %% Internal exports --export([wait/5, run/4]). +-export([tc_wait/5, tc_run/4]). -include_lib("kernel/include/file.hrl"). -include_lib("common_test/include/ct.hrl"). @@ -277,35 +277,80 @@ init_case(Config) when is_list(Config) -> %%%-------------------------------------------------- try_test(Mod, Func) -> - call(get(mgr_node), ?MODULE, run, [Mod, Func, [], []]). + tc_try(get(mgr_node), ?MODULE, tc_run, [Mod, Func, [], []]). try_test(Mod, Func, A) -> - call(get(mgr_node), ?MODULE, run, [Mod, Func, A, []]). + tc_try(get(mgr_node), ?MODULE, tc_run, [Mod, Func, A, []]). try_test(Mod, Func, A, Opts) -> - call(get(mgr_node), ?MODULE, run, [Mod, Func, A, Opts]). - -call(N,M,F,A) -> - ?DBG("call -> entry with~n" - " N: ~p~n" - " M: ~p~n" - " F: ~p~n" - " A: ~p~n" - " when~n" - " get(): ~p", - [N,M,F,A,get()]), - spawn(N, ?MODULE, wait, [self(),get(),M,F,A]), + tc_try(get(mgr_node), ?MODULE, tc_run, [Mod, Func, A, Opts]). + +%% We spawn a test case runner process on the manager node. +%% The assumption is that the manager shall do something, but +%% not all test cases have the manager perform actions. +%% In some cases we make a rpc call back to the agent node directly +%% and call something in the agent... (for example the info_test +%% test case). +tc_try(N, M, F, A) -> + ?PRINT2("tc_try -> entry with" + "~n N: ~p" + "~n M: ~p" + "~n F: ~p" + "~n A: ~p" + "~n when" + "~n get(): ~p" + "~n", [N, + M, F, A, + get()]), + case net_adm:ping(N) of + pong -> + ?PRINT2("tc_try -> ~p still running~n", [N]), + Runner = spawn(N, ?MODULE, tc_wait, [self(), get(), M, F, A]), + MRef = erlang:monitor(process, Runner), + await_tc_runner_started(Runner, MRef), + await_tc_runner_done(Runner, MRef); + pang -> + ?EPRINT2("tc_try -> ~p *not* running~n", [N]), + exit({node_not_running, N}) + end. + +await_tc_runner_started(Runner, MRef) -> receive - {done, {'EXIT', Rn}, Loc} -> - ?DBG("call -> done with exit: " - "~n Rn: ~p" - "~n Loc: ~p", [Rn, Loc]), + {'DOWN', MRef, process, Runner, Reason} -> + ?EPRINT2("TC runner start failed: " + "~n ~p~n", [Reason]), + exit({tx_runner_start_failed, Reason}); + {tc_runner_started, Runner} -> + ?PRINT2("TC runner start acknowledged~n"), + ok + after 10000 -> + erlang:demonitor(MRef, [flush]), + RunnerInfo = process_info(Runner), + ?EPRINT2("TC runner start timeout: " + "~n ~p", [RunnerInfo]), + %% If we don't get a start ack within 10 seconds, we are f*ed + exit(Runner, kill), + exit({tc_runner_start, timeout, RunnerInfo}) + end. + +await_tc_runner_done(Runner, MRef) -> + receive + {'DOWN', MRef, process, Runner, Reason} -> + ?EPRINT2("TC runner failed: " + "~n ~p~n", [Reason]), + exit({tx_runner_failed, Reason}); + {tc_runner_done, Runner, {'EXIT', Rn}, Loc} -> + ?PRINT2("call -> done with exit: " + "~n Rn: ~p" + "~n Loc: ~p" + "~n", [Rn, Loc]), put(test_server_loc, Loc), exit(Rn); - {done, Ret, _Zed} -> + {tc_runner_done, Runner, Ret, _Zed} -> ?DBG("call -> done:" "~n Ret: ~p" "~n Zed: ~p", [Ret, _Zed]), + erlang:demonitor(MRef, [flush]), case Ret of {error, Reason} -> exit(Reason); @@ -313,50 +358,56 @@ call(N,M,F,A) -> OK end end. - -wait(From, Env, M, F, A) -> - ?DBG("wait -> entry with" - "~n From: ~p" - "~n Env: ~p" - "~n M: ~p" - "~n F: ~p" - "~n A: ~p", [From, Env, M, F, A]), + + +tc_wait(From, Env, M, F, A) -> + ?PRINT2("tc_wait -> entry with" + "~n From: ~p" + "~n Env: ~p" + "~n M: ~p" + "~n F: ~p" + "~n A: ~p", [From, Env, M, F, A]), + From ! {tc_runner_started, self()}, lists:foreach(fun({K,V}) -> put(K,V) end, Env), - Rn = (catch apply(M, F, A)), - ?DBG("wait -> Rn: ~n~p", [Rn]), - From ! {done, Rn, get(test_server_loc)}, - exit(Rn). - -run(Mod, Func, Args, Opts) -> - ?DBG("run -> entry with" - "~n Mod: ~p" - "~n Func: ~p" - "~n Args: ~p" - "~n Opts: ~p", [Mod, Func, Args, Opts]), - M = get(mib_dir), - Dir = get(mgr_dir), - User = snmp_misc:get_option(user, Opts, "all-rights"), - SecLevel = snmp_misc:get_option(sec_level, Opts, noAuthNoPriv), - EngineID = snmp_misc:get_option(engine_id, Opts, "agentEngine"), + ?PRINT2("tc_wait -> env set - now run tc~n"), + Res = (catch apply(M, F, A)), + ?PRINT2("tc_wait -> tc run done: " + "~n ~p" + "~n", [Res]), + From ! {tc_runner_done, self(), Res, get(test_server_loc)}, + exit(Res). + +tc_run(Mod, Func, Args, Opts) -> + ?PRINT2("tc_run -> entry with" + "~n Mod: ~p" + "~n Func: ~p" + "~n Args: ~p" + "~n Opts: ~p" + "~n", [Mod, Func, Args, Opts]), + (catch snmp_test_mgr:stop()), % If we had a running mgr from a failed case + M = get(mib_dir), + Dir = get(mgr_dir), + User = snmp_misc:get_option(user, Opts, "all-rights"), + SecLevel = snmp_misc:get_option(sec_level, Opts, noAuthNoPriv), + EngineID = snmp_misc:get_option(engine_id, Opts, "agentEngine"), CtxEngineID = snmp_misc:get_option(context_engine_id, Opts, EngineID), - Community = snmp_misc:get_option(community, Opts, "all-rights"), - ?DBG("run -> start crypto app",[]), - _CryptoRes = ?CRYPTO_START(), - ?DBG("run -> Crypto: ~p", [_CryptoRes]), - catch snmp_test_mgr:stop(), % If we had a running mgr from a failed case - StdM = join(code:priv_dir(snmp), "mibs") ++ "/", - Vsn = get(vsn), - ?DBG("run -> config:" - "~n M: ~p" - "~n Vsn: ~p" - "~n Dir: ~p" - "~n User: ~p" - "~n SecLevel: ~p" - "~n EngineID: ~p" - "~n CtxEngineID: ~p" - "~n Community: ~p" - "~n StdM: ~p", - [M,Vsn,Dir,User,SecLevel,EngineID,CtxEngineID,Community,StdM]), + Community = snmp_misc:get_option(community, Opts, "all-rights"), + ?DBG("tc_run -> start crypto app",[]), + _CryptoRes = ?CRYPTO_START(), + ?DBG("tc_run -> Crypto: ~p", [_CryptoRes]), + StdM = join(code:priv_dir(snmp), "mibs") ++ "/", + Vsn = get(vsn), + ?PRINT2("tc_run -> config:" + "~n M: ~p" + "~n Vsn: ~p" + "~n Dir: ~p" + "~n User: ~p" + "~n SecLevel: ~p" + "~n EngineID: ~p" + "~n CtxEngineID: ~p" + "~n Community: ~p" + "~n StdM: ~p" + "~n", [M,Vsn,Dir,User,SecLevel,EngineID,CtxEngineID,Community,StdM]), case snmp_test_mgr:start([%% {agent, snmp_test_lib:hostname()}, {packet_server_debug, true}, {debug, true}, @@ -377,23 +428,23 @@ run(Mod, Func, Args, Opts) -> {ok, _Pid} -> case (catch apply(Mod, Func, Args)) of {'EXIT', Reason} -> - catch snmp_test_mgr:stop(), + (catch snmp_test_mgr:stop()), ?FAIL({apply_failed, {Mod, Func, Args}, Reason}); Res -> - catch snmp_test_mgr:stop(), + (catch snmp_test_mgr:stop()), Res end; {error, Reason} -> ?EPRINT2("Failed starting (test) manager: " "~n ~p", [Reason]), - catch snmp_test_mgr:stop(), + (catch snmp_test_mgr:stop()), ?line ?FAIL({mgr_start_error, Reason}); Err -> ?EPRINT2("Failed starting (test) manager: " "~n ~p", [Err]), - catch snmp_test_mgr:stop(), + (catch snmp_test_mgr:stop()), ?line ?FAIL({mgr_start_failure, Err}) end. diff --git a/lib/snmp/test/snmp_test_lib.hrl b/lib/snmp/test/snmp_test_lib.hrl index 335f3fff3c..dafa4cbf19 100644 --- a/lib/snmp/test/snmp_test_lib.hrl +++ b/lib/snmp/test/snmp_test_lib.hrl @@ -150,8 +150,10 @@ snmp_test_lib:print(P, ?MODULE, ?LINE, F, A)). -define(PRINT1(F, A), snmp_test_lib:print1(F, A)). +-define(PRINT1(F), ?PRINT1(F, [])). -define(EPRINT1(F, A), ?PRINT1(" " ++ F, A)). -define(PRINT2(F, A), snmp_test_lib:print2(F, A)). +-define(PRINT2(F), ?PRINT2(F, [])). -define(EPRINT2(F, A), ?PRINT2(" " ++ F, A)). -- cgit v1.2.3 From 6c2bb8d54f774970a30365e3ee94af66e099886b Mon Sep 17 00:00:00 2001 From: Micael Karlberg Date: Tue, 28 May 2019 15:15:35 +0200 Subject: [snmp|test] Replaced monitor with link Replaced the monitor (to the tc runner process) with a link. The point is that if the test case "stalls", the ts (ct) framework shall kill it (with the test case process). OTP-15764 --- lib/snmp/test/snmp_agent_test_lib.erl | 62 ++++++++++++++++++++++++----------- 1 file changed, 43 insertions(+), 19 deletions(-) diff --git a/lib/snmp/test/snmp_agent_test_lib.erl b/lib/snmp/test/snmp_agent_test_lib.erl index c1fcaf27d6..1128fc8a8c 100644 --- a/lib/snmp/test/snmp_agent_test_lib.erl +++ b/lib/snmp/test/snmp_agent_test_lib.erl @@ -276,14 +276,18 @@ init_case(Config) when is_list(Config) -> %%% configuration. %%%-------------------------------------------------- -try_test(Mod, Func) -> - tc_try(get(mgr_node), ?MODULE, tc_run, [Mod, Func, [], []]). +try_test(TcRunMod, TcRunFunc) -> + try_test(TcRunMod, TcRunFunc, []). -try_test(Mod, Func, A) -> - tc_try(get(mgr_node), ?MODULE, tc_run, [Mod, Func, A, []]). +try_test(TcRunMod, TcRunFunc, TcRunArgs) -> + try_test(TcRunMod, TcRunFunc, TcRunArgs, []). -try_test(Mod, Func, A, Opts) -> - tc_try(get(mgr_node), ?MODULE, tc_run, [Mod, Func, A, Opts]). +try_test(TcRunMod, TcRunFunc, TcRunArgs, TcRunOpts) -> + Node = get(mgr_node), + Mod = ?MODULE, + Func = tc_run, + Args = [TcRunMod, TcRunFunc, TcRunArgs, TcRunOpts], + tc_try(Node, Mod, Func, Args). %% We spawn a test case runner process on the manager node. %% The assumption is that the manager shall do something, but @@ -291,6 +295,10 @@ try_test(Mod, Func, A, Opts) -> %% In some cases we make a rpc call back to the agent node directly %% and call something in the agent... (for example the info_test %% test case). +%% We should use link (instead of monitor) in order for the test case +%% timeout cleanup (kills) should have effect on the test case runner +%% process as well. + tc_try(N, M, F, A) -> ?PRINT2("tc_try -> entry with" "~n N: ~p" @@ -304,19 +312,20 @@ tc_try(N, M, F, A) -> get()]), case net_adm:ping(N) of pong -> - ?PRINT2("tc_try -> ~p still running~n", [N]), - Runner = spawn(N, ?MODULE, tc_wait, [self(), get(), M, F, A]), - MRef = erlang:monitor(process, Runner), - await_tc_runner_started(Runner, MRef), - await_tc_runner_done(Runner, MRef); + ?PRINT2("tc_try -> ~p still running - start runner~n", [N]), + OldFlag = trap_exit(true), % Make sure we catch it + Runner = spawn_link(N, ?MODULE, tc_wait, [self(), get(), M, F, A]), + await_tc_runner_started(Runner, OldFlag), + await_tc_runner_done(Runner, OldFlag); pang -> ?EPRINT2("tc_try -> ~p *not* running~n", [N]), exit({node_not_running, N}) end. -await_tc_runner_started(Runner, MRef) -> +await_tc_runner_started(Runner, OldFlag) -> + ?PRINT2("await tc-runner (~p) start ack~n", [Runner]), receive - {'DOWN', MRef, process, Runner, Reason} -> + {'EXIT', Runner, Reason} -> ?EPRINT2("TC runner start failed: " "~n ~p~n", [Reason]), exit({tx_runner_start_failed, Reason}); @@ -324,7 +333,8 @@ await_tc_runner_started(Runner, MRef) -> ?PRINT2("TC runner start acknowledged~n"), ok after 10000 -> - erlang:demonitor(MRef, [flush]), + trap_exit(OldFlag), + unlink_and_flush_exit(Runner), RunnerInfo = process_info(Runner), ?EPRINT2("TC runner start timeout: " "~n ~p", [RunnerInfo]), @@ -333,9 +343,9 @@ await_tc_runner_started(Runner, MRef) -> exit({tc_runner_start, timeout, RunnerInfo}) end. -await_tc_runner_done(Runner, MRef) -> +await_tc_runner_done(Runner, OldFlag) -> receive - {'DOWN', MRef, process, Runner, Reason} -> + {'EXIT', Runner, Reason} -> ?EPRINT2("TC runner failed: " "~n ~p~n", [Reason]), exit({tx_runner_failed, Reason}); @@ -344,13 +354,16 @@ await_tc_runner_done(Runner, MRef) -> "~n Rn: ~p" "~n Loc: ~p" "~n", [Rn, Loc]), + trap_exit(OldFlag), + unlink_and_flush_exit(Runner), put(test_server_loc, Loc), exit(Rn); {tc_runner_done, Runner, Ret, _Zed} -> ?DBG("call -> done:" "~n Ret: ~p" "~n Zed: ~p", [Ret, _Zed]), - erlang:demonitor(MRef, [flush]), + trap_exit(OldFlag), + unlink_and_flush_exit(Runner), case Ret of {error, Reason} -> exit(Reason); @@ -358,8 +371,19 @@ await_tc_runner_done(Runner, MRef) -> OK end end. - - + +trap_exit(Flag) when is_boolean(Flag) -> + erlang:process_flag(trap_exit, Flag). + +unlink_and_flush_exit(Pid) -> + unlink(Pid), + receive + {'EXIT', Pid, _} -> + ok + after 0 -> + ok + end. + tc_wait(From, Env, M, F, A) -> ?PRINT2("tc_wait -> entry with" "~n From: ~p" -- cgit v1.2.3 From 36e61f1e23bf06664206aaba1848e36440a36e4a Mon Sep 17 00:00:00 2001 From: Micael Karlberg Date: Wed, 17 Apr 2019 17:39:54 +0200 Subject: [snmp|test] Maybe skipable (IPv6) groups Add an "os test" to the IPv6 group init. On "old" version of darwin (9.8.0) its simply to messy to figure out our IPv6 address, so its better to simply skip the IPv6 tests on those machines. OTP-15764 --- lib/snmp/test/snmp_agent_test.erl | 55 +++++++++++++++++------- lib/snmp/test/snmp_manager_test.erl | 70 ++++++++++++++++++------------ lib/snmp/test/snmp_test_lib.erl | 86 ++++++++++++++++++++++--------------- 3 files changed, 132 insertions(+), 79 deletions(-) diff --git a/lib/snmp/test/snmp_agent_test.erl b/lib/snmp/test/snmp_agent_test.erl index 71e3fa3b9a..fa7db87c38 100644 --- a/lib/snmp/test/snmp_agent_test.erl +++ b/lib/snmp/test/snmp_agent_test.erl @@ -667,22 +667,45 @@ init_per_group(GroupName, Config) -> snmp_test_lib:init_group_top_dir(GroupName, Config). init_per_group_ipv6(GroupName, Config, Init) -> - {ok, Hostname0} = inet:gethostname(), - case ct:require(ipv6_hosts) of - ok -> - case lists:member(list_to_atom(Hostname0), ct:get_config(ipv6_hosts)) of - true -> - Init( - snmp_test_lib:init_group_top_dir( - GroupName, - [{ipfamily, inet6}, - {ip, ?LOCALHOST(inet6)} - | lists:keydelete(ip, 1, Config)])); - false -> - {skip, "Host does not support IPV6"} - end; - _ -> - {skip, "Test config ipv6_hosts is missing"} + %% + %% This is a higly questionable test. + %% But until we have time to figure out what IPv6 issues + %% are actually causing the failures... + OSSkipable = [{unix, + [ + {darwin, fun(V) when (V > {9, 8, 0}) -> + %% This version is OK: No Skip + false; + (_) -> + %% This version is *not* ok: Skip + true + end} + ] + }], + %% + case ?OS_BASED_SKIP(OSSkipable) of + true -> + {skip, "Host *may* not *properly* support IPV6"}; + false -> + %% And now for the "proper" test... + case ct:require(ipv6_hosts) of + ok -> + {ok, Hostname0} = inet:gethostname(), + case lists:member(list_to_atom(Hostname0), + ct:get_config(ipv6_hosts)) of + true -> + Init( + snmp_test_lib:init_group_top_dir( + GroupName, + [{ipfamily, inet6}, + {ip, ?LOCALHOST(inet6)} + | lists:keydelete(ip, 1, Config)])); + false -> + {skip, "Host does not support IPV6"} + end; + _ -> + {skip, "Test config ipv6_hosts is missing"} + end end. end_per_group(all_tcs, Config) -> diff --git a/lib/snmp/test/snmp_manager_test.erl b/lib/snmp/test/snmp_manager_test.erl index 5b0ebf8647..8fd325192b 100644 --- a/lib/snmp/test/snmp_manager_test.erl +++ b/lib/snmp/test/snmp_manager_test.erl @@ -619,38 +619,52 @@ init_per_group(event_tests_mt = GroupName, Config) -> GroupName, [{manager_net_if_module, snmpm_net_if_mt} | Config]); init_per_group(ipv6_mt = GroupName, Config) -> - {ok, Hostname0} = inet:gethostname(), - case ct:require(ipv6_hosts) of - ok -> - case lists:member(list_to_atom(Hostname0), ct:get_config(ipv6_hosts)) of - true -> - ipv6_init( - snmp_test_lib:init_group_top_dir( - GroupName, - [{manager_net_if_module, snmpm_net_if_mt} - | Config])); - false -> - {skip, "Host does not support IPv6"} - end; - _ -> - {skip, "Test config ipv6_hosts is missing"} - end; + init_per_group_ipv6(GroupName, + [{manager_net_if_module, snmpm_net_if_mt} | Config]); init_per_group(ipv6 = GroupName, Config) -> - {ok, Hostname0} = inet:gethostname(), - case ct:require(ipv6_hosts) of - ok -> - case lists:member(list_to_atom(Hostname0), ct:get_config(ipv6_hosts)) of - true -> - ipv6_init(snmp_test_lib:init_group_top_dir(GroupName, Config)); - false -> - {skip, "Host does not support IPv6"} - end; - _ -> - {skip, "Test config ipv6_hosts is missing"} - end; + init_per_group_ipv6(GroupName, Config); init_per_group(GroupName, Config) -> snmp_test_lib:init_group_top_dir(GroupName, Config). + +init_per_group_ipv6(GroupName, Config) -> + %% + OSSkipable = [{unix, + [ + {darwin, fun(V) when (V > {9, 8, 0}) -> + %% This version is OK: No Skip + false; + (_) -> + %% This version is *not* ok: Skip + %% We need a fully qualified hostname + %% to get a proper IPv6 address (in this + %% version), but its just to messy, so + %% instead we skip this **OLD** darwin... + true + end} + ] + }], + %% + case ?OS_BASED_SKIP(OSSkipable) of + true -> + {skip, "Host *may* not *properly* support IPV6"}; + false -> + {ok, Hostname} = inet:gethostname(), + case ct:require(ipv6_hosts) of + ok -> + case lists:member(list_to_atom(Hostname), + ct:get_config(ipv6_hosts)) of + true -> + ipv6_init(snmp_test_lib:init_group_top_dir(GroupName, + Config)); + false -> + {skip, "Host does not support IPv6"} + end; + _ -> + {skip, "Test config ipv6_hosts is missing"} + end + end. + end_per_group(_GroupName, Config) -> %% Do we really need to do this? lists:keydelete(snmp_group_top_dir, 1, Config). diff --git a/lib/snmp/test/snmp_test_lib.erl b/lib/snmp/test/snmp_test_lib.erl index a483690653..9fab1a5c56 100644 --- a/lib/snmp/test/snmp_test_lib.erl +++ b/lib/snmp/test/snmp_test_lib.erl @@ -207,54 +207,70 @@ non_pc_tc_maybe_skip(Config, Condition, File, Line) end. +%% The type and spec'ing is just to increase readability +-type os_family() :: win32 | unix. +-type os_name() :: atom(). +-type os_version() :: string() | {non_neg_integer(), + non_neg_integer(), + non_neg_integer()}. +-type os_skip_check() :: fun(() -> boolean()) | + fun((os_version()) -> boolean()). +-type skippable() :: any | [os_family() | + {os_family(), os_name() | + [os_name() | {os_name(), + os_skip_check()}]}]. + +-spec os_based_skip(skippable()) -> boolean(). + os_based_skip(any) -> - io:format("os_based_skip(any) -> entry" - "~n", []), true; os_based_skip(Skippable) when is_list(Skippable) -> - io:format("os_based_skip -> entry with" - "~n Skippable: ~p" - "~n", [Skippable]), - {OsFam, OsName} = - case os:type() of - {_Fam, _Name} = FamAndName -> - FamAndName; - Fam -> - {Fam, undefined} - end, - io:format("os_based_skip -> os-type: " - "~n OsFam: ~p" - "~n OsName: ~p" - "~n", [OsFam, OsName]), + os_base_skip(Skippable, os:type()); +os_based_skip(_Crap) -> + false. + +os_base_skip(Skippable, {OsFam, OsName}) -> + os_base_skip(Skippable, OsFam, OsName); +os_base_skip(Skippable, OsFam) -> + os_base_skip(Skippable, OsFam, undefined). + +os_base_skip(Skippable, OsFam, OsName) -> + %% Check if the entire family is to be skipped + %% Example: [win32, unix] case lists:member(OsFam, Skippable) of true -> true; false -> - case lists:keysearch(OsFam, 1, Skippable) of - {value, {OsFam, OsName}} -> - true; - {value, {OsFam, OsNames}} when is_list(OsNames) -> + %% Example: [{unix, freebsd}] | [{unix, [freebsd, darwin]}] + case lists:keysearch(OsFam, 1, Skippable) of + {value, {OsFam, OsName}} -> + true; + {value, {OsFam, OsNames}} when is_list(OsNames) -> + %% OsNames is a list of: + %% [atom()|{atom(), function/0 | function/1}] case lists:member(OsName, OsNames) of true -> true; false -> - case lists:keymember(OsName, 1, OsNames) of - {value, {OsName, Check}} when is_function(Check) -> - Check(); - _ -> - false - end + os_based_skip_check(OsName, OsNames) end; - _ -> - false - end - end; -os_based_skip(_Crap) -> - io:format("os_based_skip -> entry with" - "~n _Crap: ~p" - "~n", [_Crap]), - false. + _ -> + false + end + end. +%% Performs a check via a provided fun with arity 0 or 1. +%% The argument is the result of os:version(). +os_based_skip_check(OsName, OsNames) -> + case lists:keysearch(OsName, 1, OsNames) of + {value, {OsName, Check}} when is_function(Check, 0) -> + Check(); + {value, {OsName, Check}} when is_function(Check, 1) -> + Check(os:version()); + _ -> + false + end. + %% ---------------------------------------------------------------- %% Test suite utility functions -- cgit v1.2.3 From e628d2cdf8953b8692e6b9ac0265c20bfce4b21d Mon Sep 17 00:00:00 2001 From: Micael Karlberg Date: Wed, 29 May 2019 12:10:56 +0200 Subject: [snmp|test] Improved IPv6 test checks Improve the checks for if/when we shall run the IPv6 test cases. OTP-15764 --- lib/snmp/test/snmp_agent_test.erl | 30 ++++++++----------- lib/snmp/test/snmp_manager_test.erl | 23 ++++++--------- lib/snmp/test/snmp_test_lib.erl | 58 ++++++++++++++++++++++++++++++++----- lib/snmp/test/snmp_test_lib.hrl | 8 +++-- 4 files changed, 78 insertions(+), 41 deletions(-) diff --git a/lib/snmp/test/snmp_agent_test.erl b/lib/snmp/test/snmp_agent_test.erl index fa7db87c38..860ca17cdb 100644 --- a/lib/snmp/test/snmp_agent_test.erl +++ b/lib/snmp/test/snmp_agent_test.erl @@ -687,24 +687,18 @@ init_per_group_ipv6(GroupName, Config, Init) -> true -> {skip, "Host *may* not *properly* support IPV6"}; false -> - %% And now for the "proper" test... - case ct:require(ipv6_hosts) of - ok -> - {ok, Hostname0} = inet:gethostname(), - case lists:member(list_to_atom(Hostname0), - ct:get_config(ipv6_hosts)) of - true -> - Init( - snmp_test_lib:init_group_top_dir( - GroupName, - [{ipfamily, inet6}, - {ip, ?LOCALHOST(inet6)} - | lists:keydelete(ip, 1, Config)])); - false -> - {skip, "Host does not support IPV6"} - end; - _ -> - {skip, "Test config ipv6_hosts is missing"} + %% Even if this host supports IPv6 we don't use it unless its + %% one of the configured/supported IPv6 hosts... + case (?HAS_SUPPORT_IPV6() andalso ?IS_IPV6_HOST()) of + true -> + Init( + snmp_test_lib:init_group_top_dir( + GroupName, + [{ipfamily, inet6}, + {ip, ?LOCALHOST(inet6)} + | lists:keydelete(ip, 1, Config)])); + false -> + {skip, "Host does not support IPv6"} end end. diff --git a/lib/snmp/test/snmp_manager_test.erl b/lib/snmp/test/snmp_manager_test.erl index 8fd325192b..d959d9e09b 100644 --- a/lib/snmp/test/snmp_manager_test.erl +++ b/lib/snmp/test/snmp_manager_test.erl @@ -649,22 +649,17 @@ init_per_group_ipv6(GroupName, Config) -> true -> {skip, "Host *may* not *properly* support IPV6"}; false -> - {ok, Hostname} = inet:gethostname(), - case ct:require(ipv6_hosts) of - ok -> - case lists:member(list_to_atom(Hostname), - ct:get_config(ipv6_hosts)) of - true -> - ipv6_init(snmp_test_lib:init_group_top_dir(GroupName, - Config)); - false -> - {skip, "Host does not support IPv6"} - end; - _ -> - {skip, "Test config ipv6_hosts is missing"} + %% Even if this host supports IPv6 we don't use it unless its + %% one of the configures/supported IPv6 hosts... + case (?HAS_SUPPORT_IPV6() andalso ?IS_IPV6_HOST()) of + true -> + ipv6_init(snmp_test_lib:init_group_top_dir(GroupName, Config)); + false -> + {skip, "Host does not support IPv6"} end end. - + + end_per_group(_GroupName, Config) -> %% Do we really need to do this? lists:keydelete(snmp_group_top_dir, 1, Config). diff --git a/lib/snmp/test/snmp_test_lib.erl b/lib/snmp/test/snmp_test_lib.erl index 9fab1a5c56..42f710e4cd 100644 --- a/lib/snmp/test/snmp_test_lib.erl +++ b/lib/snmp/test/snmp_test_lib.erl @@ -25,7 +25,9 @@ -export([hostname/0, hostname/1, localhost/0, localhost/1, os_type/0, sz/1, display_suite_info/1]). --export([non_pc_tc_maybe_skip/4, os_based_skip/1]). +-export([non_pc_tc_maybe_skip/4, os_based_skip/1, + has_support_ipv6/0, has_support_ipv6/1, + is_ipv6_host/0, is_ipv6_host/1]). -export([fix_data_dir/1, init_suite_top_dir/2, init_group_top_dir/2, init_testcase_top_dir/2, lookup/2, @@ -52,11 +54,12 @@ hostname() -> hostname(node()). hostname(Node) -> - from($@, atom_to_list(Node)). - -from(H, [H | T]) -> T; -from(H, [_ | T]) -> from(H, T); -from(_H, []) -> []. + case string:tokens(atom_to_list(Node), [$@]) of + [_, Host] -> + Host; + _ -> + [] + end. %% localhost() -> %% {ok, Ip} = snmp_misc:ip(net_adm:localhost()), @@ -78,7 +81,9 @@ localhost(Family) -> {error, Reason1} -> fail({getifaddrs, Reason1}, ?MODULE, ?LINE) end; - {ok, {0, _, _, _, _, _, _, _}} when (Family =:= inet6) -> + {ok, {A1, _, _, _, _, _, _, _}} when (Family =:= inet6) andalso + ((A1 =:= 0) orelse + (A1 =:= 16#fe80)) -> %% Ouch, we need to use something else case inet:getifaddrs() of {ok, IfList} -> @@ -270,8 +275,47 @@ os_based_skip_check(OsName, OsNames) -> _ -> false end. + + +%% A basic test to check if current host supports IPv6 +has_support_ipv6() -> + case inet:gethostname() of + {ok, Hostname} -> + has_support_ipv6(Hostname); + _ -> + false + end. + +has_support_ipv6(Hostname) -> + case inet:getaddr(Hostname, inet6) of + {ok, Addr} when (size(Addr) =:= 8) andalso + (element(1, Addr) =/= 0) andalso + (element(1, Addr) =/= 16#fe80) -> + true; + {ok, _} -> + false; + {error, _} -> + false + end. +is_ipv6_host() -> + case inet:gethostname() of + {ok, Hostname} -> + is_ipv6_host(Hostname); + {error, _} -> + false + end. + +is_ipv6_host(Hostname) -> + case ct:require(ipv6_hosts) of + ok -> + lists:member(list_to_atom(Hostname), ct:get_config(ipv6_hosts)); + _ -> + false + end. + + %% ---------------------------------------------------------------- %% Test suite utility functions %% diff --git a/lib/snmp/test/snmp_test_lib.hrl b/lib/snmp/test/snmp_test_lib.hrl index dafa4cbf19..c66602b779 100644 --- a/lib/snmp/test/snmp_test_lib.hrl +++ b/lib/snmp/test/snmp_test_lib.hrl @@ -49,8 +49,12 @@ snmp_test_lib:os_based_skip(Skippable)). -define(NON_PC_TC_MAYBE_SKIP(Config, Condition), snmp_test_lib:non_pc_tc_maybe_skip(Config, Condition, ?MODULE, ?LINE)). --define(SKIP(Reason), snmp_test_lib:skip(Reason, ?MODULE, ?LINE)). --define(FAIL(Reason), snmp_test_lib:fail(Reason, ?MODULE, ?LINE)). +-define(SKIP(Reason), snmp_test_lib:skip(Reason, ?MODULE, ?LINE)). +-define(FAIL(Reason), snmp_test_lib:fail(Reason, ?MODULE, ?LINE)). +-define(IS_IPV6_HOST(), snmp_test_lib:is_ipv6_host()). +-define(IS_IPV6_HOST(H), snmp_test_lib:is_ipv6_host(H)). +-define(HAS_SUPPORT_IPV6(), snmp_test_lib:has_support_ipv6()). +-define(HAS_SUPPORT_IPV6(H), snmp_test_lib:has_support_ipv6(H)). %% - Time macros - -- cgit v1.2.3 From 9e0dbd341d738e0c8a268752f131fa6ab6195ad3 Mon Sep 17 00:00:00 2001 From: Micael Karlberg Date: Wed, 29 May 2019 12:12:25 +0200 Subject: [snmp] Add verbosity printouts to the set handling Add verbosity printouts after a set operation. Both for a successful and a failed set. On one of our test hosts (a VM running an old gento), it looked like an ets or dets insert hangs. But since we only have a printout directly before, we can't tell for sure what happens. OTP-15764 --- lib/snmp/src/agent/snmpa_set.erl | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/snmp/src/agent/snmpa_set.erl b/lib/snmp/src/agent/snmpa_set.erl index 9833d6fdcc..b3a3bf0ab0 100644 --- a/lib/snmp/src/agent/snmpa_set.erl +++ b/lib/snmp/src/agent/snmpa_set.erl @@ -163,10 +163,14 @@ set_phase_two(MyVarbinds, SubagentVarbinds) -> [MyVarbinds, SubagentVarbinds]), case snmpa_set_lib:try_set(MyVarbinds) of {noError, 0} -> + ?vtrace("set phase two: (local) varbinds set ok", []), set_phase_two_subagents(SubagentVarbinds); - {ErrorStatus, Index} -> + {ErrorStatus, ErrorIndex} -> + ?vlog("set phase two: (local) varbinds set failed" + "~n ErrorStatus: ~p" + "~n ErrorIndex: ~p", [ErrorStatus, ErrorIndex]), set_phase_two_undo_subagents(SubagentVarbinds), - {ErrorStatus, Index} + {ErrorStatus, ErrorIndex} end. %%----------------------------------------------------------------- @@ -188,6 +192,7 @@ set_phase_two_subagents([{SubAgentPid, SAVbs} | SubagentVarbinds]) -> {_SAOids, Vbs} = sa_split(SAVbs), case catch snmpa_agent:subagent_set(SubAgentPid, [phase_two, set, Vbs]) of {noError, 0} -> + ?vtrace("set phase two: subagent ~p varbinds set ok", [SubAgentPid]), set_phase_two_subagents(SubagentVarbinds); {'EXIT', Reason} -> user_err("Lost contact with subagent (set)~n~w. Using genErr", @@ -195,10 +200,14 @@ set_phase_two_subagents([{SubAgentPid, SAVbs} | SubagentVarbinds]) -> set_phase_two_undo_subagents(SubagentVarbinds), {genErr, 0}; {ErrorStatus, ErrorIndex} -> + ?vlog("set phase two: subagent ~p varbinds set failed" + "~n ErrorStatus: ~p" + "~n ErrorIndex: ~p", [SubAgentPid, ErrorStatus, ErrorIndex]), set_phase_two_undo_subagents(SubagentVarbinds), {ErrorStatus, ErrorIndex} end; set_phase_two_subagents([]) -> + ?vtrace("set phase two: subagent(s) set ok", []), {noError, 0}. %%----------------------------------------------------------------- -- cgit v1.2.3