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

[Fiona Ebner <f.ebner@proxmox.com>]

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:

> 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.

> 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.

> +
> +Therefore, the expected outcome is:
> +- As node3 fails, all services are migrated to node2
> +- Two of those services will start successfully, but fa:120002 will stay in
> +  recovery, since it cannot be started on this node, but cannot be relocated to
> +  another one either due to the strict colocation rule


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