From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
by lore.proxmox.com (Postfix) with ESMTPS id 64E461FF168
for <inbox@lore.proxmox.com>; Tue, 12 Nov 2024 14:16:18 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id C334329913;
Tue, 12 Nov 2024 14:16:18 +0100 (CET)
Message-ID: <33881880-3f0b-4de1-9f7d-9bb114eb21a2@proxmox.com>
Date: Tue, 12 Nov 2024 14:15:44 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Content-Language: en-US
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Max Carrara <m.carrara@proxmox.com>
References: <20240724150511.474844-1-m.carrara@proxmox.com>
<20240724150511.474844-2-m.carrara@proxmox.com>
From: Aaron Lauterer <a.lauterer@proxmox.com>
In-Reply-To: <20240724150511.474844-2-m.carrara@proxmox.com>
X-SPAM-LEVEL: Spam detection results: 0
AWL -0.036 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [PATCH v3 pve-manager 1/5] fix #5366: ui: ceph:
services: parse and display build commit
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
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.carrara@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