From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
	by lore.proxmox.com (Postfix) with ESMTPS id 8AD121FF164
	for <inbox@lore.proxmox.com>; Fri,  9 May 2025 13:21:57 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 2231C3B7A3;
	Fri,  9 May 2025 13:22:16 +0200 (CEST)
Message-ID: <9378616f-22c7-49a9-8e76-4383a5c318b9@proxmox.com>
Date: Fri, 9 May 2025 13:22:13 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
From: Daniel Kral <d.kral@proxmox.com>
To: Fiona Ebner <f.ebner@proxmox.com>,
 Proxmox VE development discussion <pve-devel@lists.proxmox.com>
References: <20250325151254.193177-1-d.kral@proxmox.com>
 <20250325151254.193177-14-d.kral@proxmox.com>
 <2feb8499-0dfa-4a28-810a-45a1db3e436d@proxmox.com>
Content-Language: en-US
In-Reply-To: <2feb8499-0dfa-4a28-810a-45a1db3e436d@proxmox.com>
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.012 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [PATCH ha-manager 12/15] test: ha tester: add test
 cases for strict positive colocation rules
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.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