[pve-devel] [PATCH manager v2 1/3] pveceph: create mgr with mon, use nodename for id

Thomas Lamprecht t.lamprecht at proxmox.com
Wed Jul 26 10:56:34 CEST 2017


besides one minor nitpick below it looks good to me now.

On 07/26/2017 10:42 AM, Dominik Csapak wrote:
> we now want to add a ceph-mgr daemon to every node where a ceph-mon
> daemon runs, as per ceph documentation recommendation, because in
> luminous the mgr daemons will not be automatically created/started
> with a monitor anymore
> 
> we also give the createmon an optional id parameter, so that one
> can set a custom id, and make the creation/removal of the manager
> optional but the default
> 
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
> changes from v1:
> * added explaining print lines, so that a user can reconstruct what is done
>    and in the error case, where we stopped

I really like that!

> * corrected the regex for the ids (i put the ? on the wrong side of the
>    brackets)
> * optimized the id assignment and dropped the unnecessary die statement
> * reordered enabling before starting
> * added the disabling of the service before the stopping
> * added the 'mgr' part to the start/stop api calls pattern
> * added the hunk which was mistakenly in the second patch
> * renamed the 'monitoronly' to 'exclude-manager' for clarity
> 
>   PVE/API2/Ceph.pm | 106 +++++++++++++++++++++++++++++++++++++++++++++++--------
>   PVE/CephTools.pm |   2 +-
>   2 files changed, 92 insertions(+), 16 deletions(-)
> 
> diff --git a/PVE/API2/Ceph.pm b/PVE/API2/Ceph.pm
> index 105ee37a..9b9b7fec 100644
> --- a/PVE/API2/Ceph.pm
> +++ b/PVE/API2/Ceph.pm
> @@ -843,11 +843,63 @@ my $find_node_ip = sub {
>       die "unable to find local address within network '$cidr'\n";
>   };
>   
> +my $create_mgr = sub {
> +    my ($rados, $id) = @_;
> +
> +    my $clustername = PVE::CephTools::get_config('ccname');
> +    my $mgrdir = "/var/lib/ceph/mgr/$clustername-$id";
> +    my $mgrkeyring = "$mgrdir/keyring";
> +    my $mgrname = "mgr.$id";
> +
> +    die "ceph manager directory '$mgrdir' already exists\n"
> +	if -d $mgrdir;
> +
> +    print "creating manager directory '$mgrdir'\n";
> +    mkdir $mgrdir;
> +    print "creating keys for '$mgrname'\n";
> +    my $output = $rados->mon_command({ prefix => 'auth get-or-create',
> +				       entity => $mgrname,
> +				       caps => [
> +					   mon => 'allow profile mgr',
> +					   osd => 'allow *',
> +					   mds => 'allow *',
> +				       ],
> +				       format => 'plain'});
> +    PVE::Tools::file_set_contents($mgrkeyring, $output);
> +
> +    print "setting owner for directory\n";
> +    run_command(["chown", 'ceph:ceph', '-R', $mgrdir]);
> +
> +    print "enabling service 'ceph-mgr\@$id.service'\n";
> +    PVE::CephTools::ceph_service_cmd('enable', $mgrname);
> +    print "starting service 'ceph-mgr\@$id.service'\n";
> +    PVE::CephTools::ceph_service_cmd('start', $mgrname);
> +};
> +
> +my $destroy_mgr = sub {
> +    my ($mgrid) = @_;
> +
> +    my $clustername = PVE::CephTools::get_config('ccname');
> +    my $mgrname = "mgr.$mgrid";
> +    my $mgrdir = "/var/lib/ceph/mgr/$clustername-$mgrid";
> +
> +    die "ceph manager directory '$mgrname' not found\n"

rather s/mgrname/mgrdir/ ? but either not too important or fixable
one/after committing.

> +	if ! -d $mgrdir;
> +
> +    print "disabling service 'ceph-mgr\@$mgrid.service'\n";
> +    PVE::CephTools::ceph_service_cmd('disable', $mgrname);
> +    print "stopping service 'ceph-mgr\@$mgrid.service'\n";
> +    PVE::CephTools::ceph_service_cmd('stop', $mgrname);
> +
> +    print "removing manager directory '$mgrdir'\n";
> +    File::Path::remove_tree($mgrdir);
> +};
> +
>   __PACKAGE__->register_method ({
>       name => 'createmon',
>       path => 'mon',
>       method => 'POST',
> -    description => "Create Ceph Monitor",
> +    description => "Create Ceph Monitor and Manager",
>       proxyto => 'node',
>       protected => 1,
>       permissions => {
> @@ -857,6 +909,18 @@ __PACKAGE__->register_method ({
>       	additionalProperties => 0,
>   	properties => {
>   	    node => get_standard_option('pve-node'),
> +	    id => {
> +		type => 'string',
> +		optional => 1,
> +		pattern => '[a-zA-Z0-9]([a-zA-Z0-9\-]*[a-zA-Z0-9])?',
> +		description => "The ID for the monitor, when omitted the same as the nodename",
> +	    },
> +	    'exclude-manager' => {
> +		type => 'boolean',
> +		optional => 1,
> +		default => 0,
> +		description => "When set, only a monitor will be created.",
> +	    },
>   	},
>       },
>       returns => { type => 'string' },
> @@ -890,14 +954,7 @@ __PACKAGE__->register_method ({
>   	    }
>   	}
>   
> -	my $monid;
> -	for (my $i = 0; $i < 7; $i++) {
> -	    if (!$cfg->{"mon.$i"}) {
> -		$monid = $i;
> -		last;
> -	    }
> -	}
> -	die "unable to find usable monitor id\n" if !defined($monid);
> +	my $monid = $param->{id} // $param->{node};
>   
>   	my $monsection = "mon.$monid";
>   	my $ip;
> @@ -944,14 +1001,15 @@ __PACKAGE__->register_method ({
>   	    -d $mondir && die "monitor filesystem '$mondir' already exist\n";
>    
>   	    my $monmap = "/tmp/monmap";
> -	
> +
> +	    my $rados = PVE::RADOS->new(timeout => PVE::CephTools::get_config('long_rados_timeout'));
> +
>   	    eval {
>   		mkdir $mondir;
>   
>   		run_command("chown ceph:ceph $mondir") if $systemd_managed;
>   
>   		if ($moncount > 0) {
> -		    my $rados = PVE::RADOS->new(timeout => PVE::CephTools::get_config('long_rados_timeout'));
>   		    my $mapdata = $rados->mon_command({ prefix => 'mon getmap', format => 'plain' });
>   		    PVE::Tools::file_set_contents($monmap, $mapdata);
>   		} else {
> @@ -990,6 +1048,11 @@ __PACKAGE__->register_method ({
>   		}
>   		waitpid($create_keys_pid, 0);
>   	    }
> +
> +	    # create manager
> +	    if (!$param->{'exclude-manager'}) {
> +		$create_mgr->($rados, $monid);
> +	    }
>   	};
>   
>   	return $rpcenv->fork_worker('cephcreatemon', $monsection, $authuser, $worker);
> @@ -999,7 +1062,7 @@ __PACKAGE__->register_method ({
>       name => 'destroymon',
>       path => 'mon/{monid}',
>       method => 'DELETE',
> -    description => "Destroy Ceph monitor.",
> +    description => "Destroy Ceph Monitor and Manager.",
>       proxyto => 'node',
>       protected => 1,
>       permissions => {
> @@ -1011,8 +1074,15 @@ __PACKAGE__->register_method ({
>   	    node => get_standard_option('pve-node'),
>   	    monid => {
>   		description => 'Monitor ID',
> -		type => 'integer',
> +		type => 'string',
> +		pattern => '[a-zA-Z0-9]([a-zA-Z0-9\-]*[a-zA-Z0-9])?',
>   	    },
> +	    'exclude-manager' => {
> +		type => 'boolean',
> +		default => 0,
> +		optional => 1,
> +		description => "When set, removes only the monitor, not the manager"
> +	    }
>   	},
>       },
>       returns => { type => 'string' },
> @@ -1058,6 +1128,12 @@ __PACKAGE__->register_method ({
>   	    delete $cfg->{$monsection};
>   	    PVE::CephTools::write_ceph_config($cfg);
>   	    File::Path::remove_tree($mondir);
> +
> +	    # remove manager
> +	    if (!$param->{'exclude-manager'}) {
> +		eval { $destroy_mgr->($monid); };
> +		warn $@ if $@;
> +	    }
>   	};
>   
>   	return $rpcenv->fork_worker('cephdestroymon', $monsection,  $authuser, $worker);
> @@ -1081,7 +1157,7 @@ __PACKAGE__->register_method ({
>   		description => 'Ceph service name.',
>   		type => 'string',
>   		optional => 1,
> -		pattern => '(mon|mds|osd)\.[A-Za-z0-9]{1,32}',
> +		pattern => '(mon|mds|osd|mgr)\.[A-Za-z0-9]{1,32}',
>   	    },
>   	},
>       },
> @@ -1131,7 +1207,7 @@ __PACKAGE__->register_method ({
>   		description => 'Ceph service name.',
>   		type => 'string',
>   		optional => 1,
> -		pattern => '(mon|mds|osd)\.[A-Za-z0-9]{1,32}',
> +		pattern => '(mon|mds|osd|mgr)\.[A-Za-z0-9]{1,32}',
>   	    },
>   	},
>       },
> diff --git a/PVE/CephTools.pm b/PVE/CephTools.pm
> index 0d26ea3e..0c0d7c18 100644
> --- a/PVE/CephTools.pm
> +++ b/PVE/CephTools.pm
> @@ -178,7 +178,7 @@ sub ceph_service_cmd {
>   
>       if (systemd_managed()) {
>   
> -	if ($service && $service =~ m/^(mon|osd|mds|radosgw)(\.([A-Za-z0-9]{1,32}))?$/) {
> +	if ($service && $service =~ m/^(mon|osd|mds|mgr|radosgw)(\.([A-Za-z0-9\-]{1,32}))?$/) {
>   	    $service = defined($3) ? "ceph-$1\@$3" : "ceph-$1.target";
>   	} else {
>   	    $service = "ceph.target";
> 





More information about the pve-devel mailing list