[pve-devel] [RFC manager] Allow specifying cluster and public network separately

[Thomas Lamprecht]

On 08/30/2017 09:41 AM, Philip Abernethy wrote:
> ---
> We always recommend putting ceph-internal communication into a separate cluster
> network. Yet pveceph init does not allow to specify one, leading to
> difficulties if one were to follow the setup guide on the wiki, which proceeds
> to create monitors after the ceph network was initialised.
> In case you never had to, changing the IP of a monitor is a huge PITA.
> Ideally I'd like to group cluster_network and public_network so that either
> both or neither have to be specified. Also I'd like this group to be mutually
> exclusive with the network param, with the whole construct still being
> optional.
> But I'm not sure if your cmdline builder can deliver that.

I'd put that as "real" commit message, above the "---" marker, nicer than the empty
commit message, IMO.

Also a
ceph: allow speci...
as commit subject would be nice, we use cluster network for the pve cluster network
where corosync runs and have Cluster API entry points here in the manager, thus a
hint that this is for ceph here could be helpful.

> 
>   PVE/API2/Ceph.pm | 20 +++++++++++++++++++-
>   1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/PVE/API2/Ceph.pm b/PVE/API2/Ceph.pm
> index 7f709f55..6062e173 100644
> --- a/PVE/API2/Ceph.pm
> +++ b/PVE/API2/Ceph.pm
> @@ -783,7 +783,22 @@ __PACKAGE__->register_method ({
>   	properties => {
>   	    node => get_standard_option('pve-node'),
>   	    network => {
> -		description => "Use specific network for all ceph related traffic",
> +		description => "Use specific network for all ceph related traffic.\n" .
> +		"Takes precedence over public_network and cluster_network.",
> +		type => 'string', format => 'CIDR',
> +		optional => 1,
> +		maxLength => 128,
> +	    },
> +	    public_network => {
> +		description => "Use specific network for all ceph data traffic.\n" .
> +		"Specify along with cluster_network.",
> +		type => 'string', format => 'CIDR',
> +		optional => 1,
> +		maxLength => 128,
> +	    },
> +	    cluster_network => {
> +		description => "Use specific network for all internal ceph traffic.\n" .
> +		"Specify along with public_network.",
>   		type => 'string', format => 'CIDR',
>   		optional => 1,
>   		maxLength => 128,
> @@ -877,6 +892,9 @@ __PACKAGE__->register_method ({
>   	if ($param->{network}) {
>   	    $cfg->{global}->{'public network'} = $param->{network};
>   	    $cfg->{global}->{'cluster network'} = $param->{network};
> +	} elsif ($param->{public_network} && $param->{cluster_network}) {
> +	    $cfg->{global}->{'public network'} = $param->{public_network};
> +	    $cfg->{global}->{'cluster network'} = $param->{cluster_network};

If you give just one of those parameters it will get silently dropped,
at least a warning should be printed, IMO.

Maybe we could also omit one of the new params and reuse the "old" network
parameter for this too, mirroring how ceph does it.[1]

Basically just add the "cluster_network" param. If just "network" is set use
it for both, if "cluster_network" is also set, use it for the cluster network
and the "network" one for the public.

If just cluster_network is set I'd just error out, I mean, you could use it
then also for both, but that could seem a bit weird and does not mirrors ceph
behavior.[1]

With this approach we would save a option switch which makes error handling
easier, keep the old behavior if the new option is not used and mirror cephs
behavior.

[1]: <https://github.com/ceph/ceph/blob/luminous/src/common/pick_address.cc#L115>

>   	}
>   
>   	PVE::CephTools::write_ceph_config($cfg);
>