[pve-devel] [PATCH pve-manager 01/18] pvesr: add pve storage replication tool
Fabian Grünbichler
f.gruenbichler at proxmox.com
Mon May 29 12:13:07 CEST 2017
On Mon, May 29, 2017 at 11:57:48AM +0200, Dietmar Maurer wrote:
> comments inline
> >
> > On Tue, May 23, 2017 at 09:08:40AM +0200, Dietmar Maurer wrote:
> > > Just added code to configure jobs. Replication itself is not
> > > implemented.
> > >
> > > Signed-off-by: Dietmar Maurer <dietmar at proxmox.com>
> > > ---
> > > PVE/API2/Cluster.pm | 7 ++
> > > PVE/API2/Makefile | 2 +
> > > PVE/API2/Nodes.pm | 7 ++
> > > PVE/API2/Replication.pm | 93 ++++++++++++++++
> > > PVE/API2/ReplicationConfig.pm | 217 +++++++++++++++++++++++++++++++++++++
> > > PVE/CLI/Makefile | 2 +-
> > > PVE/CLI/pvesr.pm | 151 ++++++++++++++++++++++++++
> > > PVE/Makefile | 1 +
> > > PVE/Replication.pm | 242
> > > ++++++++++++++++++++++++++++++++++++++++++
> > > bin/Makefile | 2 +-
> > > bin/pvesr | 8 ++
> > > 11 files changed, 730 insertions(+), 2 deletions(-)
> > > create mode 100644 PVE/API2/Replication.pm
> > > create mode 100644 PVE/API2/ReplicationConfig.pm
> > > create mode 100644 PVE/CLI/pvesr.pm
> > > create mode 100644 PVE/Replication.pm
> > > create mode 100644 bin/pvesr
> > >
> > > diff --git a/PVE/API2/Cluster.pm b/PVE/API2/Cluster.pm
> > > index 6ce86de4..0e94e9ff 100644
> > > --- a/PVE/API2/Cluster.pm
> > > +++ b/PVE/API2/Cluster.pm
> > > @@ -22,10 +22,16 @@ use PVE::RPCEnvironment;
> > > use PVE::JSONSchema qw(get_standard_option);
> > > use PVE::Firewall;
> > > use PVE::API2::Firewall::Cluster;
> > > +use PVE::API2::ReplicationConfig;
> > >
> > > use base qw(PVE::RESTHandler);
> > >
> > > __PACKAGE__->register_method ({
> > > + subclass => "PVE::API2::ReplicationConfig",
> > > + path => 'replication',
> > > +});
> > > +
> > > +__PACKAGE__->register_method ({
> > > subclass => "PVE::API2::ClusterConfig",
> > > path => 'config',
> > > });
> > > @@ -82,6 +88,7 @@ __PACKAGE__->register_method ({
> > > { name => 'log' },
> > > { name => 'options' },
> > > { name => 'resources' },
> > > + { name => 'replication' },
> > > { name => 'tasks' },
> > > { name => 'backup' },
> > > { name => 'ha' },
> > > diff --git a/PVE/API2/Makefile b/PVE/API2/Makefile
> > > index 4dfcbb56..86d75d36 100644
> > > --- a/PVE/API2/Makefile
> > > +++ b/PVE/API2/Makefile
> > > @@ -1,6 +1,8 @@
> > > include ../../defines.mk
> > >
> > > PERLSOURCE = \
> > > + Replication.pm \
> > > + ReplicationConfig.pm \
> > > Ceph.pm \
> > > APT.pm \
> > > Subscription.pm \
> > > diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
> > > index a45ca6db..885cf116 100644
> > > --- a/PVE/API2/Nodes.pm
> > > +++ b/PVE/API2/Nodes.pm
> > > @@ -40,6 +40,7 @@ use PVE::API2::VZDump;
> > > use PVE::API2::APT;
> > > use PVE::API2::Ceph;
> > > use PVE::API2::Firewall::Host;
> > > +use PVE::API2::Replication;
> > > use Digest::MD5;
> > > use Digest::SHA;
> > > use PVE::API2::Disks;
> > > @@ -113,6 +114,11 @@ __PACKAGE__->register_method ({
> > > });
> > >
> > > __PACKAGE__->register_method ({
> > > + subclass => "PVE::API2::Replication",
> > > + path => 'replication',
> > > +});
> > > +
> > > +__PACKAGE__->register_method ({
> > > name => 'index',
> > > path => '',
> > > method => 'GET',
> > > @@ -147,6 +153,7 @@ __PACKAGE__->register_method ({
> > > { name => 'tasks' },
> > > { name => 'rrd' }, # fixme: remove?
> > > { name => 'rrddata' },# fixme: remove?
> > > + { name => 'replication' },
> > > { name => 'vncshell' },
> > > { name => 'spiceshell' },
> > > { name => 'time' },
> > > diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm
> > > new file mode 100644
> > > index 00000000..3c631e44
> > > --- /dev/null
> > > +++ b/PVE/API2/Replication.pm
> > > @@ -0,0 +1,93 @@
> > > +package PVE::API2::Replication;
> > > +
> > > +use warnings;
> > > +use strict;
> > > +
> > > +use PVE::JSONSchema qw(get_standard_option);
> > > +use PVE::RPCEnvironment;
> > > +use PVE::ReplicationConfig;
> > > +use PVE::Replication;
> > > +
> > > +use PVE::RESTHandler;
> > > +
> > > +use base qw(PVE::RESTHandler);
> > > +
> > > +__PACKAGE__->register_method ({
> > > + name => 'index',
> > > + path => '',
> > > + method => 'GET',
> > > + permissions => { user => 'all' },
> > > + description => "Directory index.",
> > > + parameters => {
> > > + additionalProperties => 0,
> > > + properties => {
> > > + node => get_standard_option('pve-node'),
> > > + },
> > > + },
> > > + returns => {
> > > + type => 'array',
> > > + items => {
> > > + type => "object",
> > > + properties => {},
> > > + },
> > > + links => [ { rel => 'child', href => "{name}" } ],
> > > + },
> > > + code => sub {
> > > + my ($param) = @_;
> > > +
> > > + return [
> > > + { name => 'status' },
> > > + ];
> > > + }});
> > > +
> > > +
> > > +__PACKAGE__->register_method ({
> > > + name => 'status',
> > > + path => 'status',
> > > + method => 'GET',
> > > + description => "List replication job status.",
> > > + permissions => {
> > > + description => "Only list jobs where you have VM.Audit permissons on
> > > /vms/<vmid>.",
> >
> > this is not really a description of the permissions ;) also a typo in
> > "permissions". maybe:
> >
> > Requires the VM.Audit permission on /vms/<vmid>.
>
> fixed in v2
>
> > also, maybe it makes sense to expose the status of a single guest as
> > well?
>
> yes, waiting for feedback from GUI development.
>
sounds good to me
> >
> > > + user => 'all',
> > > + },
> > > + protected => 1,
> > > + proxyto => 'node',
> > > + parameters => {
> > > + additionalProperties => 0,
> > > + properties => {
> > > + node => get_standard_option('pve-node'),
> > > + },
> > > + },
> > > + returns => {
> > > + type => 'array',
> > > + items => {
> > > + type => "object",
> > > + properties => {},
> > > + },
> > > + links => [ { rel => 'child', href => "{vmid}" } ],
> > > + },
> > > + code => sub {
> > > + my ($param) = @_;
> > > +
> > > + my $rpcenv = PVE::RPCEnvironment::get();
> > > + my $authuser = $rpcenv->get_user();
> > > +
> > > + my $jobs = PVE::Replication::job_status();
> > > +
> > > + my $res = [];
> > > + foreach my $id (sort keys %$jobs) {
> > > + my $d = $jobs->{$id};
> > > + my $state = delete $d->{state};
> > > + my $vmid = $d->{guest};
> > > + next if !$rpcenv->check($authuser, "/vms/$vmid", [ 'VM.Audit' ]);
> > > + $d->{id} = $id;
> > > + foreach my $k (qw(last_sync fail_count error duration)) {
> > > + $d->{$k} = $state->{$k} if defined($state->{$k});
> > > + }
> > > + push @$res, $d;
> > > + }
> > > +
> > > + return $res;
> > > + }});
> > > +
> > > +1;
> > > diff --git a/PVE/API2/ReplicationConfig.pm b/PVE/API2/ReplicationConfig.pm
> > > new file mode 100644
> > > index 00000000..147b5891
> > > --- /dev/null
> > > +++ b/PVE/API2/ReplicationConfig.pm
> > > @@ -0,0 +1,217 @@
> > > +package PVE::API2::ReplicationConfig;
> >
> > the whole file is riddled with whitespace and indentation errors. these
> > should probably be fixed before applying.
>
> fixed in v2
>
> > > +
> > > +use warnings;
> > > +use strict;
> > > +
> > > +use PVE::Tools qw(extract_param);
> > > +use PVE::Exception qw(raise_perm_exc);
> > > +use PVE::JSONSchema qw(get_standard_option);
> > > +use PVE::RPCEnvironment;
> > > +use PVE::ReplicationConfig;
> > > +
> > > +use PVE::RESTHandler;
> > > +
> > > +use base qw(PVE::RESTHandler);
> > > +
> > > +__PACKAGE__->register_method ({
> > > + name => 'index',
> > > + path => '',
> > > + method => 'GET',
> > > + description => "List replication jobs.",
> > > + permissions => {
> > > + description => "Only list jobs where you have VM.Audit permissons on
> > > /vms/<vmid>.",
> >
> > same as above for the status
>
> fixed in v2
>
> > > + user => 'all',
> > > + },
> > > + parameters => {
> > > + additionalProperties => 0,
> > > + properties => {},
> > > + },
> > > + returns => {
> > > + type => 'array',
> > > + items => {
> > > + type => "object",
> > > + properties => {},
> > > + },
> > > + links => [ { rel => 'child', href => "{vmid}" } ],
> > > + },
> > > + code => sub {
> > > + my ($param) = @_;
> > > +
> > > + my $rpcenv = PVE::RPCEnvironment::get();
> > > + my $authuser = $rpcenv->get_user();
> > > +
> > > + my $cfg = PVE::ReplicationConfig->new();
> > > +
> > > + my $res = [];
> > > + foreach my $id (sort keys %{$cfg->{ids}}) {
> > > + my $d = $cfg->{ids}->{$id};
> > > + my $vmid = $d->{guest};
> > > + next if !$rpcenv->check($authuser, "/vms/$vmid", [ 'VM.Audit' ]);
> > > + $d->{id} = $id;
> > > + push @$res, $d;
> > > + }
> > > +
> > > + return $res;
> > > + }});
> > > +
> > > +__PACKAGE__->register_method ({
> > > + name => 'read',
> > > + path => '{id}',
> > > + method => 'GET',
> > > + description => "Read replication job configuration.",
> > > + permissions => {
> > > + description => "Only list jobs where you have VM.Audit permissons on
> > > /vms/<vmid>.",
> >
> > same as for status
>
> fixed in v2
>
> > > + user => 'all',
> > > + },
> > > + parameters => {
> > > + additionalProperties => 0,
> > > + properties => {
> > > + id => get_standard_option('pve-replication-id'),
> > > + },
> > > + },
> > > + returns => { type => 'object' },
> > > + code => sub {
> > > + my ($param) = @_;
> > > +
> > > + my $rpcenv = PVE::RPCEnvironment::get();
> > > + my $authuser = $rpcenv->get_user();
> > > +
> > > + my $cfg = PVE::ReplicationConfig->new();
> > > +
> > > + my $data = $cfg->{ids}->{$param->{id}};
> > > +
> > > + die "no such replication job '$param->{id}'\n" if !defined($data);
> > > +
> > > + my $vmid = $data->{guest};
> > > +
> > > + raise_perm_exc() if !$rpcenv->check($authuser, "/vms/$vmid", [ 'VM.Audit'
> > > ]);
> > > +
> > > + $data->{id} = $param->{id};
> > > +
> > > + return $data;
> > > + }});
> > > +
> > > +__PACKAGE__->register_method ({
> > > + name => 'create',
> > > + path => '',
> > > + protected => 1,
> > > + method => 'POST',
> > > + description => "Create a new replication job",
> > > + permissions => {
> > > + check => ['perm', '/storage', ['Datastore.Allocate']],
> >
> > why? this does not seem right to me..
>
> why not?
IMHO using DataStore.Allocate as a catch all for all the storage related
things is okay (we could make it more fine-grained by manually checking
just the used storages, but that would also be more complicated).
what is missing is some kind of check for a guest related permission -
VM.Audit or VM.Config would be likely candidates?
sorry for not fleshing this out more in my initial mail.
>
> > > + },
> > > + parameters => PVE::ReplicationConfig->createSchema(),
> > > + returns => { type => 'null' },
> > > + code => sub {
> > > + my ($param) = @_;
> > > +
> > > + my $type = extract_param($param, 'type');
> > > + my $plugin = PVE::ReplicationConfig->lookup($type);
> > > + my $id = extract_param($param, 'id');
> > > +
> > > + my $code = sub {
> > > + my $cfg = PVE::ReplicationConfig->new();
> > > +
> > > + #die "replication job for guest '$param->{guest}' to target
> > > '$param->{target}' already exists\n"
> >
> > leftover? should already be handled by write_config
>
> removed in v2
>
> > > + die "replication job '$id' already exists\n"
> > > + if $cfg->{ids}->{$id};
> > > +
> > > + my $opts = $plugin->check_config($id, $param, 1, 1);
> > > +
> > > + $cfg->{ids}->{$id} = $opts;
> > > +
> > > + $cfg->write();
> > > + };
> > > +
> > > + PVE::ReplicationConfig::lock($code);
> > > +
> > > + return undef;
> > > + }});
> > > +
> > > +
> > > +__PACKAGE__->register_method ({
> > > + name => 'update',
> > > + protected => 1,
> > > + path => '{id}',
> > > + method => 'PUT',
> > > + description => "Update replication job configuration.",
> > > + permissions => {
> > > + check => ['perm', '/storage', ['Datastore.Allocate']],
> >
> > again - this does not seem right to me?
> >
> > > + },
> > > + parameters => PVE::ReplicationConfig->updateSchema(),
> > > + returns => { type => 'null' },
> > > + code => sub {
> > > + my ($param) = @_;
> > > +
> > > + my $id = extract_param($param, 'id');
> > > +
> > > + my $code = sub {
> > > + my $cfg = PVE::ReplicationConfig->new();
> > > +
> > > + my $data = $cfg->{ids}->{$id};
> > > + die "no such job '$id'\n" if !$data;
> > > +
> > > + my $plugin = PVE::ReplicationConfig->lookup($data->{type});
> > > + my $opts = $plugin->check_config($id, $param, 0, 1);
> > > +
> > > + foreach my $k (%$opts) {
> > > + $data->{$k} = $opts->{$k};
> > > + }
> > > +
> > > + $cfg->write();
> > > + };
> > > +
> > > + PVE::ReplicationConfig::lock($code);
> > > +
> > > + return undef;
> > > + }});
> > > +
> > > +__PACKAGE__->register_method ({
> > > + name => 'delete',
> > > + protected => 1,
> > > + path => '{id}',
> > > + method => 'DELETE',
> > > + description => "Delete replication job",
> > > + permissions => {
> > > + check => ['perm', '/storage', ['Datastore.Allocate']],
> >
> > and once more ;)
> >
> > > + },
> > > + parameters => {
> > > + additionalProperties => 0,
> > > + properties => {
> > > + id => get_standard_option('pve-replication-id'),
> > > + keep => {
> > > + description => "Keep replicated data at target (do not remove).",
> > > + type => 'boolean',
> > > + optional => 1,
> > > + default => 0,
> > > + },
> > > + }
> > > + },
> > > + returns => { type => 'null' },
> > > + code => sub {
> > > + my ($param) = @_;
> > > +
> > > + my $id = extract_param($param, 'id');
> > > +
> > > + my $code = sub {
> > > + my $cfg = PVE::ReplicationConfig->new();
> > > +
> > > + my $data = $cfg->{ids}->{$id};
> > > + die "no such job '$id'\n" if !$data;
> > > +
> > > + if (!$param->{keep}) {
> > > + # fixme: cleanup data at target
> > > +
> > > + }
> > > + # fixme: cleanup snapshots
> > > +
> > > + delete $cfg->{ids}->{$id};
> > > +
> > > + $cfg->write();
> > > + };
> > > +
> > > + PVE::ReplicationConfig::lock($code);
> > > +
> > > + return undef;
> > > + }});
> > > +1;
> > > diff --git a/PVE/CLI/Makefile b/PVE/CLI/Makefile
> > > index b005a8f1..3a27503d 100644
> > > --- a/PVE/CLI/Makefile
> > > +++ b/PVE/CLI/Makefile
> > > @@ -1,6 +1,6 @@
> > > include ../../defines.mk
> > >
> > > -SOURCES=vzdump.pm pvesubscription.pm pveceph.pm pveam.pm
> > > +SOURCES=vzdump.pm pvesubscription.pm pveceph.pm pveam.pm pvesr.pm
> > >
> > > all:
> > >
> > > diff --git a/PVE/CLI/pvesr.pm b/PVE/CLI/pvesr.pm
> > > new file mode 100644
> > > index 00000000..1dc24635
> > > --- /dev/null
> > > +++ b/PVE/CLI/pvesr.pm
> > > @@ -0,0 +1,151 @@
> > > +package PVE::CLI::pvesr;
> > > +
> > > +use strict;
> > > +use warnings;
> > > +use POSIX qw(strftime);
> > > +use Time::Local;
> >
> > not used?
>
> removed in v2
>
> >
> > > +use JSON;
> > > +
> > > +use PVE::JSONSchema qw(get_standard_option);
> > > +use PVE::INotify;
> > > +use PVE::RPCEnvironment;
> > > +use PVE::Tools qw(extract_param);
> > > +use PVE::SafeSyslog;
> > > +use PVE::CLIHandler;
> > > +
> > > +use PVE::Replication;
> > > +use PVE::API2::ReplicationConfig;
> > > +use PVE::API2::Replication;
> > > +
> > > +use base qw(PVE::CLIHandler);
> > > +
> > > +my $nodename = PVE::INotify::nodename();
> > > +
> > > +sub setup_environment {
> > > + PVE::RPCEnvironment->setup_default_cli_env();
> > > +}
> > > +
> > > +__PACKAGE__->register_method ({
> > > + name => 'run',
> > > + path => 'run',
> > > + method => 'POST',
> > > + description => "This method is called by the systemd-timer and executes
> > > all (or a specific) sync jobs.",
> > > + parameters => {
> > > + additionalProperties => 0,
> > > + properties => {
> > > + id => get_standard_option('pve-replication-id', { optional => 1 }),
> > > + },
> > > + },
> > > + returns => { type => 'null' },
> > > + code => sub {
> > > + my ($param) = @_;
> > > +
> > > + if (my $id = extract_param($param, 'id')) {
> > > +
> > > + PVE::Replication::run_single_job($id);
> > > +
> > > + } else {
> > > +
> > > + PVE::Replication::run_jobs();
> > > + }
> > > +
> > > + return undef;
> > > + }});
> > > +
> > > +__PACKAGE__->register_method ({
> > > + name => 'enable',
> > > + path => 'enable',
> > > + method => 'POST',
> > > + description => "Enable a replication jobs.",
> >
> > s/jobs/job/
>
> fixed in v2
>
> > > + parameters => {
> > > + additionalProperties => 0,
> > > + properties => {
> > > + id => get_standard_option('pve-replication-id'),
> > > + },
> > > + },
> > > + returns => { type => 'null' },
> > > + code => sub {
> > > + my ($param) = @_;
> > > +
> > > + $param->{disable} = 0;
> > > +
> > > + return PVE::API2::ReplicationConfig->update($param);
> > > + }});
> > > +
> > > +__PACKAGE__->register_method ({
> > > + name => 'disable',
> > > + path => 'disable',
> > > + method => 'POST',
> > > + description => "Disable a replication jobs.",
> >
> > s/jobs/job/
>
> fixed in v2
>
> > > + parameters => {
> > > + additionalProperties => 0,
> > > + properties => {
> > > + id => get_standard_option('pve-replication-id'),
> > > + },
> > > + },
> > > + returns => { type => 'null' },
> > > + code => sub {
> > > + my ($param) = @_;
> > > +
> > > + $param->{disable} = 1;
> > > +
> > > + return PVE::API2::ReplicationConfig->update($param);
> > > + }});
> > > +
> > > +my $print_job_list = sub {
> > > + my ($list) = @_;
> > > +
> > > + my $format = "%-20s %10s %-20s %10s %5s %8s\n";
> > > +
> > > + printf($format, "JobID", "GuestID", "Target", "Interval", "Rate",
> > > "Enabled");
> > > +
> > > + foreach my $job (sort { $a->{guest} <=> $b->{guest} } @$list) {
> > > + my $plugin = PVE::ReplicationConfig->lookup($job->{type});
> > > + my $tid = $plugin->get_unique_target_id($job);
> > > +
> > > + printf($format, $job->{id}, $job->{guest}, $tid,
> > > + defined($job->{interval}) ? $job->{interval} : '-',
> > > + defined($job->{rate}) ? $job->{rate} : '-',
> > > + $job->{disable} ? 'no' : 'yes'
> > > + );
> > > + }
> > > +};
> > > +
> > > +my $print_job_status = sub {
> > > + my ($list) = @_;
> > > +
> > > + my $format = "%-20s %10s %-20s %20s %10s %10s %s\n";
> > > +
> > > + printf($format, "JobID", "GuestID", "Target", "LastSync", "Duration",
> > > "FailCount", "State");
> > > +
> > > + foreach my $job (sort { $a->{guest} <=> $b->{guest} } @$list) {
> > > + my $plugin = PVE::ReplicationConfig->lookup($job->{type});
> > > + my $tid = $plugin->get_unique_target_id($job);
> > > +
> > > + my $timestr = $job->{last_sync} ?
> > > + strftime("%Y-%m-%d_%H:%M:%S", localtime($job->{last_sync})) : '-';
> > > +
> > > + printf($format, $job->{id}, $job->{guest}, $tid,
> > > + $timestr, $job->{duration} // '-',
> > > + $job->{fail_count}, , $job->{error} // 'OK');
> > > + }
> > > +};
> > > +
> > > +our $cmddef = {
> > > + status => [ 'PVE::API2::Replication', 'status', [], { node => $nodename
> > > }, $print_job_status ],
> > > +
> > > + jobs => [ 'PVE::API2::ReplicationConfig', 'index' , [], {},
> > > $print_job_list ],
> > > + read => [ 'PVE::API2::ReplicationConfig', 'read' , ['id'], {},
> > > + sub { my $res = shift; print to_json($res, { pretty => 1, canonical
> > > => 1}); }],
> >
> > shouldn't this be encode_json? (according to "perldoc JSON", when
> > talkign to the outside world, to ensure proper handling of UTF-8)
>
> encode_json() does not support canonical. But I added utf8 => 1 in v2
>
More information about the pve-devel
mailing list