From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 0B2321FF137 for ; Tue, 14 Apr 2026 15:29:46 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A2E8819C53; Tue, 14 Apr 2026 15:30:34 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Tue, 14 Apr 2026 15:30:29 +0200 Message-Id: Subject: Re: [PATCH manager] ui: ha: add disarm/re-arm button From: "Daniel Kral" To: "Dominik Rusovac" , X-Mailer: aerc 0.21.0-136-gdb9fe9896a79-dirty References: <20260414115156.188522-1-d.rusovac@proxmox.com> In-Reply-To: <20260414115156.188522-1-d.rusovac@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1776173353397 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.079 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox.com] Message-ID-Hash: KKXT34VH2EQDTMMG4SJ3TJ3K23I5NVJB X-Message-ID-Hash: KKXT34VH2EQDTMMG4SJ3TJ3K23I5NVJB X-MailFrom: d.kral@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Nice work, looks great! I added some inline comments, but functionality-wise this works as expected already. On Tue Apr 14, 2026 at 1:51 PM CEST, Dominik Rusovac wrote: > The button to disarm HA in either of the resource modes ('freeze' or > 'ignore') is disabled as long as HA is disarmed. Analogously, the > re-arming button is disabled as long as HA is not disarmed. > > The icons ('unlink' and 'link') are chosen to emphasize that > "Disarm HA" and "Re-arm HA" are complements. There may be more > suitable pairs of icons though. > > Signed-off-by: Dominik Rusovac > --- > www/manager6/ha/Status.js | 106 ++++++++++++++++++++++++++++++++++ > www/manager6/ha/StatusView.js | 8 +++ > 2 files changed, 114 insertions(+) > > diff --git a/www/manager6/ha/Status.js b/www/manager6/ha/Status.js > index b0b0feb9..d955452d 100644 > --- a/www/manager6/ha/Status.js > +++ b/www/manager6/ha/Status.js > @@ -8,6 +8,107 @@ Ext.define('PVE.ha.Status', { > align: 'stretch', > }, > =20 > + viewModel: { > + data: { > + haDisarmed: false, > + }, > + }, > + > + controller: { > + xclass: 'Ext.app.ViewController', > + > + check_ha_status: function (isDisarmed) { new method names should use cameCase according to our JS Style Guide [0] > + let vm =3D this.getViewModel(); > + vm.set('haDisarmed', isDisarmed); > + }, > + > + disarm: function (comp) { nit: maybe s/comp/menuItem/ would make it a little clearer where the information is coming from? nit: maybe s/disarm/handleDisarmButton/ would also make it clearer where it is supposed to be used (instead of a generic method here) > + let me =3D this; > + let view =3D me.getView(); > + let mode =3D comp.mode; > + > + Ext.Msg.confirm( > + gettext('Confirm'), > + mode =3D=3D=3D 'freeze' > + ? gettext("Are you sure you want to disarm HA with r= esource mode 'freeze'?") > + : gettext("Are you sure you want to disarm HA with r= esource mode 'ignore'?"), nit: the translation keys could be deduplicated with something like Ext.Msg.confirm( gettext('Confirm'), Ext.String.format( gettext("Are you sure you want to disarm HA with resource mode = '{0}'?"), mode ) ... ); Feel free to restructure the code snippet if you consider using it. I also removed the mode =3D=3D=3D 'freeze' condition with just emplacing th= e content of `mode`, but it would be nicer to use the same translation keys as for the menu items (i.e., gettext('Freeze') and gettext('Ignore')). > + function (btn) { > + if (btn !=3D=3D 'yes') { > + return; > + } > + Proxmox.Utils.API2Request({ > + url: '/cluster/ha/status/disarm-ha', > + params: { 'resource-mode': mode }, > + method: 'POST', > + waitMsgTarget: view, > + failure: (response) =3D> Ext.Msg.alert(gettext('= Error'), response.htmlStatus), > + }); > + }, > + ); > + }, > + > + rearm: function () { nit: maybe s/rearm/handleRearmButton/ here as well? > + let me =3D this; > + let view =3D me.getView(); > + > + Ext.Msg.confirm( > + gettext('Confirm'), > + gettext('Are you sure you want to re-arm HA?'), > + function (btn) { > + if (btn !=3D=3D 'yes') { > + return; > + } > + Proxmox.Utils.API2Request({ > + url: '/cluster/ha/status/arm-ha', > + params: {}, nit: the params is not needed here > + method: 'POST', > + waitMsgTarget: view, > + failure: (response) =3D> Ext.Msg.alert(gettext('= Error'), response.htmlStatus), > + }); > + }, > + ); > + }, > + }, > + > + dockedItems: [ > + { > + xtype: 'toolbar', > + dock: 'top', > + items: [ > + { > + text: gettext('Disarm HA'), > + iconCls: 'fa fa-unlink', > + bind: { > + disabled: '{haDisarmed}', > + }, > + menu: [ > + { > + text: gettext('Freeze'), > + iconCls: 'fa fa-snowflake-o', > + mode: 'freeze', > + handler: 'disarm', > + }, > + { > + text: gettext('Ignore'), > + iconCls: 'fa fa-eye-slash', > + mode: 'ignore', > + handler: 'disarm', > + }, > + ], > + }, > + { > + text: gettext('Re-arm HA'), nit: wondering whether this should just be called 'Arm HA', but no hard feelings about this. If we go for that, should be changed for the other instances as well. > + iconCls: 'fa fa-link', > + bind: { > + disabled: '{!haDisarmed}', > + }, > + handler: 'rearm', > + }, > + ], > + }, > + ], > + > + extra new-line that is removed with a `make -C www/manager6 tidy` > initComponent: function () { > var me =3D this; > =20 > @@ -30,6 +131,11 @@ Ext.define('PVE.ha.Status', { > border: 0, > collapsible: true, > padding: '0 0 20 0', > + listeners: { > + hastatuschange: function (isDisarmed) { > + me.getController().check_ha_status(isDisarmed); > + }, > + }, > }, > { > xtype: 'pveHAResourcesView', [0] https://pve.proxmox.com/wiki/Javascript_Style_Guide#Casing