From 73a9ca3d90ac393b7b8575199a3fcce855374136 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Sun, 7 Feb 2021 17:56:33 +0200 Subject: feat: add catch-all Reason variant for unknown reason Just in case the SQLite DB stores a reason this version of Obnam doesn't understand, we handle that now. --- src/backup_reason.rs | 5 ++++- src/backup_run.rs | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/backup_reason.rs b/src/backup_reason.rs index 218857c..502b79e 100644 --- a/src/backup_reason.rs +++ b/src/backup_reason.rs @@ -9,6 +9,7 @@ pub enum Reason { Changed, Unchanged, Error, + Unknown, } impl Reason { @@ -18,7 +19,8 @@ impl Reason { "new" => Reason::IsNew, "changed" => Reason::Changed, "unchanged" => Reason::Unchanged, - _ => Reason::Error, + "error" => Reason::Error, + _ => Reason::Unknown, } } } @@ -40,6 +42,7 @@ impl fmt::Display for Reason { Reason::Changed => "changed", Reason::Unchanged => "unchanged", Reason::Error => "error", + Reason::Unknown => "unknown", }; write!(f, "{}", reason) } diff --git a/src/backup_run.rs b/src/backup_run.rs index fce9a73..f97bb58 100644 --- a/src/backup_run.rs +++ b/src/backup_run.rs @@ -85,7 +85,7 @@ impl BackupRun { self.progress.found_live_file(path); let reason = self.policy.needs_backup(&old, &entry); match reason { - Reason::IsNew | Reason::Changed | Reason::Error => { + Reason::IsNew | Reason::Changed | Reason::Error | Reason::Unknown => { let ids = self .client .upload_filesystem_entry(&entry, self.buffer_size)?; -- cgit v1.2.1 From 5b863edb32314808b6823f90c03ad97da6fdbcbf Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Sun, 7 Feb 2021 17:59:53 +0200 Subject: refactor: rename Reason::Error to Reason::GenerationLookupError New name is more precise. The meaning of the enum variant hasn't changed. --- src/backup_reason.rs | 6 +++--- src/backup_run.rs | 5 ++++- src/policy.rs | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/backup_reason.rs b/src/backup_reason.rs index 502b79e..b3ce5e9 100644 --- a/src/backup_reason.rs +++ b/src/backup_reason.rs @@ -8,7 +8,7 @@ pub enum Reason { IsNew, Changed, Unchanged, - Error, + GenerationLookupError, Unknown, } @@ -19,7 +19,7 @@ impl Reason { "new" => Reason::IsNew, "changed" => Reason::Changed, "unchanged" => Reason::Unchanged, - "error" => Reason::Error, + "genlookuperror" => Reason::GenerationLookupError, _ => Reason::Unknown, } } @@ -41,7 +41,7 @@ impl fmt::Display for Reason { Reason::IsNew => "new", Reason::Changed => "changed", Reason::Unchanged => "unchanged", - Reason::Error => "error", + Reason::GenerationLookupError => "genlookuperror", Reason::Unknown => "unknown", }; write!(f, "{}", reason) diff --git a/src/backup_run.rs b/src/backup_run.rs index f97bb58..e0ed533 100644 --- a/src/backup_run.rs +++ b/src/backup_run.rs @@ -85,7 +85,10 @@ impl BackupRun { self.progress.found_live_file(path); let reason = self.policy.needs_backup(&old, &entry); match reason { - Reason::IsNew | Reason::Changed | Reason::Error | Reason::Unknown => { + Reason::IsNew + | Reason::Changed + | Reason::GenerationLookupError + | Reason::Unknown => { let ids = self .client .upload_filesystem_entry(&entry, self.buffer_size)?; diff --git a/src/policy.rs b/src/policy.rs index 032b851..8a65e09 100644 --- a/src/policy.rs +++ b/src/policy.rs @@ -42,7 +42,7 @@ impl BackupPolicy { "needs_backup: lookup in old generation returned error, ignored: {:?}: {}", new_name, err ); - Reason::Error + Reason::GenerationLookupError } }; debug!( -- cgit v1.2.1 From 3ef5bd7a3ab445509216116bb2f8009ace2b1080 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Sun, 7 Feb 2021 18:03:16 +0200 Subject: feat: if file can't be read, log that, don't end backup in error Such files won't be restored, as they'd be restored as empty file, and that would be confusing and thus bad. --- client.yaml | 2 +- src/backup_reason.rs | 3 +++ src/backup_run.rs | 30 +++++++++++++++++++++--------- src/cmd/restore.rs | 6 +++++- 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/client.yaml b/client.yaml index 99feee4..a1a63e7 100644 --- a/client.yaml +++ b/client.yaml @@ -1,5 +1,5 @@ server_url: https://localhost:8888 verify_tls_cert: false roots: - - /home/liw/tmp/Foton + - x log: obnam.log diff --git a/src/backup_reason.rs b/src/backup_reason.rs index b3ce5e9..f785dea 100644 --- a/src/backup_reason.rs +++ b/src/backup_reason.rs @@ -9,6 +9,7 @@ pub enum Reason { Changed, Unchanged, GenerationLookupError, + FileError, Unknown, } @@ -20,6 +21,7 @@ impl Reason { "changed" => Reason::Changed, "unchanged" => Reason::Unchanged, "genlookuperror" => Reason::GenerationLookupError, + "fileerror" => Reason::FileError, _ => Reason::Unknown, } } @@ -42,6 +44,7 @@ impl fmt::Display for Reason { Reason::Changed => "changed", Reason::Unchanged => "unchanged", Reason::GenerationLookupError => "genlookuperror", + Reason::FileError => "fileerror", Reason::Unknown => "unknown", }; write!(f, "{}", reason) diff --git a/src/backup_run.rs b/src/backup_run.rs index e0ed533..7bb4440 100644 --- a/src/backup_run.rs +++ b/src/backup_run.rs @@ -7,6 +7,7 @@ use crate::fsiter::{FsIterError, FsIterResult}; use crate::generation::{LocalGeneration, LocalGenerationError}; use crate::policy::BackupPolicy; use log::{info, warn}; +use std::path::Path; pub struct BackupRun { client: BackupClient, @@ -60,10 +61,7 @@ impl BackupRun { let path = &entry.pathbuf(); info!("backup: {}", path.display()); self.progress.found_live_file(path); - let ids = self - .client - .upload_filesystem_entry(&entry, self.buffer_size)?; - Ok((entry.clone(), ids, Reason::IsNew)) + backup_file(&self.client, &entry, &path, self.buffer_size, Reason::IsNew) } } } @@ -89,12 +87,9 @@ impl BackupRun { | Reason::Changed | Reason::GenerationLookupError | Reason::Unknown => { - let ids = self - .client - .upload_filesystem_entry(&entry, self.buffer_size)?; - Ok((entry.clone(), ids, reason)) + backup_file(&self.client, &entry, &path, self.buffer_size, reason) } - Reason::Unchanged | Reason::Skipped => { + Reason::Unchanged | Reason::Skipped | Reason::FileError => { let fileno = old.get_fileno(&entry.pathbuf())?; let ids = if let Some(fileno) = fileno { old.chunkids(fileno)? @@ -108,3 +103,20 @@ impl BackupRun { } } } + +fn backup_file( + client: &BackupClient, + entry: &FilesystemEntry, + path: &Path, + chunk_size: usize, + reason: Reason, +) -> BackupResult<(FilesystemEntry, Vec, Reason)> { + let ids = client.upload_filesystem_entry(&entry, chunk_size); + match ids { + Err(err) => { + warn!("error backing up {}, skipping it: {}", path.display(), err); + Ok((entry.clone(), vec![], Reason::FileError)) + } + Ok(ids) => Ok((entry.clone(), ids, reason)), + } +} diff --git a/src/cmd/restore.rs b/src/cmd/restore.rs index a0f5ec0..5d01bd4 100644 --- a/src/cmd/restore.rs +++ b/src/cmd/restore.rs @@ -1,3 +1,4 @@ +use crate::backup_reason::Reason; use crate::client::ClientConfig; use crate::client::{BackupClient, ClientError}; use crate::error::ObnamError; @@ -35,7 +36,10 @@ pub fn restore(config: &ClientConfig, gen_ref: &str, to: &Path) -> Result<(), Ob info!("restoring {} files", gen.file_count()?); let progress = create_progress_bar(gen.file_count()?, true); for file in gen.files()? { - restore_generation(&client, &gen, file.fileno(), file.entry(), &to, &progress)?; + match file.reason() { + Reason::FileError => (), + _ => restore_generation(&client, &gen, file.fileno(), file.entry(), &to, &progress)?, + } } for file in gen.files()? { if file.entry().is_dir() { -- cgit v1.2.1 From 028fd279652dbe167b12b32dff00c6a299c4fc02 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Sun, 7 Feb 2021 17:46:06 +0200 Subject: test: add scenario for live data file being unreadable --- obnam.md | 22 ++++++++++++++++++++++ subplot/data.py | 14 ++++++++++++++ subplot/data.yaml | 6 ++++++ 3 files changed, 42 insertions(+) diff --git a/obnam.md b/obnam.md index 204d6d9..cb08f13 100644 --- a/obnam.md +++ b/obnam.md @@ -1269,6 +1269,28 @@ given a manifest of the directory live restored in rest in rest.yaml then files live.yaml and rest.yaml match ~~~ +## Unreadable file + +This scenario verifies that Obnam will back up all files of live data, +even if one of them is unreadable. By inference, we assume this means +other errors on individual files also won't end the backup +prematurely. + + +~~~scenario +given an installed obnam +and a running chunk server +and a client config based on smoke.yaml +and a file live/data.dat containing some random data +and a file live/bad.dat containing some random data +and file live/bad.dat has mode 000 +when I run obnam --config smoke.yaml backup +then backup generation is GEN +when I invoke obnam --config smoke.yaml restore rest +then file live/data.dat is restored to rest +then file live/bad.dat is not restored to rest +~~~ + ## Restore latest generation This scenario verifies that the latest backup generation can be diff --git a/subplot/data.py b/subplot/data.py index f3faf2b..f7fb903 100644 --- a/subplot/data.py +++ b/subplot/data.py @@ -54,6 +54,20 @@ def _create_manifest_of_directory(ctx, dirname=None, manifest=None): open(manifest, "w").write(stdout) +def file_is_restored(ctx, filename=None, restored=None): + filename = os.path.join(restored, "./" + filename) + exists = os.path.exists(filename) + logging.debug(f"restored? {filename} {exists}") + assert exists + + +def file_is_not_restored(ctx, filename=None, restored=None): + filename = os.path.join(restored, "./" + filename) + exists = os.path.exists(filename) + logging.debug(f"restored? {filename} {exists}") + assert not exists + + def files_match(ctx, first=None, second=None): assert_eq = globals()["assert_eq"] diff --git a/subplot/data.yaml b/subplot/data.yaml index 9538daa..1636e77 100644 --- a/subplot/data.yaml +++ b/subplot/data.yaml @@ -24,3 +24,9 @@ - then: "stdout, as JSON, matches file {filename}" function: match_stdout_to_json_file + +- then: "file {filename} is restored to {restored}" + function: file_is_restored + +- then: "file {filename} is not restored to {restored}" + function: file_is_not_restored -- cgit v1.2.1