[pve-devel] [PATCH v2 cluster manager docs] Introduce cluster-wide vzdump configuration

[pve-devel] [PATCH v2 cluster manager docs] Introduce cluster-wide vzdump configuration

[Leo Nunner]

Changes from v1:
    - Remove automatic generation of cluster config from setup
    - Fix vzdump regression tests
    - Reword docs to be more straightforward

cluster:

Leo Nunner (1):
  fix #4234: vzdump: add cluster-wide configuration

 data/PVE/Cluster.pm | 1 +
 data/src/status.c   | 1 +
 2 files changed, 2 insertions(+)

manager:

Leo Nunner (2):
  fix #4235: vzdump: add cluster-wide configuration
  test: allow access to the cluster-wide 'vzdump.conf'

 PVE/VZDump.pm           | 50 +++++++++++++++++++++++++++++------------
 test/vzdump_new_test.pl |  4 +++-
 2 files changed, 39 insertions(+), 15 deletions(-)

docs:

Leo Nunner (1):
  vzdump: document the new cluster-wide config file

 vzdump.adoc | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH v2 cluster] fix #4234: vzdump: add cluster-wide configuration

[Leo Nunner]

Introduce a cluster-wide vzdump.conf file which gets filled with the
default vzdump configuration.

Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
 data/PVE/Cluster.pm | 1 +
 data/src/status.c   | 1 +
 2 files changed, 2 insertions(+)

