Am 01/07/2024 um 16:10 schrieb Max Carrara:
> This commit adds `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)
>
> 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>
> Tested-by: Lukas Wagner <l.wag...@proxmox.com>
> Reviewed-by: Lukas Wagner <l.wag...@proxmox.com>
> ---
> 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 | 14 ++++++++++++++
> www/manager6/ceph/ServiceList.js | 32 ++++++++++++++++++++++++++------
> www/manager6/ceph/Services.js | 14 +++++++++++++-
> 3 files changed, 53 insertions(+), 7 deletions(-)
>
> diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
> index 74e46694..f2fd0f7e 100644
> --- a/www/manager6/Utils.js
> +++ b/www/manager6/Utils.js
> @@ -128,6 +128,20 @@ Ext.define('PVE.Utils', {
> return undefined;
> },
>
> + parseCephBuildCommit: 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) {
> let avers = [];
> let bvers = [];
> diff --git a/www/manager6/ceph/ServiceList.js
> b/www/manager6/ceph/ServiceList.js
> index 76710146..d994aa4e 100644
> --- a/www/manager6/ceph/ServiceList.js
> +++ b/www/manager6/ceph/ServiceList.js
> @@ -102,21 +102,41 @@ Ext.define('PVE.node.CephServiceController', {
> if (value === undefined) {
> return '';
> }
> +
> + let buildCommit = PVE.Utils.parseCephBuildCommit(rec.data) ?? '';
> +
> let view = this.getView();
> - let host = rec.data.host, nodev = [0];
> + let host = rec.data.host;
> +
> + let versionNode = [0];
> + let buildCommitNode = '';
> if (view.nodeversions[host] !== undefined) {
> - nodev = view.nodeversions[host].version.parts;
> + versionNode = view.nodeversions[host].version.parts;
> + buildCommitNode = view.nodeversions[host].buildcommit;
> }
>
> + let bcChanged =
I'd prefer the more telling `buildCommitChanged` variable name here.
> + buildCommit !== '' &&
> + buildCommitNode !== '' &&
> + buildCommit !== buildCommitNode;
above hunk and...
> +
> 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 buildCommitPart = buildCommit.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');
> + buildCommitPart = `${buildCommit.substring(0,
> 9)}${arrow}${buildCommitNode.substring(0, 9)}`;
> + }
... most of the above hunk might be better factored out in a helper
function, as this is basically 1:1 duplication here and in patch 08/10.
The function could e.g. take both current and new commits as parameters
and return the rendered build commit (buildCommitPart) and a boolean about
if it should be interpreted as changed (updated?). That could be in form
of an array or object and then destructured here.
also, maybe rendered this as `build ${buildCommit.substring(0, 9)} ...` to
give some name for users to ask/talk about when wondering what this funny
hex string is.
> +
> + return `${icon}${value} (${buildCommitPart})`;
> },
>
> getMaxVersions: function(store, records, success) {
> diff --git a/www/manager6/ceph/Services.js b/www/manager6/ceph/Services.js
> index b9fc52c8..7ce289dd 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,10 @@ Ext.define('PVE.ceph.Services', {
> }
>
> if (result.version) {
> - result.statuses.push(gettext('Version') + ": " +
> result.version);
> + const buildCommitHost = metadata.node[host]?.buildcommit ||
> "";
>
> +
> + const buildCommitShort = result.buildcommit.substring(0, 9);
naming is IMO not ideal, I'd rather use something like `buildCommitInstalled`
and `buildCommitRunning` to clarify what each value actually represents.
> + result.statuses.push(gettext('Version') + `:
> ${result.version} (${buildCommitShort})`);
>
> 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 (buildCommitHost !== "" && result.buildcommit !==
> buildCommitHost) {
> + 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'),
maybe s/version/build/ or possibly s/version/build of the same release/
to better convey that the major release was unchanged but a newer build
got pulled in.
> + );
> }
> }
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
