Shovels: make changes to shovel status backward compatible

Author: dcorbacho

deps/rabbitmq_shovel/src/Elixir.RabbitMQ.CLI.Ctl.Commands.DeleteShovelCommand.erl

@@ -80,16 +80,9 @@ run([Name], #{node := Node, vhost := VHost, force := Force}) ->
                 undefined ->
                     try_force_removing(Node, VHost, Name, ActingUser),
                     {error, rabbit_data_coercion:to_binary(ErrMsg)};
-                {{_Name, _VHost}, _Type, {terminated, _}, _Metrics, _Timestamp} ->
-                    delete_shovel(ErrMsg, VHost, Name, ActingUser, Node, Node);
-                {{_Name, _VHost}, _Type, {terminated, Opts, _}, _Metrics, _Timestamp} ->
-                    HostingNode = proplists:get_value(node, Opts, Node),
-                    delete_shovel(ErrMsg, VHost, Name, ActingUser, HostingNode, Node);
                 {{_Name, _VHost}, _Type, {_State, Opts}, _Metrics, _Timestamp} ->
                     HostingNode = proplists:get_value(node, Opts, Node),
                     delete_shovel(ErrMsg, VHost, Name, ActingUser, HostingNode, Node);
-                {{_Name, _VHost}, _Type, {terminated, _}, _Timestamp} ->
-                    delete_shovel(ErrMsg, VHost, Name, ActingUser, Node, Node);
                 {{_Name, _VHost}, _Type, {_State, Opts}, _Timestamp} ->
                     HostingNode = proplists:get_value(node, Opts, Node),
                     delete_shovel(ErrMsg, VHost, Name, ActingUser, HostingNode, Node)

deps/rabbitmq_shovel/src/Elixir.RabbitMQ.CLI.Ctl.Commands.RestartShovelCommand.erl

@@ -62,17 +62,16 @@ run([Name], #{node := Node, vhost := VHost}) ->
             case rabbit_shovel_status:find_matching_shovel(VHost, Name, Xs) of
                 undefined ->
                     {error, rabbit_data_coercion:to_binary(ErrMsg)};
-                {{_Name, _VHost}, _Type, {terminated, Opts, _}, _Metrics, _Timestamp} ->
-                    restart_shovel(ErrMsg, Name, VHost, Opts);
                 {{_Name, _VHost}, _Type, {_State, Opts}, _Metrics, _Timestamp} ->
-                    restart_shovel(ErrMsg, Name, VHost, Opts);
+                    HostingNode = proplists:get_value(node, Opts, Node),
+                    restart_shovel(ErrMsg, Name, VHost, HostingNode);
                 {{_Name, _VHost}, _Type, {_State, Opts}, _Timestamp} ->
-                    restart_shovel(ErrMsg, Name, VHost, Opts)
+                    HostingNode = proplists:get_value(node, Opts, Node),
+                    restart_shovel(ErrMsg, Name, VHost, HostingNode)
             end
     end.
 
-restart_shovel(ErrMsg, Name, VHost, Opts) ->
-    {_, HostingNode} = lists:keyfind(node, 1, Opts),
+restart_shovel(ErrMsg, Name, VHost, HostingNode) ->
     case rabbit_misc:rpc_call(
         HostingNode, rabbit_shovel_util, restart_shovel, [VHost, Name]) of
         {badrpc, _} = Error ->

deps/rabbitmq_shovel/src/rabbit_shovel_status.erl

@@ -44,7 +44,7 @@
               | {terminated, term()}.
 -type info_with_node() :: starting
                         | {running, proplists:proplist()}
-                        | {terminated, proplists:proplist(), term()}.
+                        | {terminated, proplists:proplist()}.
 -type blocked_status() :: running | flow | blocked.
 -type shovel_status() :: blocked_status() | ignore.
 
@@ -219,7 +219,8 @@ inject_node_info(Node, Shovels) ->
              {Name, Type, {State, Opts}, Metrics, Timestamp};
            %% terminated
            ({Name, Type, {terminated, Reason}, Metrics, Timestamp}) ->
-             {Name, Type, {terminated, [{node, Node}], Reason}, Metrics, Timestamp};
+             {Name, Type, {terminated, [{node, Node},
+                                        {reason, Reason}]}, Metrics, Timestamp};
             %% running
            ({Name, Type, {State, Opts}, Metrics, Timestamp}) ->
              Opts1 = Opts ++ [{node, Node}],

deps/rabbitmq_shovel/test/delete_shovel_command_SUITE.erl

@@ -68,6 +68,15 @@ end_per_group(_Group, Config) ->
       rabbit_ct_client_helpers:teardown_steps() ++
       rabbit_ct_broker_helpers:teardown_steps()).
 
+init_per_testcase(Testcase, Config) when Testcase == delete_invalid_uri_another_node ->
+    case rabbit_ct_helpers:is_mixed_versions(Config) of
+        true ->
+            %% The code changes to delete shovel are compatible with older versions, however
+            %% older versions fail to delete invalid shovels
+            {skip, "not mixed versions compatible"};
+        false ->
+            rabbit_ct_helpers:testcase_started(Config, Testcase)
+    end;
 init_per_testcase(Testcase, Config) ->
     rabbit_ct_helpers:testcase_started(Config, Testcase).