[pve-devel] [PATCH manager v2 2/4] api ceph osd: add OSD index, metadata and lv-info
Dominik Csapak
d.csapak at proxmox.com
Mon Oct 17 16:29:56 CEST 2022
LGTM, some comments inline (mostly nitpicks/qestions)
On 7/6/22 15:01, Aaron Lauterer wrote:
> To get more details for a single OSD, we add two new endpoints:
> * nodes/{node}/ceph/osd/{osdid}/metadata
> * nodes/{node}/ceph/osd/{osdid}/lv-info
>
> The {osdid} endpoint itself gets a new GET handler to return the index.
>
> The metadata one provides various metadata regarding the OSD.
>
> Such as
> * process id
> * memory usage
> * info about devices used (bdev/block, db, wal)
> * size
> * disks used (sdX)
> ...
> * network addresses and ports used
> ...
>
> Memory usage and PID are retrieved from systemd while the rest can be
> retrieved from the metadata provided by Ceph.
>
> The second one (lv-info) returns the following infos for a logical
> volume:
> * creation time
> * lv name
> * lv path
> * lv size
> * lv uuid
> * vg name
>
> Possible volumes are:
> * block (default value if not provided)
> * db
> * wal
>
> 'ceph-volume' is used to gather the infos, except for the creation time
> of the LV which is retrieved via 'lvs'.
>
> Signed-off-by: Aaron Lauterer <a.lauterer at proxmox.com>
> ---
> changes since v1:
> - squashed all API commits into one
> - moved all new API endpoints into sub endpoints to {osdid}
> - {osdid} itself returns the necessary index
> - incorporated other code improvements
>
> PVE/API2/Ceph/OSD.pm | 315 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 315 insertions(+)
>
> diff --git a/PVE/API2/Ceph/OSD.pm b/PVE/API2/Ceph/OSD.pm
> index 93433b3a..170c4fd3 100644
> --- a/PVE/API2/Ceph/OSD.pm
> +++ b/PVE/API2/Ceph/OSD.pm
> @@ -5,6 +5,7 @@ use warnings;
>
> use Cwd qw(abs_path);
> use IO::File;
> +use JSON;
> use UUID;
>
> use PVE::Ceph::Tools;
> @@ -516,6 +517,320 @@ __PACKAGE__->register_method ({
> return $rpcenv->fork_worker('cephcreateosd', $devs->{dev}->{name}, $authuser, $worker);
> }});
>
> +my $OSD_DEV_RETURN_PROPS = {
> + dev_node => {
> + type => 'string',
> + description => 'Device node',
> + },
> + devices => {
> + type => 'string',
> + description => 'Physical disks used',
> + },
> + size => {
> + type => 'integer',
> + description => 'Size in bytes',
> + },
> + support_discard => {
> + type => 'boolean',
> + description => 'Discard support of the physical device',
> + },
> + type => {
> + type => 'string',
> + description => 'Type of device. For example, hdd or ssd',
> + },
> +};
> +
> +__PACKAGE__->register_method ({
> + name => 'osdindex',
> + path => '{osdid}',
> + method => 'GET',
> + permissions => { user => 'all' },
> + description => "OSD index.",
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + node => get_standard_option('pve-node'),
> + osdid => {
> + description => 'OSD ID',
> + type => 'integer',
> + },
> + },
> + },
> + returns => {
> + type => 'array',
> + items => {
> + type => "object",
> + properties => {},
> + },
> + links => [ { rel => 'child', href => "{name}" } ],
> + },
> + code => sub {
> + my ($param) = @_;
> +
> + my $result = [
> + { name => 'metadata' },
> + { name => 'lv-info' },
> + ];
> +
> + return $result;
> + }});
> +
> +__PACKAGE__->register_method ({
> + name => 'osddetails',
> + path => '{osdid}/metadata',
> + method => 'GET',
> + description => "Get OSD details",
> + proxyto => 'node',
> + protected => 1,
> + permissions => {
> + check => ['perm', '/', [ 'Sys.Audit' ], any => 1],
> + },
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + node => get_standard_option('pve-node'),
> + osdid => {
> + description => 'OSD ID',
> + type => 'integer',
> + },
> + },
> + },
> + returns => {
> + type => 'object',
> + properties => {
> + osd => {
> + type => 'object',
> + description => 'General information about the OSD',
> + properties => {
> + hostname => {
> + type => 'string',
> + description => 'Name of the host containing the OSD.',
> + },
> + id => {
> + type => 'integer',
> + description => 'ID of the OSD.',
> + },
> + mem_usage => {
> + type => 'integer',
> + description => 'Memory usage of the OSD service.',
> + },
> + osd_data => {
> + type => 'string',
> + description => "Path to the OSD's data directory.",
> + },
> + osd_objectstore => {
> + type => 'string',
> + description => 'The type of object store used.',
> + },
> + pid => {
> + type => 'integer',
> + description => 'OSD process ID.',
> + },
> + version => {
> + type => 'string',
> + description => 'Ceph version of the OSD service.',
> + },
> + front_addr => {
> + type => 'string',
> + description => 'Address and port used to talk to clients and monitors.',
> + },
> + back_addr => {
> + type => 'string',
> + description => 'Address and port used to talk to other OSDs.',
> + },
> + hb_front_addr => {
> + type => 'string',
> + description => 'Heartbeat address and port for clients and monitors.',
> + },
> + hb_back_addr => {
> + type => 'string',
> + description => 'Heartbeat address and port for other OSDs.',
> + },
> + },
> + },
> + bdev => {
> + type => 'object',
> + description => 'Data about the OSD block device',
> + properties => $OSD_DEV_RETURN_PROPS,
> + },
> + db => {
> + type => 'object',
> + description => 'Data about the DB device (optional)',
> + properties => $OSD_DEV_RETURN_PROPS,
> + optional => 1,
> + },
> + wal => {
> + type => 'object',
> + description => 'Data about the WAL device (optional)',
> + properties => $OSD_DEV_RETURN_PROPS,
> + optional => 1,
> + },
> + }
> + },
> + code => sub {
> + my ($param) = @_;
> +
> + PVE::Ceph::Tools::check_ceph_inited();
> +
> + my $osdid = $param->{osdid};
> + my $rados = PVE::RADOS->new();
> + my $metadata = $rados->mon_command({ prefix => 'osd metadata', id => int($osdid) });
> +
> + die "OSD '${osdid}' does not exists on host '${nodename}'\n"
> + if $nodename ne $metadata->{hostname};
> +
> + my $raw = '';
> + my $pid;
> + my $memory;
> + my $parser = sub { $raw .= shift };
> + my $cmd = [
> + '/bin/systemctl',
> + 'show',
> + "ceph-osd\@${osdid}.service",
> + '--property=MainPID,MemoryCurrent'
> + ];
nit: personally i'd prefer to have the value of the parameter on it's own
i.e. :
'--property', 'MainPID,MemoryCurrent'
but it's not really a blocker for me
> + run_command($cmd, errmsg => 'systemctl show error', outfunc => $parser);
> +
> + if ($raw =~ m/^MainPID=([0-9]*)MemoryCurrent=([0-9]*|\[not set\])$/s) { #untaint
> + $pid = $1;
> + $memory = $2 eq "[not set]" ? 0 : $2;
> + } else {
> + die "got unexpected data from systemctl: '${raw}'\n";
> + }
it works, but wouldn't it also be possible to parse those two
things on their own line in the parser? that way you'd not need the intermediate
$raw and would (IMHO) make the code a bit cleaner
> +
> + my $data = {
> + osd => {
> + hostname => $metadata->{hostname},
> + id => $metadata->{id},
> + mem_usage => int($memory),
> + osd_data => $metadata->{osd_data},
> + osd_objectstore => $metadata->{osd_objectstore},
> + pid => int($pid),
> + version => "$metadata->{ceph_version_short} ($metadata->{ceph_release})",
> + front_addr => $metadata->{front_addr},
> + back_addr => $metadata->{back_addr},
> + hb_front_addr => $metadata->{hb_front_addr},
> + hb_back_addr => $metadata->{hb_back_addr},
> + },
> + };
> +
> + my $get_data = sub {
> + my ($dev, $prefix) = @_;
> + $data->{$dev} = {
> + dev_node => $metadata->{"${prefix}_${dev}_dev_node"},
> + devices => $metadata->{"${prefix}_${dev}_devices"},
> + size => int($metadata->{"${prefix}_${dev}_size"}),
> + support_discard => int($metadata->{"${prefix}_${dev}_support_discard"}),
> + type => $metadata->{"${prefix}_${dev}_type"},
> + };
> + };
> +
> + $get_data->("bdev", "bluestore");
> + $get_data->("db", "bluefs") if $metadata->{bluefs_dedicated_db};
> + $get_data->("wal", "bluefs") if $metadata->{bluefs_dedicated_wal};
> +
> + return $data;
> + }});
> +
> +__PACKAGE__->register_method ({
> + name => 'osdvolume',
> + path => '{osdid}/lv-info',
> + method => 'GET',
> + description => "Get OSD volume details",
> + proxyto => 'node',
> + protected => 1,
> + permissions => {
> + check => ['perm', '/', [ 'Sys.Audit' ], any => 1],
> + },
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + node => get_standard_option('pve-node'),
> + osdid => {
> + description => 'OSD ID',
> + type => 'integer',
> + },
> + type => {
> + description => 'OSD device type',
> + type => 'string',
> + enum => ['block', 'db', 'wal'],
> + default => 'block',
> + optional => 1,
> + },
> + },
> + },
> + returns => {
> + type => 'object',
> + properties => {
> + creation_time => {
> + type => 'string',
> + description => "Creation time as reported by `lvs`.",
> + },
> + lv_name => {
> + type => 'string',
> + description => 'Name of the logical volume (LV).',
> + },
> + lv_path => {
> + type => 'string',
> + description => 'Path to the logical volume (LV).',
> + },
> + lv_size => {
> + type => 'integer',
> + description => 'Size of the logical volume (LV).',
> + },
> + lv_uuid => {
> + type => 'string',
> + description => 'UUID of the logical volume (LV).',
> + },
> + vg_name => {
> + type => 'string',
> + description => 'Name of the volume group (VG).',
> + },
> + },
> + },
> + code => sub {
> + my ($param) = @_;
> +
> + PVE::Ceph::Tools::check_ceph_inited();
> +
> + my $osdid = $param->{osdid};
> + my $type = $param->{type} // 'block';
> +
> + my $raw = '';
> + my $parser = sub { $raw .= shift };
> + my $cmd = ['/usr/sbin/ceph-volume', 'lvm', 'list', '--format', 'json'];
> + run_command($cmd, errmsg => 'ceph volume error', outfunc => $parser);
> +
> + my $result;
> + if ($raw =~ m/^(\{.*\})$/s) { #untaint
> + $result = JSON::decode_json($1);
> + } else {
> + die "got unexpected data from ceph-volume: '${raw}'\n";
> + }
> + die "OSD '${osdid}' not found in 'ceph-volume lvm list' on node '${nodename}'\n"
> + if !$result->{$osdid};
> +
> + my $lv_data = { map { $_->{type} => $_ } @{$result->{$osdid}} };
> + my $volume = $lv_data->{$type} || die "volume type '${type}' not found for OSD ${osdid}\n";
> +
> + $raw = '';
> + $cmd = ['/sbin/lvs', $volume->{lv_path}, '--reportformat', 'json', '-o', 'lv_time'];
> + run_command($cmd, errmsg => 'lvs error', outfunc => $parser);
> +
> + if ($raw =~ m/(\{.*\})$/s) { #untaint, lvs has whitespace at beginning
> + $result = JSON::decode_json($1);
> + } else {
> + die "got unexpected data from lvs: '${raw}'\n";
> + }
> +
> + my $data = { map { $_ => $volume->{$_} } qw(lv_name lv_path lv_uuid vg_name) };
> + $data->{lv_size} = int($volume->{lv_size});
> +
> + $data->{creation_time} = @{$result->{report}}[0]->{lv}[0]->{lv_time};
> +
> + return $data;
> + }});
> +
> # Check if $osdid belongs to $nodename
> # $tree ... rados osd tree (passing the tree makes it easy to test)
> sub osd_belongs_to_node {
More information about the pve-devel
mailing list