diff --git a/src/cets_status.erl b/src/cets_status.erl index 1b06d9bb..36b29e14 100644 --- a/src/cets_status.erl +++ b/src/cets_status.erl @@ -3,7 +3,12 @@ %% RPC callbacks -export([get_local_table_to_other_nodes_map_from_disco/1]). --ignore_xref([status/1, get_local_table_to_other_nodes_map_from_disco/1]). +%% Export for debugging/testing +-export([gather_data/1, format_data/1]). + +-ignore_xref([ + status/1, get_local_table_to_other_nodes_map_from_disco/1, gather_data/1, format_data/1 +]). -include_lib("kernel/include/logger.hrl"). @@ -39,19 +44,56 @@ conflict_tables := [table_name()] }. +-type status_data() :: #{ + this_node := node(), + online_nodes := [node()], + system_info := cets_discovery:system_info(), + available_nodes := [node()], + local_table_to_other_nodes_map := tab_nodes_map(), + node_to_tab_nodes_map := node_to_tab_nodes_map() +}. + -spec status(disco_name()) -> info(). status(Disco) when is_atom(Disco) -> + %% Gathering and formatting data is separated to simplify testing/debugging + Data = gather_data(Disco), + format_data(Data). + +-spec gather_data(disco_name()) -> status_data(). +gather_data(Disco) -> + ThisNode = node(), + Info = cets_discovery:system_info(Disco), + #{tables := Tables} = Info, + OnlineNodes = [ThisNode | nodes()], + AvailNodes = available_nodes(Disco, OnlineNodes), + Expected = get_local_table_to_other_nodes_map(Tables), + OtherTabNodes = get_node_to_tab_nodes_map(AvailNodes, Disco), + #{ + this_node => ThisNode, + online_nodes => OnlineNodes, + system_info => Info, + available_nodes => AvailNodes, + local_table_to_other_nodes_map => Expected, + node_to_tab_nodes_map => OtherTabNodes + }. + +%% Formats gathered data. +%% Contains pure functions logic. +-spec format_data(status_data()) -> info(). +format_data(#{ + this_node := ThisNode, + online_nodes := OnlineNodes, + system_info := Info, + available_nodes := AvailNodes, + local_table_to_other_nodes_map := Expected, + node_to_tab_nodes_map := OtherTabNodes +}) -> %% The node lists could not match for different nodes %% because they are updated periodically - #{unavailable_nodes := UnNodes, nodes := DiscoNodes, tables := Tables} = - Info = cets_discovery:system_info(Disco), + #{unavailable_nodes := UnNodes, nodes := DiscoNodes, tables := Tables} = Info, DiscoNodesSorted = lists:sort(DiscoNodes), - OnlineNodes = [node() | nodes()], - AvailNodes = available_nodes(Disco, OnlineNodes), NoDiscoNodes = remote_nodes_without_disco(DiscoNodesSorted, AvailNodes, OnlineNodes), - Expected = get_local_table_to_other_nodes_map(Tables), - OtherTabNodes = get_node_to_tab_nodes_map(AvailNodes, Disco), - JoinedNodes = joined_nodes(Expected, OtherTabNodes), + JoinedNodes = joined_nodes(ThisNode, Expected, OtherTabNodes), AllTables = all_tables(Expected, OtherTabNodes), {UnknownTables, NodesWithUnknownTables} = unknown_tables(OtherTabNodes, Tables, AllTables), {MissingTables, NodesWithMissingTables} = missing_tables(OtherTabNodes, Tables), @@ -96,9 +138,9 @@ get_node_to_tab_nodes_map(AvailNodes, Disco) -> %% Nodes that has our local tables running (but could also have some unknown tables). %% All joined nodes replicate data between each other. --spec joined_nodes(tab_nodes_map(), node_to_tab_nodes_map()) -> [node()]. -joined_nodes(Expected, OtherTabNodes) -> - ExpectedTables = maps:keys(Expected), +-spec joined_nodes(node(), tab_nodes_map(), node_to_tab_nodes_map()) -> [node()]. +joined_nodes(ThisNode, Expected, OtherTabNodes) -> + ExpectedTables = maps_keys_sorted(Expected), OtherJoined = maps:fold( fun(Node, TabNodes, Acc) -> case maps:with(ExpectedTables, TabNodes) =:= Expected of @@ -109,7 +151,7 @@ joined_nodes(Expected, OtherTabNodes) -> [], OtherTabNodes ), - lists:sort([node() | OtherJoined]). + lists:sort([ThisNode | OtherJoined]). unknown_tables(OtherTabNodes, Tables, AllTables) -> UnknownTables = ordsets:subtract(AllTables, Tables), @@ -130,7 +172,7 @@ unknown_tables(OtherTabNodes, Tables, AllTables) -> missing_tables(OtherTabNodes, LocalTables) -> Zip = maps:fold( fun(Node, TabNodes, Acc) -> - RemoteTables = maps:keys(TabNodes), + RemoteTables = maps_keys_sorted(TabNodes), MissingTables = ordsets:subtract(LocalTables, RemoteTables), case MissingTables of [] -> Acc; @@ -171,7 +213,7 @@ conflict_tables(Expected, OtherTabNodes) -> -spec all_tables(tab_nodes_map(), node_to_tab_nodes_map()) -> [table_name()]. all_tables(Expected, OtherTabNodes) -> TableNodesVariants = [Expected | maps:values(OtherTabNodes)], - TableVariants = lists:map(fun maps:keys/1, TableNodesVariants), + TableVariants = lists:map(fun maps_keys_sorted/1, TableNodesVariants), ordsets:union(TableVariants). %% Returns nodes for each table hosted on node() @@ -201,3 +243,6 @@ discovery_works(#{last_get_nodes_result := {ok, _}}) -> true; discovery_works(_) -> false. + +maps_keys_sorted(Map) -> + lists:sort(maps:keys(Map)). diff --git a/test/cets_SUITE.erl b/test/cets_SUITE.erl index 44afcb8d..2ea8cf76 100644 --- a/test/cets_SUITE.erl +++ b/test/cets_SUITE.erl @@ -149,7 +149,8 @@ cases() -> long_call_to_unknown_name_throws_pid_not_found, send_leader_op_throws_noproc, pinfo_returns_value, - pinfo_returns_undefined + pinfo_returns_undefined, + format_data_does_not_return_table_duplicates ]. only_for_logger_cases() -> @@ -2207,6 +2208,10 @@ join_interrupted_when_ping_crashes(Config) -> ?assertMatch({error, {task_failed, ping_all_failed, #{}}}, Res), meck:unload(). +format_data_does_not_return_table_duplicates(Config) -> + Res = cets_status:format_data(test_data_for_duplicate_missing_table_in_status(Config)), + ?assertMatch(#{remote_unknown_tables := [], remote_nodes_with_missing_tables := []}, Res). + %% Helper functions receive_all_logs(Id) -> @@ -2484,3 +2489,19 @@ receive_all_logs_with_log_ref(LogHandlerId, LogRef) -> after 5000 -> ct:fail({timeout, receive_all_logs_with_log_ref}) end. + +%% Gathered after Helm update +%% with cets_status:gather_data(mongoose_cets_discovery). +test_data_for_duplicate_missing_table_in_status(Config) -> + %% Create atoms in non sorted order + %% maps:keys returns keys in the atom-creation order (and not sorted). + %% Also, compiler is smart and would optimize list_to_atom("literal_string"), + %% so we do a module call to disable this optimization. + _ = list_to_atom(?MODULE:return_same("cets_external_component")), + _ = list_to_atom(?MODULE:return_same("cets_bosh")), + Name = filename:join(proplists:get_value(data_dir, Config), "status_data.txt"), + {ok, [Term]} = file:consult(Name), + Term. + +return_same(X) -> + X. diff --git a/test/cets_SUITE_data/status_data.txt b/test/cets_SUITE_data/status_data.txt new file mode 100644 index 00000000..130962eb --- /dev/null +++ b/test/cets_SUITE_data/status_data.txt @@ -0,0 +1,52 @@ +#{ + system_info => + #{ + nodes => + [ + 'mongooseim@mongooseim-0.mongooseim.default.svc.cluster.local', + 'mongooseim@mongooseim-1.mongooseim.default.svc.cluster.local' + ], + tables => [cets_bosh, cets_external_component], + unavailable_nodes => [] + }, + available_nodes => + [ + 'mongooseim@mongooseim-0.mongooseim.default.svc.cluster.local', + 'mongooseim@mongooseim-1.mongooseim.default.svc.cluster.local' + ], + local_table_to_other_nodes_map => + #{ + cets_external_component => + [ + 'mongooseim@mongooseim-0.mongooseim.default.svc.cluster.local', + 'mongooseim@mongooseim-1.mongooseim.default.svc.cluster.local' + ], + cets_bosh => + [ + 'mongooseim@mongooseim-0.mongooseim.default.svc.cluster.local', + 'mongooseim@mongooseim-1.mongooseim.default.svc.cluster.local' + ] + }, + node_to_tab_nodes_map => + #{ + 'mongooseim@mongooseim-1.mongooseim.default.svc.cluster.local' => + #{ + cets_external_component => + [ + 'mongooseim@mongooseim-0.mongooseim.default.svc.cluster.local', + 'mongooseim@mongooseim-1.mongooseim.default.svc.cluster.local' + ], + cets_bosh => + [ + 'mongooseim@mongooseim-0.mongooseim.default.svc.cluster.local', + 'mongooseim@mongooseim-1.mongooseim.default.svc.cluster.local' + ] + } + }, + online_nodes => + [ + 'mongooseim@mongooseim-0.mongooseim.default.svc.cluster.local', + 'mongooseim@mongooseim-1.mongooseim.default.svc.cluster.local' + ], + this_node => 'mongooseim@mongooseim-0.mongooseim.default.svc.cluster.local' +}.