[PATCH pve-ha-manager v3 2/2] fix #7557: introduce 'auto-rebalance' property

[Dominik Rusovac <d.rusovac@proxmox.com>]

Add 'auto-rebalance' property to HA resources config, which gives users
control over which HA resources may be moved by dynamic CRS during
automatic rebalancing.

The 'auto-rebalance' flag is set to true by default. Disabling
'auto-rebalance' for some HA resource, say vm:100, means that vm:100
will be disregarded as a migration candidate during auto-rebalancing.
Any HA resource with a positive affinity for vm:100 will be disregarded
too.

Tests validate that an entire resource bundle will be disregarded if any
resource belonging to the bundle has 'auto-rebalance' disabled.

Signed-off-by: Dominik Rusovac <d.rusovac@proxmox.com>
---

Notes:
    no changes since v2

 src/PVE/API2/HA/Resources.pm      |   6 ++
 src/PVE/API2/HA/Status.pm         |  11 ++-
 src/PVE/HA/Config.pm              |   2 +
 src/PVE/HA/Manager.pm             |  17 ++--
 src/PVE/HA/Resources.pm           |   6 ++
 src/PVE/HA/Resources/PVECT.pm     |   1 +
 src/PVE/HA/Resources/PVEVM.pm     |   1 +
 src/PVE/HA/Sim/Hardware.pm        |   1 +
 src/test/test_resource_bundles.pl | 142 ++++++++++++++++++++++++++++--
 9 files changed, 175 insertions(+), 12 deletions(-)

