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 From 989b50a633ad285b86c3549ea9ac6f62c0a13273 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Fri, 12 Mar 2021 12:31:49 +0200 Subject: feat: handle files in directories that can be read but not executed --- obnam.md | 19 +++++++++++++++++++ src/error.rs | 3 +-- src/fsiter.rs | 12 ++++++------ 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/obnam.md b/obnam.md index cffd83a..15cddb2 100644 --- a/obnam.md +++ b/obnam.md @@ -1449,6 +1449,25 @@ then file live/unreadable is restored to rest then file live/unreadable/data.dat is not restored to rest ~~~ +## Unexecutable 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/dir/data.dat containing some random data +and file live/dir has mode 600 +when I run obnam --config smoke.yaml backup +then stdout contains "live/dir" +then backup generation is GEN +when I invoke obnam --config smoke.yaml restore rest +then file live/dir is restored to rest +then file live/dir/data.dat is not restored to rest +~~~ + ## Restore latest generation This scenario verifies that the latest backup generation can be diff --git a/src/error.rs b/src/error.rs index 73d4a66..a905458 100644 --- a/src/error.rs +++ b/src/error.rs @@ -5,11 +5,10 @@ use crate::generation::{LocalGenerationError, NascentError}; use crate::genlist::GenerationListError; use std::time::SystemTimeError; use tempfile::PersistError; -use thiserror::Error; /// Define all the kinds of errors that functions corresponding to /// subcommands of the main program can return. -#[derive(Debug, Error)] +#[derive(Debug, thiserror::Error)] pub enum ObnamError { #[error(transparent)] GenerationListError(#[from] GenerationListError), diff --git a/src/fsiter.rs b/src/fsiter.rs index 57b6fd5..b778cf3 100644 --- a/src/fsiter.rs +++ b/src/fsiter.rs @@ -1,6 +1,6 @@ use crate::fsentry::{FilesystemEntry, FsEntryError}; -use log::{debug, error}; -use std::path::Path; +use log::{debug, warn}; +use std::path::{Path, PathBuf}; use walkdir::{DirEntry, IntoIter, WalkDir}; /// Iterator over file system entries in a directory tree. @@ -13,8 +13,8 @@ pub enum FsIterError { #[error(transparent)] WalkError(#[from] walkdir::Error), - #[error(transparent)] - IoError(#[from] std::io::Error), + #[error("I/O error on {0}: {1}")] + IoError(PathBuf, #[source] std::io::Error), #[error(transparent)] FsEntryError(#[from] FsEntryError), @@ -50,8 +50,8 @@ fn new_entry(e: &DirEntry) -> FsIterResult { let meta = match meta { Ok(meta) => meta, Err(err) => { - error!("failed to get metadata: {}", err); - return Err(err.into()); + warn!("failed to get metadata for {}: {}", path.display(), err); + return Err(FsIterError::IoError(path.to_path_buf(), err)); } }; let entry = FilesystemEntry::from_metadata(path, &meta)?; -- cgit v1.2.1