Re: [RFC ha-manager 16/21] sim: hardware: add set-dynamic-stats for services

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 17.02.26 um 15:17 schrieb Daniel Kral:
> From: Dominik Rusovac <d.rusovac@proxmox.com>
> 
> This adds command set-dynamic-stats, to simulate the cpu load (cpu) and
> memory usage (mem in MiB) of a service, as well as command
> set-static-stats, to configure the number of cores (maxcpu) and RAM
> (maxmem in MiB) of a service. In addition to using the designated
> command, dynamic service stats can be specified beforehand in file
> dynamic_service_stats.
> 
> Upon calling set-dynamic-stats on some service, the dynamic stats of the
> node this very service is running on will be aggregated accordingly.
> 
> Signed-off-by: Dominik Rusovac <d.rusovac@proxmox.com>
> Signed-off-by: Daniel Kral <d.kral@proxmox.com>
> ---
>  src/PVE/HA/Sim/Hardware.pm | 130 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 130 insertions(+)
> 
> diff --git a/src/PVE/HA/Sim/Hardware.pm b/src/PVE/HA/Sim/Hardware.pm
> index ec135e09..910f9718 100644

> @@ -744,6 +796,7 @@ sub get_cfs_state {
>  #   service <sid> lock/unlock [lockname]
>  #   service <sid> add <node> [<request-state=started>] [<running=0>]
>  #   service <sid> set-static-stats  <maxcpu|maxmem> <cpu cores|MiB>
> +#   service <sid> set-dynamic-stats <cpu|mem>       <load in cpu cores|usage in MiB>
>  #   service <sid> delete
>  sub sim_hardware_cmd {
>      my ($self, $cmdstr, $logid) = @_;
> @@ -911,6 +964,24 @@ sub sim_hardware_cmd {
>                  } else {
>                      die "sim_hardware_cmd: unknown target stat '$target' for '$action' command";
>                  }
> +            } elsif ($action eq 'set-dynamic-stats') {
> +                my ($target, $val) = ($params[0], $params[1]);
> +
> +                if (!$target) {
> +                    die "sim_hardware_cmd: missing target stat for '$action' command";
> +                } elsif ($target eq "cpu") {
> +                    die "sim_hardware_cmd: missing value for '$action $target' command"
> +                        if !$val;
> +
> +                    $self->set_dynamic_service_stats($sid, { $target => 0.0 + $val });
> +                } elsif ($target eq "mem") {
> +                    die "sim_hardware_cmd: missing value for '$action $target' command"
> +                        if !$val;
> +
> +                    $self->set_dynamic_service_stats($sid, { $target => $val * 1024**2 });
> +                } else {
> +                    die "sim_hardware_cmd: unknown target stat '$target' for '$action' command";
> +                }
>              } elsif ($action eq 'delete') {
>  
>                  $self->delete_service($sid);
> @@ -1135,6 +1206,27 @@ sub get_static_service_stats {

FWIW, we could avoid some code duplication between dynamic and static set
and reduce the test cmd's needed by applying something like below on top at
this point.

The change below basically leverage's the rather fresh perl 5.40's "for loop
with multiple iteration variables" [0] (which is fine to use for PVE 9, we
won't backport this to PVE 8 - where the perl version would be to old - anyway),
and merging the two branches for set-static-stats and set-dynamic-stats commands
by using a hash for mapping the conversions of the different stats/cmds (could
be written maybe even nicer).

Then such chains like:

info    220      cmdlist: execute service vm:103 set-dynamic-stats cpu 3.9 
info    220      cmdlist: execute service vm:103 set-dynamic-stats mem 6517
info    220      cmdlist: execute service vm:104 set-dynamic-stats cpu 6.7 
info    220      cmdlist: execute service vm:104 set-dynamic-stats mem 8001
info    220      cmdlist: execute service vm:105 set-dynamic-stats cpu 1.8 
info    220      cmdlist: execute service vm:105 set-dynamic-stats mem 1201
info    220      cmdlist: execute service vm:106 set-dynamic-stats cpu 2.1 
info    220      cmdlist: execute service vm:106 set-dynamic-stats mem 1211
info    220      cmdlist: execute service vm:107 set-dynamic-stats cpu 0.9 
info    220      cmdlist: execute service vm:107 set-dynamic-stats mem 1191

could become:

info    220      cmdlist: execute service vm:103 set-dynamic-stats cpu 3.9 mem 6517
info    220      cmdlist: execute service vm:104 set-dynamic-stats cpu 6.7 mem 8001
info    220      cmdlist: execute service vm:105 set-dynamic-stats cpu 1.8 mem 1201
info    220      cmdlist: execute service vm:106 set-dynamic-stats cpu 2.1 mem 1211
info    220      cmdlist: execute service vm:107 set-dynamic-stats cpu 0.9 mem 1191

I.e., get basically cut in half without really loosing expressiveness (IMO).

[0]: https://perldoc.perl.org/perlexperiment#for-loop-with-multiple-iteration-variables

----8<----
diff --git a/src/PVE/HA/Sim/Hardware.pm b/src/PVE/HA/Sim/Hardware.pm
index dc95593..e0a1cce 100644
--- a/src/PVE/HA/Sim/Hardware.pm
+++ b/src/PVE/HA/Sim/Hardware.pm
@@ -795,8 +795,8 @@ sub get_cfs_state {
 #   service <sid> stop <timeout>
 #   service <sid> lock/unlock [lockname]
 #   service <sid> add <node> [<request-state=started>] [<running=0>]
-#   service <sid> set-static-stats  <maxcpu|maxmem> <cpu cores|MiB>
-#   service <sid> set-dynamic-stats <cpu|mem>       <load in cpu cores|usage in MiB>
+#   service <sid> set-static-stats  [maxcpu <cores>] [maxmem <MiB>]
+#   service <sid> set-dynamic-stats [cpu <cores>] [mem <MiB>]
 #   service <sid> delete
 sub sim_hardware_cmd {
     my ($self, $cmdstr, $logid) = @_;
@@ -946,41 +946,29 @@ sub sim_hardware_cmd {
                     $params[2] || 0,
                 );
 
-            } elsif ($action eq 'set-static-stats') {
-                my ($target, $val) = ($params[0], $params[1]);
+            } elsif ($action eq 'set-static-stats' || $action eq 'set-dynamic-stats') {
+                die "sim_hardware_cmd: missing target stat for '$action' command"
+                    if !@params;
 
-                if (!$target) {
-                    die "sim_hardware_cmd: missing target stat for '$action' command";
-                } elsif ($target eq "maxcpu") {
-                    die "sim_hardware_cmd: missing value for '$action $target' command"
-                        if !$val;
+                my $conversions = $action eq 'set-static-stats'
+                    ? { maxcpu => sub { 0.0 + $_[0] }, maxmem => sub { $_[0] * 1024**2 } }
+                    : { cpu => sub { 0.0 + $_[0] }, mem => sub { $_[0] * 1024**2 } };
 
-                    $self->set_static_service_stats($sid, { $target => 0.0 + $val });
-                } elsif ($target eq "maxmem") {
+                my %new_stats;
+                for my ($target, $val) (@params) {
                     die "sim_hardware_cmd: missing value for '$action $target' command"
-                        if !$val;
+                        if !defined($val);
+
+                    my $convert = $conversions->{$target}
+                        or die "sim_hardware_cmd: unknown target stat '$target' for '$action' command";
 
-                    $self->set_static_service_stats($sid, { $target => $val * 1024**2 });
-                } else {
-                    die "sim_hardware_cmd: unknown target stat '$target' for '$action' command";
+                    $new_stats{$target} = $convert->($val);
                 }
-            } elsif ($action eq 'set-dynamic-stats') {
-                my ($target, $val) = ($params[0], $params[1]);
 
-                if (!$target) {
-                    die "sim_hardware_cmd: missing target stat for '$action' command";
-                } elsif ($target eq "cpu") {
-                    die "sim_hardware_cmd: missing value for '$action $target' command"
-                        if !$val;
-
-                    $self->set_dynamic_service_stats($sid, { $target => 0.0 + $val });
-                } elsif ($target eq "mem") {
-                    die "sim_hardware_cmd: missing value for '$action $target' command"
-                        if !$val;
-
-                    $self->set_dynamic_service_stats($sid, { $target => $val * 1024**2 });
+                if ($action eq 'set-static-stats') {
+                    $self->set_static_service_stats($sid, \%new_stats);
                 } else {
-                    die "sim_hardware_cmd: unknown target stat '$target' for '$action' command";
+                    $self->set_dynamic_service_stats($sid, \%new_stats);
                 }
             } elsif ($action eq 'delete') {