On 4/25/25 11:12, Fiona Ebner wrote:
Am 25.04.25 um 10:29 schrieb Daniel Kral:
On 4/24/25 15:03, Fiona Ebner wrote:
Am 25.03.25 um 16:12 schrieb Daniel Kral:
+
+ $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?
Yes, you're right, it would make more sense to have these be functions
rather than methods. In the current implementation it's rather confusing
and in the end $rules should consist of all types of rules, so $groups
and $services are hopefully not needed as separate parameters anymore
(The only usage for these are to check for HA group members).
For canonicalize(), I don't think it's a hard requirement. Can still be
useful for further optimization of course.
What do you think about something like a
sub register_rule_check {
my ($class, $check_func, $canonicalize_func, $satisfiable_func) = @_;
}
in the base plugin and then each plugin can register their checker
methods with the behavior what is done when running canonicalize(...)
and are_satisfiable(...)? These then have to go through every registered
entry in the list and call $check_func and then either
$canonicalize_func and $satisfiable_func.
I don't see how that would help with the scenario I described above
where the non-satisfiability can only be seen by knowing about
inter-dependencies between rules.
Another (simpler) option would be to just put all checker subroutines in
the base plugin, but that could get unmaintainable quite fast.
I think the helpers should go into the plugins. These can be designed to
take the constraints arising from the inter-dependency as arguments.
E.g. a helper in the location plugin, simply checking if the location
rules are satisfiable (no constraints) and returning the arising
services<->nodes constraints. A helper in the colocation plugin to check
if colocation rules are satisfiable given certain services<->nodes
constraints. The main function in the base plugin would just need to
call these two in order then.
As discussed off-list, I'll take a closer look how we can improve the
interface of the helpers between the Location and Colocation plugin
here, so that they are less coupled on one another.
Depending on how large the rule set can get, I could see some possible
improvements to factor out some of the common checks as pointed out by
Fabian and you on/off-list, so that they're only done once, but as
discussed off-list, I'll wait until their configuration variables are
settled (nofailback, enabled/disabled/conflict).
+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?
As in the previous comment, I think it would be reasonable to pass all
types of rules as there are some checks that require to check between
colocation and location rules, for example. But it would also make sense
to move these more general checks in the base plugin, so that the
checkers in the plugins have to only care about their own feasibility.
Again, IMHO we could have the plugins implement suitable helper
functions, but put the logic that knows about inter-dependencies into
the base plugin itself. Otherwise, you essentially need every plugin to
care about all others, rather than having only the common base plugin
care about all.
So design the helpers in expectation of what inter-dependencies we need
to consider (this will of course change with future rules, but we are
flexible to adapt), but don't have the plugins be concerned with other
plugins directly, i.e. they don't need to know how the constraints arise
from other rule types.
Similar to the above, having a more decoupled interface or separating
check helpers into "plugin-local" and "global" (e.g. checking
inconsistent inter-dependencies between location and colocation rules)
makes sense here.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
