From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 4FEDA1FF0AF for ; Fri, 06 Nov 2026 10:57:50 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 8A629216F0; Fri, 06 Nov 2026 10:57:21 +0100 (CET) From: Daniel Kral To: pve-devel@lists.proxmox.com Subject: [PATCH ha-manager v2 4/7] manager: apply negative resource affinity before get_node_affinity Date: Fri, 6 Nov 2026 10:56:47 +0100 Message-ID: <20261106095650.72865-5-d.kral@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20261106095650.72865-1-d.kral@proxmox.com> References: <20261106095650.72865-1-d.kral@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1793959024550 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.684 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust 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: KXMWLTXCEJHBB4GI7SUKWOZOFC3TDB3O X-Message-ID-Hash: KXMWLTXCEJHBB4GI7SUKWOZOFC3TDB3O X-MailFrom: d.kral@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: When selecting possible HA resource node placements, get_node_affinity() in select_service_node() and get_resource_migration_candidates() is responsible to determine the highest node priority class with at least one available node and the allowed set from an input node set, which must only contain available nodes to the specific HA resource. A necessary condition for a node to be considered as available to an HA resource is whether the node is in 'online' state (i.e., not 'unknown', 'maintenance', 'fence', nor 'gone'). However, negative resource affinity rules introduced a new condition, where nodes become unavailable to HA resources: another HA resource, with which the HA resource must not share the node with, is already running on the node. Currently, the negative resource affinity rules are applied on the highest node priority class selected by get_node_affinity(). This can result in an empty set if all the remaining nodes in the highest node priority class cannot be selected because of negative resource affinity constraints. Therefore, apply the negative resource affinity rules on the available node set already, so get_node_affinity() will only select node priority classes, which meet both conditions mentioned above. It should be noted that the HA stack currently does not allow an HA resource to be referenced in both resource affinity rules and explicit multi-priority node affinity rules. However, non-strict node affinity rules implicitly create multi-priority node affinity rules, because there the member nodes are in the highest node priority class, while the non-member nodes are in the least node priority class. As seen in "test-resource-affinity-with-node-affinity-strict-negative3", this makes the HA Manager move HA resources away from their preferred node, if an HA resource, that it must not share the node with, starts there. This is valid with the semantics of non-strict node affinity rules. Nonetheless, handling negative affinity rules in these scenarios should be improved in the future, e.g., by moving the starting HA resource to another available node in request_start already. Signed-off-by: Daniel Kral --- changes since v1: - move this patch up first - instead of looping through the node priority classes and effectively letting apply_negative_resource_affinity() decide the node priority class itself, remove them in $online_nodes already beforehand; this results in the same behavior and is easier to read and think about - adapt comment accordingly src/PVE/HA/Manager.pm | 20 ++++++++++++---- .../README | 5 ++-- .../log.expect | 24 ++++++++++++++----- .../README | 7 +++--- .../log.expect | 16 +++++++------ 5 files changed, 49 insertions(+), 23 deletions(-) diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm index 753da1a8..c38b1ddd 100644 --- a/src/PVE/HA/Manager.pm +++ b/src/PVE/HA/Manager.pm @@ -206,10 +206,15 @@ sub get_resource_migration_candidates { my $current_leader_node = $ss->{$leader_sid}->{node}; my $online_nodes = { map { $_ => 1 } $online_node_usage->list_nodes() }; - my (undef, $target_nodes) = get_node_affinity($node_affinity, $leader_sid, $online_nodes); my ($together, $separate) = get_resource_affinity($resource_affinity, $leader_sid, $ss, $online_nodes); - apply_negative_resource_affinity($separate, $target_nodes); + + # Remove nodes with active HA resources that must not run on the same node as $leader_sid + # here already, so get_node_affinity() acknowledges these nodes as unavailable to + # $leader_sid when determining the highest available node priority class. + apply_negative_resource_affinity($separate, $online_nodes); + + my (undef, $target_nodes) = get_node_affinity($node_affinity, $leader_sid, $online_nodes); delete $target_nodes->{$current_leader_node}; @@ -352,12 +357,18 @@ sub select_service_node { $compiled_rules->@{qw(node-affinity resource-affinity)}; my $online_nodes = { map { $_ => 1 } $online_node_usage->list_nodes() }; + + my ($together, $separate) = get_resource_affinity($resource_affinity, $sid, $ss, $online_nodes); + + # Remove nodes with active HA resources that must not run on the same node as $sid + # here already, so get_node_affinity() acknowledges these nodes as unavailable to + # $sid when determining the highest available node priority class. + apply_negative_resource_affinity($separate, $online_nodes); + my ($allowed_nodes, $pri_nodes) = get_node_affinity($node_affinity, $sid, $online_nodes); return undef if !%$pri_nodes; - my ($together, $separate) = get_resource_affinity($resource_affinity, $sid, $ss, $online_nodes); - # stay on current node if possible (avoids random migrations) if ( $node_preference eq 'none' @@ -377,7 +388,6 @@ sub select_service_node { } } - apply_negative_resource_affinity($separate, $pri_nodes); apply_positive_resource_affinity($together, $pri_nodes); return $maintenance_fallback diff --git a/src/test/test-resource-affinity-with-node-affinity-maintenance-strict-negative1/README b/src/test/test-resource-affinity-with-node-affinity-maintenance-strict-negative1/README index c6a11cec..e1fc0d04 100644 --- a/src/test/test-resource-affinity-with-node-affinity-maintenance-strict-negative1/README +++ b/src/test/test-resource-affinity-with-node-affinity-maintenance-strict-negative1/README @@ -4,5 +4,6 @@ - in a non-strict node affinity rule to node2 and node3 (equal priority), and - in a strict negative resource affinity rule with each other. -Tests whether the HA resource on node3 will stay there, even though node3 is -put in maintenance mode, because it cannot find any replacement node. +Tests whether the HA resource on node3 will correctly move to a replacement +node, which is different from the node of the other HA resource (node2), and +moves back to its previous maintenance node as soon as it's available again. diff --git a/src/test/test-resource-affinity-with-node-affinity-maintenance-strict-negative1/log.expect b/src/test/test-resource-affinity-with-node-affinity-maintenance-strict-negative1/log.expect index 5c7aa99c..1fc25206 100644 --- a/src/test/test-resource-affinity-with-node-affinity-maintenance-strict-negative1/log.expect +++ b/src/test/test-resource-affinity-with-node-affinity-maintenance-strict-negative1/log.expect @@ -30,13 +30,25 @@ info 25 node3/lrm: service status vm:102 started info 120 cmdlist: execute crm node3 enable-node-maintenance info 125 node3/lrm: status change active => maintenance info 140 node1/crm: node 'node3': state changed from 'online' => 'maintenance' -warn 140 node1/crm: service 'vm:102': cannot find a replacement node while its current node is in maintenance -warn 160 node1/crm: service 'vm:102': cannot find a replacement node while its current node is in maintenance -warn 180 node1/crm: service 'vm:102': cannot find a replacement node while its current node is in maintenance -warn 200 node1/crm: service 'vm:102': cannot find a replacement node while its current node is in maintenance +info 140 node1/crm: migrate service 'vm:102' to node 'node1' (running) +info 140 node1/crm: service 'vm:102': state changed from 'started' to 'migrate' (node = node3, target = node1) +info 141 node1/lrm: got lock 'ha_agent_node1_lock' +info 141 node1/lrm: status change wait_for_agent_lock => active +info 145 node3/lrm: service vm:102 - start migrate to node 'node1' +info 145 node3/lrm: service vm:102 - end migrate to node 'node1' +info 160 node1/crm: service 'vm:102': state changed from 'migrate' to 'started' (node = node1) +info 161 node1/lrm: starting service vm:102 +info 161 node1/lrm: service status vm:102 started info 220 cmdlist: execute crm node3 disable-node-maintenance -warn 220 node1/crm: service 'vm:102': cannot find a replacement node while its current node is in maintenance +info 225 node3/lrm: got lock 'ha_agent_node3_lock' info 225 node3/lrm: status change maintenance => active info 240 node1/crm: node 'node3': state changed from 'maintenance' => 'online' -info 240 node1/crm: service 'vm:102': clearing stale maintenance node 'node3' setting (is current node) +info 240 node1/crm: moving service 'vm:102' back to 'node3', node came back from maintenance. +info 240 node1/crm: migrate service 'vm:102' to node 'node3' (running) +info 240 node1/crm: service 'vm:102': state changed from 'started' to 'migrate' (node = node1, target = node3) +info 241 node1/lrm: service vm:102 - start migrate to node 'node3' +info 241 node1/lrm: service vm:102 - end migrate to node 'node3' +info 260 node1/crm: service 'vm:102': state changed from 'migrate' to 'started' (node = node3) +info 265 node3/lrm: starting service vm:102 +info 265 node3/lrm: service status vm:102 started info 820 hardware: exit simulation - done diff --git a/src/test/test-resource-affinity-with-node-affinity-strict-negative3/README b/src/test/test-resource-affinity-with-node-affinity-strict-negative3/README index 062fc665..581b0e9a 100644 --- a/src/test/test-resource-affinity-with-node-affinity-strict-negative3/README +++ b/src/test/test-resource-affinity-with-node-affinity-strict-negative3/README @@ -1,7 +1,8 @@ Test whether a strict negative resource affinity rule among three resources, -where two resources are restricted each to nodes they are not yet on, can be -exchanged to the nodes described by their node affinity rules, if one of the -resources is stopped. +where all resources are restricted each to nodes they are not yet on, can be +exchanged to the nodes described by their node affinity rules or fallback to +another valid configuration within the semantics of non-strict node affinity +rules, if one of the resources is stopped. The test scenario is: - vm:101, vm:102, and vm:103 should be on node2, node3 or node1 respectively diff --git a/src/test/test-resource-affinity-with-node-affinity-strict-negative3/log.expect b/src/test/test-resource-affinity-with-node-affinity-strict-negative3/log.expect index 1ed34c36..66974583 100644 --- a/src/test/test-resource-affinity-with-node-affinity-strict-negative3/log.expect +++ b/src/test/test-resource-affinity-with-node-affinity-strict-negative3/log.expect @@ -57,11 +57,13 @@ info 285 node3/lrm: service status vm:102 started info 320 cmdlist: execute service vm:101 started info 320 node1/crm: service 'vm:101': state changed from 'stopped' to 'request_start' (node = node1) info 320 node1/crm: service 'vm:101': state changed from 'request_start' to 'started' (node = node1) -info 320 node1/crm: migrate service 'vm:101' to node 'node2' (running) -info 320 node1/crm: service 'vm:101': state changed from 'started' to 'migrate' (node = node1, target = node2) -info 321 node1/lrm: service vm:101 - start migrate to node 'node2' -info 321 node1/lrm: service vm:101 - end migrate to node 'node2' -info 340 node1/crm: service 'vm:101': state changed from 'migrate' to 'started' (node = node2) -info 343 node2/lrm: starting service vm:101 -info 343 node2/lrm: service status vm:101 started +info 320 node1/crm: migrate service 'vm:103' to node 'node2' (running) +info 320 node1/crm: service 'vm:103': state changed from 'started' to 'migrate' (node = node1, target = node2) +info 321 node1/lrm: starting service vm:101 +info 321 node1/lrm: service status vm:101 started +info 321 node1/lrm: service vm:103 - start migrate to node 'node2' +info 321 node1/lrm: service vm:103 - end migrate to node 'node2' +info 340 node1/crm: service 'vm:103': state changed from 'migrate' to 'started' (node = node2) +info 343 node2/lrm: starting service vm:103 +info 343 node2/lrm: service status vm:103 started info 920 hardware: exit simulation - done -- 2.47.3