Am 25.03.25 um 16:12 schrieb Daniel Kral:
> Add a rules section config base plugin to allow users to specify
> different kinds of rules in a single configuration file.
>
> The interface is designed to allow sub plugins to implement their own
> {decode,encode}_value() methods and also offer a canonicalized version
It's not "allow" them to implement, but actually requires them to
implement it. Otherwise, it would be infinite recursion.
> of their rules with canonicalize(), i.e. with any inconsistencies
> removed and ambiguities resolved. There is also a are_satisfiable()
> method for anticipation of the verification of additions or changes to
> the rules config via the API.
---snip 8<---
> diff --git a/src/PVE/HA/Rules.pm b/src/PVE/HA/Rules.pm
> new file mode 100644
> index 0000000..bff3375
> --- /dev/null
> +++ b/src/PVE/HA/Rules.pm
> @@ -0,0 +1,118 @@
> +package PVE::HA::Rules;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::JSONSchema qw(get_standard_option);
> +use PVE::SectionConfig;
Missing include of PVE::Tools.
Nit: I'd put a blank here to separate modules from different packages
and modules from the same package.
> +use PVE::HA::Tools;
> +
> +use base qw(PVE::SectionConfig);
> +
> +# TODO Add descriptions, completions, etc.
> +my $defaultData = {
> + propertyList => {
> + type => { description => "Rule type." },
> + ruleid => get_standard_option('pve-ha-rule-id'),
> + comment => {
> + type => 'string',
> + maxLength => 4096,
> + description => "Rule description.",
> + },
Oh good, so there already is a comment property :)
---snip 8<---
> +sub foreach_service_rule {
> + my ($rules, $func, $opts) = @_;
> +
> + my $sid = $opts->{sid};
> + my $type = $opts->{type};
> +
> + my @ruleids = sort {
> + $rules->{order}->{$a} <=> $rules->{order}->{$b}
> + } keys %{$rules->{ids}};
> +
> + for my $ruleid (@ruleids) {
> + my $rule = $rules->{ids}->{$ruleid};
> +
> + next if !$rule; # invalid rules are kept undef in section config,
> delete them
s/delete/skip/ ?
> + next if $type && $rule->{type} ne $type;
> + next if $sid && !defined($rule->{services}->{$sid});
Style nit: I'd prefer defined($type) and defined($sid) in the above
expressions
> +
> + $func->($rule, $ruleid);
> + }
> +}
> +
> +sub canonicalize {
> + my ($class, $rules, $groups, $services) = @_;
> +
> + die "implement in subclass";
> +}
> +
> +sub are_satisfiable {
> + my ($class, $rules, $groups, $services) = @_;
> +
> + die "implement in subclass";
> +}
This might not be possible to implement in just the subclasses. E.g.
services 1 and 2 have strict colocation with each other, but 1 has
restricted location on node A and 2 has restricted location on node B.
I don't think it hurts to rather put the implementation here with
knowledge of all rule types and what inter-dependencies they entail. And
maybe have it be a function rather than a method then?
> +sub checked_config {
> + my ($rules, $groups, $services) = @_;
> +
> + my $types = __PACKAGE__->lookup_types();
> +
> + for my $type (@$types) {
> + my $plugin = __PACKAGE__->lookup($type);
> +
> + $plugin->canonicalize($rules, $groups, $services);
Shouldn't we rather only pass the rules that belong to the specific
plugin rather than always all?
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
