[pve-devel] [PATCH V4 manager 2/2] Send an email when a replication job fails.

[Thomas Lamprecht]

On 12/12/2017 10:23 AM, Wolfgang Link wrote:
>> Thomas Lamprecht <t.lamprecht at proxmox.com> hat am 12. Dezember 2017 um 08:20 geschrieben:
>>
>>
>> On 12/07/2017 12:07 PM, Wolfgang Link wrote:
>>> A email notification will be send for each job when the job fails.
>>> This message will only send when an error occurs and the fail count is on 1.
>>> ---
>>>  PVE/API2/Replication.pm  | 18 ++++++++++++++++--
>>>  PVE/CLI/pvesr.pm         | 11 ++++++++++-
>>>  bin/init.d/pvesr.service |  2 +-
>>>  3 files changed, 27 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm
>>> index 38449892..8c049363 100644
>>> --- a/PVE/API2/Replication.pm
>>> +++ b/PVE/API2/Replication.pm
>>> @@ -72,7 +72,7 @@ sub run_single_job {
>>>  
>>>  # passing $now and $verbose is useful for regression testing
>>>  sub run_jobs {
>>> -    my ($now, $logfunc, $verbose) = @_;
>>> +    my ($now, $logfunc, $verbose, $mail) = @_;
>>>  
>>>      my $iteration = $now // time();
>>>  
>>> @@ -83,7 +83,21 @@ sub run_jobs {
>>>  
>>>  	while (my $jobcfg = PVE::ReplicationState::get_next_job($iteration, $start_time)) {
>>>  	    my $guest_class = $lookup_guest_class->($jobcfg->{vmtype});
>>> -	    PVE::Replication::run_replication($guest_class, $jobcfg, $iteration, $start_time, $logfunc, 1, $verbose);
>>> +
>>> +	    eval {
>>> +		PVE::Replication::run_replication($guest_class, $jobcfg, $iteration, $start_time, $logfunc, $verbose);
>>> +	    };
>>> +	    if (my $err = $@) {
>>> +		warn "$jobcfg->{id}: got unexpected replication job error - $err";
>>> +		my $state = PVE::ReplicationState::read_state();
>>> +		my $jobstate = PVE::ReplicationState::extract_job_state($state, $jobcfg);
>>> +		eval {
>>> +		    PVE::Tools::sendmail('root', "Replication Job: $jobcfg->{id} failed", $err)
>>> +			if $jobstate->{fail_count} == 1 && $mail;
>>> +		};
>>
>>
>>
>>> +		warn ": $@" if $@;
>>                      ^^^^
>> Is the colon + space above intended?
> 
> Yes it is, because this is a sub-warning of the warning before.

OK.

>>
>>> +	    };
>>             ^^^
>> nit: semicolon not needed for if branch.
> 
> This semicolon belongs to the eval clause.
> 

No, it does *not*, vim tells me otherwise when matching parenthesis.


>> Looks OK otherwise:
>> Reviewed-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
>>
>>
>>> +
>>>  	    $start_time = $now // time();
>>>  	}
>>>      };
         ^^^
This is the eval one