Re: [pve-devel] [PATCH ha-manager 12/15] test: ha tester: add test cases for strict positive colocation rules

[Daniel Kral <d.kral@proxmox.com>]

On 4/28/25 15:51, Fiona Ebner wrote:
> Am 25.03.25 um 16:12 schrieb Daniel Kral:
>> Add test cases for strict positive colocation rules, i.e. where services
>> must be kept on the same node together. These verify the behavior of the
>> services in strict positive colocation rules in case of a failover of
>> their assigned nodes in the following scenarios:
>>
>> - 2 pos. colocated services in a 3 node cluster; 1 node failing
>> - 3 pos. colocated services in a 3 node cluster; 1 node failing
>> - 3 pos. colocated services in a 3 node cluster; 1 node failing, but the
>>    recovery node cannot start one of the services
>>
>> Signed-off-by: Daniel Kral <d.kral@proxmox.com>
> 
> Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
> 
> Again minor nits with the descriptions:

ACK

> 
>> diff --git a/src/test/test-colocation-strict-together2/README b/src/test/test-colocation-strict-together2/README
>> new file mode 100644
>> index 0000000..c1abf68
>> --- /dev/null
>> +++ b/src/test/test-colocation-strict-together2/README
>> @@ -0,0 +1,11 @@
>> +Test whether a strict positive colocation rule makes three services migrate to
>> +the same recovery node in case of a failover of their previously assigned node.
>> +
>> +The test scenario is:
>> +- vm:101, vm:102, and vm:103 must be kept together
>> +- vm:101, vm:102, and vm:103 are all currently running on node3
>> +- node1 has a higher service count than node2 to test that the rule is applied
>> +  even though it would be usually balanced between both remaining nodes
> 
> Nit: The balancing would also happen if the service count would be the
> same on the two nodes, the sentence makes it sound like that it's a
> requirement for this test.

Right, I'll simplify the description and test case in general as it 
doesn't need the same requirements as the strict negative counterpart.

I'll make it clearer / correct it in the next revision.

> 
>> diff --git a/src/test/test-colocation-strict-together3/README b/src/test/test-colocation-strict-together3/README
>> new file mode 100644
>> index 0000000..5332696
>> --- /dev/null
>> +++ b/src/test/test-colocation-strict-together3/README
>> @@ -0,0 +1,17 @@
>> +Test whether a strict positive colocation rule makes three services migrate to
>> +the same recovery node in case of a failover of their previously assigned node.
>> +If one of those fail to start on the recovery node (e.g. insufficient
>> +resources), the failing service will be kept on the recovery node.
>> +
>> +The test scenario is:
>> +- vm:101, vm:102, and fa:120002 must be kept together
>> +- vm:101, vm:102, and fa:120002 are all currently running on node3
>> +- fa:120002 will fail to start on node2
>> +- node1 has a higher service count than node2 to test that the rule is applied
>> +  even though it would be usually balanced between both remaining nodes
> 
> Nit: The balancing would also happen if the service count would be the
> same on the two nodes, the sentence makes it sound like that it's a
> requirement for this test. You do need it since the failure for the 'fa'
> service will happen on node 2 however, so you should mention that instead.

Thanks, I will!


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel