Move common peer discovery functions to rabbit_nodes

lock_id/1 and lock_retries/0 get used by peer discovery backends
classic, k8s, and aws. Ideally, these two functions live in app
rabbitmq_peer_discovery_common. However, the classic backend (and
therefore app rabbit) cannot depend on app
rabbitmq_peer_discovery_common since the latter already depends on the
former. We don't want cyclic dependencies between Erlang apps.
That's why we move these two functions into module rabbit_nodes.

We could have moved them into app rabbit_common instead. However that module is
mainly used for sharing code across server and client. These two
functions are used only in the server.

Author: ansd

deps/rabbit/src/rabbit_nodes.erl

@@ -15,6 +15,7 @@
          boot/0]).
 -export([persistent_cluster_id/0, seed_internal_cluster_id/0, seed_user_provided_cluster_name/0]).
 -export([all_running_with_hashes/0]).
+-export([lock_id/1, lock_retries/0]).
 
 -include_lib("kernel/include/inet.hrl").
 -include_lib("rabbit_common/include/rabbit.hrl").
@@ -23,6 +24,12 @@
 
 -define(INTERNAL_CLUSTER_ID_PARAM_NAME, internal_cluster_id).
 
+% Retries as passed to https://erlang.org/doc/man/global.html#set_lock-3
+% To understand how retries map to the timeout, read
+% https://github.com/erlang/otp/blob/d256ae477014158a49bb860b283df9c040011197/lib/kernel/src/global.erl#L2062-L2075
+% 80 corresponds to a timeout of ca 300 seconds.
+-define(DEFAULT_LOCK_RETRIES, 80).
+
 %%----------------------------------------------------------------------------
 %% API
 %%----------------------------------------------------------------------------
@@ -160,3 +167,16 @@ await_running_count_with_retries(TargetCount, Retries) ->
 -spec all_running_with_hashes() -> #{non_neg_integer() => node()}.
 all_running_with_hashes() ->
     maps:from_list([{erlang:phash2(Node), Node} || Node <- all_running()]).
+
+-spec lock_id(Node :: node()) -> {ResourceId :: string(), LockRequesterId :: node()}.
+lock_id(Node) ->
+  {cookie_hash(), Node}.
+
+-spec lock_retries() -> integer().
+lock_retries() ->
+  case application:get_env(rabbit, cluster_formation) of
+      {ok, PropList} ->
+        proplists:get_value(internal_lock_retries, PropList, ?DEFAULT_LOCK_RETRIES);
+      undefined ->
+          ?DEFAULT_LOCK_RETRIES
+  end.

deps/rabbit/src/rabbit_peer_discovery_classic_config.erl

@@ -13,8 +13,6 @@
 -export([list_nodes/0, supports_registration/0, register/0, unregister/0,
          post_registration/0, lock/1, unlock/1]).
 
-%% TODO ansd: see todo below
--define(DEFAULT_LOCK_RETRIES, 80).
 %%
 %% API
 %%
@@ -31,26 +29,15 @@ list_nodes() ->
 -spec lock(Node :: node()) -> {ok, {ResourceId :: string(), LockRequesterId :: node()}} | {error, Reason :: string()}.
 
 lock(Node) ->
-  %% TODO ansd: instead of below lines, ideally we reuse functions rabbit_peer_discovery_util:lock_id/1 and
-  %% rabbit_peer_discovery_util:lock_retries/0
-  %% When using these two functions, executing tests failed since node couldn't boot: undef rabbit_peer_discovery_util:lock_id/1
-  %% Can the rabbit app depend on the rabbit_peer_discovery_common_app?
-  %% If not, should we move these two functions to some other place (e.g. rabbit_common)?
-  LockId = {rabbit_nodes:cookie_hash(), Node},
-  Retries = case application:get_env(rabbit, cluster_formation) of
-              {ok, PropList} ->
-                proplists:get_value(internal_lock_retries, PropList, ?DEFAULT_LOCK_RETRIES);
-              undefined ->
-                ?DEFAULT_LOCK_RETRIES
-            end,
-
   Nodes = node_list(),
   case lists:member(Node, Nodes) of
     false when Nodes =/= [] ->
       rabbit_log:warning("Local node ~s is not part of configured nodes ~p. "
                       "This might lead to incorrect cluster formation.", [Node, Nodes]);
     _ -> ok
   end,
