* [PATCH manager 1/2] fix #7589: rules: allow setting false 'disable' parameter
@ 2026-05-21 14:20 Dominik Rusovac
2026-05-21 14:20 ` [PATCH manager 2/2] test: rules_cfg: declare explicitly not disabled rules Dominik Rusovac
0 siblings, 1 reply; 2+ messages in thread
From: Dominik Rusovac @ 2026-05-21 14:20 UTC (permalink / raw)
To: pve-devel
Allow setting 'disabled' parameter of rule to false via API.
Rules used to be excluded if respective 'disable' parameter did not
exist, regardless of its value. Keep a rule's 'disable' parameter and
properly check on its value.
Either of the options to disable a rule, removing 'disable' as well as
setting it to false, should work now.
Signed-off-by: Dominik Rusovac <d.rusovac@proxmox.com>
---
Notes:
to rule out potential regressions, I modified all tests relating to
rule_cfgs such that any enabled rule had the 'disabled' parameter
explicitly set to 0, and verified that all of the tests passed
src/PVE/API2/HA/Rules.pm | 4 ----
src/PVE/CLI/ha_manager.pm | 2 +-
src/PVE/HA/Rules.pm | 2 +-
3 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/src/PVE/API2/HA/Rules.pm b/src/PVE/API2/HA/Rules.pm
index 0b44d51..5f0ce04 100644
--- a/src/PVE/API2/HA/Rules.pm
+++ b/src/PVE/API2/HA/Rules.pm
@@ -273,8 +273,6 @@ __PACKAGE__->register_method({
my $type = extract_param($param, 'type');
my $ruleid = extract_param($param, 'rule');
- delete $param->{disable} if !$param->{disable};
-
my $plugin = PVE::HA::Rules->lookup($type);
my $opts = $plugin->check_config($ruleid, $param, 1, 1);
@@ -325,8 +323,6 @@ __PACKAGE__->register_method({
my $digest = extract_param($param, 'digest');
my $delete = extract_param($param, 'delete');
- delete $param->{disable} if !$param->{disable};
-
if ($delete) {
$delete = [PVE::Tools::split_list($delete)];
}
diff --git a/src/PVE/CLI/ha_manager.pm b/src/PVE/CLI/ha_manager.pm
index 6625de6..cc4361f 100644
--- a/src/PVE/CLI/ha_manager.pm
+++ b/src/PVE/CLI/ha_manager.pm
@@ -259,7 +259,7 @@ our $cmddef = {
'enabled', 'state', 'rule', 'type', 'resources', 'comment',
];
for my $rule (@$data) {
- $rule->{enabled} = int(!exists($rule->{disable}));
+ $rule->{enabled} = int(!$rule->{disable});
$rule->{state} = $rule->{errors} ? 'ignored (conflicts)' : 'in use';
}
PVE::CLIFormatter::print_api_result($data, $schema, $props_to_print, $options);
diff --git a/src/PVE/HA/Rules.pm b/src/PVE/HA/Rules.pm
index 3a14eeb..90625af 100644
--- a/src/PVE/HA/Rules.pm
+++ b/src/PVE/HA/Rules.pm
@@ -563,7 +563,7 @@ sub foreach_rule {
next if !$rule; # skip invalid rules
next if defined($sid) && !defined($rule->{resources}->{$sid});
next if defined($opts{type}) && $rule->{type} ne $opts{type};
- next if $opts{exclude_disabled_rules} && exists($rule->{disable});
+ next if $opts{exclude_disabled_rules} && $rule->{disable};
$func->($rule, $ruleid);
}
--
2.47.3
^ permalink raw reply related [flat|nested] 2+ messages in thread
* [PATCH manager 2/2] test: rules_cfg: declare explicitly not disabled rules
2026-05-21 14:20 [PATCH manager 1/2] fix #7589: rules: allow setting false 'disable' parameter Dominik Rusovac
@ 2026-05-21 14:20 ` Dominik Rusovac
0 siblings, 0 replies; 2+ messages in thread
From: Dominik Rusovac @ 2026-05-21 14:20 UTC (permalink / raw)
To: pve-devel
Adopt tests 'rules_cfgs/inconsistent*' and explicitly set 'disabled' to
false for any enabled rule to check that rules are still compiled
correctly.
Signed-off-by: Dominik Rusovac <d.rusovac@proxmox.com>
---
...-disabled-node-resource-affinity-rules.cfg | 88 ++++++++++++
...ed-node-resource-affinity-rules.cfg.expect | 133 ++++++++++++++++++
...y-not-disabled-resource-affinity-rules.cfg | 32 +++++
...isabled-resource-affinity-rules.cfg.expect | 22 +++
4 files changed, 275 insertions(+)
create mode 100644 src/test/rules_cfgs/explicitly-not-disabled-node-resource-affinity-rules.cfg
create mode 100644 src/test/rules_cfgs/explicitly-not-disabled-node-resource-affinity-rules.cfg.expect
create mode 100644 src/test/rules_cfgs/explicitly-not-disabled-resource-affinity-rules.cfg
create mode 100644 src/test/rules_cfgs/explicitly-not-disabled-resource-affinity-rules.cfg.expect
diff --git a/src/test/rules_cfgs/explicitly-not-disabled-node-resource-affinity-rules.cfg b/src/test/rules_cfgs/explicitly-not-disabled-node-resource-affinity-rules.cfg
new file mode 100644
index 0000000..e5a158a
--- /dev/null
+++ b/src/test/rules_cfgs/explicitly-not-disabled-node-resource-affinity-rules.cfg
@@ -0,0 +1,88 @@
+# Case 1: Do not remove a positive resource affinity rule, where there is exactly one node to keep them together.
+node-affinity: vm101-vm102-must-be-on-node1
+ resources vm:101,vm:102
+ nodes node1
+ strict 1
+ disable 0
+
+resource-affinity: vm101-vm102-must-be-kept-together
+ resources vm:101,vm:102
+ affinity positive
+ disable 0
+
+# Case 2: Do not remove a negative resource affinity rule, where there are exactly enough nodes available to keep them apart.
+node-affinity: vm201-must-be-on-node1
+ resources vm:201
+ nodes node1
+ strict 1
+ disable 0
+
+node-affinity: vm202-must-be-on-node2
+ resources vm:202
+ nodes node2
+ strict 1
+ disable 0
+
+resource-affinity: vm201-vm202-must-be-kept-separate
+ resources vm:201,vm:202
+ affinity negative
+ disable 0
+
+# Case 3: Remove positive resource affinity rules, where two resources are restricted to different nodes.
+node-affinity: vm301-must-be-on-node1
+ resources vm:301
+ nodes node1
+ strict 1
+ disable 0
+
+node-affinity: vm301-must-be-on-node2
+ resources vm:302
+ nodes node2
+ strict 1
+ disable 0
+
+resource-affinity: vm301-vm302-must-be-kept-together
+ resources vm:301,vm:302
+ affinity positive
+ disable 0
+
+# Case 4: Remove negative resource affinity rules, where two resources are restricted to less nodes than needed to keep them apart.
+node-affinity: vm401-must-be-on-node1
+ resources vm:401
+ nodes node1
+ strict 1
+ disable 0
+
+node-affinity: vm402-must-be-on-node1
+ resources vm:402
+ nodes node1
+ strict 1
+ disable 0
+
+resource-affinity: vm401-vm402-must-be-kept-separate
+ resources vm:401,vm:402
+ affinity negative
+ disable 0
+
+# Case 5: Remove positive resource affinity rules, which have overlapping HA resources, and are restricted with two different node affinity rules.
+node-affinity: vm501-must-be-on-node1
+ resources vm:501
+ nodes node1
+ strict 1
+ disable 0
+
+node-affinity: vm503-must-be-on-node2
+ resources vm:503
+ nodes node2
+ strict 1
+ disable 0
+
+resource-affinity: vm501-vm502-must-be-kept-together
+ resources vm:501,vm:502
+ affinity positive
+ disable 0
+
+resource-affinity: vm502-vm503-must-be-kept-together
+ resources vm:502,vm:503
+ affinity positive
+ disable 0
diff --git a/src/test/rules_cfgs/explicitly-not-disabled-node-resource-affinity-rules.cfg.expect b/src/test/rules_cfgs/explicitly-not-disabled-node-resource-affinity-rules.cfg.expect
new file mode 100644
index 0000000..5e32e06
--- /dev/null
+++ b/src/test/rules_cfgs/explicitly-not-disabled-node-resource-affinity-rules.cfg.expect
@@ -0,0 +1,133 @@
+--- Log ---
+Drop rule 'vm301-must-be-on-node1', because at least one resource is in a positive resource affinity rule and there are other resources in at least one other node affinity rule already.
+Drop rule 'vm301-must-be-on-node2', because at least one resource is in a positive resource affinity rule and there are other resources in at least one other node affinity rule already.
+Drop rule 'vm301-vm302-must-be-kept-together', because resources are in multiple node affinity rules.
+Drop rule 'vm401-must-be-on-node1', because at least one resource is in a negative resource affinity rule and this rule would restrict these to less nodes than available to the resources.
+Drop rule 'vm401-vm402-must-be-kept-separate', because two or more resources are restricted to less nodes than available to the resources.
+Drop rule 'vm402-must-be-on-node1', because at least one resource is in a negative resource affinity rule and this rule would restrict these to less nodes than available to the resources.
+Drop rule 'vm501-must-be-on-node1', because at least one resource is in a positive resource affinity rule and there are other resources in at least one other node affinity rule already.
+Drop rule 'vm501-vm502-must-be-kept-together', because resources are in multiple node affinity rules.
+Drop rule 'vm502-vm503-must-be-kept-together', because resources are in multiple node affinity rules.
+Drop rule 'vm503-must-be-on-node2', because at least one resource is in a positive resource affinity rule and there are other resources in at least one other node affinity rule already.
+--- Config ---
+{
+ "digest" : "0c16b9fb9c5a0f7824f1bfbd13b1bd96010d7550",
+ "ids" : {
+ "vm101-vm102-must-be-kept-together" : {
+ "affinity" : "positive",
+ "disable" : 0,
+ "resources" : {
+ "vm:101" : 1,
+ "vm:102" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "vm101-vm102-must-be-on-node1" : {
+ "disable" : 0,
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ }
+ },
+ "resources" : {
+ "vm:101" : 1,
+ "vm:102" : 1
+ },
+ "strict" : 1,
+ "type" : "node-affinity"
+ },
+ "vm201-must-be-on-node1" : {
+ "disable" : 0,
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ }
+ },
+ "resources" : {
+ "vm:201" : 1
+ },
+ "strict" : 1,
+ "type" : "node-affinity"
+ },
+ "vm201-vm202-must-be-kept-separate" : {
+ "affinity" : "negative",
+ "disable" : 0,
+ "resources" : {
+ "vm:201" : 1,
+ "vm:202" : 1
+ },
+ "type" : "resource-affinity"
+ },
+ "vm202-must-be-on-node2" : {
+ "disable" : 0,
+ "nodes" : {
+ "node2" : {
+ "priority" : 0
+ }
+ },
+ "resources" : {
+ "vm:202" : 1
+ },
+ "strict" : 1,
+ "type" : "node-affinity"
+ }
+ },
+ "order" : {
+ "vm101-vm102-must-be-kept-together" : 2,
+ "vm101-vm102-must-be-on-node1" : 1,
+ "vm201-must-be-on-node1" : 3,
+ "vm201-vm202-must-be-kept-separate" : 5,
+ "vm202-must-be-on-node2" : 4
+ }
+}
+--- Compiled Config ---
+{
+ "node-affinity" : {
+ "vm:101" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ }
+ }
+ },
+ "vm:102" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ }
+ }
+ },
+ "vm:201" : {
+ "nodes" : {
+ "node1" : {
+ "priority" : 0
+ }
+ }
+ },
+ "vm:202" : {
+ "nodes" : {
+ "node2" : {
+ "priority" : 0
+ }
+ }
+ }
+ },
+ "resource-affinity" : {
+ "negative" : {
+ "vm:201" : {
+ "vm:202" : 1
+ },
+ "vm:202" : {
+ "vm:201" : 1
+ }
+ },
+ "positive" : {
+ "vm:101" : {
+ "vm:102" : 1
+ },
+ "vm:102" : {
+ "vm:101" : 1
+ }
+ }
+ }
+}
diff --git a/src/test/rules_cfgs/explicitly-not-disabled-resource-affinity-rules.cfg b/src/test/rules_cfgs/explicitly-not-disabled-resource-affinity-rules.cfg
new file mode 100644
index 0000000..88feb0b
--- /dev/null
+++ b/src/test/rules_cfgs/explicitly-not-disabled-resource-affinity-rules.cfg
@@ -0,0 +1,32 @@
+# Case 1: Remove positive and negative resource affinity rules, which share two or more ha resources.
+resource-affinity: keep-apart1
+ resources vm:102,vm:103
+ affinity negative
+ disable 0
+
+resource-affinity: keep-apart2
+ resources vm:102,vm:104,vm:106
+ affinity negative
+ disable 0
+
+resource-affinity: stick-together1
+ resources vm:101,vm:102,vm:103,vm:104,vm:106
+ affinity positive
+ disable 0
+
+# Case 2: Remove positive and negative resource affinity rules, which share two or more HA resources with the positive
+# resource affinity being split in two.
+resource-affinity: split-stick-together1
+ resources vm:201,vm:202
+ affinity positive
+ disable 0
+
+resource-affinity: split-stick-together2
+ resources vm:202,vm:203
+ affinity positive
+ disable 0
+
+resource-affinity: split-keep-apart1
+ resources vm:201,vm:203
+ affinity negative
+ disable 0
diff --git a/src/test/rules_cfgs/explicitly-not-disabled-resource-affinity-rules.cfg.expect b/src/test/rules_cfgs/explicitly-not-disabled-resource-affinity-rules.cfg.expect
new file mode 100644
index 0000000..019159c
--- /dev/null
+++ b/src/test/rules_cfgs/explicitly-not-disabled-resource-affinity-rules.cfg.expect
@@ -0,0 +1,22 @@
+--- Log ---
+Drop rule 'keep-apart1', because rule shares two or more resources with a positive resource affinity rule.
+Drop rule 'keep-apart2', because rule shares two or more resources with a positive resource affinity rule.
+Drop rule 'split-keep-apart1', because rule shares two or more resources with a positive resource affinity rule.
+Drop rule 'split-stick-together1', because rule shares two or more resources with a negative resource affinity rule.
+Drop rule 'split-stick-together2', because rule shares two or more resources with a negative resource affinity rule.
+Drop rule 'stick-together1', because rule shares two or more resources with a negative resource affinity rule.
+Drop rule 'stick-together1', because rule shares two or more resources with a negative resource affinity rule.
+--- Config ---
+{
+ "digest" : "d4de6b15d65b32e975e51766e23a4a25a950ff2e",
+ "ids" : {},
+ "order" : {}
+}
+--- Compiled Config ---
+{
+ "node-affinity" : {},
+ "resource-affinity" : {
+ "negative" : {},
+ "positive" : {}
+ }
+}
--
2.47.3
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-05-21 14:21 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-21 14:20 [PATCH manager 1/2] fix #7589: rules: allow setting false 'disable' parameter Dominik Rusovac
2026-05-21 14:20 ` [PATCH manager 2/2] test: rules_cfg: declare explicitly not disabled rules Dominik Rusovac
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.