From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 0EE371FF150 for ; Mon, 06 Jul 2026 15:00:25 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id D56DF2144C; Mon, 06 Jul 2026 15:00:24 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Mon, 06 Jul 2026 15:00:20 +0200 Message-Id: From: "Daniel Kral" To: "David Riley" , Subject: Re: [PATCH pve-cluster v2 09/10] fix #7294: cluster: helpers: add cluster-wide version assertion X-Mailer: aerc 0.21.0-147-g43ac7b685014-dirty References: <20260626131035.112374-1-d.riley@proxmox.com> <20260626131035.112374-10-d.riley@proxmox.com> In-Reply-To: <20260626131035.112374-10-d.riley@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1783342814323 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.019 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: FNO4DMLO57PG3GQ4ZWTHPZPR4M3DPVUN X-Message-ID-Hash: FNO4DMLO57PG3GQ4ZWTHPZPR4M3DPVUN X-MailFrom: d.kral@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Fri Jun 26, 2026 at 3:10 PM CEST, David Riley wrote: > Add function to assert a minimum version requirement for the whole > cluster. > > Evaluates against the cluster configuration. Dies if any node is > offline, or running an older version, ensuring cluster-wide feature > compatibility before allowing an operation to proceed. > > Prevents older nodes from truncating newly structured configuration > entries. > > Link: https://bugzilla.proxmox.com/show_bug.cgi?id=3D7294 > Signed-off-by: David Riley > --- > src/PVE/Cluster.pm | 43 +++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 41 insertions(+), 2 deletions(-) > > diff --git a/src/PVE/Cluster.pm b/src/PVE/Cluster.pm > index 034b78c..3ecde03 100644 > --- a/src/PVE/Cluster.pm > +++ b/src/PVE/Cluster.pm > @@ -20,7 +20,7 @@ use PVE::IPCC; > use PVE::JSONSchema; > use PVE::Network; > use PVE::SafeSyslog; > -use PVE::Tools qw(run_command); > +use PVE::Tools qw(run_command pvecfg_min_version); > =20 > use PVE::Cluster::IPCConst; > =20 > @@ -30,7 +30,9 @@ our @EXPORT_OK =3D qw( > cfs_read_file > cfs_write_file > cfs_register_file > - cfs_lock_file); > + cfs_lock_file > + assert_min_cluster_version This export is not needed anymore if the function is always called as PVE::Cluster::assert_min_cluster_version(). > +); > =20 > # x509 certificate utils > =20 > @@ -929,4 +931,41 @@ sub cfs_rename_db_unsafe { > rename $dbfile, "$dbfile.$ctime.bak" or warn "failed to rename old c= onfig database - $!\n"; > } > =20 > +# Asserts that all configured nodes in the cluster meet a minimum versio= n requirement. > +# Evaluates against the full, static cluster node list rather than only = live nodes. > +# Aborts if any node is offline, unreadable, or running an > +# outdated version, preventing partial feature rollouts from corrupting = cluster state. > +# NOTE: Uses PVE::Tools::pvecfg_min_version, which currently checks basi= c major.minor.release > +# triples. If gatekeeping a feature right around a major release transit= ion, be aware that it > +# does not yet lexically sort trailing Debian revision suffixes (e.g., d= istinguishing '9.0.0' as > +# newer than '9.0.0~22'). > +sub assert_min_cluster_version { > + my ($major, $minor, $release) =3D @_; > + > + my $nodestat =3D get_node_kv('version-info') || {}; > + my $nodelist =3D get_nodelist() || []; nit: get_node_kv() will always return an empty hash ref {} and get_nodelist() will always return an empty array ref [], so the fallbacks aren't really needed > + > + for my $node (sort @$nodelist) { > + my $raw_json =3D $nodestat->{$node}; > + > + if (!defined($raw_json)) { > + die "cannot execute operation: cluster node '$node' is offli= ne or not reporting its" > + . " version. All nodes must be online to verify cluster-= wide feature support.\n"; > + } > + > + my $vinfo =3D eval { decode_json($raw_json) }; > + if ($@ || !$vinfo || !$vinfo->{version}) { > + die "cannot execute operation: failed to parse cluster versi= on string for node" > + . " '$node'.\n"; > + } > + > + if (!pvecfg_min_version($vinfo->{version}, $major, $minor, $rele= ase)) { > + die "cannot execute operation: cluster node '$node' runs an = older version of Proxmox" > + . " VE ($vinfo->{version}) than the required $major.$min= or.$release. Please" > + . " upgrade all nodes first.\n"; > + } > + } > + return 1; > +} > + > 1; Modulo the nits, consider this: Reviewed-by: Daniel Kral