From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id DB1011FF0E4 for ; Tue, 28 Jul 2026 15:59:49 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 2AAE02143D; Tue, 28 Jul 2026 15:59:49 +0200 (CEST) Message-ID: <1219b0fa-c7c8-4749-9069-9cb440338e91@proxmox.com> Date: Tue, 28 Jul 2026 15:59:39 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird From: Christian Ebner Subject: Re: [PATCH proxmox 1/3] system-report: add crate for shared report generation To: Erik Fastermann , pbs-devel@lists.proxmox.com, pdm-devel@lists.proxmox.com References: <20260714083156.99794-1-e.fastermann@proxmox.com> <20260714083156.99794-2-e.fastermann@proxmox.com> Content-Language: en-US, de-DE In-Reply-To: <20260714083156.99794-2-e.fastermann@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1785247144151 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.157 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) KAM_SHORT 0.001 Use of a URL Shortener for very short URL RCVD_IN_DNSWL_LOW -0.7 Sender listed at https://www.dnswl.org/, low trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: BYHWYDBK73TZGYVVBC2HPGGSGY2CWZ36 X-Message-ID-Hash: BYHWYDBK73TZGYVVBC2HPGGSGY2CWZ36 X-MailFrom: c.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On 7/14/26 10:31 AM, Erik Fastermann wrote: > Factor the shared report generation from PBS and PDM into a new > proxmox-system-report crate, so both generate their reports from a > single implementation. > > The crate defines the common section order (FILES, COMMANDS, > FUNCTIONS) and the set of general commands run on every product; > products pass their own additional files, commands and functions on > top. It produces no report on its own, so the user-visible changes > land in the respective server packages. > > Suggested-by: Lukas Wagner > Suggested-by: Christian Ebner > Signed-off-by: Erik Fastermann > --- > Cargo.toml | 2 + > proxmox-system-report/Cargo.toml | 16 ++ > proxmox-system-report/debian/copyright | 18 ++ > proxmox-system-report/debian/debcargo.toml | 7 + > proxmox-system-report/src/lib.rs | 235 +++++++++++++++++++++ > 5 files changed, 278 insertions(+) > create mode 100644 proxmox-system-report/Cargo.toml > create mode 100644 proxmox-system-report/debian/copyright > create mode 100644 proxmox-system-report/debian/debcargo.toml > create mode 100644 proxmox-system-report/src/lib.rs > > diff --git a/Cargo.toml b/Cargo.toml > index ef407166..1732444b 100644 > --- a/Cargo.toml > +++ b/Cargo.toml > @@ -59,6 +59,7 @@ members = [ > "proxmox-subscription", > "proxmox-sys", > "proxmox-syslog-api", > + "proxmox-system-report", > "proxmox-systemd", > "proxmox-tfa", > "proxmox-time", > @@ -177,6 +178,7 @@ proxmox-io = { version = "1.2.1", path = "proxmox-io" } > proxmox-lang = { version = "1.5", path = "proxmox-lang" } > proxmox-log = { version = "1.0.0", path = "proxmox-log" } > proxmox-login = { version = "1.0.0", path = "proxmox-login" } > +proxmox-network-api = { version = "1.0.5", path = "proxmox-network-api" } > proxmox-network-types = { version = "1.0.2", path = "proxmox-network-types" } > proxmox-parallel-handler = { version = "1.0.0", path = "proxmox-parallel-handler" } > proxmox-pgp = { version = "1.0.0", path = "proxmox-pgp" } > diff --git a/proxmox-system-report/Cargo.toml b/proxmox-system-report/Cargo.toml > new file mode 100644 > index 00000000..84a87c76 > --- /dev/null > +++ b/proxmox-system-report/Cargo.toml > @@ -0,0 +1,16 @@ > +[package] > +name = "proxmox-system-report" > +description = "Shared system report functionality" > +version = "0.1.0" nit: let's start with 1.0.0, otherwise this could be mis-interpreted as not production-ready. > + > +authors.workspace = true > +edition.workspace = true > +exclude.workspace = true > +homepage.workspace = true > +license.workspace = true > +repository.workspace = true > +rust-version.workspace = true > + > +[dependencies] > +proxmox-network-api = { workspace = true, features = ["impl"] } > +proxmox-sys.workspace = true comment: these dependencies must also be declared in debian/control (still missing and should be included to build deb packages with their dependencies), so apt can resolve and install these in case no other package does. > diff --git a/proxmox-system-report/debian/copyright b/proxmox-system-report/debian/copyright > new file mode 100644 > index 00000000..77952eba > --- /dev/null > +++ b/proxmox-system-report/debian/copyright > @@ -0,0 +1,18 @@ > +Format: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/ > + > +Files: > + * > +Copyright: 2019 - 2026 Proxmox Server Solutions GmbH question: not sure about the copyright timespan here, but since the factored out code lived in proxmox-backup and that has 2019-2026 this is fine I guess? > +License: AGPL-3.0-or-later > + This program is free software: you can redistribute it and/or modify it under > + the terms of the GNU Affero General Public License as published by the Free > + Software Foundation, either version 3 of the License, or (at your option) any > + later version. > + . > + This program is distributed in the hope that it will be useful, but WITHOUT > + ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS > + FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more > + details. > + . > + You should have received a copy of the GNU Affero General Public License along > + with this program. If not, see . > diff --git a/proxmox-system-report/debian/debcargo.toml b/proxmox-system-report/debian/debcargo.toml > new file mode 100644 > index 00000000..b7864cdb > --- /dev/null > +++ b/proxmox-system-report/debian/debcargo.toml > @@ -0,0 +1,7 @@ > +overlay = "." > +crate_src_path = ".." > +maintainer = "Proxmox Support Team " > + > +[source] > +vcs_git = "git://git.proxmox.com/git/proxmox.git" > +vcs_browser = "https://git.proxmox.com/?p=proxmox.git" > diff --git a/proxmox-system-report/src/lib.rs b/proxmox-system-report/src/lib.rs > new file mode 100644 > index 00000000..92ca8e5d > --- /dev/null > +++ b/proxmox-system-report/src/lib.rs > @@ -0,0 +1,235 @@ > +//! Shared functionality to generate system reports. > + > +#![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))] > +#![deny(unsafe_code)] > +#![deny(missing_docs)] > + > +use std::fmt::Write; > +use std::path::Path; > +use std::process::Command; > + > +use proxmox_network_api::NetworkInterfaceType; > + > +/// A group of files to include: `(group_name, [path, ...])`. > +pub type FileGroup = (&'static str, Vec<&'static str>); > + > +/// A command to run: `(command, [arg, ...])`. > +pub type CommandSpec = (&'static str, Vec<&'static str>); nit: IMHO this should be StaticArgsCommandSpec, and the docstring state that this is for commands with static, compile time known arguments ... > + > +/// A command to run with owned arguments: `(command, [arg, ...])`. > +type DynamicCommandSpec = (&'static str, Vec); ... and this DynamicArgsCommmandSpec, the docstring mentioning that these commands are to be run with dynamically generated arguments produced on runtime. > + > +/// A function to run and its label: `(description, function)`. > +pub type FunctionMapping = (&'static str, fn() -> String); > + > +fn get_top_processes() -> String { > + let (exe, args) = ("top", vec!["-b", "-c", "-w512", "-n", "1", "-o", "TIME"]); > + let output = Command::new(exe).args(&args).output(); > + let output = match output { > + Ok(output) => String::from_utf8_lossy(&output.stdout).to_string(), > + Err(err) => err.to_string(), > + }; > + let output = output.lines().take(30).collect::>().join("\n"); > + format!("$ `{exe} {}`\n```\n{output}\n```", args.join(" ")) > +} > + > +fn files() -> Vec { nit: in PBS and PDM, the files() helper generates also the list of additional, product specific file groups. So might be better to call this product_common_files() or product_agnostic_files()? > + vec![( > + "General System Info", > + vec![ > + "/etc/hostname", > + "/etc/hosts", > + "/etc/network/interfaces", > + "/etc/apt/sources.list", > + "/etc/apt/sources.list.d/", > + "/proc/pressure/", > + ], > + )] > +} > + > +fn commands() -> Vec { nit: same as for files(), the command() helper should reflect that these are product common commands, so maybe rename this to product_common_commands()? > + vec![ > + ("proxmox-boot-tool", vec!["status"]), > + ("df", vec!["-h", "-T"]), > + ( > + "lsblk", > + vec![ > + "--ascii", > + "-M", > + "-o", > + "+HOTPLUG,ROTA,PHY-SEC,FSTYPE,MODEL,TRAN", > + ], > + ), > + ("bash", vec!["-c", "ls -l /dev/disk/by-*/"]), > + ("zpool", vec!["status"]), > + ("zfs", vec!["list"]), > + ("zarcstat", vec![]), > + ("dmidecode", vec!["-t", "bios"]), > + ("lscpu", vec![]), > + ("lspci", vec!["-nnk"]), > + ("ip", vec!["-details", "-statistics", "a"]), > + ("ip", vec!["-4", "route", "show"]), > + ("ip", vec!["-6", "route", "show"]), comment: these commands depend on external executables. So there should be a dependency in debian/control as well. > + ] > +} > + > +fn dynamic_commands() -> Vec { nit: same as files() and commands() above ... > + let mut commands = Vec::new(); > + > + match proxmox_network_api::config() { > + Ok((config, _)) => { > + for (name, iface) in config.interfaces { > + if iface.interface_type == NetworkInterfaceType::Eth { > + commands.push(("ethtool", vec![name])); > + } > + } > + } > + Err(err) => { > + eprintln!("failed to query network interfaces: {err}"); > + } > + } > + > + commands > +} > + > +fn function_calls() -> Vec { nit: same as for others above, this are the product_common_function_calls() > + vec![("System Load & Uptime", get_top_processes)] > +} > + > +fn get_file_content(file: impl AsRef) -> String { > + use proxmox_sys::fs::file_read_optional_string; > + let content = match file_read_optional_string(&file) { > + Ok(Some(content)) => content, > + Ok(None) => String::from("# file does not exist"), > + Err(err) => err.to_string(), > + }; > + let file_name = file.as_ref().display(); > + format!("`$ cat '{file_name}'`\n```\n{}\n```", content.trim_end()) > +} > + > +fn get_directory_content(path: impl AsRef) -> String { > + let read_dir_iter = match std::fs::read_dir(&path) { > + Ok(iter) => iter, > + Err(err) => { > + return format!( > + "`$ cat '{}*'`\n```\n# read dir failed - {err}\n```", > + path.as_ref().display(), > + ); > + } > + }; > + let mut out = String::new(); > + let mut first = true; > + for entry in read_dir_iter { > + let entry = match entry { > + Ok(entry) => entry, > + Err(err) => { > + let _ = writeln!(out, "error during read-dir - {err}"); > + continue; > + } > + }; > + let path = entry.path(); > + if path.is_file() { > + if first { > + let _ = writeln!(out, "{}", get_file_content(path)); > + first = false; > + } else { > + let _ = writeln!(out, "\n{}", get_file_content(path)); > + } > + } else { > + let _ = writeln!(out, "skipping sub-directory `{}`", path.display()); > + } > + } > + out > +} > + > +fn get_command_output(exe: &str, args: &[&str]) -> String { > + let output = Command::new(exe) > + .env("PROXMOX_OUTPUT_NO_BORDER", "1") > + .args(args) > + .output(); > + let output = match output { > + Ok(output) => { > + let mut out = String::from_utf8_lossy(&output.stdout) > + .trim_end() > + .to_string(); > + let stderr = String::from_utf8_lossy(&output.stderr) > + .trim_end() > + .to_string(); > + if !stderr.is_empty() { > + let _ = writeln!(out, "\n```\nSTDERR:\n```\n{stderr}"); > + } > + out > + } > + Err(err) => err.to_string(), > + }; > + format!("$ `{exe} {}`\n```\n{output}\n```", args.join(" ")) > +} > + > +/// Generate a system report as a Markdown-like document. > +/// > +/// The report has three top-level sections, always emitted in this order: > +/// `FILES`, `COMMANDS` and `FUNCTIONS`. The project-specific entries passed in > +/// are merged with a set of built-in, general-purpose entries common to all > +/// products. > +/// > +/// Missing files, unreadable directories and commands that fail to spawn are > +/// reported inline instead of aborting the report. > +pub fn generate_report( > + project_files: Vec, > + project_commands: Vec, > + project_function_calls: Vec, > +) -> String { > + // We deliberately output the shared files before the project specific files > + // to preserve the legacy report ordering. > + let file_contents = files() > + .iter() > + .chain(&project_files) > + .map(|group| { > + let (group, files) = group; > + let group_content = files > + .iter() > + .map(|file_name| { > + let path = Path::new(file_name); > + if path.is_dir() { > + get_directory_content(path) > + } else { > + get_file_content(file_name) > + } > + }) > + .collect::>() > + .join("\n\n"); > + > + format!("### {group}\n\n{group_content}") > + }) > + .collect::>() > + .join("\n\n"); > + > + let static_command_outputs = project_commands > + .into_iter() > + .chain(commands()) > + .map(|(command, args)| get_command_output(command, &args)); > + > + let dynamic_command_outputs = dynamic_commands().into_iter().map(|(command, args)| { > + let args: Vec<_> = args.iter().map(String::as_str).collect(); > + get_command_output(command, &args) > + }); > + > + let command_outputs = static_command_outputs > + .chain(dynamic_command_outputs) > + .collect::>() > + .join("\n\n"); > + > + let function_outputs = project_function_calls > + .iter() > + .chain(&function_calls()) > + .map(|(desc, function)| { > + let output = function(); > + format!("#### {desc}\n{}\n", output.trim_end()) > + }) > + .collect::>() > + .join("\n\n"); > + > + format!( > + "## FILES\n\n{file_contents}\n## COMMANDS\n\n{command_outputs}\n## FUNCTIONS\n\n{function_outputs}\n" > + ) > +}