[pve-devel] [PATCH manager 0/2] fix ceph version handling in gui

[pve-devel] [PATCH manager 0/2] fix ceph version handling in gui

[Dominik Csapak]

we removed the broadcast of the legacy kv 'ceph-version', but our
gui and api were not completely  adapted to the new versions yet
(it seems we forgot)

the ui patch fixes the dashboard and service lists, but the osd panel
is still broken (that needs the api patch)

alternatively to the api patch, we could revert fb0cb9b7, but
as we already rolled that change out to pve-no-subscription,
i see no point to it really.


Dominik Csapak (2):
  api: ceph: fix getting ceph versions
  ui: ceph: fix version handling

 PVE/API2/Ceph/OSD.pm              | 5 ++---
 PVE/API2/Cluster/Ceph.pm          | 5 +----
 www/manager6/ceph/OSD.js          | 6 +++---
 www/manager6/ceph/Services.js     | 8 ++++----
 www/manager6/ceph/StatusDetail.js | 8 ++++----
 5 files changed, 14 insertions(+), 18 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH manager 1/2] api: ceph: fix getting ceph versions

[Dominik Csapak]

since commit:
fb0cb9b7 ("ceph services: drop broadcasting legacy version pmxcfs KV")

the 'ceph-version' kv is not broadcasted anymore, so we should not query it.
instead use get_ceph_versions

