one very small nit inline at the end

On  2024-07-24  17:05, Max Carrara wrote:
The build commit 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 (build: 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 (build: 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
icons related to cluster health 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.

This requires the introduction of two helper functions:

1. `PVE.Utils.parseCephBuildCommit`, 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)

2. `PVE.Utils.renderCephBuildCommit`, which is used to determine how
    to render a service's current build commit and which accompanying
    icon to choose.

Signed-off-by: Max Carrara <m.carr...@proxmox.com>
Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=5366
---
Changes v2 --> v3:
   * add new `renderCephBuildCommit` helper function (thanks Thomas!)
   * add docstrings for helpers
   * use less ambiguous variable names (thanks Thomas!)
   * put 'build: ' in front of build commit when rendering (thanks Thomas!)
   * handle no rendered build commit being available in MGR / MON lists,
     returning the icon + version without the commit instead
   * make the modified logic in Services.js more readable
   * reword message about differing builds in the overview
   * reword commit title & message
   * add 'Fixes' trailer
   * remove outdated R-b and T-b trailers

Changes v1 --> v2:
   * use camelCase instead of snake_case (thanks Lukas!)
   * use more descriptive variable names (thanks Lukas!)
   * use `let` instead of `const` for variables where applicable (thanks Lukas!)

  www/manager6/Utils.js            | 107 +++++++++++++++++++++++++++++++
  www/manager6/ceph/ServiceList.js |  33 +++++++---
  www/manager6/ceph/Services.js    |  19 +++++-
  3 files changed, 148 insertions(+), 11 deletions(-)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index db86fa9a..1d42be34 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -128,6 +128,113 @@ Ext.define('PVE.Utils', {
        return undefined;
      },
+ /**
+     * Parses a Ceph build commit from its version string.

[...]
getMaxVersions: function(store, records, success) {
diff --git a/www/manager6/ceph/Services.js b/www/manager6/ceph/Services.js
index dfafee43..ff6f80e9 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.parseCephBuildCommit(metadata[type][id]),
                    service: metadata[type][id].service,
                    addr: metadata[type][id].addr || metadata[type][id].addrs 
|| Proxmox.Utils.unknownText,
                };
@@ -181,7 +182,15 @@ Ext.define('PVE.ceph.Services', {
                }
if (result.version) {
-                   result.statuses.push(gettext('Version') + ": " + 
result.version);
+                   let installedBuildCommit = metadata.node[host]?.buildcommit 
?? '';
+                   let runningBuildCommit = result.buildcommit ?? '';
+
+                   let statusLine = gettext('Version') + `: ${result.version}`;
+                   if (runningBuildCommit) {
+                       statusLine += ` (build: 
${runningBuildCommit.substring(0, 9)})`;
+                   }
+
+                   result.statuses.push(statusLine);
if (PVE.Utils.compare_ceph_versions(result.version, maxversion) !== 0) {
                        let host_version = metadata.node[host]?.version?.parts || 
metadata.version?.[host] || "";
@@ -202,6 +211,14 @@ Ext.define('PVE.ceph.Services', {
                                gettext('Other cluster members use a newer 
version of this service, please upgrade and restart'),
                            );
                        }
+                   } else if (installedBuildCommit !== "" && 
runningBuildCommit !== installedBuildCommit) {
+                       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 build of this release was 
installed but old build still running, please restart'),

Maybe phrase it like:

A newer build of this release was installed, but the old build is still running. Please restart the service.

But that is really just a nice to have thing.

+                       );
                    }
                }



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to