Re: applied: [PATCH-SERIES docs/ha-manager/manager v3 00/16] Negative Node Affinity Rules

["Daniel Kral" <d.kral@proxmox.com>]

On Fri Jul 24, 2026 at 12:02 AM CEST, Thomas Lamprecht wrote:
> Applied, thanks! 
>
> I added a few fixes on top of manager and ha-manager, would be nice if you
> could recheck them. The HA manager ones:
>
> - The "negative rule covers all cluster nodes" check compared node counts,
>   which only holds while the rule's nodes are a subset of the cluster. A rule
>   still naming a since-removed node inflates the count and gets dropped even
>   though its complement is non-empty. It now takes the actual complement via
>   set_difference(), like the inversion transform already does.

That's a very good catch, thank you for adding the fix-ups!

>
> - The consistency check between negative resource affinity and node affinity
>   rules runs before the negative->positive inversion, so it read a negative
>   rule's nodes as the allowed set and rejected feasible setups (e.g. keep vm:1
>   and vm:2 apart, both off node1, on three nodes). It now uses the complement
>   there too; added a test for the feasible and the still-infeasible case.

Here as well, I'm sorry that I've missed it since I've gone through the
checks here, but unfortunately it slipped right past me..

The changes looks good to me though and there are no other global checks
that need special cases for negative node affinity rules currently,
because

1. in check_single_priority_node_affinity_in_resource_affinity_rules
   the same checks are true for negative node affinity rules as for
   those non-zero priority classes aren't allowed anyway

2. in check_single_node_affinity_per_positive_resource_affinity_rule it
   is also true as the strict/non-strict option in negative node
   affinity rules doesn't allow for a simple interpretation of multiple
   node affinity rules existing either

>
> Plus a trivial cleanup dropping a redundant node-set copy in the inversion.
>
> The root cause seems to be that negative rules only get inverted after all
> checks ran, so every inter-plugin check has to special-case them.  Longer term
> it'd be probably cleaner to invert right after the negative-specific checks and
> before the general ones, so downstream only ever sees positive rules and this
> whole class of "check saw the avoided set" issues can't happen. Needs a bit of
> check/transform phase reordering though, better done on its own, so that's why
> I took that in as is with the smaller targeted follow-up.

I have considered to interleave checks with transformations in the past
for these patches [0] [1] as we need to special case positive resource affinity
rules already as well, but I scratched the idea before sending those
tree's patches and went with introducing a helper for these special
cases [2].

I didn't do the interleave for those patches around [0] and [1], because
I only considered arbitrary interleaving then, which is rather messy and
costly for doing quick checks for the API routes. Another smaller issue
was to properly map the transformed rules back to the original rules so
that the users (and the web interface dialog) could identify the rules
which caused any troubles.

However, I think we could go forward with a fixed pipeline with a
pre-check-transformation pass and a post-check-transformation pass,
where it's much more straightforward to know which logic to run for the
check_feasibility() and for the additional transform() and compile().

That could make those special cases for positive resource affinity rules
and negative node affinity rules nicer to work with in the end.
I'm prioritizing the work on making the CRM/LRM aware of non-HA
resources and everything that goes with that change currently, but I can
certainly send a patch series for this if there's no other takers in the
mean time.

[0] https://git.proxmox.com/?p=pve-ha-manager.git;a=commitdiff;h=e79f4b4ae436c1a26238a4585e68c53886169ba7
[1] https://git.proxmox.com/?p=pve-ha-manager.git;a=commitdiff;h=4f09820fa23aa417dc15070bc2aaa50859c855f1
[2] https://git.proxmox.com/?p=pve-ha-manager.git;a=commitdiff;h=0f6309d4330a9b86cf6ed57d0398277a0075076a

>
> Manager were mostly smaller issues/nits, see git.

Thanks for those as well, will keep browser compatibility better in
check for future patches.

The approach with using the select listener is also much nicer than
doing the selection inversion on apply from the two-way binding of the
useNodePriority property.