[pbs-devel] [PATCH proxmox-backup] add version API call
Stefan Reiter
s.reiter at proxmox.com
Wed Nov 4 14:08:27 CET 2020
In general, you should only call list_installed_apt_packages once, since
it has to iterate the entire installed package list every time - which
is slow, putting '--verbose' takes noticeable longer than without for no
good reason.
I would add 'proxmox-backup' and 'proxmox-backup-server' to PACKAGES as
well, then run list_apt_packages once, something like:
let pbs_packages = tools::apt::list_installed_apt_packages(
|fd|
(
// only package versions that are actually installed
fd.installed_version.is_some() &&
fd.active_version == fd.installed_version.unwrap()
) && (
fd.package.starts_with("pve-kernel") ||
PACKAGES.contains(&fd.package)
),
None,
);
(completely off the top of my head :) )
Then only operate on that list, which should contain everything.
Some other stuff inline too.
On 11/4/20 12:41 PM, Mira Limbeck wrote:
> Adds an API call that returns all relevant packages, same as the version
> API call in PVE.
>
> In addition extens proxmox-backup-manager with the 'version' command
> that prints the packages in a format similar to pveversion.
>
> Signed-off-by: Mira Limbeck <m.limbeck at proxmox.com>
> ---
> src/api2/node.rs | 148 +++++++++++++++++++++++++++++-
> src/api2/types/mod.rs | 13 +++
> src/bin/proxmox-backup-manager.rs | 40 +++++++-
> src/tools/apt.rs | 3 +
> 4 files changed, 202 insertions(+), 2 deletions(-)
>
> diff --git a/src/api2/node.rs b/src/api2/node.rs
> index a19bea7e..8b0b439a 100644
> --- a/src/api2/node.rs
> +++ b/src/api2/node.rs
> @@ -19,7 +19,7 @@ use proxmox::tools::websocket::WebSocket;
> use proxmox::{identity, sortable};
>
> use crate::api2::types::*;
> -use crate::config::acl::PRIV_SYS_CONSOLE;
> +use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_SYS_CONSOLE};
> use crate::server::WorkerTask;
> use crate::tools;
> use crate::tools::ticket::{self, Empty, Ticket};
> @@ -305,6 +305,151 @@ fn upgrade_to_websocket(
> .boxed()
> }
>
> +#[api(
> + input: {
> + properties: {
> + node: {
> + schema: NODE_SCHEMA,
> + },
> + verbose: {
> + description: "Enable verbose output.",
> + type: Boolean,
> + optional: true,
> + default: false,
> + },
> + },
> + },
> + returns: {
> + description: "List of packages and their installed version.",
> + type: Array,
> + items: {
> + type: PackageInfo,
> + },
> + },
> + access: {
> + permission: &Permission::Privilege(&[], PRIV_SYS_AUDIT, false),
> + },
> +)]
> +/// Get package information for important Proxmox packages.
> +pub fn get_version(verbose: bool) -> Result<Value, Error> {
> + const PACKAGES: [&str; 9] = [
> + "ifupdown2",
> + "libjs-extjs",
> + "proxmox-backup-docs",
> + "proxmox-backup-client",
> + "proxmox-mini-journalreader",
> + "proxmox-widget-toolkit",
> + "pve-xtermjs",
> + "smartmontools",
> + "zfsutils-linux",
> + ];
> +
> + let running_kernel = nix::sys::utsname::uname().release().to_owned();
> + let mut packages: Vec<PackageInfo> = Vec::new();
> + let proxmox_backup =
> + tools::apt::list_installed_apt_packages(|fd| fd.package == "proxmox-backup", None);
> + packages.push(
> + proxmox_backup
> + .iter()
> + .map(|pkg| PackageInfo {
> + name: pkg.package.clone(),
> + version: pkg.old_version.clone(),
> + comment: Some(format!("running kernel: {}", &running_kernel)),
> + })
> + .next()
> + .unwrap_or(PackageInfo {
> + name: "proxmox-backup".to_owned(),
> + version: "not correctly installed".to_owned(),
> + comment: Some(format!("running kernel: {}", &running_kernel)),
> + }),
> + );
> +
> + if !verbose {
> + return Ok(json!(packages));
> + }
> +
> + let proxmox_backup_server =
> + tools::apt::list_installed_apt_packages(|fd| fd.package == "proxmox-backup-server", None);
> + packages.push(
> + proxmox_backup_server
> + .iter()
> + .map(|pkg| PackageInfo {
> + name: pkg.package.clone(),
> + version: pkg.old_version.clone(),
> + comment: Some(format!("running version: {}", &pkg.version)),
'pkg.version' refers to the one that would be installed on upgrade. The
one running is always the one installed, so old_version is enough.
> + })
> + .next()
> + .unwrap_or(PackageInfo {
> + name: "proxmox-backup-server".to_owned(),
> + version: "not correctly installed".to_owned(),
> + comment: None,
> + }),
> + );
> +
> + let mut pve_kernel_pkgs = tools::apt::list_installed_apt_packages(
> + |fd| {
> + if fd.package.starts_with("pve-kernel") {
> + return true;
> + }
> + false
> + },
> + None,
> + );
> + let cache = apt_pkg_native::Cache::get_singleton();
Just a side note because it bit me before: Cache::get_singleton() locks
a mutex, so this only works because Rust is smart enough to drop 'cache'
before the next call to list_installed_apt_packages.
> + pve_kernel_pkgs.sort_by(|left, right| {
> + cache.compare_versions(&left.old_version, &right.old_version).reverse()
> + });
> +
> + for pkg in pve_kernel_pkgs.iter().map(|pkg| PackageInfo {
> + name: pkg.package.clone(),
> + version: if pkg.old_version.is_empty() {
> + "not correctly installed".to_owned()
This would currently only trigger for new kernel packages that are
released but not yet installed. It should not sound like an error IMO.
> + } else {
> + pkg.old_version.clone()
> + },
> + comment: None,
> + }) {
> + packages.push(pkg);
> + }
> +
> + let mut pbs_pkgs = tools::apt::list_installed_apt_packages(
> + |fd| {
> + if PACKAGES.contains(&fd.package) {
> + return true;
> + }
> + false
> + },
> + None,
> + );
> + pbs_pkgs.sort_by(|left, right| left.package.cmp(&right.package));
> +
> + // add packages we're interested in, but are not installed
> + // and the installed ones returned by list_installed_apt_packages
> + for pkg in PACKAGES.iter() {
> + let apt_pkg = pbs_pkgs.iter().find(|item| &item.package == pkg);
> + if apt_pkg.is_some() {
> + let apt_pkg = apt_pkg.unwrap();
> + packages.push(PackageInfo {
> + name: apt_pkg.package.clone(),
> + version: if apt_pkg.old_version.is_empty() {
> + "not correctly installed".to_owned()
I'd put a different message here, to distinguish from the 'else' case below.
Though both this and the kernel case above cannot happen if you use the
filter Fn from my proposal, since it only returns installed package
versions.
> + } else {
> + apt_pkg.old_version.clone()
> + },
> + comment: None,
> + });
> + } else {
> + packages.push(PackageInfo {
> + name: pkg.to_string(),
> + version: "not correctly installed".to_owned(),
> + comment: None,
> + });
> + }
> + }
> +
> + Ok(json!(packages))
> +}
> +
> pub const SUBDIRS: SubdirMap = &[
> ("apt", &apt::ROUTER),
> ("disks", &disks::ROUTER),
> @@ -320,6 +465,7 @@ pub const SUBDIRS: SubdirMap = &[
> ("tasks", &tasks::ROUTER),
> ("termproxy", &Router::new().post(&API_METHOD_TERMPROXY)),
> ("time", &time::ROUTER),
> + ("version", &Router::new().get(&API_METHOD_GET_VERSION)),
> (
> "vncwebsocket",
> &Router::new().upgrade(&API_METHOD_WEBSOCKET),
> diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
> index 7ee89f57..f52b1d37 100644
> --- a/src/api2/types/mod.rs
> +++ b/src/api2/types/mod.rs
> @@ -1155,6 +1155,19 @@ pub struct APTUpdateInfo {
> pub change_log_url: String,
> }
>
> +#[api()]
> +#[derive(Serialize, Deserialize)]
> +#[serde(rename_all = "PascalCase")]
> +/// Pair of package name and version with optional comment.
> +pub struct PackageInfo {
> + /// Package name
> + pub name: String,
> + /// Package version
> + pub version: String,
> + /// Optional comment
> + pub comment: Option<String>,
> +}
> +
> #[api()]
> #[derive(Debug, Copy, Clone, PartialEq, Serialize, Deserialize)]
> #[serde(rename_all = "lowercase")]
> diff --git a/src/bin/proxmox-backup-manager.rs b/src/bin/proxmox-backup-manager.rs
> index 7499446b..bdfdfdfa 100644
> --- a/src/bin/proxmox-backup-manager.rs
> +++ b/src/bin/proxmox-backup-manager.rs
> @@ -363,6 +363,42 @@ async fn report() -> Result<Value, Error> {
> Ok(Value::Null)
> }
>
> +#[api(
> + input: {
> + properties: {
> + verbose: {
> + description: "Enable verbose output.",
> + type: Boolean,
> + optional: true,
> + default: false,
> + },
> + },
> + },
> +)]
> +/// Get package information for important Proxmox packages.
> +async fn get_version(param: Value) -> Result<Value, Error> {
> + let client = connect()?;
> +
> + let node = proxmox::tools::nodename();
> +
> + let path = format!("api2/json/nodes/{}/version", node);
> +
> + let mut result = client.get(&path, Some(param)).await?;
> +
> + let data = result["data"].take();
> + let packages: Vec<PackageInfo> = serde_json::from_value(data)?;
> +
> + for pkg in packages {
> + if pkg.comment.is_some() {
> + println!("{}: {} ({})", pkg.name, pkg.version, pkg.comment.unwrap());
> + } else {
> + println!("{}: {}", pkg.name, pkg.version);
> + }
> + }
> +
> + Ok(Value::Null)
> +}
> +
> fn main() {
>
> proxmox_backup::tools::setup_safe_path_env();
> @@ -396,7 +432,9 @@ fn main() {
> )
> .insert("report",
> CliCommand::new(&API_METHOD_REPORT)
> - );
> + )
> + .insert("version",
> + CliCommand::new(&API_METHOD_GET_VERSION));
>
>
>
> diff --git a/src/tools/apt.rs b/src/tools/apt.rs
> index 5800e0a2..fab8f998 100644
> --- a/src/tools/apt.rs
> +++ b/src/tools/apt.rs
> @@ -136,6 +136,8 @@ fn get_changelog_url(
> }
>
> pub struct FilterData<'a> {
> + // package name
> + pub package: &'a str,
> // this is version info returned by APT
> pub installed_version: Option<&'a str>,
> pub candidate_version: &'a str,
> @@ -270,6 +272,7 @@ where
> let mut long_desc = "".to_owned();
>
> let fd = FilterData {
> + package: package.as_str(),
> installed_version: current_version.as_deref(),
> candidate_version: &candidate_version,
> active_version: &version,
>
More information about the pbs-devel
mailing list