diff --git a/data/PVE/Cluster.pm b/data/PVE/Cluster.pm
index 0154aae..efca58f 100644
--- a/data/PVE/Cluster.pm
+++ b/data/PVE/Cluster.pm
@@ -45,6 +45,7 @@ my $dbbackupdir = "/var/lib/pve-cluster/backup";
 # using a computed version and only those can be used by the cfs_*_file methods
 my $observed = {
     'vzdump.cron' => 1,
+    'vzdump.conf' => 1,
     'jobs.cfg' => 1,
     'storage.cfg' => 1,
     'datacenter.cfg' => 1,
diff --git a/data/src/status.c b/data/src/status.c
index 5e1e841..9e520a5 100644
--- a/data/src/status.c
+++ b/data/src/status.c
@@ -89,6 +89,7 @@ static memdb_change_t memdb_change_array[] = {
 	{ .path = "priv/ipam.db" },
 	{ .path = "datacenter.cfg" },
 	{ .path = "vzdump.cron" },
+	{ .path = "vzdump.conf" },
 	{ .path = "jobs.cfg" },
 	{ .path = "ha/crm_commands" },
 	{ .path = "ha/manager_status" },
-- 
2.30.2

[pve-devel] [PATCH v2 manager 1/2] fix #4235: vzdump: add cluster-wide configuration

[Leo Nunner]

The different config files get merged in the following order:

/etc/vzdump.conf <- /etc/pve/vzdump.conf <- default values

So the local configuration takes the highest precedence, then the
cluster-wide config, and finally the default values.

Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
 PVE/VZDump.pm | 50 ++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 36 insertions(+), 14 deletions(-)

diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index a04837e7..d0ab03ac 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -13,7 +13,7 @@ use IPC::Open3;
 use POSIX qw(strftime);
 use Time::Local;
 
-use PVE::Cluster qw(cfs_read_file);
+use PVE::Cluster qw(cfs_register_file cfs_read_file);
 use PVE::DataCenterConfig;
 use PVE::Exception qw(raise_param_exc);
 use PVE::HA::Config;
@@ -258,6 +258,32 @@ sub check_vmids {
     return $res;
 }
 
+sub parse_vzdump_conf {
+    my ($fn, $content) = @_;
+
+    return {} if !defined($content);
+
+    my $conf_schema = { type => 'object', properties => $confdesc_for_defaults };
+    my $res = PVE::JSONSchema::parse_config($conf_schema, $fn, $content);
+    if (my $excludes = $res->{'exclude-path'}) {
+        $res->{'exclude-path'} = PVE::Tools::split_args($excludes);
+    }
+    if (defined($res->{mailto})) {
+        my @mailto = split_list($res->{mailto});
+        $res->{mailto} = [ @mailto ];
+    }
+    $parse_prune_backups_maxfiles->($res, "options in '$fn'");
+    parse_performance($res);
+
+    if (defined($res->{storage} && defined($res->{dumpdir}))) {
+        debugmsg('warn', "both 'storage' and 'dumpdir' defined in $fn - ignoring 'dumpdir'");
+        delete $res->{dumpdir};
+    }
+
+    return $res;
+}
+
+cfs_register_file("vzdump.conf", \&parse_vzdump_conf);
 
 sub read_vzdump_defaults {
 
@@ -266,7 +292,7 @@ sub read_vzdump_defaults {
     my $defaults = {
 	map {
 	    my $default = $confdesc->{$_}->{default};
-	     defined($default) ? ($_ => $default) : ()
+	    defined($default) ? ($_ => $default) : ()
 	} keys %$confdesc_for_defaults
     };
     $parse_prune_backups_maxfiles->($defaults, "defaults in VZDump schema");
@@ -274,26 +300,22 @@ sub read_vzdump_defaults {
 
     my $raw;
     eval { $raw = PVE::Tools::file_get_contents($fn); };
-    return $defaults if $@;
 
-    my $conf_schema = { type => 'object', properties => $confdesc_for_defaults };
-    my $res = PVE::JSONSchema::parse_config($conf_schema, $fn, $raw);
-    if (my $excludes = $res->{'exclude-path'}) {
-	$res->{'exclude-path'} = PVE::Tools::split_args($excludes);
-    }
-    if (defined($res->{mailto})) {
-	my @mailto = split_list($res->{mailto});
-	$res->{mailto} = [ @mailto ];
+    my $res = parse_vzdump_conf($fn, $raw);
+
+    # Merge in cluster-wide config
+    my $cluster_conf = cfs_read_file("vzdump.conf");
+    foreach my $key (keys %$cluster_conf) {
+	$res->{$key} = $cluster_conf->{$key} if !defined($res->{$key});
     }
-    $parse_prune_backups_maxfiles->($res, "options in '$fn'");
-    parse_performance($res);
 
+    # Merge in defaults
     foreach my $key (keys %$defaults) {
 	$res->{$key} = $defaults->{$key} if !defined($res->{$key});
     }
 
     if (defined($res->{storage}) && defined($res->{dumpdir})) {
-	debugmsg('warn', "both 'storage' and 'dumpdir' defined in '$fn' - ignoring 'dumpdir'");
+	debugmsg('warn', "both 'storage' and 'dumpdir' defined in config - ignoring 'dumpdir'");
 	delete $res->{dumpdir};
     }
 
-- 
2.30.2

[pve-devel] [PATCH v2 manager 2/2] test: allow access to the cluster-wide 'vzdump.conf'

[Leo Nunner]

The hardcoded filename check for cluster file access needs to include
'vzdump.conf', since vzdump now always tries to read that file when
parsing configurations.

Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
 test/vzdump_new_test.pl | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/test/vzdump_new_test.pl b/test/vzdump_new_test.pl
index 8cd73075..64f8e0a1 100755
--- a/test/vzdump_new_test.pl
+++ b/test/vzdump_new_test.pl
@@ -58,7 +58,9 @@ $pve_cluster_module->mock(
     get_config => sub {
 	my ($filename) = @_;
 
-	die "unexpected filename '$filename'\n" if $filename ne 'storage.cfg';
+	if(!grep(/^$filename$/, ('storage.cfg', 'vzdump.conf'))) {
+	    die "unexpected filename '$filename'\n";
+	}
 	return $storage_config;
     },
     # never update during the tests
-- 
2.30.2

[pve-devel] [PATCH v2 docs] vzdump: document the new cluster-wide config file

[Leo Nunner]

+ change the wording from "Global configuration" to "Node-wide
configuration"

Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
 vzdump.adoc | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/vzdump.adoc b/vzdump.adoc
index ce46529..6c2023d 100644
--- a/vzdump.adoc
+++ b/vzdump.adoc
@@ -473,13 +473,18 @@ entire process happens transparently from a user's point of view.
 Configuration
 -------------
 
-Global configuration is stored in `/etc/vzdump.conf`. The file uses a
-simple colon separated key/value format. Each line has the following
-format:
+Node-wide configuration is stored in `/etc/vzdump.conf`. A cluster-wide
+configuration can be found at `/etc/pve/vzdump.conf`. The configuration files
+take precedence in the following order: first, the job/CLI options are
+read. If an option is not defined there, it is taken from the node-wide
+configuration, and, if not defined there either, it gets looked up in the
+cluster-wide configuration. Should it not be defined there either, the default
+value is used. The files use a simple colon separated key/value format. Each
+line has the following format:
 
  OPTION: value
 
-Blank lines in the file are ignored, and lines starting with a `#`
+Blank lines in the files are ignored, and lines starting with a `#`
 character are treated as comments and are also ignored. Values from
 this file are used as default, and can be overwritten on the command
 line.
-- 
2.30.2

[pve-devel] applied: [PATCH v2 cluster] fix #4234: vzdump: add cluster-wide configuration

[Thomas Lamprecht]

Am 07/03/2023 um 13:11 schrieb Leo Nunner:
> Introduce a cluster-wide vzdump.conf file which gets filled with the
> default vzdump configuration.
> 
> Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
> ---
>  data/PVE/Cluster.pm | 1 +
>  data/src/status.c   | 1 +
>  2 files changed, 2 insertions(+)
> 
>

applied, this one already as the manager one then requires bumping it's
depends on pve-cluster to the version containing this commit, thanks!