From 1641ecbc113be14365d3e0097e61d709807772cf Mon Sep 17 00:00:00 2001 From: Daniil Fedotov Date: Thu, 8 Jun 2017 18:15:21 +0100 Subject: [PATCH] Improve wait command. Do not call await_startup on rabbitmq node, instead do it on rabbitmqctl node, so even if net_ticktime is low, it won't fail with nodedown. --- .../lib/rabbitmq/cli/core/exit_codes.ex | 2 ++ .../lib/rabbitmq/cli/ctl/commands/wait_command.ex | 14 +++++++------- deps/rabbitmq_cli/lib/rabbitmqctl.ex | 10 ++++++++++ deps/rabbitmq_cli/mix.exs | 2 +- .../test/plugins/set_plugins_command_test.exs | 1 - deps/rabbitmq_cli/test/wait_command_test.exs | 15 ++++++++++----- 6 files changed, 30 insertions(+), 14 deletions(-) diff --git a/deps/rabbitmq_cli/lib/rabbitmq/cli/core/exit_codes.ex b/deps/rabbitmq_cli/lib/rabbitmq/cli/core/exit_codes.ex index 5e461ac80e..aab3bf49d8 100644 --- a/deps/rabbitmq_cli/lib/rabbitmq/cli/core/exit_codes.ex +++ b/deps/rabbitmq_cli/lib/rabbitmq/cli/core/exit_codes.ex @@ -43,6 +43,8 @@ defmodule RabbitMQ.CLI.Core.ExitCodes do def exit_code_for({:validation_failure, _}), do: exit_usage() def exit_code_for({:badrpc, :timeout}), do: exit_tempfail() def exit_code_for({:badrpc, {:timeout, _}}), do: exit_tempfail() + def exit_code_for(:timeout), do: exit_tempfail() + def exit_code_for({:timeout, _}), do: exit_tempfail() def exit_code_for({:badrpc, :nodedown}), do: exit_unavailable() def exit_code_for({:error, _}), do: exit_software() diff --git a/deps/rabbitmq_cli/lib/rabbitmq/cli/ctl/commands/wait_command.ex b/deps/rabbitmq_cli/lib/rabbitmq/cli/ctl/commands/wait_command.ex index b4106c3943..bb479df4d2 100644 --- a/deps/rabbitmq_cli/lib/rabbitmq/cli/ctl/commands/wait_command.ex +++ b/deps/rabbitmq_cli/lib/rabbitmq/cli/ctl/commands/wait_command.ex @@ -30,9 +30,9 @@ defmodule RabbitMQ.CLI.Ctl.Commands.WaitCommand do def validate([_|_] = args, _) when length(args) > 1, do: {:validation_failure, :too_many_args} def validate([_], %{pid: _}), do: {:validation_failure, "Cannot specify both pid and pidfile"} - def validate([], %{pid: _}), do: :ok - def validate([], _), do: {:validation_failure, "No pid or pidfile specified"} - def validate([_], _), do: :ok + def validate([], %{pid: _} = opts), do: RabbitMQ.CLI.Ctl.Validators.rabbit_is_loaded([], opts) + def validate([], _), do: {:validation_failure, "No pid or pidfile specified"} + def validate([_], opts), do: RabbitMQ.CLI.Ctl.Validators.rabbit_is_loaded([], opts) def switches(), do: [pid: :integer] @@ -128,9 +128,9 @@ defmodule RabbitMQ.CLI.Ctl.Commands.WaitCommand do end defp wait_for_application(node_name, :rabbit_and_plugins) do - case :rpc.call(node_name, :rabbit, :await_startup, []) do - :ok -> :ok; - {:badrpc, reason} -> {:error, {:badrpc, reason}} + case :rabbit.await_startup(node_name) do + {:badrpc, err} -> {:error, {:badrpc, err}}; + other -> other end end @@ -149,7 +149,7 @@ defmodule RabbitMQ.CLI.Ctl.Commands.WaitCommand do case is_os_process_alive(pid) do true -> case Node.ping(node_name) do - :pong -> :ok; + :pong -> :ok :pang -> {:error, :pang} end; false -> {:error, :process_not_running} diff --git a/deps/rabbitmq_cli/lib/rabbitmqctl.ex b/deps/rabbitmq_cli/lib/rabbitmqctl.ex index e53e8d8c47..95cd08fbd7 100644 --- a/deps/rabbitmq_cli/lib/rabbitmqctl.ex +++ b/deps/rabbitmq_cli/lib/rabbitmqctl.ex @@ -267,6 +267,16 @@ defmodule RabbitMQCtl do {:error, ExitCodes.exit_code_for(result), "Error: operation #{op} on node #{opts[:node]} timed out. Timeout: #{to}"} end + defp format_error({:error, {:timeout, to} = result}, opts, module) do + op = CommandModules.module_to_command(module) + {:error, ExitCodes.exit_code_for(result), + "Error: operation #{op} on node #{opts[:node]} timed out. Timeout: #{to}"} + end + defp format_error({:error, :timeout = result}, opts, module) do + op = CommandModules.module_to_command(module) + {:error, ExitCodes.exit_code_for(result), + "Error: operation #{op} on node #{opts[:node]} timed out. Timeout: #{opts[:timeout]}"} + end defp format_error({:error, err} = result, _, _) do string_err = Helpers.string_or_inspect(err) {:error, ExitCodes.exit_code_for(result), "Error:\n#{string_err}"} diff --git a/deps/rabbitmq_cli/mix.exs b/deps/rabbitmq_cli/mix.exs index a21d701069..94af13880d 100644 --- a/deps/rabbitmq_cli/mix.exs +++ b/deps/rabbitmq_cli/mix.exs @@ -50,7 +50,7 @@ defmodule RabbitMQCtl.MixfileBase do defp add_modules(app, :test) do - # There are issue with building a package without this line ¯\_(ツ)_/¯ + # There are issues with building a package without this line ¯\_(ツ)_/¯ Mix.Project.get path = Mix.Project.compile_path mods = modules_from(Path.wildcard("#{path}/*.beam")) diff --git a/deps/rabbitmq_cli/test/plugins/set_plugins_command_test.exs b/deps/rabbitmq_cli/test/plugins/set_plugins_command_test.exs index 376648cbeb..165978c4a4 100644 --- a/deps/rabbitmq_cli/test/plugins/set_plugins_command_test.exs +++ b/deps/rabbitmq_cli/test/plugins/set_plugins_command_test.exs @@ -19,7 +19,6 @@ defmodule SetPluginsCommandTest do @command RabbitMQ.CLI.Plugins.Commands.SetCommand - #RABBITMQ_PLUGINS_DIR=~/dev/master/deps RABBITMQ_ENABLED_PLUGINS_FILE=/var/folders/cl/jnydxpf92rg76z05m12hlly80000gq/T/rabbitmq-test-instances/rabbit/enabled_plugins RABBITMQ_HOME=~/dev/master/deps/rabbit ./rabbitmq-plugins list_plugins setup_all do RabbitMQ.CLI.Core.Distribution.start() diff --git a/deps/rabbitmq_cli/test/wait_command_test.exs b/deps/rabbitmq_cli/test/wait_command_test.exs index a61b11ed1d..5e8b730ff5 100644 --- a/deps/rabbitmq_cli/test/wait_command_test.exs +++ b/deps/rabbitmq_cli/test/wait_command_test.exs @@ -22,14 +22,13 @@ defmodule WaitCommandTest do setup_all do RabbitMQ.CLI.Core.Distribution.start() + rabbitmq_home = :rabbit_misc.rpc_call(get_rabbit_hostname(), :code, :lib_dir, [:rabbit]) - - :ok + {:ok, opts: %{node: get_rabbit_hostname(), + rabbitmq_home: rabbitmq_home, + timeout: 500}} end - setup do - {:ok, opts: %{node: get_rabbit_hostname(), timeout: 500}} - end test "validate: cannot have both pid and pidfile", context do {:validation_failure, "Cannot specify both pid and pidfile"} = @@ -45,6 +44,12 @@ defmodule WaitCommandTest do assert @command.validate(["pid_file", "extra"], context[:opts]) == {:validation_failure, :too_many_args} end + test "validate: failure to load rabbit application is reported as an error", context do + options = Map.merge(Map.delete(context[:opts], :rabbitmq_home), %{pid: 123}) + {:validation_failure, {:unable_to_load_rabbit, _}} = + @command.validate([], options) + end + test "run: times out waiting for non-existent pid file", context do {:error, {:timeout, _}} = @command.run(["pid_file"], context[:opts]) |> Enum.to_list |> List.last end