* [pve-devel] [PATCH pve-cluster 1/3] jobs: move base plugin from pve-manager
2022-03-22 7:34 [pve-devel] [PATCH-SERIES] move jobs from pve-manager Hannes Laimer
@ 2022-03-22 7:34 ` Hannes Laimer
[not found] ` <<20220322073412.30562-2-h.laimer@proxmox.com>
2022-03-22 7:34 ` [pve-devel] [PATCH pve-guest-common 2/3] jobs: move VZDump " Hannes Laimer
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Hannes Laimer @ 2022-03-22 7:34 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
data/PVE/Jobs/Makefile | 11 ++++
data/PVE/Jobs/Plugin.pm | 101 +++++++++++++++++++++++++++++++++++++
data/PVE/Makefile | 2 +-
debian/pve-cluster.install | 1 +
4 files changed, 114 insertions(+), 1 deletion(-)
create mode 100644 data/PVE/Jobs/Makefile
create mode 100644 data/PVE/Jobs/Plugin.pm
diff --git a/data/PVE/Jobs/Makefile b/data/PVE/Jobs/Makefile
new file mode 100644
index 0000000..4320369
--- /dev/null
+++ b/data/PVE/Jobs/Makefile
@@ -0,0 +1,11 @@
+PVEDIR=${DESTDIR}/usr/share/perl5/PVE
+
+SOURCES=Plugin.pm
+
+.PHONY: install
+install: ${SOURCES}
+ install -d -m 0755 ${PVEDIR}/Jobs
+ for f in ${SOURCES}; do install -D -m 0644 $$f ${PVEDIR}/Jobs/$$f; done
+
+.PHONY: clean
+clean:
diff --git a/data/PVE/Jobs/Plugin.pm b/data/PVE/Jobs/Plugin.pm
new file mode 100644
index 0000000..6098360
--- /dev/null
+++ b/data/PVE/Jobs/Plugin.pm
@@ -0,0 +1,101 @@
+package PVE::Jobs::Plugin;
+
+use strict;
+use warnings;
+
+use PVE::Cluster qw(cfs_register_file);
+
+use base qw(PVE::SectionConfig);
+
+cfs_register_file(
+ 'jobs.cfg',
+ sub { __PACKAGE__->parse_config(@_); },
+ sub { __PACKAGE__->write_config(@_); }
+);
+
+my $defaultData = {
+ propertyList => {
+ type => { description => "Section type." },
+ id => {
+ description => "The ID of the VZDump job.",
+ type => 'string',
+ format => 'pve-configid',
+ maxLength => 64,
+ },
+ enabled => {
+ description => "Determines if the job is enabled.",
+ type => 'boolean',
+ default => 1,
+ optional => 1,
+ },
+ schedule => {
+ description => "Backup schedule. The format is a subset of `systemd` calendar events.",
+ type => 'string', format => 'pve-calendar-event',
+ maxLength => 128,
+ },
+ comment => {
+ optional => 1,
+ type => 'string',
+ description => "Description for the Job.",
+ maxLength => 512,
+ },
+ },
+};
+
+sub private {
+ return $defaultData;
+}
+
+sub parse_config {
+ my ($class, $filename, $raw) = @_;
+
+ my $cfg = $class->SUPER::parse_config($filename, $raw);
+
+ foreach my $id (sort keys %{$cfg->{ids}}) {
+ my $data = $cfg->{ids}->{$id};
+
+ $data->{id} = $id;
+ $data->{enabled} //= 1;
+
+ if (defined($data->{comment})) {
+ $data->{comment} = PVE::Tools::decode_text($data->{comment});
+ }
+ }
+
+ return $cfg;
+}
+
+# call the plugin specific decode/encode code
+sub decode_value {
+ my ($class, $type, $key, $value) = @_;
+
+ my $plugin = __PACKAGE__->lookup($type);
+ return $plugin->decode_value($type, $key, $value);
+}
+
+sub encode_value {
+ my ($class, $type, $key, $value) = @_;
+
+ my $plugin = __PACKAGE__->lookup($type);
+ return $plugin->encode_value($type, $key, $value);
+}
+
+sub write_config {
+ my ($class, $filename, $cfg) = @_;
+
+ for my $job (values $cfg->{ids}->%*) {
+ if (defined($job->{comment})) {
+ $job->{comment} = PVE::Tools::encode_text($job->{comment});
+ }
+ }
+
+ $class->SUPER::write_config($filename, $cfg);
+}
+
+sub run {
+ my ($class, $cfg) = @_;
+ # implement in subclass
+ die "not implemented";
+}
+
+1;
diff --git a/data/PVE/Makefile b/data/PVE/Makefile
index 8ea5383..eecf9f9 100644
--- a/data/PVE/Makefile
+++ b/data/PVE/Makefile
@@ -10,7 +10,7 @@ PVE_VENDORARCH=${DESTDIR}/${PERL_VENDORARCH}/auto/PVE/IPCC
PERL_DOC_INC_DIRS:=..
-SUBDIRS=Cluster CLI API2
+SUBDIRS=Cluster CLI API2 Jobs
SOURCES=IPCC.pm Cluster.pm Corosync.pm RRD.pm DataCenterConfig.pm SSHInfo.pm
all:
diff --git a/debian/pve-cluster.install b/debian/pve-cluster.install
index f66cd06..eb15b55 100644
--- a/debian/pve-cluster.install
+++ b/debian/pve-cluster.install
@@ -3,6 +3,7 @@ usr/bin/create_pmxcfs_db
usr/bin/pmxcfs
usr/lib/
usr/share/man/man8/pmxcfs.8
+usr/share/perl5/PVE/Jobs/Plugin.pm
usr/share/perl5/PVE/Cluster.pm
usr/share/perl5/PVE/Cluster/IPCConst.pm
usr/share/perl5/PVE/IPCC.pm
--
2.30.2
^ permalink raw reply [flat|nested] 7+ messages in thread* [pve-devel] [PATCH pve-guest-common 2/3] jobs: move VZDump plugin from pve-manager
2022-03-22 7:34 [pve-devel] [PATCH-SERIES] move jobs from pve-manager Hannes Laimer
2022-03-22 7:34 ` [pve-devel] [PATCH pve-cluster 1/3] jobs: move base plugin " Hannes Laimer
@ 2022-03-22 7:34 ` Hannes Laimer
2022-04-08 7:40 ` Fabian Grünbichler
2022-03-22 7:34 ` [pve-devel] [PATCH pve-manager 3/3] jobs: move to pve-cluster and pve-guest-common Hannes Laimer
2022-04-08 7:44 ` [pve-devel] [PATCH-SERIES] move jobs from pve-manager Fabian Grünbichler
3 siblings, 1 reply; 7+ messages in thread
From: Hannes Laimer @ 2022-03-22 7:34 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/Makefile | 4 +
src/PVE/Jobs.pm | 282 +++++++++++++++++++++++++++++++++++++++++
src/PVE/Jobs/VZDump.pm | 87 +++++++++++++
3 files changed, 373 insertions(+)
create mode 100644 src/PVE/Jobs.pm
create mode 100644 src/PVE/Jobs/VZDump.pm
diff --git a/src/Makefile b/src/Makefile
index baa2688..853b562 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -9,6 +9,7 @@ install: PVE
install -m 0644 PVE/GuestHelpers.pm ${PERL5DIR}/PVE/
install -m 0644 PVE/AbstractConfig.pm ${PERL5DIR}/PVE/
install -m 0644 PVE/AbstractMigrate.pm ${PERL5DIR}/PVE/
+ install -m 0644 PVE/Jobs.pm ${PERL5DIR}/PVE/
install -m 0644 PVE/ReplicationConfig.pm ${PERL5DIR}/PVE/
install -m 0644 PVE/ReplicationState.pm ${PERL5DIR}/PVE/
install -m 0644 PVE/Replication.pm ${PERL5DIR}/PVE/
@@ -17,6 +18,9 @@ install: PVE
install -d ${PERL5DIR}/PVE/VZDump
install -m 0644 PVE/VZDump/Plugin.pm ${PERL5DIR}/PVE/VZDump/
install -m 0644 PVE/VZDump/Common.pm ${PERL5DIR}/PVE/VZDump/
+ install -d ${PERL5DIR}/PVE/Jobs
+ install -m 0644 PVE/Jobs/VZDump.pm ${PERL5DIR}/PVE/Jobs/
+
.PHONY: check
check:
diff --git a/src/PVE/Jobs.pm b/src/PVE/Jobs.pm
new file mode 100644
index 0000000..ba3685e
--- /dev/null
+++ b/src/PVE/Jobs.pm
@@ -0,0 +1,282 @@
+package PVE::Jobs;
+
+use strict;
+use warnings;
+use JSON;
+
+use PVE::Cluster qw(cfs_read_file cfs_lock_file);
+use PVE::Jobs::Plugin;
+use PVE::Jobs::VZDump;
+use PVE::Tools;
+
+PVE::Jobs::VZDump->register();
+PVE::Jobs::Plugin->init();
+
+my $state_dir = "/var/lib/pve-manager/jobs";
+my $lock_dir = "/var/lock/pve-manager";
+
+my $get_state_file = sub {
+ my ($jobid, $type) = @_;
+ return "$state_dir/$type-$jobid.json";
+};
+
+my $default_state = {
+ state => 'created',
+ time => 0,
+};
+
+# lockless, since we use file_get_contents, which is atomic
+sub read_job_state {
+ my ($jobid, $type) = @_;
+ my $path = $get_state_file->($jobid, $type);
+ return if ! -e $path;
+
+ my $raw = PVE::Tools::file_get_contents($path);
+
+ return $default_state if $raw eq '';
+
+ # untaint $raw
+ if ($raw =~ m/^(\{.*\})$/) {
+ return decode_json($1);
+ }
+
+ die "invalid json data in '$path'\n";
+}
+
+sub lock_job_state {
+ my ($jobid, $type, $sub) = @_;
+
+ my $filename = "$lock_dir/$type-$jobid.lck";
+
+ my $res = PVE::Tools::lock_file($filename, 10, $sub);
+ die $@ if $@;
+
+ return $res;
+}
+
+my $get_job_task_status = sub {
+ my ($state) = @_;
+
+ if (!defined($state->{upid})) {
+ return; # not started
+ }
+
+ my ($task, $filename) = PVE::Tools::upid_decode($state->{upid}, 1);
+ die "unable to parse worker upid - $state->{upid}\n" if !$task;
+ die "no such task\n" if ! -f $filename;
+
+ my $pstart = PVE::ProcFSTools::read_proc_starttime($task->{pid});
+ if ($pstart && $pstart == $task->{pstart}) {
+ return; # still running
+ }
+
+ return PVE::Tools::upid_read_status($state->{upid});
+};
+
+# checks if the job is already finished if it was started before and
+# updates the statefile accordingly
+sub update_job_stopped {
+ my ($jobid, $type) = @_;
+
+ # first check unlocked to save time,
+ my $state = read_job_state($jobid, $type);
+ return if !defined($state) || $state->{state} ne 'started'; # removed or not started
+
+ if (defined($get_job_task_status->($state))) {
+ lock_job_state($jobid, $type, sub {
+ my $state = read_job_state($jobid, $type);
+ return if !defined($state) || $state->{state} ne 'started'; # removed or not started
+
+ my $new_state = {
+ state => 'stopped',
+ msg => $get_job_task_status->($state) // 'internal error',
+ upid => $state->{upid},
+ };
+
+ if ($state->{updated}) { # save updated time stamp
+ $new_state->{updated} = $state->{updated};
+ }
+
+ my $path = $get_state_file->($jobid, $type);
+ PVE::Tools::file_set_contents($path, encode_json($new_state));
+ });
+ }
+}
+
+# must be called when the job is first created
+sub create_job {
+ my ($jobid, $type) = @_;
+
+ lock_job_state($jobid, $type, sub {
+ my $state = read_job_state($jobid, $type) // $default_state;
+
+ if ($state->{state} ne 'created') {
+ die "job state already exists\n";
+ }
+
+ $state->{time} = time();
+
+ my $path = $get_state_file->($jobid, $type);
+ PVE::Tools::file_set_contents($path, encode_json($state));
+ });
+}
+
+# to be called when the job is removed
+sub remove_job {
+ my ($jobid, $type) = @_;
+ my $path = $get_state_file->($jobid, $type);
+ unlink $path;
+}
+
+# checks if the job can be started and sets the state to 'starting'
+# returns 1 if the job can be started, 0 otherwise
+sub starting_job {
+ my ($jobid, $type) = @_;
+
+ # first check unlocked to save time
+ my $state = read_job_state($jobid, $type);
+ return 0 if !defined($state) || $state->{state} eq 'started'; # removed or already started
+
+ lock_job_state($jobid, $type, sub {
+ my $state = read_job_state($jobid, $type);
+ return 0 if !defined($state) || $state->{state} eq 'started'; # removed or already started
+
+ my $new_state = {
+ state => 'starting',
+ time => time(),
+ };
+
+ my $path = $get_state_file->($jobid, $type);
+ PVE::Tools::file_set_contents($path, encode_json($new_state));
+ });
+ return 1;
+}
+
+sub started_job {
+ my ($jobid, $type, $upid, $msg) = @_;
+
+ lock_job_state($jobid, $type, sub {
+ my $state = read_job_state($jobid, $type);
+ return if !defined($state); # job was removed, do not update
+ die "unexpected state '$state->{state}'\n" if $state->{state} ne 'starting';
+
+ my $new_state;
+ if (defined($msg)) {
+ $new_state = {
+ state => 'stopped',
+ msg => $msg,
+ time => time(),
+ };
+ } else {
+ $new_state = {
+ state => 'started',
+ upid => $upid,
+ };
+ }
+
+ my $path = $get_state_file->($jobid, $type);
+ PVE::Tools::file_set_contents($path, encode_json($new_state));
+ });
+}
+
+# will be called when the job schedule is updated
+sub updated_job_schedule {
+ my ($jobid, $type) = @_;
+ lock_job_state($jobid, $type, sub {
+ my $old_state = read_job_state($jobid, $type) // $default_state;
+
+ $old_state->{updated} = time();
+
+ my $path = $get_state_file->($jobid, $type);
+ PVE::Tools::file_set_contents($path, encode_json($old_state));
+ });
+}
+
+sub get_last_runtime {
+ my ($jobid, $type) = @_;
+
+ my $state = read_job_state($jobid, $type) // $default_state;
+
+ return $state->{updated} if defined($state->{updated});
+
+ if (my $upid = $state->{upid}) {
+ my ($task) = PVE::Tools::upid_decode($upid, 1);
+ die "unable to parse worker upid\n" if !$task;
+ return $task->{starttime};
+ }
+
+ return $state->{time} // 0;
+}
+
+sub run_jobs {
+ synchronize_job_states_with_config();
+
+ my $jobs_cfg = cfs_read_file('jobs.cfg');
+ my $nodename = PVE::INotify::nodename();
+
+ foreach my $id (sort keys %{$jobs_cfg->{ids}}) {
+ my $cfg = $jobs_cfg->{ids}->{$id};
+ my $type = $cfg->{type};
+ my $schedule = delete $cfg->{schedule};
+
+ # only schedule local jobs
+ next if defined($cfg->{node}) && $cfg->{node} ne $nodename;
+
+ eval { update_job_stopped($id, $type) };
+ if (my $err = $@) {
+ warn "could not update job state, skipping - $err\n";
+ next;
+ }
+
+ next if defined($cfg->{enabled}) && !$cfg->{enabled}; # only schedule actually enabled jobs
+
+ my $last_run = get_last_runtime($id, $type);
+ my $calspec = PVE::CalendarEvent::parse_calendar_event($schedule);
+ my $next_sync = PVE::CalendarEvent::compute_next_event($calspec, $last_run) // 0;
+
+ next if time() < $next_sync; # not yet its (next) turn
+
+ my $plugin = PVE::Jobs::Plugin->lookup($type);
+ if (starting_job($id, $type)) {
+ my $upid = eval { $plugin->run($cfg) };
+ if (my $err = $@) {
+ warn $@ if $@;
+ started_job($id, $type, undef, $err);
+ } elsif ($upid eq 'OK') { # some jobs return OK immediately
+ started_job($id, $type, undef, 'OK');
+ } else {
+ started_job($id, $type, $upid);
+ }
+ }
+ }
+}
+
+# creates and removes statefiles for job configs
+sub synchronize_job_states_with_config {
+ cfs_lock_file('jobs.cfg', undef, sub {
+ my $data = cfs_read_file('jobs.cfg');
+
+ for my $id (keys $data->{ids}->%*) {
+ my $job = $data->{ids}->{$id};
+ my $type = $job->{type};
+ my $jobstate = read_job_state($id, $type);
+ create_job($id, $type) if !defined($jobstate);
+ }
+
+ PVE::Tools::dir_glob_foreach($state_dir, '(.*?)-(.*).json', sub {
+ my ($path, $type, $id) = @_;
+
+ if (!defined($data->{ids}->{$id})) {
+ remove_job($id, $type);
+ }
+ });
+ });
+ die $@ if $@;
+}
+
+sub setup_dirs {
+ mkdir $state_dir;
+ mkdir $lock_dir;
+}
+
+1;
diff --git a/src/PVE/Jobs/VZDump.pm b/src/PVE/Jobs/VZDump.pm
new file mode 100644
index 0000000..44fe33d
--- /dev/null
+++ b/src/PVE/Jobs/VZDump.pm
@@ -0,0 +1,87 @@
+package PVE::Jobs::VZDump;
+
+use strict;
+use warnings;
+
+use PVE::INotify;
+use PVE::VZDump::Common;
+use PVE::API2::VZDump;
+use PVE::Cluster;
+use PVE::JSONSchema;
+
+use base qw(PVE::Jobs::Plugin);
+
+sub type {
+ return 'vzdump';
+}
+
+my $props = PVE::VZDump::Common::json_config_properties();
+
+sub properties {
+ return $props;
+}
+
+sub options {
+ my $options = {
+ enabled => { optional => 1 },
+ schedule => {},
+ comment => { optional => 1 },
+ };
+ foreach my $opt (keys %$props) {
+ if ($props->{$opt}->{optional}) {
+ $options->{$opt} = { optional => 1 };
+ } else {
+ $options->{$opt} = {};
+ }
+ }
+
+ return $options;
+}
+
+sub decode_value {
+ my ($class, $type, $key, $value) = @_;
+
+ if ($key eq 'prune-backups' && !ref($value)) {
+ $value = PVE::JSONSchema::parse_property_string(
+ 'prune-backups',
+ $value,
+ );
+ }
+
+ return $value;
+}
+
+sub encode_value {
+ my ($class, $type, $key, $value) = @_;
+
+ if ($key eq 'prune-backups' && ref($value) eq 'HASH') {
+ $value = PVE::JSONSchema::print_property_string(
+ $value,
+ 'prune-backups',
+ );
+ }
+
+ return $value;
+}
+
+sub run {
+ my ($class, $conf) = @_;
+
+ # remove all non vzdump related options
+ foreach my $opt (keys %$conf) {
+ delete $conf->{$opt} if !defined($props->{$opt});
+ }
+
+ my $retention = $conf->{'prune-backups'};
+ if ($retention && ref($retention) eq 'HASH') { # fixup, its required as string parameter
+ $conf->{'prune-backups'} = PVE::JSONSchema::print_property_string($retention, 'prune-backups');
+ }
+
+ $conf->{quiet} = 1; # do not write to stdout/stderr
+
+ PVE::Cluster::cfs_update(); # refresh vmlist
+
+ return PVE::API2::VZDump->vzdump($conf);
+}
+
+1;
--
2.30.2
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [pve-devel] [PATCH pve-guest-common 2/3] jobs: move VZDump plugin from pve-manager
2022-03-22 7:34 ` [pve-devel] [PATCH pve-guest-common 2/3] jobs: move VZDump " Hannes Laimer
@ 2022-04-08 7:40 ` Fabian Grünbichler
0 siblings, 0 replies; 7+ messages in thread
From: Fabian Grünbichler @ 2022-04-08 7:40 UTC (permalink / raw)
To: Proxmox VE development discussion
On March 22, 2022 8:34 am, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> src/Makefile | 4 +
> src/PVE/Jobs.pm | 282 +++++++++++++++++++++++++++++++++++++++++
> src/PVE/Jobs/VZDump.pm | 87 +++++++++++++
> 3 files changed, 373 insertions(+)
> create mode 100644 src/PVE/Jobs.pm
> create mode 100644 src/PVE/Jobs/VZDump.pm
>
> diff --git a/src/Makefile b/src/Makefile
> index baa2688..853b562 100644
> --- a/src/Makefile
> +++ b/src/Makefile
[..]
> --- /dev/null
> +++ b/src/PVE/Jobs.pm
> @@ -0,0 +1,282 @@
> +package PVE::Jobs;
> +
> +use strict;
> +use warnings;
> +use JSON;
> +
> +use PVE::Cluster qw(cfs_read_file cfs_lock_file);
> +use PVE::Jobs::Plugin;
> +use PVE::Jobs::VZDump;
> +use PVE::Tools;
> +
> +PVE::Jobs::VZDump->register();
> +PVE::Jobs::Plugin->init();
so replication would also need to move here when it gets converted to a
regular job instead of being called directly by pvescheduler
currently those get called via PVE::API2::Replication->run_jobs(), which
lives in pve-manager..
> +
> +my $state_dir = "/var/lib/pve-manager/jobs";
> +my $lock_dir = "/var/lock/pve-manager";
any thoughts about migrating these? if not possible, some rationale for
why they have to stay until XXX would be good :)
> +
> +my $get_state_file = sub {
> + my ($jobid, $type) = @_;
> + return "$state_dir/$type-$jobid.json";
> +};
> +
[..]
> diff --git a/src/PVE/Jobs/VZDump.pm b/src/PVE/Jobs/VZDump.pm
> new file mode 100644
> index 0000000..44fe33d
> --- /dev/null
> +++ b/src/PVE/Jobs/VZDump.pm
> @@ -0,0 +1,87 @@
> +package PVE::Jobs::VZDump;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::INotify;
> +use PVE::VZDump::Common;
> +use PVE::API2::VZDump;
and this (like PVE::API2::Replication mentioned above) lives in
pve-manager as well and in turn calls other code that also lives in
pve-manager, mainly
PVE::VZDump
-> PVE::VZDump::QemuServer (qemu-server, which is above pve-guest-common
in the dep tree)
-> PVE::VZDump::LXC (pve-container, same problem)
-> PVE::HA::Config & PVE::HA::Env::PVE2 (already involved in a cycle
with pve-container / qemu-server, that would just need some look whether
it makes stuff worse or not)
-> PVE::VZDump::Common & PVE::VZDump::Plugin (pve-guest-common, so okay)
-> PVE::API2Tools (pve-manager)
so that whole chain can't move to pve-guest-common, and the 'use'
statement and call to the API handler can't be there either.
a few approaches that might help:
- handwave the issue away by calling the vzdump binary instead of the
API handler (extra process that needs to be tracked, no way to
preserve authuser if we do manual job execution, .., kinda 'meh')
- split the plugin definition (which we want to move as low in the
dependency tree as possible so that more other packages can use it for
job modification) from the 'run' sub for the job type (e.g., by
allowing to specify it in the scheduler/pve-manager if the plugin
doesn't have one, a simple map of plugin type -> run sub could work)
- other great ideas? ;)
like I said, the same issue also applies to replication once we convert
that to jobs, so any solution would need to take that into account
(e.g., the two I mentioned above would work for that as well - via
'pvesr' call or leaving the run part in pve-manager and only moving the
rest to pve-guest-common).
> +use PVE::Cluster;
> +use PVE::JSONSchema;
> +
> +use base qw(PVE::Jobs::Plugin);
> +
> +sub type {
> + return 'vzdump';
> +}
> +
> +my $props = PVE::VZDump::Common::json_config_properties();
> +
[..]
> +
> +sub run {
> + my ($class, $conf) = @_;
> +
> + # remove all non vzdump related options
> + foreach my $opt (keys %$conf) {
> + delete $conf->{$opt} if !defined($props->{$opt});
> + }
> +
> + my $retention = $conf->{'prune-backups'};
> + if ($retention && ref($retention) eq 'HASH') { # fixup, its required as string parameter
> + $conf->{'prune-backups'} = PVE::JSONSchema::print_property_string($retention, 'prune-backups');
> + }
> +
> + $conf->{quiet} = 1; # do not write to stdout/stderr
> +
> + PVE::Cluster::cfs_update(); # refresh vmlist
> +
> + return PVE::API2::VZDump->vzdump($conf);
this call here is the problematic one!
> +}
> +
> +1;
> --
> 2.30.2
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [pve-devel] [PATCH pve-manager 3/3] jobs: move to pve-cluster and pve-guest-common
2022-03-22 7:34 [pve-devel] [PATCH-SERIES] move jobs from pve-manager Hannes Laimer
2022-03-22 7:34 ` [pve-devel] [PATCH pve-cluster 1/3] jobs: move base plugin " Hannes Laimer
2022-03-22 7:34 ` [pve-devel] [PATCH pve-guest-common 2/3] jobs: move VZDump " Hannes Laimer
@ 2022-03-22 7:34 ` Hannes Laimer
2022-04-08 7:44 ` [pve-devel] [PATCH-SERIES] move jobs from pve-manager Fabian Grünbichler
3 siblings, 0 replies; 7+ messages in thread
From: Hannes Laimer @ 2022-03-22 7:34 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
PVE/Jobs.pm | 282 ---------------------------------------------
PVE/Jobs/Makefile | 16 ---
PVE/Jobs/Plugin.pm | 101 ----------------
PVE/Jobs/VZDump.pm | 87 --------------
PVE/Makefile | 3 +-
5 files changed, 1 insertion(+), 488 deletions(-)
delete mode 100644 PVE/Jobs.pm
delete mode 100644 PVE/Jobs/Makefile
delete mode 100644 PVE/Jobs/Plugin.pm
delete mode 100644 PVE/Jobs/VZDump.pm
diff --git a/PVE/Jobs.pm b/PVE/Jobs.pm
deleted file mode 100644
index ba3685ec..00000000
--- a/PVE/Jobs.pm
+++ /dev/null
@@ -1,282 +0,0 @@
-package PVE::Jobs;
-
-use strict;
-use warnings;
-use JSON;
-
-use PVE::Cluster qw(cfs_read_file cfs_lock_file);
-use PVE::Jobs::Plugin;
-use PVE::Jobs::VZDump;
-use PVE::Tools;
-
-PVE::Jobs::VZDump->register();
-PVE::Jobs::Plugin->init();
-
-my $state_dir = "/var/lib/pve-manager/jobs";
-my $lock_dir = "/var/lock/pve-manager";
-
-my $get_state_file = sub {
- my ($jobid, $type) = @_;
- return "$state_dir/$type-$jobid.json";
-};
-
-my $default_state = {
- state => 'created',
- time => 0,
-};
-
-# lockless, since we use file_get_contents, which is atomic
-sub read_job_state {
- my ($jobid, $type) = @_;
- my $path = $get_state_file->($jobid, $type);
- return if ! -e $path;
-
- my $raw = PVE::Tools::file_get_contents($path);
-
- return $default_state if $raw eq '';
-
- # untaint $raw
- if ($raw =~ m/^(\{.*\})$/) {
- return decode_json($1);
- }
-
- die "invalid json data in '$path'\n";
-}
-
-sub lock_job_state {
- my ($jobid, $type, $sub) = @_;
-
- my $filename = "$lock_dir/$type-$jobid.lck";
-
- my $res = PVE::Tools::lock_file($filename, 10, $sub);
- die $@ if $@;
-
- return $res;
-}
-
-my $get_job_task_status = sub {
- my ($state) = @_;
-
- if (!defined($state->{upid})) {
- return; # not started
- }
-
- my ($task, $filename) = PVE::Tools::upid_decode($state->{upid}, 1);
- die "unable to parse worker upid - $state->{upid}\n" if !$task;
- die "no such task\n" if ! -f $filename;
-
- my $pstart = PVE::ProcFSTools::read_proc_starttime($task->{pid});
- if ($pstart && $pstart == $task->{pstart}) {
- return; # still running
- }
-
- return PVE::Tools::upid_read_status($state->{upid});
-};
-
-# checks if the job is already finished if it was started before and
-# updates the statefile accordingly
-sub update_job_stopped {
- my ($jobid, $type) = @_;
-
- # first check unlocked to save time,
- my $state = read_job_state($jobid, $type);
- return if !defined($state) || $state->{state} ne 'started'; # removed or not started
-
- if (defined($get_job_task_status->($state))) {
- lock_job_state($jobid, $type, sub {
- my $state = read_job_state($jobid, $type);
- return if !defined($state) || $state->{state} ne 'started'; # removed or not started
-
- my $new_state = {
- state => 'stopped',
- msg => $get_job_task_status->($state) // 'internal error',
- upid => $state->{upid},
- };
-
- if ($state->{updated}) { # save updated time stamp
- $new_state->{updated} = $state->{updated};
- }
-
- my $path = $get_state_file->($jobid, $type);
- PVE::Tools::file_set_contents($path, encode_json($new_state));
- });
- }
-}
-
-# must be called when the job is first created
-sub create_job {
- my ($jobid, $type) = @_;
-
- lock_job_state($jobid, $type, sub {
- my $state = read_job_state($jobid, $type) // $default_state;
-
- if ($state->{state} ne 'created') {
- die "job state already exists\n";
- }
-
- $state->{time} = time();
-
- my $path = $get_state_file->($jobid, $type);
- PVE::Tools::file_set_contents($path, encode_json($state));
- });
-}
-
-# to be called when the job is removed
-sub remove_job {
- my ($jobid, $type) = @_;
- my $path = $get_state_file->($jobid, $type);
- unlink $path;
-}
-
-# checks if the job can be started and sets the state to 'starting'
-# returns 1 if the job can be started, 0 otherwise
-sub starting_job {
- my ($jobid, $type) = @_;
-
- # first check unlocked to save time
- my $state = read_job_state($jobid, $type);
- return 0 if !defined($state) || $state->{state} eq 'started'; # removed or already started
-
- lock_job_state($jobid, $type, sub {
- my $state = read_job_state($jobid, $type);
- return 0 if !defined($state) || $state->{state} eq 'started'; # removed or already started
-
- my $new_state = {
- state => 'starting',
- time => time(),
- };
-
- my $path = $get_state_file->($jobid, $type);
- PVE::Tools::file_set_contents($path, encode_json($new_state));
- });
- return 1;
-}
-
-sub started_job {
- my ($jobid, $type, $upid, $msg) = @_;
-
- lock_job_state($jobid, $type, sub {
- my $state = read_job_state($jobid, $type);
- return if !defined($state); # job was removed, do not update
- die "unexpected state '$state->{state}'\n" if $state->{state} ne 'starting';
-
- my $new_state;
- if (defined($msg)) {
- $new_state = {
- state => 'stopped',
- msg => $msg,
- time => time(),
- };
- } else {
- $new_state = {
- state => 'started',
- upid => $upid,
- };
- }
-
- my $path = $get_state_file->($jobid, $type);
- PVE::Tools::file_set_contents($path, encode_json($new_state));
- });
-}
-
-# will be called when the job schedule is updated
-sub updated_job_schedule {
- my ($jobid, $type) = @_;
- lock_job_state($jobid, $type, sub {
- my $old_state = read_job_state($jobid, $type) // $default_state;
-
- $old_state->{updated} = time();
-
- my $path = $get_state_file->($jobid, $type);
- PVE::Tools::file_set_contents($path, encode_json($old_state));
- });
-}
-
-sub get_last_runtime {
- my ($jobid, $type) = @_;
-
- my $state = read_job_state($jobid, $type) // $default_state;
-
- return $state->{updated} if defined($state->{updated});
-
- if (my $upid = $state->{upid}) {
- my ($task) = PVE::Tools::upid_decode($upid, 1);
- die "unable to parse worker upid\n" if !$task;
- return $task->{starttime};
- }
-
- return $state->{time} // 0;
-}
-
-sub run_jobs {
- synchronize_job_states_with_config();
-
- my $jobs_cfg = cfs_read_file('jobs.cfg');
- my $nodename = PVE::INotify::nodename();
-
- foreach my $id (sort keys %{$jobs_cfg->{ids}}) {
- my $cfg = $jobs_cfg->{ids}->{$id};
- my $type = $cfg->{type};
- my $schedule = delete $cfg->{schedule};
-
- # only schedule local jobs
- next if defined($cfg->{node}) && $cfg->{node} ne $nodename;
-
- eval { update_job_stopped($id, $type) };
- if (my $err = $@) {
- warn "could not update job state, skipping - $err\n";
- next;
- }
-
- next if defined($cfg->{enabled}) && !$cfg->{enabled}; # only schedule actually enabled jobs
-
- my $last_run = get_last_runtime($id, $type);
- my $calspec = PVE::CalendarEvent::parse_calendar_event($schedule);
- my $next_sync = PVE::CalendarEvent::compute_next_event($calspec, $last_run) // 0;
-
- next if time() < $next_sync; # not yet its (next) turn
-
- my $plugin = PVE::Jobs::Plugin->lookup($type);
- if (starting_job($id, $type)) {
- my $upid = eval { $plugin->run($cfg) };
- if (my $err = $@) {
- warn $@ if $@;
- started_job($id, $type, undef, $err);
- } elsif ($upid eq 'OK') { # some jobs return OK immediately
- started_job($id, $type, undef, 'OK');
- } else {
- started_job($id, $type, $upid);
- }
- }
- }
-}
-
-# creates and removes statefiles for job configs
-sub synchronize_job_states_with_config {
- cfs_lock_file('jobs.cfg', undef, sub {
- my $data = cfs_read_file('jobs.cfg');
-
- for my $id (keys $data->{ids}->%*) {
- my $job = $data->{ids}->{$id};
- my $type = $job->{type};
- my $jobstate = read_job_state($id, $type);
- create_job($id, $type) if !defined($jobstate);
- }
-
- PVE::Tools::dir_glob_foreach($state_dir, '(.*?)-(.*).json', sub {
- my ($path, $type, $id) = @_;
-
- if (!defined($data->{ids}->{$id})) {
- remove_job($id, $type);
- }
- });
- });
- die $@ if $@;
-}
-
-sub setup_dirs {
- mkdir $state_dir;
- mkdir $lock_dir;
-}
-
-1;
diff --git a/PVE/Jobs/Makefile b/PVE/Jobs/Makefile
deleted file mode 100644
index 6023c3ba..00000000
--- a/PVE/Jobs/Makefile
+++ /dev/null
@@ -1,16 +0,0 @@
-include ../../defines.mk
-
-PERLSOURCE = \
- Plugin.pm\
- VZDump.pm
-
-all:
-
-.PHONY: clean
-clean:
- rm -rf *~
-
-.PHONY: install
-install: ${PERLSOURCE}
- install -d ${PERLLIBDIR}/PVE/Jobs
- install -m 0644 ${PERLSOURCE} ${PERLLIBDIR}/PVE/Jobs
diff --git a/PVE/Jobs/Plugin.pm b/PVE/Jobs/Plugin.pm
deleted file mode 100644
index 6098360b..00000000
--- a/PVE/Jobs/Plugin.pm
+++ /dev/null
@@ -1,101 +0,0 @@
-package PVE::Jobs::Plugin;
-
-use strict;
-use warnings;
-
-use PVE::Cluster qw(cfs_register_file);
-
-use base qw(PVE::SectionConfig);
-
-cfs_register_file(
- 'jobs.cfg',
- sub { __PACKAGE__->parse_config(@_); },
- sub { __PACKAGE__->write_config(@_); }
-);
-
-my $defaultData = {
- propertyList => {
- type => { description => "Section type." },
- id => {
- description => "The ID of the VZDump job.",
- type => 'string',
- format => 'pve-configid',
- maxLength => 64,
- },
- enabled => {
- description => "Determines if the job is enabled.",
- type => 'boolean',
- default => 1,
- optional => 1,
- },
- schedule => {
- description => "Backup schedule. The format is a subset of `systemd` calendar events.",
- type => 'string', format => 'pve-calendar-event',
- maxLength => 128,
- },
- comment => {
- optional => 1,
- type => 'string',
- description => "Description for the Job.",
- maxLength => 512,
- },
- },
-};
-
-sub private {
- return $defaultData;
-}
-
-sub parse_config {
- my ($class, $filename, $raw) = @_;
-
- my $cfg = $class->SUPER::parse_config($filename, $raw);
-
- foreach my $id (sort keys %{$cfg->{ids}}) {
- my $data = $cfg->{ids}->{$id};
-
- $data->{id} = $id;
- $data->{enabled} //= 1;
-
- if (defined($data->{comment})) {
- $data->{comment} = PVE::Tools::decode_text($data->{comment});
- }
- }
-
- return $cfg;
-}
-
-# call the plugin specific decode/encode code
-sub decode_value {
- my ($class, $type, $key, $value) = @_;
-
- my $plugin = __PACKAGE__->lookup($type);
- return $plugin->decode_value($type, $key, $value);
-}
-
-sub encode_value {
- my ($class, $type, $key, $value) = @_;
-
- my $plugin = __PACKAGE__->lookup($type);
- return $plugin->encode_value($type, $key, $value);
-}
-
-sub write_config {
- my ($class, $filename, $cfg) = @_;
-
- for my $job (values $cfg->{ids}->%*) {
- if (defined($job->{comment})) {
- $job->{comment} = PVE::Tools::encode_text($job->{comment});
- }
- }
-
- $class->SUPER::write_config($filename, $cfg);
-}
-
-sub run {
- my ($class, $cfg) = @_;
- # implement in subclass
- die "not implemented";
-}
-
-1;
diff --git a/PVE/Jobs/VZDump.pm b/PVE/Jobs/VZDump.pm
deleted file mode 100644
index 44fe33dc..00000000
--- a/PVE/Jobs/VZDump.pm
+++ /dev/null
@@ -1,87 +0,0 @@
-package PVE::Jobs::VZDump;
-
-use strict;
-use warnings;
-
-use PVE::INotify;
-use PVE::VZDump::Common;
-use PVE::API2::VZDump;
-use PVE::Cluster;
-use PVE::JSONSchema;
-
-use base qw(PVE::Jobs::Plugin);
-
-sub type {
- return 'vzdump';
-}
-
-my $props = PVE::VZDump::Common::json_config_properties();
-
-sub properties {
- return $props;
-}
-
-sub options {
- my $options = {
- enabled => { optional => 1 },
- schedule => {},
- comment => { optional => 1 },
- };
- foreach my $opt (keys %$props) {
- if ($props->{$opt}->{optional}) {
- $options->{$opt} = { optional => 1 };
- } else {
- $options->{$opt} = {};
- }
- }
-
- return $options;
-}
-
-sub decode_value {
- my ($class, $type, $key, $value) = @_;
-
- if ($key eq 'prune-backups' && !ref($value)) {
- $value = PVE::JSONSchema::parse_property_string(
- 'prune-backups',
- $value,
- );
- }
-
- return $value;
-}
-
-sub encode_value {
- my ($class, $type, $key, $value) = @_;
-
- if ($key eq 'prune-backups' && ref($value) eq 'HASH') {
- $value = PVE::JSONSchema::print_property_string(
- $value,
- 'prune-backups',
- );
- }
-
- return $value;
-}
-
-sub run {
- my ($class, $conf) = @_;
-
- # remove all non vzdump related options
- foreach my $opt (keys %$conf) {
- delete $conf->{$opt} if !defined($props->{$opt});
- }
-
- my $retention = $conf->{'prune-backups'};
- if ($retention && ref($retention) eq 'HASH') { # fixup, its required as string parameter
- $conf->{'prune-backups'} = PVE::JSONSchema::print_property_string($retention, 'prune-backups');
- }
-
- $conf->{quiet} = 1; # do not write to stdout/stderr
-
- PVE::Cluster::cfs_update(); # refresh vmlist
-
- return PVE::API2::VZDump->vzdump($conf);
-}
-
-1;
diff --git a/PVE/Makefile b/PVE/Makefile
index 48b85d33..0071fab1 100644
--- a/PVE/Makefile
+++ b/PVE/Makefile
@@ -1,6 +1,6 @@
include ../defines.mk
-SUBDIRS=API2 Status CLI Service Ceph Jobs
+SUBDIRS=API2 Status CLI Service Ceph
PERLSOURCE = \
API2.pm \
@@ -11,7 +11,6 @@ PERLSOURCE = \
CertHelpers.pm \
ExtMetric.pm \
HTTPServer.pm \
- Jobs.pm \
NodeConfig.pm \
Report.pm \
VZDump.pm
--
2.30.2
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [pve-devel] [PATCH-SERIES] move jobs from pve-manager
2022-03-22 7:34 [pve-devel] [PATCH-SERIES] move jobs from pve-manager Hannes Laimer
` (2 preceding siblings ...)
2022-03-22 7:34 ` [pve-devel] [PATCH pve-manager 3/3] jobs: move to pve-cluster and pve-guest-common Hannes Laimer
@ 2022-04-08 7:44 ` Fabian Grünbichler
3 siblings, 0 replies; 7+ messages in thread
From: Fabian Grünbichler @ 2022-04-08 7:44 UTC (permalink / raw)
To: Proxmox VE development discussion
On March 22, 2022 8:34 am, Hannes Laimer wrote:
> The VZDump plugin was moved to pve-guest-common because it is used by
> both CTs and VMs. Since the Jobs.pm contains helpers that depend on the
> VZDump plugin, it was also moved to pve-guest-common. The base jobs
> plugin was moved to pve-cluster since it does not really depend on
> anything and might be used in many places.
>
> Version bumps are needed.
actually (although it's pretty obvious in this case given the
description above and the diff stats below ;):
version bumps, versioned dependencies and breaks+replaces since files
are moved from one package to another while keeping their paths!
pve-manager depends on bumped pve-cluster and bumped pve-guest-common (using moved files)
pve-guest-common depends on bumped pve-cluster (using moved files)
pve-guest-common breaks & replaces old pve-manager (taking over files)
pve-cluster breaks & replaces old pve-manager (taking over file)
replace pve-cluster with new binary package possibly (see patch #1)
but alas, this requires some more work/thought anyway - see patch #2.
>
> pve-cluster:
>
> Hannes Laimer (1):
> jobs: move base plugin from pve-manager
>
> data/PVE/Jobs/Makefile | 11 ++++
> data/PVE/Jobs/Plugin.pm | 101 +++++++++++++++++++++++++++++++++++++
> data/PVE/Makefile | 2 +-
> debian/pve-cluster.install | 1 +
> 4 files changed, 114 insertions(+), 1 deletion(-)
> create mode 100644 data/PVE/Jobs/Makefile
> create mode 100644 data/PVE/Jobs/Plugin.pm
>
>
> pve-guest-common:
>
> Hannes Laimer (1):
> jobs: move VZDump plugin from pve-manager
>
> src/Makefile | 4 +
> src/PVE/Jobs.pm | 282 +++++++++++++++++++++++++++++++++++++++++
> src/PVE/Jobs/VZDump.pm | 87 +++++++++++++
> 3 files changed, 373 insertions(+)
> create mode 100644 src/PVE/Jobs.pm
> create mode 100644 src/PVE/Jobs/VZDump.pm
>
>
> pve-manager:
>
> Hannes Laimer (1):
> jobs: move to pve-cluster and pve-guest-common
>
> PVE/Jobs.pm | 282 ---------------------------------------------
> PVE/Jobs/Makefile | 16 ---
> PVE/Jobs/Plugin.pm | 101 ----------------
> PVE/Jobs/VZDump.pm | 87 --------------
> PVE/Makefile | 3 +-
> 5 files changed, 1 insertion(+), 488 deletions(-)
> delete mode 100644 PVE/Jobs.pm
> delete mode 100644 PVE/Jobs/Makefile
> delete mode 100644 PVE/Jobs/Plugin.pm
> delete mode 100644 PVE/Jobs/VZDump.pm
>
> --
> 2.30.2
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread