On Wed Jun 5, 2024 at 4:48 PM CEST, Lukas Wagner wrote:
>
>
> On 2024-04-30 17:28, Max Carrara wrote:
> > This commit adds `PVE.Utils.parse_ceph_buildcommit`, which can be used
> > to get the full hash "eccf199d..." in parentheses from a string like
> > the following:
> >
> > ceph version 17.2.7 (eccf199d63457659c09677399928203b7903c888) quincy
> > (stable)
> >
> > This hash is displayed and taken into account when comparing monitor
> > and manager versions in the client. Specifically, the shortened build
> > commit is now displayed in parentheses next to the version for both
> > monitors and managers like so:
> >
> > 18.2.2 (abcd1234)
> >
> > Should the build commit of the running service differ from the one
> > that's installed on the host, the newer build commit will also be
> > shown in parentheses:
> >
> > 18.2.2 (abcd1234 -> 5678fedc)
> >
> > The icon displayed for running a service with an outdated build is the
> > same as for running an outdated version. The conditional display of
> > cluster health-related icons remains the same otherwise.
> >
> > The Ceph summary view also displays the hash and will show a warning
> > if a service is running with a build commit that doesn't match the one
> > that's advertised by the host.
> >
> > Signed-off-by: Max Carrara <m.carr...@proxmox.com>
> > ---
> > www/manager6/Utils.js | 14 ++++++++++++++
> > www/manager6/ceph/ServiceList.js | 28 ++++++++++++++++++++++------
> > www/manager6/ceph/Services.js | 14 +++++++++++++-
> > 3 files changed, 49 insertions(+), 7 deletions(-)
> >
> > diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
> > index 74e46694..435c79d7 100644
> > --- a/www/manager6/Utils.js
> > +++ b/www/manager6/Utils.js
> > @@ -128,6 +128,20 @@ Ext.define('PVE.Utils', {
> > return undefined;
> > },
> >
> > + parse_ceph_buildcommit: function(service) {
> > + if (service.ceph_version) {
> > + // See PVE/Ceph/Tools.pm - get_local_version
> > + const match = service.ceph_version.match(
> > + /^ceph.*\sv?(?:\d+(?:\.\d+)+)\s+(?:\(([a-zA-Z0-9]+)\))/,
> > + );
> > + if (match) {
> > + return match[1];
> > + }
> > + }
> > +
> > + return undefined;
> > + },
> > +
> > compare_ceph_versions: function(a, b) {
>
> style nit: new functions/vars should use camelCase, see our JS styleguide [1]
Oh woops, thanks for pointing this out!
>
> > let avers = [];
> > let bvers = [];
> > diff --git a/www/manager6/ceph/ServiceList.js
> > b/www/manager6/ceph/ServiceList.js
> > index 76710146..30f455ed 100644
> > --- a/www/manager6/ceph/ServiceList.js
> > +++ b/www/manager6/ceph/ServiceList.js
> > @@ -102,21 +102,37 @@ Ext.define('PVE.node.CephServiceController', {
> > if (value === undefined) {
> > return '';
> > }
> > +
> > + const bc = PVE.Utils.parse_ceph_buildcommit(rec.data) ?? '';
>
> I'd prefer a longer variable name here, e.g. buildCommit - bc is a bit too
> terse for my
> taste :)
Fair ;)
>
> > +
> > let view = this.getView();
> > - let host = rec.data.host, nodev = [0];
> > + const host = rec.data.host;
>
> style nit: we only really use `const` for proper constants, see our JS
> guidelines
Again thanks for pointing this out!
>
> > +
> > + let versionNode = [0];
> > + let bcNode = '';
> > if (view.nodeversions[host] !== undefined) {
> > - nodev = view.nodeversions[host].version.parts;
> > + versionNode = view.nodeversions[host].version.parts;
> > + bcNode = view.nodeversions[host].buildcommit;
> > }
> >
> > + let bcChanged = bc !== '' && bcNode !== '' && bc !== bcNode;
> > let icon = '';
> > - if (PVE.Utils.compare_ceph_versions(view.maxversion, nodev) > 0) {
> > + if (PVE.Utils.compare_ceph_versions(view.maxversion, versionNode) > 0) {
> > icon = PVE.Utils.get_ceph_icon_html('HEALTH_UPGRADE');
> > - } else if (PVE.Utils.compare_ceph_versions(nodev, value) > 0) {
> > + } else if (PVE.Utils.compare_ceph_versions(versionNode, value) > 0) {
> > icon = PVE.Utils.get_ceph_icon_html('HEALTH_OLD');
> > - } else if (view.mixedversions) {
> > + } else if (view.mixedversions && !bcChanged) {
> > icon = PVE.Utils.get_ceph_icon_html('HEALTH_OK');
> > }
> > - return icon + value;
> > +
> > + let bcPart = bc.substring(0, 9);
> > + if (bcChanged) {
> > + const arrow = '<i class="fa fa-fw fa-long-arrow-right"></i>';
> > + icon ||= PVE.Utils.get_ceph_icon_html('HEALTH_OLD');
> > + bcPart = `${bc.substring(0, 9)}${arrow}${bcNode.substring(0, 9)}`;
> > + }
> > +
> > + return `${icon}${value} (${bcPart})`;
> > },
> >
> > getMaxVersions: function(store, records, success) {
> > diff --git a/www/manager6/ceph/Services.js b/www/manager6/ceph/Services.js
> > index b9fc52c8..410b28bf 100644
> > --- a/www/manager6/ceph/Services.js
> > +++ b/www/manager6/ceph/Services.js
> > @@ -155,6 +155,7 @@ Ext.define('PVE.ceph.Services', {
> > title: metadata[type][id].name || name,
> > host: host,
> > version: PVE.Utils.parse_ceph_version(metadata[type][id]),
> > + buildcommit:
> > PVE.Utils.parse_ceph_buildcommit(metadata[type][id]),
> > service: metadata[type][id].service,
> > addr: metadata[type][id].addr || metadata[type][id].addrs
> > || Proxmox.Utils.unknownText,
> > };
> > @@ -181,7 +182,10 @@ Ext.define('PVE.ceph.Services', {
> > }
> >
> > if (result.version) {
> > - result.statuses.push(gettext('Version') + ": " +
> > result.version);
> > + const host_buildcommit = metadata.node[host]?.buildcommit
> > || "";
> > +
> > + const buildcommit_short = result.buildcommit.substring(0,
> > 9);
> > + result.statuses.push(gettext('Version') + `:
> > ${result.version} (${buildcommit_short})`);
> >
> > if (PVE.Utils.compare_ceph_versions(result.version,
> > maxversion) !== 0) {
> > let host_version = metadata.node[host]?.version?.parts
> > || metadata.version?.[host] || "";
> > @@ -202,6 +206,14 @@ Ext.define('PVE.ceph.Services', {
> > gettext('Other cluster members use a newer
> > version of this service, please upgrade and restart'),
> > );
> > }
> > + } else if (host_buildcommit !== "" && result.buildcommit
> > !== host_buildcommit) {
> > + if (result.health > healthstates.HEALTH_OLD) {
> > + result.health = healthstates.HEALTH_OLD;
> > + }
> > + result.messages.push(
> > + PVE.Utils.get_ceph_icon_html('HEALTH_OLD', true) +
> > + gettext('A newer version was installed but old
> > version still running, please restart'),
> > + );
> > }
> > }
> >
>
> [1] https://pve.proxmox.com/wiki/Javascript_Style_Guide#Casing
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
