From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 681938977F for ; Mon, 17 Oct 2022 16:30:31 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 320B2205BF for ; Mon, 17 Oct 2022 16:30:01 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Mon, 17 Oct 2022 16:29:58 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 78C2244A51; Mon, 17 Oct 2022 16:29:58 +0200 (CEST) Message-ID: <2051d376-8fb0-74eb-5e3e-19c0d189f574@proxmox.com> Date: Mon, 17 Oct 2022 16:29:56 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:106.0) Gecko/20100101 Thunderbird/106.0 Content-Language: en-US To: Proxmox VE development discussion , Aaron Lauterer References: <20220706130126.282308-1-a.lauterer@proxmox.com> <20220706130126.282308-3-a.lauterer@proxmox.com> From: Dominik Csapak In-Reply-To: <20220706130126.282308-3-a.lauterer@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.068 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) 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 manager v2 2/4] api ceph osd: add OSD index, metadata and lv-info X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 17 Oct 2022 14:30:31 -0000 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 > --- > 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 {