Merge pull request #6445 from emqx/fix-badrpc-monitor-collect

fix(monitor): fix return value on badrpc

apps/emqx/src/emqx_config.erl

@@ -248,7 +248,7 @@ init_load(SchemaMod) ->
     init_load(SchemaMod, ConfFiles).
 
 %% @doc Initial load of the given config files.
-%% NOTE: The order of the files is significant, configs from files orderd
+%% NOTE: The order of the files is significant, configs from files ordered
 %% in the rear of the list overrides prior values.
 -spec init_load(module(), [string()] | binary() | hocon:config()) -> ok.
 init_load(SchemaMod, Conf) when is_list(Conf) orelse is_binary(Conf) ->

apps/emqx_dashboard/src/emqx_dashboard_admin.erl

@@ -211,6 +211,7 @@ hash(Password) ->
 sha256(SaltBin, Password) ->
     crypto:hash('sha256', <<SaltBin/binary, Password/binary>>).
 
+-spec(add_default_user() -> {ok, map() | empty | default_user_exists } | {error, any()}).
 add_default_user() ->
     add_default_user(binenv(default_username), binenv(default_password)).
 
@@ -218,7 +219,7 @@ binenv(Key) ->
     iolist_to_binary(emqx_conf:get([emqx_dashboard, Key], "")).
 
 add_default_user(Username, Password) when ?EMPTY_KEY(Username) orelse ?EMPTY_KEY(Password) ->
-    ok;
+    {ok, empty};
 
 add_default_user(Username, Password) ->
     case lookup_user(Username) of

apps/emqx_dashboard/src/emqx_dashboard_monitor_api.erl

@@ -29,6 +29,8 @@
                   , sent
                   , dropped]).
 
+-define(EMPTY_COLLECTION, {0, 0, 0, 0}).
+
 api_spec() ->
     {[ monitor_api()
      , monitor_nodes_api()
@@ -175,7 +177,7 @@ current_counters(get, _Params) ->
     {200, Response}.
 
 format_current_metrics(Collects) ->
-    format_current_metrics(Collects, {0,0,0,0}).
+    format_current_metrics(Collects, ?EMPTY_COLLECTION).
 format_current_metrics([], Acc) ->
     Acc;
 format_current_metrics([{Received, Sent, Sub, Conn} | Collects],
@@ -217,7 +219,7 @@ get_collect(Node) when Node =:= node() ->
     emqx_dashboard_collection:get_collect();
 get_collect(Node) ->
     case rpc:call(Node, emqx_dashboard_collection, get_collect, []) of
-        {badrpc, _Reason} -> #{};
+        {badrpc, _Reason} -> ?EMPTY_COLLECTION;
         Res -> Res
     end.
 

apps/emqx_dashboard/test/emqx_dashboard_monitor_api_SUITE.erl

@@ -0,0 +1,121 @@
+%%--------------------------------------------------------------------
+%% Copyright (c) 2020-2021 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_dashboard_monitor_api_SUITE).
+
+-compile(nowarn_export_all).
+-compile(export_all).
+
+-include_lib("eunit/include/eunit.hrl").
+-include_lib("common_test/include/ct.hrl").
+-include_lib("emqx/include/emqx.hrl").
+-include("emqx_dashboard.hrl").
+
+all() ->
+    emqx_common_test_helpers:all(?MODULE).
+
+init_per_testcase(t_badrpc_collect, Config) ->
+    Cluster = cluster_specs(2),
+    Apps = [emqx_modules, emqx_dashboard],
+    Nodes = [N1, N2] =  lists:map(fun(Spec) -> start_slave(Spec, Apps) end, Cluster),
+    %% form the cluster
+    ok = rpc:call(N2, mria, join, [N1]),
+    %% Wait until all nodes are healthy:
+    [rpc:call(Node, mria_rlog, wait_for_shards, [[?DASHBOARD_SHARD], 5000])
+     || Node <- Nodes],
+    [ {nodes, Nodes}
+    , {apps, Apps}
+    | Config];
+init_per_testcase(_, Config) ->
+    Config.
+
+end_per_testcase(t_badrpc_collect, Config) ->
+    Apps = ?config(apps, Config),
+    Nodes = ?config(nodes, Config),
+    lists:foreach(fun(Node) -> stop_slave(Node, Apps) end, Nodes),
+    ok;
+end_per_testcase(_, _Config) ->
+    ok.
+
+t_badrpc_collect(Config) ->
+    [N1, N2] = ?config(nodes, Config),
+    %% simulate badrpc on one node
+    ok = rpc:call(N2, meck, new, [emqx_dashboard_collection, [no_history, no_link]]),
+    %% we don't mock the `emqx_dashboard_collection:get_collect/0' to
+    %% provoke the `badrpc' error.
+    ?assertMatch(
+       {200, #{nodes := 2}},
+       rpc:call(N1, emqx_dashboard_monitor_api, current_counters, [get, #{}])),
+    ok = rpc:call(N2, meck, unload, [emqx_dashboard_collection]),
+    ok.
+
+%%------------------------------------------------------------------------------
+%% Internal functions
+%%------------------------------------------------------------------------------
+
+cluster_specs(NumNodes) ->
+    BaseGenRpcPort = 9000,
+    Specs0 = [#{ name => node_name(N)
+               , num => N
+               }
+              || N <- lists:seq(1, NumNodes)],
+    GenRpcPorts = maps:from_list([{node_id(Name), {tcp, BaseGenRpcPort + N}}
+                                  || #{name := Name, num := N} <- Specs0]),
+    [ Spec#{env => [ {gen_rpc, tcp_server_port, BaseGenRpcPort + N}
+                   , {gen_rpc, client_config_per_node, {internal, GenRpcPorts}}
+                   ]}
+      || Spec = #{num := N} <- Specs0].
+
+node_name(N) ->
+    list_to_atom("n" ++ integer_to_list(N)).
+
+node_id(Name) ->
+    list_to_atom(lists:concat([Name, "@", host()])).
+
+start_slave(Spec = #{ name := Name}, Apps) ->
+    CommonBeamOpts = "+S 1:1 ", % We want VMs to only occupy a single core
+    {ok, Node} = slave:start_link(host(), Name, CommonBeamOpts ++ ebin_path()),
+    setup_node(Node, Spec, Apps),
+    Node.
+
+stop_slave(Node, Apps) ->
+    ok = rpc:call(Node, emqx_common_test_helpers, start_apps, [Apps]),
+    slave:stop(Node).
+
+host() ->
+    [_, Host] = string:tokens(atom_to_list(node()), "@"), Host.
+
+ebin_path() ->
+    string:join(["-pa" | lists:filter(fun is_lib/1, code:get_path())], " ").
+
+is_lib(Path) ->
+    string:prefix(Path, code:lib_dir()) =:= nomatch.
+
+setenv(Node, Env) ->
+    [rpc:call(Node, application, set_env, [App, Key, Val]) || {App, Key, Val} <- Env].
+
+setup_node(Node, _Spec = #{env := Env}, Apps) ->
+    %% load these before starting ekka and such
+    [rpc:call(Node, application, load, [App]) || App <- [gen_rpc, emqx_conf, emqx]],
+    setenv(Node, Env),
+    EnvHandler =
+        fun(emqx) ->
+                application:set_env(emqx, boot_modules, [router, broker]);
+           (_) ->
+                ok
+        end,
+    ok = rpc:call(Node, emqx_common_test_helpers, start_apps, [Apps, EnvHandler]),
+    ok.