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 33C3A1FF16F for <inbox@lore.proxmox.com>; Tue, 29 Apr 2025 11:39:04 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A79898136; Tue, 29 Apr 2025 11:39:13 +0200 (CEST) Message-ID: <325a5da0-a89c-4ab8-9a04-a69868d09052@proxmox.com> Date: Tue, 29 Apr 2025 11:39:09 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Proxmox VE development discussion <pve-devel@lists.proxmox.com> References: <20250325151254.193177-1-d.kral@proxmox.com> <20250325151254.193177-11-d.kral@proxmox.com> <67d201c9-64ca-4392-b166-aa14cb2bae48@proxmox.com> <e95e915b-3153-428f-9d8c-64e60d0c9689@proxmox.com> Content-Language: en-US From: Daniel Kral <d.kral@proxmox.com> In-Reply-To: <e95e915b-3153-428f-9d8c-64e60d0c9689@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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. 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 09/15] manager: apply colocation rules when selecting service nodes 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 16:33, Fiona Ebner wrote: > Am 28.04.25 um 14:26 schrieb Fiona Ebner: >> Am 25.03.25 um 16:12 schrieb Daniel Kral: >>> + >>> + delete $allowed_nodes->{$node}; >>> + } >>> + } elsif (scalar keys %$possible_nodes) { >>> + # limit to the possible nodes the service should be on, if there are any. >>> + for my $node (keys %$allowed_nodes) { >>> + next if exists($possible_nodes->{$node}); >>> + >>> + delete $allowed_nodes->{$node}; >> >> This seems wrong. Non-strict rules should not limit the allowed nodes. >> See below for more on this. > > Ah, if there are no possible nodes at all, then the allowed nodes are > not modified at all. This is what makes the loose tests work. This > "secret" here really needs to be properly documented ;) Yes definitely, I'm working on making the logic clearer here. I think another improvement we could make here is along with "merging" get_colocated_services(...) and get_colocation_preference(...), they could already structure together in a strict and non-strict part, so the apply_*_colocation_rules(...) helpers don't have to handle that anymore and just could focus on applying them in the end. If we go for that route, we could reduce apply_positive_colocation_rules(...) to something like this: sub apply_positive_colocation_rules { my ($together, $allowed_nodes) = @_; my $possible_nodes = { $together->{strict}->%* }; # Consider loose nodes if there are no strict nodes $possible_nodes = PVE::HA::Tools::set_intersect($allowed_nodes, $together->{loose}) if !%$possible_nodes; # If there are no strict nodes or the loose nodes would result in an empty $allowed_nodes, apply nothing return if !%$possible_nodes; for my $node (keys %$allowed_nodes) { next if exists($possible_nodes->{$node}); delete $allowed_nodes->{$node}; } } > > It still would be nice to think about which kind of interaction with > scoring we want exactly. Currently it's the number 1 I mentioned, i.e. > "prefer loose colocation over scoring no matter what". Can be fine to > start out too, just means we'd need to introduce an option/tunable if we > ever want to change it. I'll see if I find a good indicator that is understandable for users and results in something deterministic from our side to be testable. But AFAICS the current config interface would need an extra tunable anyway to express some kind of factor which controls how much the loose colocation rule anyway, so I think that would fit better in a follow-up series - especially if users request it :). _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel