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 ECE451FF13C for ; Thu, 16 Apr 2026 11:20:59 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id C43E3270ED; Thu, 16 Apr 2026 11:20:59 +0200 (CEST) Message-ID: Date: Thu, 16 Apr 2026 11:20:54 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Subject: Re: [PATCH manager v2] ui: ha: add disarm/re-arm button To: Daniel Kral , Dominik Rusovac , Dominik Csapak , pve-devel@lists.proxmox.com References: <20260415064118.33290-1-d.rusovac@proxmox.com> <900084a5-9466-4a98-b653-2c97fc863f38@proxmox.com> Content-Language: en-US, de-DE From: Thomas Lamprecht In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1776331176523 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.002 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 Message-ID-Hash: NLZJ4GPYH26I7ACVVE3GHBI5N7TMY34B X-Message-ID-Hash: NLZJ4GPYH26I7ACVVE3GHBI5N7TMY34B X-MailFrom: t.lamprecht@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: On 16/04/2026 09:32, Daniel Kral wrote: > On Wed Apr 15, 2026 at 3:32 PM CEST, 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 >> >>> >>> 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) > > Good catch! > > Hm, we also more-or-less fake the instant state change of the HA > resources: If the HA resource's state is changed in the web interface, > which is written to the resources.cfg file, we reload the HA status > model right after it. The status API then returns some interleaved > version, which instantly respects the requested state from the config > (see PVE::HA::Tools::get_verbose_service_state()). > > The reason this doesn't happen for the disarm/arm state is because these > need to wait to be processed by the HA Manager first... The same goes > for other things handled with CRM commands, such things as migrating HA > resources or putting nodes in maintenance mode. But for these things we > can't really fake it because whether these actions are accepted at all > depend on the HA Manager entirely. > > Ideally, we would have some callback mechanism here (would benefit some > other things for the HA stack as well). We could compare the request state (config + queued CRM commands) with the current status and show pending states, with that we should be able to solve better relaying any such queued changes, be it resource level ones or HA LRM/CRM ones. > In the meantime I think a worker polling the ha state for the requested > condition would do good here to give users feedback when the disarming > process is finished. This should also handle if the HA Manager doesn't > process the command at all for some reason. can be OK, but rather more work and also "just" a entry in the task log, if a lot is going on in a cluster, i.e. multiple tasks are running, this might be easily missed as well. Biggest benefit of it would be actually having some more visible task log of whom triggered a HA change when, as the system log is not fully complete (missing user) and as much more gets logged there from the whole system it's also more crowded. Such a task log would IMO a orthogonal change to the showing pending changes one though, and might be better solved with a targeted audit system/log, as adding a task log just for that might be a bit overkill and slightly misusing the task log system for something it wasn't exactly designed for (one can squint and it will seem appropriate, but a structured audit log would be much better and solve some other pain points and user requests). Would be nice to have, but not a blocker for this. The pending changes is blowing up scope a bit and given that we already have the pattern for resource request state changes, I'd also not see that as blocker here, but definitively should get improved soon. > > [snip] > >>>> + { >>>> + 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 >> > > IMO this should be consistent with the other panels (e.g. OSD where we > also have mutually exclusive buttons like 'Start'/'Stop' and > 'In'/'Out'). +1 w.r.t. consistency with other panels. > I think the nice thing about having both visible at the same time is > that users can immediately see where they have to click when they want > to arm the HA stack again and vice versa and don't have to find out by a > rather 'dangerous' action. Especially since the buttons have different > behaviors with disarming having two modes while arming being a single, > simple button. I mean, both have a confirmation prompt, so IMO OK as is w.r.t. basic UI/UX safety.