[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