@@ -79,45 +79,60 @@ is_authorized(ReqData, Context) ->
7979
8080delete_resource (ReqData , # context {user = # user {username = Username }}= Context ) ->
8181 VHost = rabbit_mgmt_util :id (vhost , ReqData ),
82- Reply = case rabbit_mgmt_util :id (name , ReqData ) of
83- none ->
84- false ;
85- Name ->
86- case get_shovel_node (VHost , Name , ReqData , Context ) of
87- undefined -> rabbit_log :error (" Could not find shovel data for shovel '~ts ' in vhost: '~ts '" , [Name , VHost ]),
88- case is_restart (ReqData ) of
89- true ->
90- false ;
91- % % this is a deletion attempt
92- false ->
93- % % if we do not know the node, use the local one
94- try_delete (node (), VHost , Name , Username ),
95- true
82+ case rabbit_mgmt_util :id (name , ReqData ) of
83+ none ->
84+ {false , ReqData , Context };
85+ Name ->
86+ case get_shovel_node (VHost , Name , ReqData , Context ) of
87+ undefined -> rabbit_log :error (" Could not find shovel data for shovel '~ts ' in vhost: '~ts '" , [Name , VHost ]),
88+ case is_restart (ReqData ) of
89+ true ->
90+ {false , ReqData , Context };
91+ % % this is a deletion attempt
92+ false ->
93+ % % if we do not know the node, use the local one
94+ case try_delete (node (), VHost , Name , Username ) of
95+ true -> {true , ReqData , Context };
96+ % % NOTE: that how it was before, try_delete return was ignored and true returned ¯\_(ツ)_/¯
97+ false -> {true , ReqData , Context };
98+ locked -> Reply = cowboy_req :reply (405 , #{<<" content-type" >> => <<" text/plain" >>},
99+ " Protected" , ReqData ),
100+ {halt , Reply , Context };
101+ % % NOTE: that how it was before, try_delete return was ignored and true returned ¯\_(ツ)_/¯
102+ error -> {true , ReqData , Context }
103+ end
104+ end ;
105+ Node ->
106+ % % We must distinguish between a delete and a restart
107+ case is_restart (ReqData ) of
108+ true ->
109+ rabbit_log :info (" Asked to restart shovel '~ts ' in vhost '~ts ' on node '~s '" , [Name , VHost , Node ]),
110+ try erpc :call (Node , rabbit_shovel_util , restart_shovel , [VHost , Name ], ? SHOVEL_CALLS_TIMEOUT_MS ) of
111+ ok -> {true , ReqData , Context };
112+ {error , not_found } ->
113+ rabbit_log :error (" Could not find shovel data for shovel '~s ' in vhost: '~s '" , [Name , VHost ]),
114+ {false , ReqData , Context }
115+ catch _ :Reason ->
116+ rabbit_log :error (" Failed to restart shovel '~s ' on vhost '~s ', reason: ~p " ,
117+ [Name , VHost , Reason ]),
118+ {false , ReqData , Context }
96119 end ;
97- Node ->
98- % % We must distinguish between a delete and a restart
99- case is_restart (ReqData ) of
100- true ->
101- rabbit_log :info (" Asked to restart shovel '~ts ' in vhost '~ts ' on node '~s '" , [Name , VHost , Node ]),
102- try erpc :call (Node , rabbit_shovel_util , restart_shovel , [VHost , Name ], ? SHOVEL_CALLS_TIMEOUT_MS ) of
103- ok -> true ;
104- {error , not_found } ->
105- rabbit_log :error (" Could not find shovel data for shovel '~s ' in vhost: '~s '" , [Name , VHost ]),
106- false
107- catch _ :Reason ->
108- rabbit_log :error (" Failed to restart shovel '~s ' on vhost '~s ', reason: ~p " ,
109- [Name , VHost , Reason ]),
110- false
111- end ;
112-
113- _ ->
114- try_delete (Node , VHost , Name , Username ),
115- true
116120
121+ _ ->
122+ case try_delete (Node , VHost , Name , Username ) of
123+ true -> {true , ReqData , Context };
124+ % % NOTE: that how it was before, try_delete return was ignored and true returned ¯\_(ツ)_/¯
125+ false -> {true , ReqData , Context };
126+ locked -> Reply = cowboy_req :reply (405 , #{<<" content-type" >> => <<" text/plain" >>},
127+ " Protected" , ReqData ),
128+ {halt , Reply , Context };
129+ % % NOTE: that how it was before, try_delete return was ignored and true returned ¯\_(ツ)_/¯
130+ error -> {true , ReqData , Context }
117131 end
132+
118133 end
119- end ,
120- { Reply , ReqData , Context } .
134+ end
135+ end .
121136
122137% %--------------------------------------------------------------------
123138
@@ -168,7 +183,7 @@ find_matching_shovel(VHost, Name, Shovels) ->
168183 undefined
169184 end .
170185
171- -spec try_delete (node (), vhost :name (), any (), rabbit_types :username ()) -> boolean () .
186+ -spec try_delete (node (), vhost :name (), any (), rabbit_types :username ()) -> true | false | locked | error .
172187try_delete (Node , VHost , Name , Username ) ->
173188 rabbit_log :info (" Asked to delete shovel '~ts ' in vhost '~ts ' on node '~s '" , [Name , VHost , Node ]),
174189 % % this will clear the runtime parameter, the ultimate way of deleting a dynamic Shovel eventually. MK.
@@ -177,8 +192,13 @@ try_delete(Node, VHost, Name, Username) ->
177192 {error , not_found } ->
178193 rabbit_log :error (" Could not find shovel data for shovel '~s ' in vhost: '~s '" , [Name , VHost ]),
179194 false
180- catch _ :Reason ->
195+ catch
196+ _ :{exception , {amqp_error , resource_locked , Reason , _ }} ->
181197 rabbit_log :error (" Failed to delete shovel '~s ' on vhost '~s ', reason: ~p " ,
182198 [Name , VHost , Reason ]),
183- false
199+ locked ;
200+ _ :Reason ->
201+ rabbit_log :error (" Failed to delete shovel '~s ' on vhost '~s ', reason: ~p " ,
202+ [Name , VHost , Reason ]),
203+ error
184204 end .
0 commit comments