* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox