From 0fcb8f314a054e4c92e49461f1ae2d9392756638 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Fri, 12 Mar 2021 11:12:46 +0200 Subject: feat: show warnings for any problems backing up files Previously, we either ignored it or aborted the backup. Neither is good. Now we ignore the problem, except to show a warning at the end of the backup run. --- obnam.md | 19 +++++++++++++++++++ src/cmd/backup.rs | 40 ++++++++++++++++++++++++++++++---------- src/generation.rs | 19 ++++++++++++++----- subplot/client.py | 5 +++++ subplot/client.yaml | 1 + subplot/data.py | 6 +++++- 6 files changed, 74 insertions(+), 16 deletions(-) diff --git a/obnam.md b/obnam.md index bfaff5d..cffd83a 100644 --- a/obnam.md +++ b/obnam.md @@ -1430,6 +1430,25 @@ then file live/data.dat is restored to rest then file live/bad.dat is not restored to rest ~~~ +## Unreadable directory + +This scenario verifies that Obnam will skip a file in a directory it +can't read. Obnam should warn about that, but not give an error. + +~~~scenario +given an installed obnam +and a running chunk server +and a client config based on smoke.yaml +and a file live/unreadable/data.dat containing some random data +and file live/unreadable has mode 000 +when I run obnam --config smoke.yaml backup +then stdout contains "live/unreadable" +then backup generation is GEN +when I invoke obnam --config smoke.yaml restore rest +then file live/unreadable is restored to rest +then file live/unreadable/data.dat is not restored to rest +~~~ + ## Restore latest generation This scenario verifies that the latest backup generation can be diff --git a/src/cmd/backup.rs b/src/cmd/backup.rs index 4dc9370..547775c 100644 --- a/src/cmd/backup.rs +++ b/src/cmd/backup.rs @@ -1,5 +1,5 @@ use crate::backup_progress::BackupProgress; -use crate::backup_run::{IncrementalBackup, InitialBackup}; +use crate::backup_run::{BackupError, IncrementalBackup, InitialBackup}; use crate::chunkid::ChunkId; use crate::client::{BackupClient, ClientConfig}; use crate::error::ObnamError; @@ -17,18 +17,28 @@ pub fn backup(config: &ClientConfig) -> Result<(), ObnamError> { let client = BackupClient::new(config)?; let genlist = client.list_generations()?; - let (gen_id, file_count) = match genlist.resolve("latest") { + let (gen_id, file_count, warnings) = match genlist.resolve("latest") { Err(_) => initial_backup(&config, &client)?, Ok(old_ref) => incremental_backup(&old_ref, &config, &client)?, }; - report_stats(&runtime, file_count, &gen_id)?; + for w in warnings.iter() { + println!("warning: {}", w); + } + + report_stats(&runtime, file_count, &gen_id, warnings.len())?; Ok(()) } -fn report_stats(runtime: &SystemTime, file_count: i64, gen_id: &ChunkId) -> Result<(), ObnamError> { +fn report_stats( + runtime: &SystemTime, + file_count: i64, + gen_id: &ChunkId, + num_warnings: usize, +) -> Result<(), ObnamError> { println!("status: OK"); + println!("warnings: {}", num_warnings); println!("duration: {}", runtime.elapsed()?.as_secs()); println!("file-count: {}", file_count); println!("generation-id: {}", gen_id); @@ -38,34 +48,41 @@ fn report_stats(runtime: &SystemTime, file_count: i64, gen_id: &ChunkId) -> Resu fn initial_backup( config: &ClientConfig, client: &BackupClient, -) -> Result<(ChunkId, i64), ObnamError> { +) -> Result<(ChunkId, i64, Vec), ObnamError> { let run = InitialBackup::new(config, &client)?; let newtemp = NamedTempFile::new()?; + let mut all_warnings = vec![]; let count = { + println!("create nascent"); info!("fresh backup without a previous generation"); let mut new = NascentGeneration::create(newtemp.path())?; for root in &config.roots { let iter = FsIterator::new(root); - new.insert_iter(iter.map(|entry| run.backup(entry)))?; + let warnings = new.insert_iter(iter.map(|entry| run.backup(entry)))?; + for w in warnings { + all_warnings.push(w); + } } new.file_count() }; + run.drop(); let progress = BackupProgress::upload_generation(); let gen_id = client.upload_generation(newtemp.path(), SQLITE_CHUNK_SIZE)?; progress.finish(); - Ok((gen_id, count)) + Ok((gen_id, count, all_warnings)) } fn incremental_backup( old_ref: &str, config: &ClientConfig, client: &BackupClient, -) -> Result<(ChunkId, i64), ObnamError> { +) -> Result<(ChunkId, i64, Vec), ObnamError> { let mut run = IncrementalBackup::new(config, &client)?; let newtemp = NamedTempFile::new()?; + let mut all_warnings = vec![]; let count = { info!("incremental backup based on {}", old_ref); let oldtemp = NamedTempFile::new()?; @@ -75,7 +92,10 @@ fn incremental_backup( let mut new = NascentGeneration::create(newtemp.path())?; for root in &config.roots { let iter = FsIterator::new(root); - new.insert_iter(iter.map(|entry| run.backup(entry, &old)))?; + let warnings = new.insert_iter(iter.map(|entry| run.backup(entry, &old)))?; + for w in warnings { + all_warnings.push(w); + } } new.file_count() }; @@ -84,5 +104,5 @@ fn incremental_backup( let gen_id = client.upload_generation(newtemp.path(), SQLITE_CHUNK_SIZE)?; progress.finish(); - Ok((gen_id, count)) + Ok((gen_id, count, all_warnings)) } diff --git a/src/generation.rs b/src/generation.rs index 4655c17..240f46a 100644 --- a/src/generation.rs +++ b/src/generation.rs @@ -2,6 +2,7 @@ use crate::backup_reason::Reason; use crate::backup_run::{BackupError, BackupResult}; use crate::chunkid::ChunkId; use crate::fsentry::FilesystemEntry; +use log::debug; use rusqlite::Connection; use std::path::{Path, PathBuf}; @@ -64,15 +65,23 @@ impl NascentGeneration { pub fn insert_iter<'a>( &mut self, entries: impl Iterator, Reason)>>, - ) -> NascentResult<()> { + ) -> NascentResult> { let t = self.conn.transaction()?; + let mut warnings = vec![]; for r in entries { - let (e, ids, reason) = r?; - self.fileno += 1; - sql::insert_one(&t, e, self.fileno, &ids[..], reason)?; + match r { + Err(err) => { + debug!("ignoring backup error {}", err); + warnings.push(err); + } + Ok((e, ids, reason)) => { + self.fileno += 1; + sql::insert_one(&t, e, self.fileno, &ids[..], reason)?; + } + } } t.commit()?; - Ok(()) + Ok(warnings) } } diff --git a/subplot/client.py b/subplot/client.py index 0e724b8..966324e 100644 --- a/subplot/client.py +++ b/subplot/client.py @@ -11,6 +11,11 @@ def install_obnam(ctx): runcmd_prepend_to_path(ctx, dirname=os.path.join(srcdir, "target", "debug")) +def uninstall_obnam(ctx): + runcmd_run = globals()["runcmd_run"] + runcmd_run(ctx, ["chmod", "-R", "u+rwX", "."]) + + def configure_client(ctx, filename=None): get_file = globals()["get_file"] diff --git a/subplot/client.yaml b/subplot/client.yaml index eef4714..b76b96a 100644 --- a/subplot/client.yaml +++ b/subplot/client.yaml @@ -1,5 +1,6 @@ - given: "an installed obnam" function: install_obnam + cleanup: uninstall_obnam - given: "a client config based on {filename}" function: configure_client diff --git a/subplot/data.py b/subplot/data.py index 9396215..1455bf4 100644 --- a/subplot/data.py +++ b/subplot/data.py @@ -73,7 +73,11 @@ def file_is_restored(ctx, filename=None, restored=None): def file_is_not_restored(ctx, filename=None, restored=None): filename = os.path.join(restored, "./" + filename) - exists = os.path.exists(filename) + logging.info(r"verifying that {filename} does not exist") + try: + exists = os.path.exists(filename) + except PermissionError: + exists = False logging.debug(f"restored? {filename} {exists}") assert not exists -- cgit v1.2.1