On July 31, 2019 1:23 pm, Stefan Reiter wrote: > Since we cannot easily proxy the vzdump command to other nodes, we > expect the client to call this on all nodes they want to run the backup > job on (and use standard proxyto=>'node'). > > Signed-off-by: Stefan Reiter <s.rei...@proxmox.com> > --- > > I wasn't too sure regarding permissions, so I left it at the rather strict > Sys.Modify. This way, at least no one can use this to do stuff they couldn't > before (as in: if you had Sys.Modify permissions, you could trigger them > "immediately" before this patch too, by setting the day and time to "now" and > waiting a few minutes). > > PVE/API2/Backup.pm | 61 +++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 60 insertions(+), 1 deletion(-) > > diff --git a/PVE/API2/Backup.pm b/PVE/API2/Backup.pm > index ec6b4541..bf9a3330 100644 > --- a/PVE/API2/Backup.pm > +++ b/PVE/API2/Backup.pm > @@ -9,7 +9,7 @@ use PVE::Tools qw(extract_param); > use PVE::Cluster qw(cfs_register_file cfs_lock_file cfs_read_file > cfs_write_file); > use PVE::RESTHandler; > use PVE::RPCEnvironment; > -use PVE::JSONSchema; > +use PVE::JSONSchema qw(get_standard_option); > use PVE::Storage; > use PVE::Exception qw(raise_param_exc); > use PVE::VZDump; > @@ -491,4 +491,63 @@ __PACKAGE__->register_method({ > die "$@" if ($@); > }}); > > +# this method is most useful when called on all nodes in parallel > +# (to correctly mimic scheduled behaviour) > +__PACKAGE__->register_method({ > + name => 'run_job', > + path => '{id}/{node}/run',
this is under /cluster/backup, so I'd like a bit more input on whether we want to put this API call here since it's node specific.. another approach would be to call Backup->read_job and VZDump->vzdump directly from the client, since we most likely already have the job data the former might even be not needed, and we would not need to introduce a new API call unless I am missing something? > + method => 'POST', > + description => "Run vzdump backup job definition on a specified node > immediately.", > + permissions => { > + check => ['perm', '/', ['Sys.Modify']], > + }, > + protected => 1, > + proxyto => 'node', > + parameters => { > + additionalProperties => 0, > + properties => { > + id => { > + type => 'string', > + description => "The job ID.", > + maxLength => 50, since this would now be the fourth time in this file, it might make sense to refactor it into at least a local variable > + }, > + node => get_standard_option('pve-node') > + }, > + }, > + returns => { type => 'string' }, > + code => sub { > + my ($param) = @_; > + > + my $run_job = sub { > + my $data = cfs_read_file('vzdump.cron'); > + my $jobs = $data->{jobs} || []; > + > + my $found_job; > + foreach my $job (@$jobs) { > + if ($job->{id} eq $param->{id}) { > + $found_job = $job; > + last; > + } > + } this could also be a single grep ;) > + > + raise_param_exc({ id => "No such job '$param->{id}'" }) > + if !$found_job; the whole block is basically the read_job API method, maybe we could factor that out and/or reuse it (and clean it up regarding dead code :-P)? > + > + # remove scheduling-specific parameters > + delete $found_job->{enabled}; > + delete $found_job->{starttime}; > + delete $found_job->{dow}; > + delete $found_job->{id}; > + > + $found_job->{lockwait} = 0; > + > + # returns a upid > + return PVE::API2::VZDump->vzdump($found_job); missing use for that API path (not a problem now, but might hit us when this gets called from somewhere other then pveproxy/pvedaemon). > + }; > + my $val = cfs_lock_file('vzdump.cron', undef, $run_job); do we really need this? we don't modify the crontab file, we just read it.. we could basically my $job = __PACKAGE__->read_job($param); [..] return PVE::API2::VZDump->vzdump($job); unless I am missing something? > + die "$@" if ($@); > + > + return $val; > + }}); > + > 1; > -- > 2.20.1 > > > _______________________________________________ > pve-devel mailing list > pve-devel@pve.proxmox.com > https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel