[snmp|manager|test] Problems starting (slave) nodes

When we fail to start a (slave) node with reason timeout,
its possible that it actually succeeded, which will cause
the following test case(s) init to fail (with 'already started',
since it which also try to start this node).
But since we don't know if we get this because for this reason
or because the previous test case failed to clean up after
itself, we cannot assume that the state of the node is ok
(and therefor use it). So, we attempt to stop it, and try
start it again (*one* time).

This has been observed on one (slooow) VM host.

5 files changed, 58 insertions, 31 deletions

diff --git a/lib/snmp/test/Makefile b/lib/snmp/test/Makefile
index a9142d911d..d9b01536ea 100644
--- a/lib/snmp/test/Makefile
+++ b/lib/snmp/test/Makefile
@@ -180,7 +180,7 @@ emakebuild: $(EMAKEFILE)
 targets: mib $(EMAKEFILE) 
 	erl -make
 
-old_targets: $(TARGET_FILES) $(TEST_SERVER_TARGETS)
+old_targets: mib $(TARGET_FILES) $(TEST_SERVER_TARGETS)
 
 $(EMAKEFILE):  Makefile
 	$(MAKE_EMAKE) $(ERL_COMPILE_FLAGS) -o$(EBIN) '*_SUITE_make' > $(EMAKEFILE)
diff --git a/lib/snmp/test/snmp_agent_test_lib.erl b/lib/snmp/test/snmp_agent_test_lib.erl
index b1c29c4235..8e54bcf3b8 100644
--- a/lib/snmp/test/snmp_agent_test_lib.erl
+++ b/lib/snmp/test/snmp_agent_test_lib.erl
@@ -1340,7 +1340,7 @@ do_expect2(Check, Type, Err, Idx, ExpVBs, To)
 	    io_format_expect("got timeout (16) when system events:"
                              "~n   ~p", [SysEvs]),
             if
-                (SysEvs =/= []) ->
+                (SysEvs =:= []) ->
                     Error;
                 true ->
                     {skip, {system_events, SysEvs}}
diff --git a/lib/snmp/test/snmp_manager_test.erl b/lib/snmp/test/snmp_manager_test.erl
index d959d9e09b..7cd3eae0c7 100644
--- a/lib/snmp/test/snmp_manager_test.erl
+++ b/lib/snmp/test/snmp_manager_test.erl
@@ -1,7 +1,7 @@
 %% 
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2003-2017. All Rights Reserved.
+%% Copyright Ericsson AB 2003-2019. 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.
@@ -4841,7 +4841,7 @@ inform2(Config) when is_list(Config) ->
 			  "~n   ~p", [Addr]),
 			ok;
 		    {snmp_notification, inform2_tag1, {no_response, Addr}} ->
-			p("<ERROR> received expected \"no response\" "
+			e("Received unexpected \"no response\" "
 			  "notification from: "
 			  "~n   ~p", [Addr]),
 			{error, no_response}
@@ -4975,7 +4975,7 @@ inform3(Config) when is_list(Config) ->
 			  "~n   ~p", [Addr]),
 			ok;
 		    {snmp_notification, inform3_tag1, {got_response, Addr}} ->
