Re: [PATCH manager v2] ui: ha: add disarm/re-arm button

[Dominik Csapak <d.csapak@proxmox.com>]

what i miss with using these is some feedback when i activated them.

This is probably more due to the backend decision, but
when clicking arm/disarm, there is nothing happening in the gui at
first, no spinning icon, no change in state..

I think there are a few possible solutions on this, but
i'm not super deep in the ha stack, so sorry in advance if
some can't work:
* use a worker that waits for the change of the ha state, e.g.
   via polling. this could directly be shown in the gui
* mark the requested state immediately somewhere and return
   it with the overall ha state, so we  see in the gui what is happening
* 'fake' the progress until we see a status change
   (i don't really like this, since it's prone to errors when e.g.
    one admin disarms, the other arms again, but the gui does not
    get an updated state in the meantime)


other comments inline

On 4/15/26 8:40 AM, 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 button
> to arm HA is disabled as long as HA is not disarmed.
> 
> The icons ('unlink' and 'link') are chosen to emphasize that "Disarm HA"
> and "Arm HA" are complements. There may be more suitable pairs of icons
> though.
> 
> Signed-off-by: Dominik Rusovac <d.rusovac@proxmox.com>
> ---
> changes since v1:
> * use camel case for function names
> * choose clearer parameter: 'comp' -> 'menuItem'
> * choose clearer function names:
>    - 'disarm' -> 'handleDisarmButton'
>    - 'rearm'  -> 'handleArmButton'
> * deduplicate translation keys
> * use same translation keys as for menu items
> * remove obsolete 'params'
> * change button text "Re-arm HA" to "Arm HA"
> * remove extra new-line
> 
>   www/manager6/ha/Status.js     | 104 ++++++++++++++++++++++++++++++++++
>   www/manager6/ha/StatusView.js |   8 +++
>   2 files changed, 112 insertions(+)
> 
> diff --git a/www/manager6/ha/Status.js b/www/manager6/ha/Status.js
> index b0b0feb9..5c65d52d 100644
> --- a/www/manager6/ha/Status.js
> +++ b/www/manager6/ha/Status.js
> @@ -8,6 +8,105 @@ Ext.define('PVE.ha.Status', {
>           align: 'stretch',
>       },
>   
> +    viewModel: {
> +        data: {
> +            haDisarmed: false,
> +        },
> +    },
> +
> +    controller: {
> +        xclass: 'Ext.app.ViewController',
> +
> +        checkHaStatus: function (isDisarmed) {
> +            let vm = this.getViewModel();
> +            vm.set('haDisarmed', isDisarmed);
> +        },
> +

this method does not 'check' it 'sets' the HA status?

IMHO a better name would be:

setHaStatus
or similar


also, this is only used once, so it's probably better
to just call the vm.set inline

> +        handleDisarmButton: function (menuItem) {
> +            let me = this;
> +            let view = me.getView();
> +
> +            Ext.Msg.confirm(
> +                gettext('Confirm'),
> +                Ext.String.format(
> +                    gettext("Are you sure you want to disarm HA with resource mode '{0}'?"),
> +                    menuItem.text,
> +                ),
> +                function (btn) {
> +                    if (btn !== 'yes') {
> +                        return;
> +                    }
> +                    Proxmox.Utils.API2Request({
> +                        url: '/cluster/ha/status/disarm-ha',
> +                        params: { 'resource-mode': menuItem.mode },
> +                        method: 'POST',
> +                        waitMsgTarget: view,
> +                        failure: (response) => Ext.Msg.alert(gettext('Error'), response.htmlStatus),
> +                    });
> +                },
> +            );
> +        },
> +
> +        handleArmButton: function () {
> +            let me = this;
> +            let view = me.getView();
> +
> +            Ext.Msg.confirm(
> +                gettext('Confirm'),
> +                gettext('Are you sure you want to arm HA?'),
> +                function (btn) {
> +                    if (btn !== 'yes') {
> +                        return;
> +                    }
> +                    Proxmox.Utils.API2Request({
> +                        url: '/cluster/ha/status/arm-ha',
> +                        method: 'POST',
> +                        waitMsgTarget: view,
> +                        failure: (response) => Ext.Msg.alert(gettext('Error'), response.htmlStatus),
> +                    });
> +                },
> +            );
> +        },
> +    },
> +
> +    dockedItems: [
> +        {
> +            xtype: 'toolbar',
> +            dock: 'top',
> +            items: [

it should be able to shorten that to just using 'tbar'

tbar: [
     { ... }, // button 1
     { ... }, // button 2
],

> +                {
> +                    text: gettext('Disarm HA'),
> +                    iconCls: 'fa fa-unlink',
> +                    bind: {
> +                        disabled: '{haDisarmed}',
> +                    },
> +                    menu: [
> +                        {
> +                            text: gettext('Freeze'),
> +                            iconCls: 'fa fa-snowflake-o',
> +                            mode: 'freeze',
> +                            handler: 'handleDisarmButton',
> +                        },
> +                        {
> +                            text: gettext('Ignore'),
> +                            iconCls: 'fa fa-eye-slash',
> +                            mode: 'ignore',
> +                            handler: 'handleDisarmButton',
> +                        },
> +                    ],
> +                },
> +                {
> +                    text: gettext('Arm HA'),
> +                    iconCls: 'fa fa-link',
> +                    bind: {
> +                        disabled: '{!haDisarmed}',
> +                    },
> +                    handler: 'handleArmButton',
> +                },
> +            ],

i'm not totally against having two buttons here, but since
there is always only one or the other active, wouldn't
it make more sense to hide the button that
can't do anything? (though i admit we do most often show the
unusable buttons across our gui, so it's fine too)

At least i would probably switch the position of these,
since the more  (for the lack of a better word) 'affirming' action
is usually at the beginning (like 'add' or 'start') and the other
options comes later (like 'remove' or 'shutdown')

so

| Arm | Disarm |

reads more logical to me than

| Disarm | Arm |

> +        },
> +    ],
> +
>       initComponent: function () {
>           var me = this;
>   
> @@ -30,6 +129,11 @@ Ext.define('PVE.ha.Status', {
>                   border: 0,
>                   collapsible: true,
>                   padding: '0 0 20 0',
> +                listeners: {
> +                    hastatuschange: function (isDisarmed) {
> +                        me.getController().checkHaStatus(isDisarmed);
> +                    },
> +                },
>               },
>               {
>                   xtype: 'pveHAResourcesView',
> diff --git a/www/manager6/ha/StatusView.js b/www/manager6/ha/StatusView.js
> index bc2da71f..4cbe85a1 100644
> --- a/www/manager6/ha/StatusView.js
> +++ b/www/manager6/ha/StatusView.js
> @@ -42,6 +42,13 @@ Ext.define(
>                   },
>               });
>   
> +            me.rstore.on('load', function () {
> +                let fencing = store.findRecord('type', 'fencing');
> +                let disarmed = fencing && fencing.get('armed-state') === 'disarmed';
> +
> +                me.fireEvent('hastatuschange', disarmed);

just a remark, not even a nit:

the usual convention in extjs is that the first parameter of an event is
the component itself, so it would usually be

me.fireEvent('foo', me, <remaining arguments>);

we (and extjs itself) don't always adhere to that, so it's fine
here, but wanted to note it anyway

> +            });
> +
>               Ext.apply(me, {
>                   store: store,
>                   stateful: false,
> @@ -105,6 +112,7 @@ Ext.define(
>                           return PVE.data.ResourceStore.guestName(vmid);
>                       },
>                   },
> +                'armed-state',
>               ],
>               idProperty: 'id',
>           });