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

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

On 4/15/26 3:30 PM, Dominik Rusovac wrote:
> thx for the comments! I will send a v3
> 
> On Wed Apr 15, 2026 at 2:32 PM CEST, Dominik Csapak wrote:
>> 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..
> 
> regarding feedback after activating either of them: one can see the
> Status changing in the Status panel, which exactly traces what's going
> on for every node, in particular it reveals the armed-state in the
> Status of fencing
> 
> if that's not enough feedback, I'd look into other possible solutions in
> more detail

on my test machine, it multiple seconds from clicking ok to
the status actually changing, so this might be confusing, especially
if the cluster/connection is not the fastest and the updates
take longer to arrive.

> 
>>
>> 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(+)
>>>
> 
> [snip]
> 
>>> +    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
>>
> 
> thx, I will inline it
> 
> [snip]
> 
>>> +
>>> +    dockedItems: [
>>> +        {
>>> +            xtype: 'toolbar',
>>> +            dock: 'top',
>>> +            items: [
>>
>> it should be able to shorten that to just using 'tbar'
>>
>> tbar: [
>>       { ... }, // button 1
>>       { ... }, // button 2
>> ],
>>
> 
> indeed, thx, will use 'tbar' instead
> 
>>> +                {
>>> +                    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)
>>
> 
> tbh, I just tried to adhere to what I found in other places of the gui
> 
> if hiding the unusable button is more desirable, I can go for that
> option instead
> 
>> 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 |
>>
> 
> in retrospect, I think my reasoning behind this positioning was rather
> about the more 'obvious' or 'natural' action, that is: HA is
> from the get-go (and also usually) armed and can only be (re-)armed,
> once it's been disarmed - just like stopping something is only possible
> once this very something's started
> 
> I see your reasoning though, and if we do not go for hiding either of
> the buttons, I do not mind changing the positioning
> 
>>> +        },
>>> +    ],
>>> +
> 
> [snip]
> 
>>> +            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
>>
> 
> thx for this informative remark
> 
> [snip]