Merge branch 'error-ignore' into 'main'

Ignore files that can't be read during backup

Closes #27

See merge request larswirzenius/obnam!90

8 files changed, 83 insertions, 16 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/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 <GEN> 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/src/backup_reason.rs b/src/backup_reason.rs
index 218857c..f785dea 100644
--- a/src/backup_reason.rs
+++ b/src/backup_reason.rs
@@ -8,7 +8,9 @@ pub enum Reason {
     IsNew,
     Changed,
     Unchanged,
-    Error,
+    GenerationLookupError,
+    FileError,
+    Unknown,
 }
 
 impl Reason {
@@ -18,7 +20,9 @@ impl Reason {
             "new" => Reason::IsNew,
             "changed" => Reason::Changed,
             "unchanged" => Reason::Unchanged,
-            _ => Reason::Error,
+            "genlookuperror" => Reason::GenerationLookupError,
+            "fileerror" => Reason::FileError,
+            _ => Reason::Unknown,
         }
     }
 }
@@ -39,7 +43,9 @@ impl fmt::Display for Reason {
             Reason::IsNew => "new",
             Reason::Changed => "changed",
             Reason::Unchanged => "unchanged",
-            Reason::Error => "error",
+            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 fce9a73..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)
             }
         }
     }
@@ -85,13 +83,13 @@ impl BackupRun {
                 self.progress.found_live_file(path);
                 let reason = self.policy.needs_backup(&old, &entry);
                 match reason {
-                    Reason::IsNew | Reason::Changed | Reason::Error => {
-                        let ids = self
-                            .client
-                            .upload_filesystem_entry(&entry, self.buffer_size)?;
-                        Ok((entry.clone(), ids, reason))
+                    Reason::IsNew
+                    | Reason::Changed
+                    | Reason::GenerationLookupError
+                    | Reason::Unknown => {
+                        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)?
@@ -105,3 +103,20 @@ impl BackupRun {
         }
     }
 }
+
+fn backup_file(
+    client: &BackupClient,
+    entry: &FilesystemEntry,
+    path: &Path,
+    chunk_size: usize,
+    reason: Reason,
+) -> BackupResult<(FilesystemEntry, Vec<ChunkId>, 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() {
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!(
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