[pve-devel] [PATCH ha-manager 05/15] rules: add colocation rule plugin

[Daniel Kral]

On 4/29/25 11:15, Fiona Ebner wrote:
> Am 29.04.25 um 10:37 schrieb Daniel Kral:
>> On 4/25/25 16:06, Fiona Ebner wrote:
>>> Am 11.04.25 um 13:04 schrieb Daniel Kral:
>>>> On 4/3/25 14:16, Fabian Grünbichler wrote:
>>>>> On March 25, 2025 4:12 pm, Daniel Kral wrote:
>>>> Also, I was curious about how that would work out for the case where a
>>>> negative colocation rule was defined for three nodes with those rules
>>>> split into three rules (essentially a cycle dependence). This should in
>>>> theory have the same semantics as the above rule set:
>>>>
>>>> colocation: stick-together1
>>>>       services vm:101,vm:104
>>>>       affinity together
>>>>       strict 1
>>>>
>>>> colocation: stick-together2
>>>>       services vm:104,vm:102
>>>>       affinity together
>>>>       strict 1
>>>>
>>>> colocation: very-lonely-services1
>>>>       services vm:101,vm:102
>>>>       affinity separate
>>>>       strict 1
>>>>
>>>> colocation: very-lonely-services2
>>>>       services vm:102,vm:103
>>>>       affinity separate
>>>>       strict 1
>>>>
>>>> colocation: very-lonely-services3
>>>>       services vm:101,vm:103
>>>>       affinity separate
>>>>       strict 1
>>>>
>>>> Without the merge of positive rules, 'check_inner_consistency' would
>>>> again not detect the inconsistency here. But with the merge correctly
>>>> applied before checking the consistency, this would be resolved and the
>>>> effective rule set would be:
>>>
>>> I suppose the effective rule set would still also contain the two
>>> 'together' rules, or?
>>
>> No, here it would not. I found it would be most fair or reasonable that
>> if a positive and a negative colocation rule contradict each other to
>> drop both of them. Here the conflicts are
>>
>> stick-together1 -- very-lonely-services1
>> stick-together2 -- very-lonely-services1
>>
>> so all three of them will be dropped from the rule set.
>>
>> Seeing this again here, such cases definitely benefit from the immediate
>> response with the 'conflict'/'ineffective' state to show users that
>> those won't be applied instead of only logging it.
> 
> I don't think dropping all conflicting rules is best. Say you have a
> rule between 100 services and that conflicts with a rule with just 2
> services. Dropping the latter only is much preferred then IMHO. In
> general, I'd argue that the more rules we can still honor, the better
> from a user perspective. I don't think it's worth going out of our way
> though and introduce much complexity to minimize it, because conflicts
> are usually prevented while configuring already.

Yes, that would be a worst-case scenario there and if users run into it 
often enough dropping the rules should definitely optimize for reducing 
the amount of rules that are dropped.

I'll keep in mind but that would be more of a follow-up enhancement as 
you already pointed out.

> 
>>>> colocation: very-lonely-services2
>>>>       services vm:102,vm:103
>>>>       affinity separate
>>>>       strict 1
>>>>
>>>> colocation: very-lonely-services3
>>>>       services vm:101,vm:103
>>>>       affinity separate
>>>>       strict 1
>>>>
>>>> It could be argued, that the negative colocation rules should be merged
>>>> in a similar manner here, as there's now a "effective" difference in the
>>>> semantics of the above rule sets, as the negative colocation rule
>>>> between vm 101 and vm 103 and vm 102 and vm 103 remains.
>>>>
>>>> What do you think?
>>>
>>> I don't think there's a particular need to also merge negative rules
>>> between services (when they form a complete graph). It won't make a
>>> difference if there are no conflicts with positive rules and in edge
>>> cases when there are conflicts (which usually gets caught while editing
>>> the rules), it's better to drop fewer rules, so not merging is an
>>> advantage. Or do you have a particular advantage in favor of merging in
>>> mind?
>>
>> Yes, I think so too.
>>
>> There's quite the semantic difference between positive and negative
>> colocation rules here. "Connected" positive colocation relationships
>> (strict ones in particular) must be co-located in the end anyway, so it
>> makes sense to merge them. Negative colocation relationships must be
>> defined in a "circular" way and might just happen by coincidence for
>> small scenarios.
>>
>> But one thing that just struck me is that what if the user intentionally
>> wrote them as separate rules? Then it might be confusing that all rules
>> are dropped and not just the minimal amount that contradict other
>> rules... Then check_inner_consistency() would just drop the minimal
>> amount of rules that need to be dropped as in the above example.
>>
>> It would be a softer interpretation of the rules indeed, but it might
>> benefit the user in the end and make things easier to follow from the
>> user perspective. If there's no opposition to that, I'd tend to drop the
>> merging for any rules after all.
> 
> Having conflicts is already a bit of an edge case, so I don't think we
> need to go out of our way to avoid merging of positive rules. But if it
> doesn't increase the complexity much, it's fine either way IMHO.

I also agree that the conflicts are more of an edge case anyway. Not 
merging could increase the run time a little bit per rule, since we 
iterate over them for every call of get_colocation_preference(...) now, 
but there's still caching / optimization potential in that regard...

I think we already discussed this off-list but if there's an argument 
that the current data structures are becoming a bottleneck then we could 
improve this by not iterating over all rules themselves but already 
compiling the constraints for each service statically and then only 
filtering out what isn't relevant now (e.g. because the node is offline, 
a colocated service is offline/not yet assigned, etc.).