diff --git a/src/PVE/API2/HA/Resources.pm b/src/PVE/API2/HA/Resources.pm
index 3f973c4..b96572b 100644
--- a/src/PVE/API2/HA/Resources.pm
+++ b/src/PVE/API2/HA/Resources.pm
@@ -142,6 +142,12 @@ __PACKAGE__->register_method({
                 optional => 1,
                 default => 1,
             },
+            'auto-rebalance' => {
+                description => "HA resource may be migrated during automatic rebalancing.",
+                type => 'boolean',
+                optional => 1,
+                default => 1,
+            },
             group => get_standard_option('pve-ha-group-id', { optional => 1 }),
             max_restart => {
                 description => "Maximal number of tries to restart the service on"
diff --git a/src/PVE/API2/HA/Status.pm b/src/PVE/API2/HA/Status.pm
index 5d0e572..095b2c7 100644
--- a/src/PVE/API2/HA/Status.pm
+++ b/src/PVE/API2/HA/Status.pm
@@ -121,6 +121,12 @@ __PACKAGE__->register_method({
                     optional => 1,
                     default => 1,
                 },
+                'auto-rebalance' => {
+                    description => "HA resource may be migrated during automatic rebalancing.",
+                    type => 'boolean',
+                    optional => 1,
+                    default => 1,
+                },
                 max_relocate => {
                     description => "For type 'service'.",
                     type => "integer",
@@ -340,7 +346,10 @@ __PACKAGE__->register_method({
             # also return common resource attributes
             if (defined($sc)) {
                 $data->{request_state} = $sc->{state};
-                foreach my $key (qw(group max_restart max_relocate failback comment)) {
+
+                my @exported_service_properties =
+                    qw(group max_restart max_relocate failback comment auto-rebalance);
+                for my $key (@exported_service_properties) {
                     $data->{$key} = $sc->{$key} if defined($sc->{$key});
                 }
             }
diff --git a/src/PVE/HA/Config.pm b/src/PVE/HA/Config.pm
index eb1f9b7..bee1ac7 100644
--- a/src/PVE/HA/Config.pm
+++ b/src/PVE/HA/Config.pm
@@ -118,8 +118,10 @@ my sub checked_resources_config {
         $d->{state} = 'started' if !defined($d->{state});
         $d->{state} = 'started' if $d->{state} eq 'enabled'; # backward compatibility
         $d->{failback} = 1 if !defined($d->{failback});
+        $d->{'auto-rebalance'} = 1 if !defined($d->{'auto-rebalance'});
         $d->{max_restart} = 1 if !defined($d->{max_restart});
         $d->{max_relocate} = 1 if !defined($d->{max_relocate});
+
         if (PVE::HA::Resources->lookup($d->{type})) {
             if (my $vmd = $vmlist->{ids}->{$name}) {
                 $d->{node} = $vmd->{node};
diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index 46a5bf3..57bb4e8 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -131,20 +131,22 @@ sub update_crs_scheduler_mode {
     return;
 }
 
-# Returns a hash of lists, which contain the running, non-moving HA resource
+# Returns a hash of lists, which contain the running, movable, non-moving HA resource
 # bundles, which are on the same node, implied by the strict positive resource
 # affinity rules.
 #
 # Each resource bundle has a leader, which is the alphabetically first running
 # HA resource in the resource bundle and also the key of each resource bundle
 # in the returned hash.
-sub get_active_stationary_resource_bundles {
-    my ($ss, $resource_affinity) = @_;
+sub get_active_stationary_movable_resource_bundles {
+    my ($ss, $sc, $resource_affinity) = @_;
 
     my $resource_bundles = {};
 OUTER: for my $sid (sort keys %$ss) {
         # do not consider non-started resource as 'active' leading resource
         next if $ss->{$sid}->{state} ne 'started';
+        # do not consider resource if it is not movable
+        next if !$sc->{$sid}->{'auto-rebalance'};
 
         my @resources = ($sid);
         my $nodes = { $ss->{$sid}->{node} => 1 };
@@ -159,6 +161,8 @@ OUTER: for my $sid (sort keys %$ss) {
                 next OUTER if $state eq 'migrate' || $state eq 'relocate';
                 # do not add non-started resource to active bundle
                 next if $state ne 'started';
+                # do not consider stationary bundle if a dependent resource is not movable
+                next OUTER if !$sc->{$csid}->{'auto-rebalance'};
 
                 $nodes->{$node} = 1;
 
@@ -185,12 +189,13 @@ OUTER: for my $sid (sort keys %$ss) {
 sub get_resource_migration_candidates {
     my ($self) = @_;
 
-    my ($ss, $compiled_rules, $online_node_usage) =
-        $self->@{qw(ss compiled_rules online_node_usage)};
+    my ($ss, $sc, $compiled_rules, $online_node_usage) =
+        $self->@{qw(ss sc compiled_rules online_node_usage)};
     my ($node_affinity, $resource_affinity) =
         $compiled_rules->@{qw(node-affinity resource-affinity)};
 
-    my $resource_bundles = get_active_stationary_resource_bundles($ss, $resource_affinity);
+    my $resource_bundles =
+        get_active_stationary_movable_resource_bundles($ss, $sc, $resource_affinity);
 
     my @compact_migration_candidates = ();
     for my $leader_sid (sort keys %$resource_bundles) {
diff --git a/src/PVE/HA/Resources.pm b/src/PVE/HA/Resources.pm
index 4238d9b..df7c1ff 100644
--- a/src/PVE/HA/Resources.pm
+++ b/src/PVE/HA/Resources.pm
@@ -71,6 +71,12 @@ EODESC
             optional => 1,
             default => 1,
         },
+        'auto-rebalance' => {
+            description => "HA resource may be migrated during automatic rebalancing",
+            type => 'boolean',
+            optional => 1,
+            default => 1,
+        },
         max_restart => {
             description => "Maximal number of tries to restart the resource on"
                 . " a node after its start failed. When reached, the HA manager will try to"
diff --git a/src/PVE/HA/Resources/PVECT.pm b/src/PVE/HA/Resources/PVECT.pm
index 0943d5e..177b907 100644
--- a/src/PVE/HA/Resources/PVECT.pm
+++ b/src/PVE/HA/Resources/PVECT.pm
@@ -38,6 +38,7 @@ sub options {
         group => { optional => 1 },
         comment => { optional => 1 },
         failback => { optional => 1 },
+        'auto-rebalance' => { optional => 1 },
         max_restart => { optional => 1 },
         max_relocate => { optional => 1 },
     };
diff --git a/src/PVE/HA/Resources/PVEVM.pm b/src/PVE/HA/Resources/PVEVM.pm
index dceb08e..8753271 100644
--- a/src/PVE/HA/Resources/PVEVM.pm
+++ b/src/PVE/HA/Resources/PVEVM.pm
@@ -38,6 +38,7 @@ sub options {
         group => { optional => 1 },
         comment => { optional => 1 },
         failback => { optional => 1 },
+        'auto-rebalance' => { optional => 1 },
         max_restart => { optional => 1 },
         max_relocate => { optional => 1 },
     };
diff --git a/src/PVE/HA/Sim/Hardware.pm b/src/PVE/HA/Sim/Hardware.pm
index 4ebe2ad..e1a9ddd 100644
--- a/src/PVE/HA/Sim/Hardware.pm
+++ b/src/PVE/HA/Sim/Hardware.pm
@@ -115,6 +115,7 @@ sub read_service_config {
         $d->{state} = 'disabled' if !$d->{state};
         $d->{state} = 'started' if $d->{state} eq 'enabled'; # backward compatibility
         $d->{failback} = 1 if !defined($d->{failback});
+        $d->{'auto-rebalance'} = 1 if !defined($d->{'auto-rebalance'});
         $d->{max_restart} = 1 if !defined($d->{max_restart});
         $d->{max_relocate} = 1 if !defined($d->{max_relocate});
     }
diff --git a/src/test/test_resource_bundles.pl b/src/test/test_resource_bundles.pl
index d38dc51..bbfdbab 100755
--- a/src/test/test_resource_bundles.pl
+++ b/src/test/test_resource_bundles.pl
@@ -8,7 +8,7 @@ use Test::More;
 
 use PVE::HA::Manager;
 
-my $get_active_stationary_resource_bundle_tests = [
+my $get_active_stationary_movable_resource_bundle_tests = [
     {
         description => "trivial resource bundles",
         services => {
@@ -21,6 +21,10 @@ my $get_active_stationary_resource_bundle_tests = [
                 node => 'node1',
             },
         },
+        service_config => {
+            'vm:101' => { 'auto-rebalance' => 1 },
+            'vm:102' => { 'auto-rebalance' => 1 },
+        },
         resource_affinity => {
             positive => {},
             negative => {},
@@ -46,6 +50,10 @@ my $get_active_stationary_resource_bundle_tests = [
                 node => 'node1',
             },
         },
+        service_config => {
+            'vm:101' => { 'auto-rebalance' => 1 },
+            'vm:102' => { 'auto-rebalance' => 1 },
+        },
         resource_affinity => {
             positive => {
                 'vm:101' => {
@@ -79,6 +87,11 @@ my $get_active_stationary_resource_bundle_tests = [
                 node => 'node1',
             },
         },
+        service_config => {
+            'vm:101' => { 'auto-rebalance' => 1 },
+            'vm:102' => { 'auto-rebalance' => 1 },
+            'vm:103' => { 'auto-rebalance' => 1 },
+        },
         resource_affinity => {
             positive => {
                 'vm:101' => {
@@ -118,6 +131,11 @@ my $get_active_stationary_resource_bundle_tests = [
                 node => 'node1',
             },
         },
+        service_config => {
+            'vm:101' => { 'auto-rebalance' => 1 },
+            'vm:102' => { 'auto-rebalance' => 1 },
+            'vm:103' => { 'auto-rebalance' => 1 },
+        },
         resource_affinity => {
             positive => {
                 'vm:101' => {
@@ -159,6 +177,11 @@ my $get_active_stationary_resource_bundle_tests = [
                 target => 'node1',
             },
         },
+        service_config => {
+            'vm:101' => { 'auto-rebalance' => 1 },
+            'vm:102' => { 'auto-rebalance' => 1 },
+            'vm:103' => { 'auto-rebalance' => 1 },
+        },
         resource_affinity => {
             positive => {
                 'vm:101' => {
@@ -196,6 +219,11 @@ my $get_active_stationary_resource_bundle_tests = [
                 node => 'node3',
             },
         },
+        service_config => {
+            'vm:101' => { 'auto-rebalance' => 1 },
+            'vm:102' => { 'auto-rebalance' => 1 },
+            'vm:103' => { 'auto-rebalance' => 1 },
+        },
         resource_affinity => {
             positive => {
                 'vm:101' => {
@@ -215,18 +243,122 @@ my $get_active_stationary_resource_bundle_tests = [
         },
         resource_bundles => {},
     },
+    {
+        description => "singleton resource bundle with disabled auto-rebalance",
+        services => {
+            'vm:101' => {
+                state => 'started',
+                node => 'node1',
+            },
+            'vm:102' => {
+                state => 'started',
+                node => 'node1',
+            },
+        },
+        service_config => {
+            'vm:101' => { 'auto-rebalance' => 0 },
+            'vm:102' => { 'auto-rebalance' => 1 },
+        },
+        resource_affinity => {
+            positive => {},
+            negative => {},
+        },
+        resource_bundles => {
+            'vm:102' => [
+                'vm:102',
+            ],
+        },
+    },
+    {
+        description => "resource bundle leader with disabled auto-rebalance",
+        services => {
+            'vm:101' => {
+                state => 'started',
+                node => 'node1',
+            },
+            'vm:102' => {
+                state => 'started',
+                node => 'node1',
+            },
+            'ct:103' => {
+                state => 'started',
+                node => 'node2',
+            },
+        },
+        service_config => {
+            'vm:101' => { 'auto-rebalance' => 0 },
+            'vm:102' => { 'auto-rebalance' => 1 },
+            'ct:103' => { 'auto-rebalance' => 1 },
+        },
+        resource_affinity => {
+            positive => {
+                'vm:101' => {
+                    'vm:102' => 1,
+                },
+                'vm:102' => {
+                    'vm:101' => 1,
+                },
+            },
+            negative => {},
+        },
+        resource_bundles => {
+            'ct:103' => [
+                'ct:103',
+            ],
+        },
+    },
+    {
+        description => "some member of resource bundle with disabled auto-rebalance",
+        services => {
+            'vm:101' => {
+                state => 'started',
+                node => 'node1',
+            },
+            'vm:102' => {
+                state => 'started',
+                node => 'node1',
+            },
+            'ct:103' => {
+                state => 'started',
+                node => 'node2',
+            },
+        },
+        service_config => {
+            'vm:101' => { 'auto-rebalance' => 1 },
+            'vm:102' => { 'auto-rebalance' => 0 },
+            'ct:103' => { 'auto-rebalance' => 1 },
+        },
+        resource_affinity => {
+            positive => {
+                'vm:101' => {
+                    'vm:102' => 1,
+                },
+                'vm:102' => {
+                    'vm:101' => 1,
+                },
+            },
+            negative => {},
+        },
+        resource_bundles => {
+            'ct:103' => [
+                'ct:103',
+            ],
+        },
+    },
 ];
 
 my $tests = [
-    @$get_active_stationary_resource_bundle_tests,
+    @$get_active_stationary_movable_resource_bundle_tests,
 ];
 
 plan(tests => scalar($tests->@*));
 
-for my $case ($get_active_stationary_resource_bundle_tests->@*) {
-    my ($ss, $resource_affinity) = $case->@{qw(services resource_affinity)};
+for my $case ($get_active_stationary_movable_resource_bundle_tests->@*) {
+    my ($ss, $sc, $resource_affinity) = $case->@{qw(services service_config resource_affinity)};
 
-    my $result = PVE::HA::Manager::get_active_stationary_resource_bundles($ss, $resource_affinity);
+    my $result = PVE::HA::Manager::get_active_stationary_movable_resource_bundles(
+        $ss, $sc, $resource_affinity,
+    );
 
     is_deeply($result, $case->{resource_bundles}, $case->{description});
 }
-- 
2.47.3