+  LockId = rabbit_nodes:lock_id(Node),
+  Retries = rabbit_nodes:lock_retries(),
   case global:set_lock(LockId, Nodes, Retries) of
     true ->
       {ok, LockId};

deps/rabbitmq_peer_discovery_aws/src/rabbit_peer_discovery_aws.erl

@@ -125,8 +125,8 @@ lock(Node) ->
       case lists:member(Node, Nodes) of
         true ->
           rabbit_log:info("Will try to lock connecting to nodes ~p", [Nodes]),
-          LockId = ?UTIL_MODULE:lock_id(Node),
-          Retries = ?UTIL_MODULE:lock_retries(),
+          LockId = rabbit_nodes:lock_id(Node),
+          Retries = rabbit_nodes:lock_retries(),
           case global:set_lock(LockId, Nodes, Retries) of
             true ->
               {ok, LockId};

deps/rabbitmq_peer_discovery_common/include/rabbit_peer_discovery.hrl

@@ -16,12 +16,6 @@
 % by `httpc`
 -define(DEFAULT_HTTP_TIMEOUT, 2250).
 
-% Retries as passed to https://erlang.org/doc/man/global.html#set_lock-3
-% To understand how retries map to the timeout, read
-% https://github.com/erlang/otp/blob/d256ae477014158a49bb860b283df9c040011197/lib/kernel/src/global.erl#L2062-L2075
-% 80 corresponds to a timeout of ca 300 seconds.
--define(DEFAULT_LOCK_RETRIES, 80).
-
 -type peer_discovery_config_value() :: atom() | integer() | string() | undefined.
 
 -record(peer_discovery_config_entry_meta,

deps/rabbitmq_peer_discovery_common/src/rabbit_peer_discovery_util.erl

@@ -21,9 +21,7 @@
          as_proplist/1,
          as_map/1,
          stringify_error/1,
-         maybe_backend_configured/4,
-         lock_id/1,
-         lock_retries/0
+         maybe_backend_configured/4
         ]).
 
 -include_lib("kernel/include/logger.hrl").
@@ -428,16 +426,3 @@ as_list(Value) ->
      [Value],
      #{domain => ?RMQLOG_DOMAIN_PEER_DIS}),
   Value.
-
--spec lock_id(Node :: node()) -> {ResourceId :: string(), LockRequesterId :: node()}.
-lock_id(Node) ->
-  {rabbit_nodes:cookie_hash(), Node}.
-
--spec lock_retries() -> integer().
-lock_retries() ->
-  case application:get_env(rabbit, cluster_formation) of
-      {ok, PropList} ->
-        proplists:get_value(internal_lock_retries, PropList, ?DEFAULT_LOCK_RETRIES);
-      undefined ->
-          ?DEFAULT_LOCK_RETRIES
-  end.

deps/rabbitmq_peer_discovery_k8s/src/rabbit_peer_discovery_k8s.erl

@@ -72,8 +72,8 @@ lock(Node) ->
       case lists:member(Node, Nodes) of
         true ->
           rabbit_log:info("Will try to lock connecting to nodes ~p", [Nodes]),
-          LockId = ?UTIL_MODULE:lock_id(Node),
-          Retries = ?UTIL_MODULE:lock_retries(),
+          LockId = rabbit_nodes:lock_id(Node),
+          Retries = rabbit_nodes:lock_retries(),
           case global:set_lock(LockId, Nodes, Retries) of
             true ->
               {ok, LockId};