-			p("<ERROR> received unexpected \"got response\" "
+			e("Received unexpected \"got response\" "
 			  "notification from: "
 			  "~n   ~p", 
 			  [Addr]),
@@ -5262,7 +5262,7 @@ inform_swarm_collector(N, SentAckCnt, RecvCnt, RespCnt, Timeout) ->
 		    inform_swarm_collector(N, SentAckCnt, RecvCnt+1, RespCnt, 
 					   Timeout);
 		{Err, Idx, VBs} ->
-		    p("<ERROR> unexpected error status: "
+		    e("Unexpected error status: "
 		      "~n   Err: ~p"
 		      "~n   Idx: ~p"
 		      "~n   VBs: ~p", [Err, Idx, VBs]),
@@ -5281,7 +5281,7 @@ inform_swarm_collector(N, SentAckCnt, RecvCnt, RespCnt, Timeout) ->
 
 	%% The agent did not received ack from the manager in time 
 	{snmp_notification, inform2_tag1, {no_response, Addr}} ->
-	    p("<ERROR> received expected \"no response\" notification "
+	    e("Received expected \"no response\" notification "
 	      "from: "
 	      "~n   ~p", [Addr]),
 	    Reason = {no_response, Addr, {N, SentAckCnt, RecvCnt, RespCnt}},
@@ -5458,10 +5458,10 @@ command_handler([{No, Desc, Cmd}|Cmds]) ->
             p("command_handler -> ~w: ok",[No]),
             command_handler(Cmds);
         {error, Reason} ->
-            p("<ERROR> command_handler -> ~w error: ~n~p",[No, Reason]),
+            e("Command_handler -> ~w error: ~n~p",[No, Reason]),
             ?line ?FAIL({command_failed, No, Reason});
         Error ->
-            p("<ERROR> command_handler -> ~w unexpected: ~n~p",[No, Error]),
+            e("Command_handler -> ~w unexpected: ~n~p",[No, Error]),
             ?line ?FAIL({unexpected_command_result, No, Error})
     end.
 
@@ -6327,6 +6327,8 @@ start_manager_node() ->
     start_node(snmp_manager).
 
 start_node(Name) ->
+    start_node(Name, true).
+start_node(Name, Retry) ->
     Pa   = filename:dirname(code:which(?MODULE)),
     Args = case init:get_argument('CC_TEST') of
                {ok, [[]]} ->
@@ -6337,30 +6339,47 @@ start_node(Name) ->
                       ""
               end,
     A = Args ++ " -pa " ++ Pa,
-    case (catch ?START_NODE(Name, A)) of
+    try ?START_NODE(Name, A) of
 	{ok, Node} ->
 	    Node;
-	Else ->
-	    ?line ?FAIL(Else)
+	{error, timeout} ->
+            e("Failed starting node ~p: timeout", [Name]),
+	    ?line ?FAIL({error_starting_node, Name, timeout});
+	{error, {already_running, Node}} when (Retry =:= true) ->
+            %% Ouch
+            %% Either we previously failed to (properly) stop the node
+            %% or it was a failed start, that reported failure (for instance
+            %% timeout) but actually succeeded. Regardless, we don't know
+            %% the state of this node, so (try) stop it and then (re-) try
+            %% start again.
+            e("Failed starting node ~p: Already Running - try stop", [Node]),
+            case ?STOP_NODE(Node) of
+                true ->
+                    p("Successfully stopped old node ~p", [Node]),
+                    start_node(Name, false);
+                false ->
+                    e("Failed stop old node ~p", [Node]),
+                    ?line ?FAIL({error_starting_node, Node, Retry, already_running})
+            end;
+	{error, {already_running, Node}} ->
+            e("Failed starting node ~p: Already Running", [Node]),
+            ?line ?FAIL({error_starting_node, Node, Retry, already_running});
+	{error, Reason} ->
+            e("Failed starting node ~p: ~p", [Name, Reason]),
+	    ?line ?FAIL({error_starting_node, Name, Reason})
+    catch
+        exit:{suite_failed, Reason} ->
+            e("(suite) Failed starting node ~p: ~p", [Name, Reason]),
+            ?line ?FAIL({failed_starting_node, Name, Reason})
     end.
 
-stop_node(Node) ->
-    rpc:cast(Node, erlang, halt, []),
-    await_stopped(Node, 5).
 
-await_stopped(Node, 0) ->
-    p("await_stopped -> ~p still exist: giving up", [Node]),
-    ok;
-await_stopped(Node, N) ->
-    Nodes = erlang:nodes(),
-    case lists:member(Node, Nodes) of
-	true ->
-	    p("await_stopped -> ~p still exist: ~w", [Node, N]),
-	    ?SLEEP(1000),
-	    await_stopped(Node, N-1);
-	false ->
-	    p("await_stopped -> ~p gone: ~w", [Node, N]),
-	    ok
+stop_node(Node) ->
+    case ?STOP_NODE(Node) of
+        true ->
+            ok;
+        false ->
+            ?line ?FAIL({failed_stop_node, Node})
     end.
     
  
@@ -6605,12 +6624,15 @@ rcall(Node, Mod, Func, Args) ->
 
 %% ------
 
+e(F, A) ->
+    p("<ERROR> " ++ F, A).
+
 p(F) ->
     p(F, []).
  
 p(F, A) ->
     p(get(tname), F, A).
- 
+
 p(TName, F, A) ->
     io:format("*** [~w][~s] ***"
               "~n   " ++ F ++ "~n", [TName, formated_timestamp()|A]).
diff --git a/lib/snmp/test/snmp_test_lib.erl b/lib/snmp/test/snmp_test_lib.erl
index 42f710e4cd..35bc535a80 100644
--- a/lib/snmp/test/snmp_test_lib.erl
+++ b/lib/snmp/test/snmp_test_lib.erl
@@ -36,7 +36,7 @@
 -export([hours/1, minutes/1, seconds/1, sleep/1]).
 -export([flush_mqueue/0, trap_exit/0, trap_exit/1]).
 -export([ping/1, local_nodes/0, nodes_on/1]).
--export([start_node/2]).
+-export([start_node/2, stop_node/1]).
 -export([is_app_running/1, 
 	 is_crypto_running/0, is_mnesia_running/0, is_snmp_running/0]).
 -export([crypto_start/0, crypto_support/0]).
@@ -512,10 +512,14 @@ nodes_on(Host) when is_list(Host) ->
 
 
 start_node(Name, Args) ->
-    Opts = [{cleanup,false}, {args,Args}],
+    Opts = [{cleanup, false}, {args, Args}],
     test_server:start_node(Name, slave, Opts).
 
 
+stop_node(Node) ->
+    test_server:stop_node(Node).
+
+
 %% ----------------------------------------------------------------
 %% Application and Crypto utility functions
 %% 
diff --git a/lib/snmp/test/snmp_test_lib.hrl b/lib/snmp/test/snmp_test_lib.hrl
index c66602b779..99b86a928c 100644
--- a/lib/snmp/test/snmp_test_lib.hrl
+++ b/lib/snmp/test/snmp_test_lib.hrl
@@ -92,6 +92,7 @@
 -define(LNODES(),           snmp_test_lib:local_nodes()).
 -define(NODES(H),           snmp_test_lib:nodes_on(H)).
 -define(START_NODE(N,A),    snmp_test_lib:start_node(N,A)).
+-define(STOP_NODE(N),       snmp_test_lib:stop_node(N)).
 
 
 %% - Application and Crypto utility macros -