From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 879751FF0A7 for ; Wed, 16 Sep 2026 11:58:57 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 5233D212D9; Wed, 16 Sep 2026 11:58:57 +0200 (CEST) From: Robert Obkircher To: pbs-devel@lists.proxmox.com Subject: [RFC proxmox-backup] tests: datastore pull sync Date: Wed, 16 Sep 2026 11:42:11 +0200 Message-ID: <20260916094315.108422-1-r.obkircher@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1789552727543 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.538 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) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium 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: GTW6XH24WM6ILMLYHCMTNU6IO6D5JD3T X-Message-ID-Hash: GTW6XH24WM6ILMLYHCMTNU6IO6D5JD3T X-MailFrom: r.obkircher@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 Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: This is a very rough draft of an integration test that could quickly check whether pull-sync copies snapshots correctly. It was just an experiment to see what it would take, and I'm not really planning on developing this further. The main problem that would have to be solved is faking the global state read by CachedUserInfo and the hard-coded directory used for chunk locks. There are also further TODO comments inline. Signed-off-by: Robert Obkircher --- Cargo.toml | 3 + pbs-config/src/cached_user_info.rs | 6 ++ pbs-datastore/src/backup_info.rs | 3 +- src/server/mod.rs | 1 + src/server/pull.rs | 34 +++++++ src/server/sync.rs | 1 + tests/datastore.rs | 157 +++++++++++++++++++++++++++++ 7 files changed, 204 insertions(+), 1 deletion(-) create mode 100644 tests/datastore.rs diff --git a/Cargo.toml b/Cargo.toml index f3b67ba79..fd8597d62 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -174,6 +174,9 @@ xdg = "2.2" zstd = "0.13" zstd-safe = "7" +[dev-dependencies] +tempfile.workspace = true + [dependencies] anyhow.workspace = true async-trait.workspace = true diff --git a/pbs-config/src/cached_user_info.rs b/pbs-config/src/cached_user_info.rs index 511c8f160..c88dee5d3 100644 --- a/pbs-config/src/cached_user_info.rs +++ b/pbs-config/src/cached_user_info.rs @@ -38,6 +38,12 @@ static CACHED_CONFIG: LazyLock> = LazyLock::new(|| { impl CachedUserInfo { /// Returns a cached instance (up to 5 seconds old). pub fn new() -> Result, Error> { + // TODO why does this even work? + return Ok(Arc::new(Self { + user_cfg: Arc::new(SectionConfigData::new()), + acl_tree: Arc::new(AclTree::new()), + })); + let now = epoch_i64(); let version_cache = ConfigVersionCache::new()?; diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs index be4ec8b3e..0ff05c726 100644 --- a/pbs-datastore/src/backup_info.rs +++ b/pbs-datastore/src/backup_info.rs @@ -27,7 +27,8 @@ use crate::move_journal; use crate::s3::S3_CONTENT_PREFIX; use crate::{DataBlob, DataStore, DatastoreBackend}; -pub const DATASTORE_LOCKS_DIR: &str = "/run/proxmox-backup/locks"; +// TODO conditional compilation? +pub const DATASTORE_LOCKS_DIR: &str = "./run/proxmox-backup/locks"; pub const PROTECTED_MARKER_FILENAME: &str = ".protected"; proxmox_schema::const_regex! { diff --git a/src/server/mod.rs b/src/server/mod.rs index b89f657c8..332c1a4c1 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -36,6 +36,7 @@ pub mod auth; pub mod metric_collection; pub(crate) mod pull; +pub use pull::test_pull; pub(crate) mod push; pub(crate) mod sync; pub use sync::do_sync_job; diff --git a/src/server/pull.rs b/src/server/pull.rs index 4eb5bcf11..eba3d5c8a 100644 --- a/src/server/pull.rs +++ b/src/server/pull.rs @@ -169,6 +169,40 @@ impl PullParameters { } } +/// TODO: instead of exposing pub(crate) functionality like this, we could just +/// move the test here, because access to CARGO_TARGET_TEPDIR doesn't seem +/// worth it. +pub async fn test_pull(target: Arc, source: Arc) -> Result<(), Error> { + let source = Arc::new(LocalSource { + store: source, + ns: BackupNamespace::root(), + }); + + let backend = target.backend().unwrap(); + let target = PullTarget { + store: target, + ns: BackupNamespace::root(), + backend, + }; + + let parameters = PullParameters { + source, + target, + owner: Authid::root_auth_id().clone(), + remove_vanished: false, + max_depth: None, + group_filter: vec![], + transfer_last: None, + encrypted_only: false, + verified_only: false, + resync_corrupt: false, + worker_threads: None, + crypt_configs: vec![], + }; + + pull_store(parameters).await.map(|_| ()) +} + async fn pull_index_chunks( chunk_reader: Arc, target: Arc, diff --git a/src/server/sync.rs b/src/server/sync.rs index 11f30d318..8b73a13a9 100644 --- a/src/server/sync.rs +++ b/src/server/sync.rs @@ -555,6 +555,7 @@ impl SyncSource for LocalSource { .filter_map(|info| { let owner = match backup_group.get_owner() { Ok(owner) => owner, + // TODO should this really silently skip errors? Err(_) => return None, }; Some(backup_info_to_snapshot_list_item(info, &owner)) diff --git a/tests/datastore.rs b/tests/datastore.rs new file mode 100644 index 000000000..6f0391cf3 --- /dev/null +++ b/tests/datastore.rs @@ -0,0 +1,157 @@ +use std::collections::BTreeSet; +use std::fs; +use std::path::{Path, PathBuf}; +use std::sync::Arc; + +use nix::unistd::{Uid, User}; + +use pbs_api_types::DatastoreFSyncLevel; +use pbs_datastore::{BackupManifest, ChunkStore, DataStore, data_blob::DataChunkBuilder}; +use proxmox_backup::server::test_pull; + +// TODO: move to tests/tempdir/mod.rs so it can be used by all integration tests +mod tempdir { + use std::panic::Location; + + use tempfile::TempDir; + + /// Create a unique temporary directory in `/tmp` or `CARGO_TARGET_TMPDIR`. + /// + /// The directory name encodes the caller location to simplify debugging. + #[track_caller] + pub fn create() -> TempDir { + let caller = Location::caller(); + + // PathBuf can't be used here in case of cross compilation. + let file = caller.file().replace(['/', '\\'], "-"); + + // example: /tmp/proxmox-backup_tests-datastore.rs_66_15_uJbcgv + let package = env!("CARGO_PKG_NAME"); // TODO: this wouldn't work in a separate crate + let prefix = format!("{package}_{file}_{}_{}_", caller.line(), caller.column()); + + // try `env::temp_dir()` first because it is faster (1.5 s vs 6.4 s for a test that creates 2 datastores) + TempDir::with_prefix(&prefix) + .or_else(|_| TempDir::with_prefix_in(prefix, env!("CARGO_TARGET_TMPDIR"))) + .unwrap() + } +} + +fn create_local_data_store(parent_dir: &Path, name: &str) -> Arc { + let path = parent_dir.join(name); + + let user = proxmox_product_config::get_api_user(); + + let _ = ChunkStore::create(name, &path, user.uid, user.gid, DatastoreFSyncLevel::None).unwrap(); + + unsafe { DataStore::open_path(name, &path, None) }.expect("we just created it") +} + +fn try_init_logger() { + let _ = proxmox_log::Logger::from_env("PBS_LOG", proxmox_log::LevelFilter::INFO) + .stderr() // TODO: this prints the output even if the test succeeds + .init(); +} + +#[test] +fn test_pull_local() { + try_init_logger(); + let dir = tempdir::create(); + + let user = User::from_uid(Uid::current()).unwrap().unwrap(); + proxmox_product_config::init(user.clone(), user); + + let source = create_local_data_store(dir.path(), "source_store"); + let target = create_local_data_store(dir.path(), "target_store"); + + write_dummy_backup(source.clone()); + + proxmox_async::runtime::block_on(test_pull(target.clone(), source.clone())).unwrap(); + + compare_datastores(source, target); +} + +fn write_dummy_backup(store: Arc) { + let ns = BackupNamespace::root(); + let snapshot = BackupDir { + group: BackupGroup { + ty: BackupType::Host, + id: "devel".to_string(), + }, + time: 1788951590, + }; + let backup_dir = store.backup_dir(ns.clone(), snapshot.clone()).unwrap(); + fs::create_dir_all(backup_dir.full_path()).unwrap(); + + let authid = Authid::root_auth_id(); + store + .set_owner(&ns, &snapshot.group, authid, false) + .unwrap(); + + let backend = store.backend().unwrap(); + + let (blob, digest) = DataChunkBuilder::new(&[0; 3]).build().unwrap(); + let _ = store.insert_chunk(&blob, &digest, &backend).unwrap(); + + let fidx_path = backup_dir.full_path().join("test.img.fidx"); + let mut w = store + .create_fixed_writer(&fidx_path, Some(3), 4096) + .unwrap(); + w.add_chunk(0, 3, &digest).unwrap(); + let csum = w.close().unwrap(); + + use pbs_api_types::*; + let mut m = BackupManifest::new(snapshot.clone()); + let name = BackupArchiveName::from_path(fidx_path).unwrap(); + // TODO 3 and csum might not be the correct arguments + m.add_file(&name, 3, csum, CryptMode::None).unwrap(); + + store + .add_blob( + MANIFEST_BLOB_NAME.as_ref(), + backup_dir, + m.to_data_blob(None).unwrap(), + &backend, + ) + .unwrap(); +} + +fn compare_datastores(source: Arc, target: Arc) { + let s = list_all(&source.base_path()); + let t = list_all(&target.base_path()); + let s_extra: BTreeSet<_> = s.difference(&t).cloned().collect(); + let t_extra: BTreeSet<_> = t.difference(&s).cloned().collect(); + + let mut differences = vec![]; + for p in s.intersection(&t) { + let s = fs::read(source.base_path().join(p)).map_err(|e| e.to_string()); + let t = fs::read(target.base_path().join(p)).map_err(|e| e.to_string()); + if s != t { + differences.push(p); + } + } + assert!( + s_extra.is_empty() && t_extra.is_empty() && differences.is_empty(), + "s-t: {s_extra:?}\nt-s: {t_extra:?}\ndiff(s&t): {differences:?}" + ); +} + +fn list_all(root: &Path) -> BTreeSet { + fn list_recursive(path: PathBuf, results: &mut Vec) { + match fs::read_dir(&path) { + Ok(d) => { + for entry in d { + list_recursive(entry.unwrap().path(), results); + } + } + Err(e) => assert_eq!(e.kind(), std::io::ErrorKind::NotADirectory), + }; + results.push(path); + } + + let mut results = vec![]; + list_recursive(root.into(), &mut results); + results + .iter() + .map(|r| r.strip_prefix(root).unwrap().into()) + .collect() +} -- 2.47.3