also drop the other legacy keys for the versions

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/API2/Ceph/OSD.pm     | 5 ++---
 PVE/API2/Cluster/Ceph.pm | 5 +----
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/PVE/API2/Ceph/OSD.pm b/PVE/API2/Ceph/OSD.pm
index fbbc2139..a8f48304 100644
--- a/PVE/API2/Ceph/OSD.pm
+++ b/PVE/API2/Ceph/OSD.pm
@@ -106,7 +106,7 @@ __PACKAGE__->register_method ({
 	my $osdmetadata_res = $rados->mon_command({ prefix => 'osd metadata' });
 	my $osdmetadata = { map { $_->{id} => $_ } @$osdmetadata_res };
 
-	my $hostversions = PVE::Cluster::get_node_kv("ceph-version");
+	my $hostversions = PVE::Ceph::Services::get_ceph_versions();
 
 	my $nodes = {};
 	my $newnodes = {};
@@ -176,7 +176,7 @@ __PACKAGE__->register_method ({
 	    }
 
 	    if ($name && $e->{type} eq 'host') {
-		$new->{version} = $hostversions->{$name};
+		$new->{version} = $hostversions->{$name}->{version}->{str};
 	    }
 	}
 
@@ -195,7 +195,6 @@ __PACKAGE__->register_method ({
 		leaf =>  0,
 		children => $realroots
 	    },
-	    versions => $hostversions, # for compatibility
 	};
 
 	$data->{flags} = $flags if $flags; # we want this for the noout flag
diff --git a/PVE/API2/Cluster/Ceph.pm b/PVE/API2/Cluster/Ceph.pm
index 1cc5aff4..7f825003 100644
--- a/PVE/API2/Cluster/Ceph.pm
+++ b/PVE/API2/Cluster/Ceph.pm
@@ -74,10 +74,7 @@ __PACKAGE__->register_method ({
 
 	my $scope = $param->{scope} // 'all';
 
-	my $res = {
-	    # FIXME: remove with 7.0 depreacated by structured 'versions'
-	    version => PVE::Cluster::get_node_kv("ceph-version"),
-	};
+	my $res = {};
 
 	if (defined(my $versions = PVE::Ceph::Services::get_ceph_versions())) {
 	    $res->{node} = $versions;
-- 
2.30.2

[pve-devel] [PATCH manager 2/2] ui: ceph: fix version handling

[Dominik Csapak]

it seems we did not prepare the gui enough for the api changes.
we have to use the node specific versions, not the global 'versions'
object.

also use PVE.Utils.compare_ceph_versions everywhere, since some versions
are strings and others are the parts of the version (e.g. ["16", "2, "6"])

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/ceph/OSD.js          | 6 +++---
 www/manager6/ceph/Services.js     | 8 ++++----
 www/manager6/ceph/StatusDetail.js | 8 ++++----
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/www/manager6/ceph/OSD.js b/www/manager6/ceph/OSD.js
index aea38d6c..b297d3ae 100644
--- a/www/manager6/ceph/OSD.js
+++ b/www/manager6/ceph/OSD.js
@@ -334,7 +334,7 @@ Ext.define('PVE.node.CephOsdTree', {
 			    return;
 			}
 
-			if (node.version !== maxversion && maxversion !== "0") {
+			if (PVE.Utils.compare_ceph_versions(node.version, maxversion) !== 0 && maxversion !== "0") {
 			    mixedversions = true;
 			}
 
@@ -503,8 +503,8 @@ Ext.define('PVE.node.CephOsdTree', {
 	    let icon = "";
 	    let version = value || "";
 	    let maxversion = vm.get('maxversion');
-	    if (value && value !== maxversion) {
-		if (rec.data.type === 'host' || versions[rec.data.host] !== maxversion) {
+	    if (value && PVE.Utils.compare_ceph_versions(value, maxversion) !== 0) {
+		if (rec.data.type === 'host' || PVE.Utils.compare_ceph_versions(versions[rec.data.host] || "", maxversion) !== 0) {
 		    icon = PVE.Utils.get_ceph_icon_html('HEALTH_UPGRADE');
 		} else {
 		    icon = PVE.Utils.get_ceph_icon_html('HEALTH_OLD');
diff --git a/www/manager6/ceph/Services.js b/www/manager6/ceph/Services.js
index 6cd75618..4fc9d0af 100644
--- a/www/manager6/ceph/Services.js
+++ b/www/manager6/ceph/Services.js
@@ -50,9 +50,9 @@ Ext.define('PVE.ceph.Services', {
 	// order guarantee since es2020, but browsers did so before. Note, integers would break it.
 	const healthmap = Object.keys(healthstates);
 	let maxversion = "00.0.00";
-	Object.values(metadata.version || {}).forEach(function(version) {
-	    if (PVE.Utils.compare_ceph_versions(version, maxversion) > 0) {
-		maxversion = version;
+	Object.values(metadata.node || {}).forEach(function(node) {
+	    if (PVE.Utils.compare_ceph_versions(node?.version?.parts, maxversion) > 0) {
+		maxversion = node?.version?.parts;
 	    }
 	});
 	var quorummap = status && status.quorum_names ? status.quorum_names : [];
@@ -183,7 +183,7 @@ Ext.define('PVE.ceph.Services', {
 		if (result.version) {
 		    result.statuses.push(gettext('Version') + ": " + result.version);
 
-		    if (result.version !== maxversion) {
+		    if (PVE.Utils.compare_ceph_versions(result.version, maxversion) !== 0) {
 			if (metadata.version[host] === maxversion) {
 			    if (result.health > healthstates.HEALTH_OLD) {
 				result.health = healthstates.HEALTH_OLD;
diff --git a/www/manager6/ceph/StatusDetail.js b/www/manager6/ceph/StatusDetail.js
index 1ca185f6..dbc55afe 100644
--- a/www/manager6/ceph/StatusDetail.js
+++ b/www/manager6/ceph/StatusDetail.js
@@ -192,9 +192,9 @@ Ext.define('PVE.ceph.StatusDetail', {
 	me.suspendLayout = true;
 
 	let maxversion = "0";
-	Object.values(metadata.version || {}).forEach(function(version) {
-	    if (PVE.Utils.compare_ceph_versions(version, maxversion) > 0) {
-		maxversion = version;
+	Object.values(metadata.node || {}).forEach(function(node) {
+	    if (PVE.Utils.compare_ceph_versions(node?.version?.parts, maxversion) > 0) {
+		maxversion = node?.version?.parts;
 	    }
 	});
 
@@ -203,7 +203,7 @@ Ext.define('PVE.ceph.StatusDetail', {
 	if (metadata.osd) {
 	    metadata.osd.forEach(function(osd) {
 		let version = PVE.Utils.parse_ceph_version(osd);
-		if (version !== maxversion) {
+		if (PVE.Utils.compare_ceph_versions(version, maxversion) !== 0) {
 		    oldosds.push({
 			id: osd.id,
 			version: version,
-- 
2.30.2

Re: [pve-devel] [PATCH manager 0/2] fix ceph version handling in gui

[Thomas Lamprecht]

On 10.11.21 13:15, Dominik Csapak wrote:
> we removed the broadcast of the legacy kv 'ceph-version', but our
> gui and api were not completely  adapted to the new versions yet
> (it seems we forgot)
> 
> the ui patch fixes the dashboard and service lists, but the osd panel
> is still broken (that needs the api patch)
> 
> alternatively to the api patch, we could revert fb0cb9b7, but

I do not see that as alternative, I see that as extra.

* apply this patches to actually be able to remove broadcasting in 8.0
* revert fb0cb9b7 to avoid users that still would upgrade from running into
  any issue

> as we already rolled that change out to pve-no-subscription,
> i see no point to it really.

isn't that the point of that repo exactly? ;)

[pve-devel] applied-series: [PATCH manager 0/2] fix ceph version handling in gui

[Thomas Lamprecht]

On 10.11.21 13:15, Dominik Csapak wrote:
> we removed the broadcast of the legacy kv 'ceph-version', but our
> gui and api were not completely  adapted to the new versions yet
> (it seems we forgot)
> 
> the ui patch fixes the dashboard and service lists, but the osd panel
> is still broken (that needs the api patch)
> 
> alternatively to the api patch, we could revert fb0cb9b7, but
> as we already rolled that change out to pve-no-subscription,
> i see no point to it really.
> 
> 
> Dominik Csapak (2):
>   api: ceph: fix getting ceph versions
>   ui: ceph: fix version handling
> 
>  PVE/API2/Ceph/OSD.pm              | 5 ++---
>  PVE/API2/Cluster/Ceph.pm          | 5 +----
>  www/manager6/ceph/OSD.js          | 6 +++---
>  www/manager6/ceph/Services.js     | 8 ++++----
>  www/manager6/ceph/StatusDetail.js | 8 ++++----
>  5 files changed, 14 insertions(+), 18 deletions(-)
> 


applied series and reverted fb0cb9b7 for now, we should be able to remove with
8.0 (or sooner, it's not _that_ hard of a break) thanks!