Re: [PATCH proxmox-backup 2/3] report: use shared proxmox-system-report crate

[Christian Ebner <c.ebner@proxmox.com>]

On 7/14/26 10:31 AM, Erik Fastermann wrote:
> Move the common report logic to the proxmox-system-report crate and
> keep only the PBS specific files, commands and the Datastores
> function here.
> 
> User-visible output changes:
> - df now also shows the filesystem type (df -h -T), matching PDM
> - adds the IPv4 and IPv6 route tables (ip -4/-6 route show),
>    previously PDM only
> 
> The section order (FILES, COMMANDS, FUNCTIONS) and all product
> specific entries are unchanged.
> 
> Suggested-by: Lukas Wagner <l.wagner@proxmox.com>
> Suggested-by: Christian Ebner <c.ebner@proxmox.com>
> Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
> ---
>   Cargo.toml           |   2 +
>   src/server/report.rs | 213 +++----------------------------------------
>   2 files changed, 15 insertions(+), 200 deletions(-)
> 
> diff --git a/Cargo.toml b/Cargo.toml
> index a625370cf..fb7904d38 100644
> --- a/Cargo.toml
> +++ b/Cargo.toml
> @@ -94,6 +94,7 @@ proxmox-sortable-macro = "1"
>   proxmox-subscription = { version = "1.0.2", features = [ "api-types" ] }
>   proxmox-syslog-api = { version = "1.1", features = [ "impl" ] }
>   proxmox-sys = "1"
> +proxmox-system-report = "0.1"

nit: as motivated in patch 1, lets start with 1.0 here.... also, this 
requires a build dependency in debian/control on the corresponding dev 
package, so it is pulled in by apt.

>   proxmox-systemd = "1.0.1"
>   proxmox-tfa = { version = "6.0.3", features = [ "api", "api-types" ] }
>   proxmox-time = "2"
> @@ -252,6 +253,7 @@ proxmox-sortable-macro.workspace = true
>   proxmox-subscription.workspace = true
>   proxmox-syslog-api.workspace = true
>   proxmox-sys = { workspace = true, features = [ "timer" ] }
> +proxmox-system-report.workspace = true
>   proxmox-systemd.workspace = true
>   proxmox-tfa.workspace = true
>   proxmox-time.workspace = true
> diff --git a/src/server/report.rs b/src/server/report.rs
> index 297718d2e..442a8573d 100644
> --- a/src/server/report.rs
> +++ b/src/server/report.rs
> @@ -1,33 +1,7 @@
> -use std::fmt::Write;
> -use std::path::Path;
> -use std::process::Command;
> +use proxmox_system_report::{CommandSpec, FileGroup, FunctionMapping};
>   
> -use proxmox_network_api::NetworkInterfaceType;
> -
> -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::<Vec<&str>>().join("\n");
> -    format!("$ `{exe} {}`\n```\n{output}\n```", args.join(" "))
> -}
> -
> -fn files() -> Vec<(&'static str, Vec<&'static str>)> {
> +fn files() -> Vec<FileGroup> {

nit: maybe better call this pbs_specific_files() or 
product_specific_files(), just to make it clearer that this is only part 
of what's to be generated.

>       vec![
> -        (
> -            "General System Info",
> -            vec![
> -                "/etc/hostname",
> -                "/etc/hosts",
> -                "/etc/network/interfaces",
> -                "/etc/apt/sources.list",
> -                "/etc/apt/sources.list.d/",
> -                "/proc/pressure/",
> -            ],
> -        ),
>           (
>               "Datastores & Remotes",
>               vec!["/etc/proxmox-backup/datastore.cfg"],
> @@ -65,192 +39,31 @@ fn files() -> Vec<(&'static str, Vec<&'static str>)> {
>       ]
>   }
>   
> -fn commands() -> Vec<(&'static str, Vec<&'static str>)> {
> +fn commands() -> Vec<CommandSpec> {

nit: same as above for files()

>       vec![
> -        //  ("<command>", vec![<arg [, arg]>])
>           ("date", vec!["-R"]),
>           ("proxmox-backup-manager", vec!["versions", "--verbose"]),
>           ("proxmox-backup-manager", vec!["subscription", "get"]),
>           ("proxmox-backup-manager", vec!["ldap", "list"]),
>           ("proxmox-backup-manager", vec!["openid", "list"]),
> -        ("proxmox-boot-tool", vec!["status"]),
> -        ("df", vec!["-h"]),
> -        (
> -            "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"]),
>       ]
>   }
>   
> -fn dynamic_commands() -> Vec<(&'static str, Vec<String>)> {
> -    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
> -}
> -
> -// (description, function())
> -type FunctionMapping = (&'static str, fn() -> String);
> -
>   fn function_calls() -> Vec<FunctionMapping> {

nit: same as above for files() and commands()...

> -    vec![
> -        ("Datastores", || {
> -            let config = match pbs_config::datastore::config() {
> -                Ok((config, _digest)) => config,
> -                _ => return String::from("could not read datastore config"),
> -            };
> -
> -            let mut list = Vec::new();
> -            for store in config.sections.keys() {
> -                list.push(store.as_str());
> -            }
> -            format!("```\n{}\n```", list.join(", "))
> -        }),
> -        ("System Load & Uptime", get_top_processes),
> -    ]
> -}
> -
> -fn get_file_content(file: impl AsRef<Path>) -> 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<Path>) -> 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;
> -            }
> +    vec![("Datastores", || {
> +        let config = match pbs_config::datastore::config() {
> +            Ok((config, _digest)) => config,
> +            _ => return String::from("could not read datastore config"),
>           };
> -        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: &Vec<&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
> +        let mut list = Vec::new();
> +        for store in config.sections.keys() {
> +            list.push(store.as_str());
>           }
> -        Err(err) => err.to_string(),
> -    };
> -    format!("$ `{exe} {}`\n```\n{output}\n```", args.join(" "))
> +        format!("```\n{}\n```", list.join(", "))
> +    })]
>   }
>   
>   pub fn generate_report() -> String {
> -    let file_contents = files()
> -        .iter()
> -        .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::<Vec<String>>()
> -                .join("\n\n");
> -
> -            format!("### {group}\n\n{group_content}")
> -        })
> -        .collect::<Vec<String>>()
> -        .join("\n\n");
> -
> -    let static_command_outputs = commands()
> -        .into_iter()
> -        .map(|(command, args)| get_command_output(command, &args));
> -
> -    let dynamic_command_outputs = dynamic_commands().into_iter().map(|(command, args)| {
> -        let args = args.iter().map(String::as_str).collect();
> -        get_command_output(command, &args)
> -    });
> -
> -    let command_outputs = static_command_outputs
> -        .chain(dynamic_command_outputs)
> -        .collect::<Vec<String>>()
> -        .join("\n\n");
> -
> -    let function_outputs = function_calls()
> -        .iter()
> -        .map(|(desc, function)| {
> -            let output = function();
> -            format!("#### {desc}\n{}\n", output.trim_end())
> -        })
> -        .collect::<Vec<String>>()
> -        .join("\n\n");
> -
> -    format!(
> -        "## FILES\n\n{file_contents}\n## COMMANDS\n\n{command_outputs}\n## FUNCTIONS\n\n{function_outputs}\n"
> -    )
> +    proxmox_system_report::generate_report(files(), commands(), function_calls())
>   }