[PATCH manager v2 0/1] fix #5327: ceph: osd: create: accept multiple Ceph networks

[PATCH manager v2 0/1] fix #5327: ceph: osd: create: accept multiple Ceph networks

[Kefu Chai]

Hi,

this is v2 of my patch [1] for creating OSDs on clusters with more
than one Ceph network. Sorry, when I sent v1 I missed that Max had
already posted a fix for the same bug back in July [2], so now there
are two patches for #5327 on the list. I only noticed while looking
at #8087, which reports the same problem.

Both patches split the network list and accept the node if any of the
subnets has a local address, but they split it differently. Ceph takes
commas and spaces as separators, and the config in #5327 uses a space:

  cluster_network = 192.168.254.10/24 192.168.0.100/24

If I read it right, Max's patch only splits on commas, so this value
would still go to `ip address show to` as one string and fail. This
patch uses split_list(), like monitor creation does, so both work.

With this patch, an empty "cluster_network =" also falls back to
public_network, like Ceph does, instead of skipping the check. And the
error message names the option it checked, so it doesn't say "cluster
network" when the value came from public_network.

Max's patch also warns about a subnet it can't look up and moves on,
which looks intended. Mine fails there, unless an earlier subnet
already matched. Ceph won't start with a network it can't parse
anyway, so I think failing is fine, but I'm happy to switch to a
warning if that's preferred. And if Max's version is the one to keep,
I can drop this and send the space handling on top of his.

I tested it on a three-node lab cluster with comma- and
space-separated lists, a list of /32s like in #8087, a list without
any local subnet, and an empty cluster_network.

Changes since v1:
- add "fix #5327" and mention #8087
- mention in the commit message that spaces work as separators too
- rebase onto current master

[1] https://lore.proxmox.com/pve-devel/20261009031555.3414265-1-k.chai@proxmox.com/
[2] https://lore.proxmox.com/pve-devel/20260728113729.2136104-1-m.carrara@proxmox.com/

Kefu Chai (1):
  fix #5327: ceph: osd: create: accept multiple Ceph networks

 PVE/API2/Ceph/OSD.pm | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

-- 
2.47.3

[PATCH manager v2 1/1] fix #5327: ceph: osd: create: accept multiple Ceph networks

[Kefu Chai]

public_network and cluster_network can hold a list of subnets,
separated by commas or spaces. Monitor creation has supported that
since commit 0b6a2838 [0], but the OSD create check still passed the
whole value to `ip address show to`. ip rejects a list, so OSD
creation failed on every node of a cluster with more than one subnet.

Split the value like monitor creation does, and accept the node if it
has an address in any of the subnets. An OSD only binds to one
address, so one match is enough. While at it, name the checked option
in the error message. It always said "cluster network", even when the
value came from the public_network fallback.

This was also reported as #8087.

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=2422

Signed-off-by: Kefu Chai <k.chai@proxmox.com>
---
 PVE/API2/Ceph/OSD.pm | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/PVE/API2/Ceph/OSD.pm b/PVE/API2/Ceph/OSD.pm
index 7ff006d2..539b55bb 100644
--- a/PVE/API2/Ceph/OSD.pm
+++ b/PVE/API2/Ceph/OSD.pm
@@ -6,6 +6,7 @@ use warnings;
 use Cwd qw(abs_path);
 use IO::File;
 use JSON;
+use List::Util qw(any);
 use UUID;
 
 use PVE::Ceph::Tools;
@@ -340,17 +341,18 @@ __PACKAGE__->register_method({
 
         my $ceph_conf = cfs_read_file('ceph.conf');
 
+        my $osd_net_opt = 'cluster_network';
         my $osd_network = $ceph_conf->{global}->{cluster_network};
-        $osd_network //= $ceph_conf->{global}->{public_network}; # fallback
-
-        if ($osd_network) { # check only if something is configured
-            my $cluster_net_ips = PVE::Network::get_local_ip_from_cidr($osd_network);
-            if (scalar(@$cluster_net_ips) < 1) {
-                my $osd_net_obj = PVE::Network::IP_from_cidr($osd_network);
-                my $osd_base_cidr = $osd_net_obj->{ip} . "/" . $osd_net_obj->{prefixlen};
+        if (!$osd_network) { # fallback, also when empty like ceph does
+            $osd_net_opt = 'public_network';
+            $osd_network = $ceph_conf->{global}->{public_network};
+        }
 
+        my @osd_nets = PVE::Tools::split_list($osd_network);
+        if (@osd_nets) { # check only if something is configured
+            if (!any { scalar(@{ PVE::Network::get_local_ip_from_cidr($_) }) } @osd_nets) {
                 die
-                    "No address from ceph cluster network (${osd_base_cidr}) found on node '$nodename'. "
+                    "No address from ceph $osd_net_opt ($osd_network) found on node '$nodename'. "
                     . "Check your network config.\n";
             }
         }
-- 
2.47.3