Just noted a couple of typos inline
On 6/20/25 16:31, Daniel Kral wrote:
> Add a rules base plugin to allow users to specify different kinds of HA
> rules in a single configuration file, which put constraints on the HA
> Manager's behavior.
>
> Rule checkers can be registered for every plugin with the
> register_check(...) method and are used for checking the feasibility of
> the rule set with check_feasibility(...), and creating a feasible rule
> set with canonicalize(...).
>
> Signed-off-by: Daniel Kral <d.k...@proxmox.com>
> ---
> changes since v1:
> - added documentation
> - added state property to enable/disable rules (and allow a
> tri-state 'contradictory' state for the API endpoint there)
> - replace `{encode,decode}_value` plugin calls to
> `{encode,decode}_plugin_value` with default implementation to make
> these implementations optional for rule plugins
> - introduce `set_rule_defaults` to set optional fields with defaults
> in later patches
> - renamed `are_satisfiable` to `check_feasibility`
> - factored `canonicalize` and `checked_config` to only
> `canonicalize`
> - refactored the feasibility checks to be registered as individual
> checkers with `register_check`
> - refactored the canonicalization for plugins to be a single call to
> plugin_canonicalize (if implemented)
> - added rule auto completion
>
> debian/pve-ha-manager.install | 1 +
> src/PVE/HA/Makefile | 2 +-
> src/PVE/HA/Rules.pm | 445 ++++++++++++++++++++++++++++++++++
> src/PVE/HA/Tools.pm | 22 ++
> 4 files changed, 469 insertions(+), 1 deletion(-)
> create mode 100644 src/PVE/HA/Rules.pm
>
> diff --git a/debian/pve-ha-manager.install b/debian/pve-ha-manager.install
> index 0ffbd8d..9bbd375 100644
> --- a/debian/pve-ha-manager.install
> +++ b/debian/pve-ha-manager.install
> @@ -32,6 +32,7 @@
> /usr/share/perl5/PVE/HA/Resources.pm
> /usr/share/perl5/PVE/HA/Resources/PVECT.pm
> /usr/share/perl5/PVE/HA/Resources/PVEVM.pm
> +/usr/share/perl5/PVE/HA/Rules.pm
> /usr/share/perl5/PVE/HA/Tools.pm
> /usr/share/perl5/PVE/HA/Usage.pm
> /usr/share/perl5/PVE/HA/Usage/Basic.pm
> diff --git a/src/PVE/HA/Makefile b/src/PVE/HA/Makefile
> index 8c91b97..489cbc0 100644
> --- a/src/PVE/HA/Makefile
> +++ b/src/PVE/HA/Makefile
> @@ -1,4 +1,4 @@
> -SIM_SOURCES=CRM.pm Env.pm Groups.pm Resources.pm LRM.pm Manager.pm \
> +SIM_SOURCES=CRM.pm Env.pm Groups.pm Rules.pm Resources.pm LRM.pm Manager.pm \
> NodeStatus.pm Tools.pm FenceConfig.pm Fence.pm Usage.pm
>
> SOURCES=${SIM_SOURCES} Config.pm
> diff --git a/src/PVE/HA/Rules.pm b/src/PVE/HA/Rules.pm
> new file mode 100644
> index 0000000..e1c84bc
> --- /dev/null
> +++ b/src/PVE/HA/Rules.pm
> @@ -0,0 +1,445 @@
> +package PVE::HA::Rules;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::JSONSchema qw(get_standard_option);
> +use PVE::Tools;
> +
> +use PVE::HA::Tools;
> +
> +use base qw(PVE::SectionConfig);
> +
> +=head1 NAME
> +
> +PVE::HA::Rules - Base Plugin for HA Rules
> +
> +=head1 SYNOPSIS
> +
> + use base qw(PVE::HA::Rules);
> +
> +=head1 DESCRIPTION
> +
> +This package provides the capability to have different types of rules in the
> +same config file, which put constraints or other rules that the HA Manager
> must
> +or should follow.
> +
> +Since rules can interfere with each other, i.e., rules can make other rules
> +invalid or infeasible, this package also provides the capability to check for
> +the feasibility of the rules within a rule plugin and between different rule
> +plugins and also prune the rule set in such a way, that becomes feasible
> again.
> +
> +This packages inherits its config-related methods from
> C<L<PVE::SectionConfig>>
> +and therefore rule plugins need to implement methods from there as well.
> +
> +=head1 USAGE
> +
> +Each I<rule plugin> is required to implement the methods C<L<type()>>,
> +C<L<properties()>>, and C<L<options>> from the C<L<PVE::SectionConfig>> to
> +extend the properties of this I<base plugin> with plugin-specific properties.
> +
> +=head2 REGISTERING CHECKS
> +
> +In order to C<L<< register|/$class->register_check(...) >>> checks for a rule
> +plugin, the plugin can override the
> +C<L<< get_plugin_check_arguments()|/$class->get_plugin_check_arguments(...)
> >>>
> +method, which allows the plugin's checkers to pass plugin-specific subsets of
> +specific rules, which are relevant to the checks.
> +
> +The following example shows a plugin's implementation of its
> +C<L<< get_plugin_check_arguments()|/$class->get_plugin_check_arguments(...)
> >>>
> +and a trivial check, which will render all rules defining a comment
> erroneous,
> +and blames these errors on the I<comment> property:
> +
> + sub get_plugin_check_arguments {
> + my ($class, $rules) = @_;
> +
> + my @ruleids = sort {
> + $rules->{order}->{$a} <=> $rules->{order}->{$b}
> + } keys %{$rules->{ids}};
> +
> + my $result = {
> + custom_rules => {},
> + };
> +
> + for my $ruleid (@ruleids) {
> + my $rule = $rules->{ids}->{$ruleid};
> +
> + $result->{custom_rules}->{$ruleid} = $rule if
> defined($rule->{comment});
> + }
> +
> + return $result;
> + }
> +
> + __PACKAGE__->register_check(
> + sub {
> + my ($args) = @_;
> +
> + return [ sort keys $args->{custom_rules}->%* ];
> + },
> + sub {
> + my ($ruleids, $errors) = @_;
> +
> + for my $ruleid (@$ruleids) {
> + push @{$errors->{$ruleid}->{comment}},
> + "rule is ineffective, because I said so.";
> + }
> + }
> + );
> +
> +=head1 METHODS
> +
> +=cut
> +
> +my $defaultData = {
> + propertyList => {
> + type => {
> + description => "HA rule type.",
> + },
> + rule => get_standard_option(
> + 'pve-ha-rule-id',
> + {
> + completion => \&PVE::HA::Tools::complete_rule,
> + optional => 0,
> + },
> + ),
> + state => {
> + description => "State of the HA rule.",
> + type => 'string',
> + enum => [
> + 'enabled', 'disabled',
> + ],
> + default => 'enabled',
> + optional => 1,
> + },
> + comment => {
> + description => "HA rule description.",
> + type => 'string',
> + maxLength => 4096,
> + optional => 1,
> + },
> + },
> +};
> +
> +sub private {
> + return $defaultData;
> +}
> +
> +=head3 $class->decode_plugin_value(...)
> +
> +=head3 $class->decode_plugin_value($type, $key, $value)
> +
> +B<OPTIONAL:> Can be implemented in a I<rule plugin>.
> +
> +Called during base plugin's C<decode_value()> in order to extend the
> +deserialization for plugin-specific values which need it (e.g. lists).
> +
> +If it is not overrridden by the I<rule plugin>, then it does nothing to
> +C<$value> by default.
> +
> +=cut
> +
> +sub decode_plugin_value {
> + my ($class, $type, $key, $value) = @_;
> +
> + return $value;
> +}
> +
> +sub decode_value {
> + my ($class, $type, $key, $value) = @_;
> +
> + if ($key eq 'comment') {
> + return PVE::Tools::decode_text($value);
> + }
> +
> + my $plugin = $class->lookup($type);
> + return $plugin->decode_plugin_value($type, $key, $value);
> +}
> +
> +=head3 $class->encode_plugin_value(...)
> +
> +=head3 $class->encode_plugin_value($type, $key, $value)
> +
> +B<OPTIONAL:> Can be implemented in a I<rule plugin>.
> +
> +Called during base plugin's C<encode_value()> in order to extend the
> +serialization for plugin-specific values which need it (e.g. lists).
> +
> +If it is not overrridden by the I<rule plugin>, then it does nothing to
nit: there's an extra r
> +C<$value> by default.
> +
> +=cut
> +
> +sub encode_plugin_value {
> + my ($class, $type, $key, $value) = @_;
> +
> + return $value;
> +}
> +
> +sub encode_value {
> + my ($class, $type, $key, $value) = @_;
> +
> + if ($key eq 'comment') {
> + return PVE::Tools::encode_text($value);
> + }
> +
> + my $plugin = $class->lookup($type);
> + return $plugin->encode_plugin_value($type, $key, $value);
> +}
> +
> +sub parse_section_header {
> + my ($class, $line) = @_;
> +
> + if ($line =~ m/^(\S+):\s*(\S+)\s*$/) {
> + my ($type, $ruleid) = (lc($1), $2);
> + my $errmsg = undef; # set if you want to skip whole section
> + eval { PVE::JSONSchema::pve_verify_configid($ruleid); };
> + $errmsg = $@ if $@;
> + my $config = {}; # to return additional attributes
> + return ($type, $ruleid, $errmsg, $config);
> + }
> + return undef;
> +}
> +
> +# General rule helpers
> +
> +=head3 $class->set_rule_defaults($rule)
> +
> +Sets the optional properties in the C<$rule>, which have default values, but
> +haven't been explicitly set yet.
> +
> +=cut
> +
> +sub set_rule_defaults : prototype($$) {
> + my ($class, $rule) = @_;
> +
> + $rule->{state} = 'enabled' if !defined($rule->{state});
> +
> + if (my $plugin = $class->lookup($rule->{type})) {
> + my $properties = $plugin->properties();
> +
> + for my $prop (keys %$properties) {
> + next if defined($rule->{$prop});
> + next if !$properties->{$prop}->{default};
> + next if !$properties->{$prop}->{optional};
> +
> + $rule->{$prop} = $properties->{$prop}->{default};
> + }
> + }
> +}
> +
> +# Rule checks definition and methods
> +
> +my $types = [];
> +my $checkdef;
> +
> +sub register {
> + my ($class) = @_;
> +
> + $class->SUPER::register($class);
> +
> + # store order in which plugin types are registered
> + push @$types, $class->type();
> +}
> +
> +=head3 $class->register_check(...)
> +
> +=head3 $class->register_check($check_func, $collect_errors_func)
> +
> +Used to register rule checks for a rule plugin.
> +
> +=cut
> +
> +sub register_check : prototype($$$) {
> + my ($class, $check_func, $collect_errors_func) = @_;
> +
> + my $type = eval { $class->type() };
> + $type = 'global' if $@; # check registered here in the base plugin
> +
> + push @{ $checkdef->{$type} }, [
> + $check_func, $collect_errors_func,
> + ];
> +}
> +
> +=head3 $class->get_plugin_check_arguments(...)
> +
> +=head3 $class->get_plugin_check_arguments($rules)
> +
> +B<OPTIONAL:> Can be implemented in the I<rule plugin>.
> +
> +Returns a hash of subsets of rules, which are passed to the plugin's
> +C<L<< registered checks|/$class->register_check(...) >>> so that the
> +creation of these can be shared inbetween rule check implementations.
> +
> +=cut
> +
> +sub get_plugin_check_arguments : prototype($$) {
> + my ($class, $rules) = @_;
> +
> + return {};
> +}
> +
> +=head3 $class->get_check_arguments(...)
> +
> +=head3 $class->get_check_arguments($rules)
> +
> +Returns the union of the plugin's subsets of rules, which are passed to the
> +plugin's C<L<< registered checks|/$class->register_check(...) >>> so that the
> +creation of these can be shared inbetween rule check implementations.
> +
> +=cut
> +
> +sub get_check_arguments : prototype($$) {
> + my ($class, $rules) = @_;
> +
> + my $global_args = {};
> +
> + for my $type (@$types) {
> + my $plugin = $class->lookup($type);
> + my $plugin_args = eval { $plugin->get_plugin_check_arguments($rules)
> };
> + next if $@; # plugin doesn't implement
> get_plugin_check_arguments(...)
> +
> + $global_args = { $global_args->%*, $plugin_args->%* };
> + }
> +
> + return $global_args;
> +}
> +
> +=head3 $class->check_feasibility($rules)
> +
> +Checks whether the given C<$rules> are feasible by running all checks, which
> +were registered with C<L<< register_check()|/$class->register_check(...) >>>,
> +and returns a hash map of errorneous rules.
nit: s/errorneous/erroneous
> +
> +The checks are run in the order in which the rule plugins were registered,
> +while global checks, i.e. checks between different rule types, are run at the
> +very last.
> +
> +=cut
> +
> +sub check_feasibility : prototype($$) {
> + my ($class, $rules) = @_;
> +
> + my $global_errors = {};
> + my $removable_ruleids = [];
> +
> + my $global_args = $class->get_check_arguments($rules);
> +
> + for my $type (@$types, 'global') {
> + for my $entry (@{ $checkdef->{$type} }) {
> + my ($check, $collect_errors) = @$entry;
> +
> + my $errors = $check->($global_args);
> + $collect_errors->($errors, $global_errors);
> + }
> + }
> +
> + return $global_errors;
> +}
> +
> +=head3 $class->plugin_canonicalize($rules)
> +
> +B<OPTIONAL:> Can be implemented in the I<rule plugin>.
> +
> +Modifies the C<$rules> to a plugin-specific canonical form.
> +
> +=cut
> +
> +sub plugin_canonicalize : prototype($$) {
> + my ($class, $rules) = @_;
> +}
> +
> +=head3 $class->canonicalize($rules)
> +
> +Modifies C<$rules> to contain only feasible rules.
> +
> +This is done by running all checks, which were registered with
> +C<L<< register_check()|/$class->register_check(...) >>> and removing any
> +rule, which makes the rule set infeasible.
> +
> +Returns a list of messages with the reasons why rules were removed.
> +
> +=cut
> +
> +sub canonicalize : prototype($$) {
> + my ($class, $rules) = @_;
> +
> + my $messages = [];
> + my $global_errors = $class->check_feasibility($rules);
> +
> + for my $ruleid (keys %$global_errors) {
> + delete $rules->{ids}->{$ruleid};
> + delete $rules->{order}->{$ruleid};
> + }
> +
> + for my $ruleid (sort keys %$global_errors) {
> + for my $opt (sort keys %{ $global_errors->{$ruleid} }) {
> + for my $message (@{ $global_errors->{$ruleid}->{$opt} }) {
> + push @$messages, "Drop rule '$ruleid', because $message.\n";
> + }
> + }
> + }
> +
> + for my $type (@$types) {
> + my $plugin = $class->lookup($type);
> + eval { $plugin->plugin_canonicalize($rules) };
> + next if $@; # plugin doesn't implement plugin_canonicalize(...)
> + }
> +
> + return $messages;
> +}
> +
> +=head1 FUNCTIONS
> +
> +=cut
> +
> +=head3 foreach_rule(...)
> +
> +=head3 foreach_rule($rules, $func [, $opts])
> +
> +Filters the given C<$rules> according to the C<$opts> and loops over the
> +resulting rules in the order as defined in the section config and executes
> +C<$func> with the parameters C<L<< ($rule, $ruleid) >>>.
> +
> +The filter properties for C<$opts> are:
> +
> +=over
> +
> +=item C<$type>
> +
> +Limits C<$rules> to those which are of rule type C<$type>.
> +
> +=item C<$state>
> +
> +Limits C<$rules> to those which are in the rule state C<$state>.
> +
> +=back
> +
> +=cut
> +
> +sub foreach_rule : prototype($$;$) {
> + my ($rules, $func, $opts) = @_;
> +
> + my $type = $opts->{type};
> + my $state = $opts->{state};
> +
> + my @ruleids = sort {
> + $rules->{order}->{$a} <=> $rules->{order}->{$b}
> + } keys %{ $rules->{ids} };
> +
> + for my $ruleid (@ruleids) {
> + my $rule = $rules->{ids}->{$ruleid};
> +
> + next if !$rule; # skip invalid rules
> + next if defined($type) && $rule->{type} ne $type;
> +
> + # rules are enabled by default
> + my $rule_state = defined($rule->{state}) ? $rule->{state} :
> 'enabled';
> +
> + next if defined($state) && $rule_state ne $state;
> +
> + $func->($rule, $ruleid);
> + }
> +}
> +
> +1;
> diff --git a/src/PVE/HA/Tools.pm b/src/PVE/HA/Tools.pm
> index a01ac38..767659f 100644
> --- a/src/PVE/HA/Tools.pm
> +++ b/src/PVE/HA/Tools.pm
> @@ -112,6 +112,15 @@ PVE::JSONSchema::register_standard_option(
> },
> );
>
> +PVE::JSONSchema::register_standard_option(
> + 'pve-ha-rule-id',
> + {
> + description => "HA rule identifier.",
> + type => 'string',
> + format => 'pve-configid',
> + },
> +);
> +
> sub read_json_from_file {
> my ($filename, $default) = @_;
>
> @@ -292,4 +301,17 @@ sub complete_group {
> return $res;
> }
>
> +sub complete_rule {
> + my ($cmd, $pname, $cur) = @_;
> +
> + my $cfg = PVE::HA::Config::read_rules_config();
> +
> + my $res = [];
> + foreach my $rule (keys %{ $cfg->{ids} }) {
> + push @$res, $rule;
> + }
> +
> + return $res;
> +}
> +
> 1;
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
