New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(bridge_v2_api): avoid calling nodes that do not support minimum bpapi #12472
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -106,7 +106,6 @@ | |
-define(KAFKA_BRIDGE_UPDATE(Name, Connector), | ||
maps:without([<<"name">>, <<"type">>], ?KAFKA_BRIDGE(Name, Connector)) | ||
). | ||
-define(KAFKA_BRIDGE_UPDATE(Name), ?KAFKA_BRIDGE_UPDATE(Name, ?ACTION_CONNECTOR_NAME)). | ||
|
||
-define(SOURCE_TYPE_STR, "mqtt"). | ||
-define(SOURCE_TYPE, <<?SOURCE_TYPE_STR>>). | ||
|
@@ -1477,7 +1476,7 @@ t_cluster_later_join_metrics(Config) -> | |
?assertMatch( | ||
{ok, 200, #{ | ||
<<"metrics">> := #{<<"success">> := _}, | ||
<<"node_metrics">> := [#{<<"metrics">> := #{}}, #{<<"metrics">> := #{}} | _] | ||
<<"node_metrics">> := [#{<<"metrics">> := #{}} | _] | ||
}}, | ||
request_json(get, uri([?ACTIONS_ROOT, ActionID, "metrics"]), Config) | ||
), | ||
|
@@ -1512,3 +1511,47 @@ t_raw_config_response_defaults(Config) -> | |
) | ||
), | ||
ok. | ||
|
||
t_older_version_nodes_in_cluster(matrix) -> | ||
[ | ||
[cluster, actions], | ||
[cluster, sources] | ||
]; | ||
t_older_version_nodes_in_cluster(Config) -> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this testcase going to be relevant in the future releases? If not, perhaps a manual one-off test is enough? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this still might be relevant for any rolling upgrade scenario. |
||
[_, Kind | _] = group_path(Config), | ||
PrimaryNode = ?config(node, Config), | ||
OtherNode = maybe_get_other_node(Config), | ||
?assertNotEqual(OtherNode, PrimaryNode), | ||
Name = atom_to_binary(?FUNCTION_NAME), | ||
?check_trace( | ||
begin | ||
#{api_root_key := APIRootKey} = get_common_values(Kind, Name), | ||
erpc:call(PrimaryNode, fun() -> | ||
meck:new(emqx_bpapi, [no_history, passthrough, no_link]), | ||
meck:expect(emqx_bpapi, supported_version, fun(N, Api) -> | ||
case N =:= OtherNode of | ||
true -> 1; | ||
false -> meck:passthrough([N, Api]) | ||
end | ||
end) | ||
end), | ||
erpc:call(OtherNode, fun() -> | ||
meck:new(emqx_bridge_v2, [no_history, passthrough, no_link]), | ||
meck:expect(emqx_bridge_v2, list, fun(_ConfRootKey) -> | ||
error(should_not_be_called) | ||
end) | ||
end), | ||
?assertMatch( | ||
{ok, 200, _}, | ||
request_json( | ||
get, | ||
uri([APIRootKey]), | ||
Config | ||
) | ||
), | ||
ok | ||
end, | ||
[] | ||
), | ||
|
||
ok. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Fixed an issue that could lead to some read operations on `/api/v5/actions/` and `/api/v5/sources/` to return 500 while rolling upgrades are underway. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Returns a single node result now, because BPAPI might not be ready / not have announced the new node's APIs yet, and thus is rejected when choosing the nodes.