Re: [pve-devel] [PATCH ha-manager 2/2] add api getter/setter for node maintenance mode

From: "Daniel Kral" <d.kral@proxmox.com>
To: "Thomas Skinner" <thomas@atskinner.net>,
	"Thomas Lamprecht" <t.lamprecht@proxmox.com>
Cc: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH ha-manager 2/2] add api getter/setter for node maintenance mode
Date: Mon, 01 Sep 2025 10:52:33 +0200	[thread overview]
Message-ID: <DCHC47ASHB2A.168OVVJ2II58C@proxmox.com> (raw)
In-Reply-To: <CALn9RMcFSeGEbf9qdYEbysfd_cDLeHY_P7r4OnyV008Um=gasQ@mail.gmail.com>

On Fri Aug 29, 2025 at 10:13 PM CEST, Thomas Skinner wrote:
> On Wed, Aug 27, 2025 at 3:25 AM Daniel Kral <d.kral@proxmox.com> wrote:
>> Hm, I've only looked at the PVE API now, but one similar feature is the
>> PBS's maintenance mode for datastores [0], which are set by a single,
>> enum property 'maintenance-mode' in a PUT request.
>>
>> Maybe we could add the same PUT method to @Thomas' proposed
>> /nodes/{node}/maintenance API endpoint, because AFAICT for non-HA use
>> cases we'd need some way to enable and disable maintenance mode there
>> too. That's also cleaner than introducing two very similar endpoints
>> with quite long, weirdly-cased names.
>>
>> [0] https://pbs.proxmox.com/docs/api-viewer/index.html#/config/datastore/{name}
>>
>
> If the API for maintenance mode is under the /nodes/{node}/maintenance
> as @Thomas proposed, does it still make sense to put the code for the
> API into the ha-manager package instead of the manager package? I like
> the idea of the enum under this endpoint e.g. something like PUT
> /nodes/{node}/maintenance --maintenance-mode enable|disable. To be
> more specific to HA, could use something like --lrm-maintenance
> enable|disable that way the maintenance-mode enum could stay free if
> it was wanted for the more generic maintenance mode and it would
> convey to the user that it was specific to HA only.

If it is under /nodes/{node}/maintenance, then yes, I also think it
should go in the pve-manager package.

IMO for the maintenance-mode parameter it's fine to make it generic
already but stating in the description that it's only affecting the HA
for now, as introducing `lrm-maintenance` might cause confusion later on
when users want to put the whole node under maintenance.. Except if
there are use cases to only 'partially' put the node in maintenance
through the API.

Then it should be rather straight-forward to implement the functionality
with issuing the CRM commands to the HA Manager with

    run_command(['ha-manager', 'crm-command', 'node-maintenance', $maintenance_mode]);

where $maintenance_mode is either 'enable' or 'disable'.

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel