[pbs-devel] [RFC PATCH v2 proxmox-backup 3/3] client: Add `--protected` CLI flag to backup command
Christoph Heiss
c.heiss at proxmox.com
Wed Jan 25 13:18:58 CET 2023
This implements setting the backup as protected after finishing it,
either using the newly introduced API parameter on the `/finish`
endpoint, falling back to the "old" way using the separate endpoint,
thus supporting this in a backwards-compatible way.
The same caveat as in bug #4289 [0] applies here as well: If the
datastore has "Verify New Snapshots" set, the latter way might fail to
set the backup as protected.
[0] https://bugzilla.proxmox.com/show_bug.cgi?id=4289
Signed-off-by: Christoph Heiss <c.heiss at proxmox.com>
---
Note: BackupWriter::start() actually does fine with just a &HttpClient,
since HttpClient::start_h2_connection() then .clone()'s it anyway and
not consume it.
Changes v1 -> v2:
* Moved BackupWriter::finish adaption to this patch
* Only set API parameter if available using new server feature mechanism
* Implement best-effort backwards-compatible way for older server
examples/upload-speed.rs | 2 +-
pbs-client/src/backup_writer.rs | 6 +--
pbs-client/src/tools/mod.rs | 25 ++++++++-
proxmox-backup-client/src/benchmark.rs | 2 +-
proxmox-backup-client/src/main.rs | 72 ++++++++++++++++++++------
5 files changed, 86 insertions(+), 21 deletions(-)
diff --git a/examples/upload-speed.rs b/examples/upload-speed.rs
index f9fc52a8..e4b570ec 100644
--- a/examples/upload-speed.rs
+++ b/examples/upload-speed.rs
@@ -18,7 +18,7 @@ async fn upload_speed() -> Result<f64, Error> {
let backup_time = proxmox_time::epoch_i64();
let client = BackupWriter::start(
- client,
+ &client,
None,
datastore,
&BackupNamespace::root(),
diff --git a/pbs-client/src/backup_writer.rs b/pbs-client/src/backup_writer.rs
index be6da2a6..4ecc6622 100644
--- a/pbs-client/src/backup_writer.rs
+++ b/pbs-client/src/backup_writer.rs
@@ -76,7 +76,7 @@ impl BackupWriter {
// FIXME: extract into (flattened) parameter struct?
#[allow(clippy::too_many_arguments)]
pub async fn start(
- client: HttpClient,
+ client: &HttpClient,
crypt_config: Option<Arc<CryptConfig>>,
datastore: &str,
ns: &BackupNamespace,
@@ -165,10 +165,10 @@ impl BackupWriter {
self.h2.upload("PUT", path, param, content_type, data).await
}
- pub async fn finish(self: Arc<Self>) -> Result<(), Error> {
+ pub async fn finish(self: Arc<Self>, param: Option<Value>) -> Result<(), Error> {
let h2 = self.h2.clone();
- h2.post("finish", None)
+ h2.post("finish", param)
.map_ok(move |_| {
self.abort.abort();
})
diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
index fa1d5092..140ef9c7 100644
--- a/pbs-client/src/tools/mod.rs
+++ b/pbs-client/src/tools/mod.rs
@@ -15,7 +15,9 @@ use proxmox_router::cli::{complete_file_name, shellword_split};
use proxmox_schema::*;
use proxmox_sys::fs::file_get_json;
-use pbs_api_types::{Authid, BackupNamespace, RateLimitConfig, UserWithTokens, BACKUP_REPO_URL};
+use pbs_api_types::{
+ Authid, BackupNamespace, RateLimitConfig, ServerFeature, UserWithTokens, BACKUP_REPO_URL,
+};
use crate::{BackupRepository, HttpClient, HttpClientOptions};
@@ -502,6 +504,27 @@ pub async fn complete_namespace_do(
result
}
+/// Returns a list of (backwards-incompatible) features supported by the server.
+pub async fn get_supported_server_features(client: &HttpClient) -> Vec<ServerFeature> {
+ match client.get("api2/json/version", None).await {
+ Ok(mut result) if result["data"]["features"].is_array() => {
+ match serde_json::from_value::<Vec<ServerFeature>>(result["data"]["features"].take()) {
+ Ok(features) => features,
+ Err(e) => {
+ log::warn!("failed to deserialize feature list: {}", e);
+ vec![]
+ }
+ }
+ }
+ // Old server with no feature advertising yet.
+ Ok(_) => vec![],
+ Err(e) => {
+ log::error!("could not connect to server - {}", e);
+ vec![]
+ }
+ }
+}
+
pub fn base_directories() -> Result<xdg::BaseDirectories, Error> {
xdg::BaseDirectories::with_prefix("proxmox-backup").map_err(Error::from)
}
diff --git a/proxmox-backup-client/src/benchmark.rs b/proxmox-backup-client/src/benchmark.rs
index b3047308..1262fb46 100644
--- a/proxmox-backup-client/src/benchmark.rs
+++ b/proxmox-backup-client/src/benchmark.rs
@@ -229,7 +229,7 @@ async fn test_upload_speed(
log::debug!("Connecting to backup server");
let client = BackupWriter::start(
- client,
+ &client,
crypt_config.clone(),
repo.store(),
&BackupNamespace::root(),
diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
index 55198108..bab67207 100644
--- a/proxmox-backup-client/src/main.rs
+++ b/proxmox-backup-client/src/main.rs
@@ -25,15 +25,16 @@ use pxar::accessor::{MaybeReady, ReadAt, ReadAtOperation};
use pbs_api_types::{
Authid, BackupDir, BackupGroup, BackupNamespace, BackupPart, BackupType, CryptMode,
Fingerprint, GroupListItem, HumanByte, PruneJobOptions, PruneListItem, RateLimitConfig,
- SnapshotListItem, StorageStatus, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA, BACKUP_TIME_SCHEMA,
- BACKUP_TYPE_SCHEMA, TRAFFIC_CONTROL_BURST_SCHEMA, TRAFFIC_CONTROL_RATE_SCHEMA,
+ ServerFeature, SnapshotListItem, StorageStatus, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA,
+ BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA, TRAFFIC_CONTROL_BURST_SCHEMA,
+ TRAFFIC_CONTROL_RATE_SCHEMA,
};
use pbs_client::catalog_shell::Shell;
use pbs_client::tools::{
complete_archive_name, complete_auth_id, complete_backup_group, complete_backup_snapshot,
complete_backup_source, complete_chunk_size, complete_group_or_snapshot,
complete_img_archive_name, complete_namespace, complete_pxar_archive_name, complete_repository,
- connect, connect_rate_limited, extract_repository_from_value,
+ connect, connect_rate_limited, extract_repository_from_value, get_supported_server_features,
key_source::{
crypto_parameters, format_key_source, get_encryption_key_password, KEYFD_SCHEMA,
KEYFILE_SCHEMA, MASTER_PUBKEY_FD_SCHEMA, MASTER_PUBKEY_FILE_SCHEMA,
@@ -663,6 +664,12 @@ fn spawn_catalog_upload(
optional: true,
default: false,
},
+ "protected": {
+ type: Boolean,
+ description: "Set backup as protected after upload.",
+ optional: true,
+ default: false,
+ },
}
}
)]
@@ -716,6 +723,7 @@ async fn create_backup(
let empty = Vec::new();
let exclude_args = param["exclude"].as_array().unwrap_or(&empty);
+ let protected = param["protected"].as_bool().unwrap_or(false);
let mut pattern_list = Vec::with_capacity(exclude_args.len());
for entry in exclude_args {
@@ -837,6 +845,9 @@ async fn create_backup(
log::info!("Client name: {}", proxmox_sys::nodename());
+ let server_features = get_supported_server_features(&client).await;
+ log::debug!("Supported server features: {:?}", server_features);
+
let start_time = std::time::Instant::now();
log::info!(
@@ -876,8 +887,8 @@ async fn create_backup(
}
};
- let client = BackupWriter::start(
- client,
+ let writer = BackupWriter::start(
+ &client,
crypt_config.clone(),
repo.store(),
&backup_ns,
@@ -887,7 +898,7 @@ async fn create_backup(
)
.await?;
- let download_previous_manifest = match client.previous_backup_time().await {
+ let download_previous_manifest = match writer.previous_backup_time().await {
Ok(Some(backup_time)) => {
log::info!(
"Downloading previous manifest ({})",
@@ -906,7 +917,7 @@ async fn create_backup(
};
let previous_manifest = if download_previous_manifest {
- match client.download_previous_manifest().await {
+ match writer.download_previous_manifest().await {
Ok(previous_manifest) => {
match previous_manifest.check_fingerprint(crypt_config.as_ref().map(Arc::as_ref)) {
Ok(()) => Some(Arc::new(previous_manifest)),
@@ -951,7 +962,7 @@ async fn create_backup(
};
log_file("config file", &filename, &target);
- let stats = client
+ let stats = writer
.upload_blob_from_file(&filename, &target, upload_options)
.await?;
manifest.add_file(target, stats.size, stats.csum, crypto.mode)?;
@@ -965,7 +976,7 @@ async fn create_backup(
};
log_file("log file", &filename, &target);
- let stats = client
+ let stats = writer
.upload_blob_from_file(&filename, &target, upload_options)
.await?;
manifest.add_file(target, stats.size, stats.csum, crypto.mode)?;
@@ -974,7 +985,7 @@ async fn create_backup(
// start catalog upload on first use
if catalog.is_none() {
let catalog_upload_res =
- spawn_catalog_upload(client.clone(), crypto.mode == CryptMode::Encrypt)?;
+ spawn_catalog_upload(writer.clone(), crypto.mode == CryptMode::Encrypt)?;
catalog = Some(catalog_upload_res.catalog_writer);
catalog_result_rx = Some(catalog_upload_res.result);
}
@@ -1001,7 +1012,7 @@ async fn create_backup(
};
let stats = backup_directory(
- &client,
+ &writer,
&filename,
&target,
chunk_size_opt,
@@ -1024,7 +1035,7 @@ async fn create_backup(
};
let stats =
- backup_image(&client, &filename, &target, chunk_size_opt, upload_options)
+ backup_image(&writer, &filename, &target, chunk_size_opt, upload_options)
.await?;
manifest.add_file(target, stats.size, stats.csum, crypto.mode)?;
}
@@ -1060,7 +1071,7 @@ async fn create_backup(
encrypt: false,
..UploadOptions::default()
};
- let stats = client
+ let stats = writer
.upload_blob_from_data(rsa_encrypted_key, target, options)
.await?;
manifest.add_file(target.to_string(), stats.size, stats.csum, crypto.mode)?;
@@ -1078,11 +1089,42 @@ async fn create_backup(
encrypt: false,
..UploadOptions::default()
};
- client
+ writer
.upload_blob_from_data(manifest.into_bytes(), MANIFEST_BLOB_NAME, options)
.await?;
- client.finish().await?;
+
+ let mut params = json!({});
+
+ let has_protected_param = server_features.contains(&ServerFeature::FinishHasProtectedParam);
+ if has_protected_param {
+ params["protected"] = json!(protected);
+ }
+
+ writer.finish(Some(params)).await?;
+
+ // In case the server does not support the new `protected` parameter and the user has requested
+ // to set the backup as protected, fall back to the separate API endpoint as a "best-effort"
+ // solution. This might fail for larger backups if the datastore has "Verify New Snapshots"
+ // enabled.
+ if !has_protected_param && protected {
+ let path = format!("api2/json/admin/datastore/{}/protected", repo.store());
+
+ let mut params = json!({
+ "backup-id": backup_id,
+ "backup-time": backup_time,
+ "backup-type": backup_type,
+ "protected": protected,
+ });
+
+ if backup_ns.is_root() {
+ params["ns"] = json!(backup_ns);
+ }
+
+ if let Err(e) = client.put(&path, Some(params)).await {
+ log::warn!("Unable to set backup as protected: {}", e);
+ }
+ }
let end_time = std::time::Instant::now();
let elapsed = end_time.duration_since(start_time);
--
2.34.1
More information about the pbs-devel
mailing list