public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH v4 proxmox-backup 00/10] add job based verify scheduling
@ 2020-10-20  9:10 Hannes Laimer
  2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 01/10] rename VERIFY_SCHEDULE_SCHEMA to VERIFICATION_SCHEDULE_SCHEMA Hannes Laimer
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: Hannes Laimer @ 2020-10-20  9:10 UTC (permalink / raw)
  To: pbs-devel

Replaces the first implementation of scheduled verification with a new
job-based version with additional options that may be specified through
the web ui.

Options available for verification jobs:
 * schedule when to run the job
 * set datastore on which the job should run
 * set a number of days after which a verification becomes "outdated"
    empty => verifications are valid forever
 * specify if already successfuly verified snapshots should be verified
    again even if they're not outdated(failed ones will always be done)

v4:
 * squashed patches
 * rebased
 * no build-breaking patch
 * correct old config files in postinst

v3:
 * restructure do_verification_job function
 * renamed 'verify' in config to 'verification'
 * add cbind to 'Days valid' field in frontend

v2:
 * added 'Ignire Verified' column in WebUI
 * rebased onto master
 * log number of planned verification
 * ignore_verified is not optional anymore
 * adjusted default column width for 'Days valid'
 * failed verifications won't be verified again
 * use proxmox::try_block! in worker

Hannes Laimer (10):
  rename VERIFY_SCHEDULE_SCHEMA to VERIFICATION_SCHEDULE_SCHEMA
  api2: add verification job config endpoint
  api2: add verification admin endpoint and do_verification_job function
  proxy: add scheduling for verification jobs
  set a different worker_type based on what is going to be
    verified(snapshot, group, ds)
  ui: add verification job view
  ui: add verification job edit window
  ui: add task descriptions for the different types of verification(job,
    snapshot, group, ds)
  api proxy: remove old verification scheduling
  postinst: correct invalid old datastore configs

 debian/postinst                 |   2 +
 src/api2/admin.rs               |   4 +-
 src/api2/admin/datastore.rs     |   5 +-
 src/api2/admin/verify.rs        | 107 ++++++++++++
 src/api2/config.rs              |   2 +
 src/api2/config/datastore.rs    |  24 ---
 src/api2/config/verify.rs       | 274 +++++++++++++++++++++++++++++++
 src/api2/types/mod.rs           |  12 +-
 src/backup/verify.rs            |  96 +++++++++++
 src/bin/proxmox-backup-proxy.rs | 120 ++++----------
 src/config.rs                   |   1 +
 src/config/datastore.rs         |   6 -
 src/config/verify.rs            | 189 +++++++++++++++++++++
 www/Makefile                    |   2 +
 www/NavigationTree.js           |   6 +
 www/Utils.js                    |   5 +-
 www/config/DataStoreConfig.js   |   2 +-
 www/config/VerifyView.js        | 280 ++++++++++++++++++++++++++++++++
 www/window/DataStoreEdit.js     |   9 -
 www/window/VerifyJobEdit.js     |  93 +++++++++++
 20 files changed, 1108 insertions(+), 131 deletions(-)
 create mode 100644 src/api2/admin/verify.rs
 create mode 100644 src/api2/config/verify.rs
 create mode 100644 src/config/verify.rs
 create mode 100644 www/config/VerifyView.js
 create mode 100644 www/window/VerifyJobEdit.js

-- 
2.20.1





^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pbs-devel] [PATCH v4 proxmox-backup 01/10] rename VERIFY_SCHEDULE_SCHEMA to VERIFICATION_SCHEDULE_SCHEMA
  2020-10-20  9:10 [pbs-devel] [PATCH v4 proxmox-backup 00/10] add job based verify scheduling Hannes Laimer
@ 2020-10-20  9:10 ` Hannes Laimer
  2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 02/10] api2: add verification job config endpoint Hannes Laimer
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Hannes Laimer @ 2020-10-20  9:10 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
broken config files will be corrected by postinst script in later patch,
renamed in order to avoid build breaking patches later

 src/api2/config/datastore.rs | 4 ++--
 src/api2/types/mod.rs        | 2 +-
 src/config/datastore.rs      | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index 5ab637ce..ad5c03a6 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -77,7 +77,7 @@ pub fn list_datastores(
             },
             "verify-schedule": {
                 optional: true,
-                schema: VERIFY_SCHEDULE_SCHEMA,
+                schema: VERIFICATION_SCHEDULE_SCHEMA,
             },
             "keep-last": {
                 optional: true,
@@ -216,7 +216,7 @@ pub enum DeletableProperty {
             },
             "verify-schedule": {
                 optional: true,
-                schema: VERIFY_SCHEDULE_SCHEMA,
+                schema: VERIFICATION_SCHEDULE_SCHEMA,
             },
             "keep-last": {
                 optional: true,
diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
index 75b68879..5a30bb89 100644
--- a/src/api2/types/mod.rs
+++ b/src/api2/types/mod.rs
@@ -302,7 +302,7 @@ pub const PRUNE_SCHEDULE_SCHEMA: Schema = StringSchema::new(
     .format(&ApiStringFormat::VerifyFn(crate::tools::systemd::time::verify_calendar_event))
     .schema();
 
-pub const VERIFY_SCHEDULE_SCHEMA: Schema = StringSchema::new(
+pub const VERIFICATION_SCHEDULE_SCHEMA: Schema = StringSchema::new(
     "Run verify job at specified schedule.")
     .format(&ApiStringFormat::VerifyFn(crate::tools::systemd::time::verify_calendar_event))
     .schema();
diff --git a/src/config/datastore.rs b/src/config/datastore.rs
index aaf977a7..3284a63d 100644
--- a/src/config/datastore.rs
+++ b/src/config/datastore.rs
@@ -46,7 +46,7 @@ pub const DIR_NAME_SCHEMA: Schema = StringSchema::new("Directory name").schema()
         },
         "verify-schedule": {
             optional: true,
-            schema: VERIFY_SCHEDULE_SCHEMA,
+            schema: VERIFICATION_SCHEDULE_SCHEMA,
         },
         "keep-last": {
             optional: true,
-- 
2.20.1





^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pbs-devel] [PATCH v4 proxmox-backup 02/10] api2: add verification job config endpoint
  2020-10-20  9:10 [pbs-devel] [PATCH v4 proxmox-backup 00/10] add job based verify scheduling Hannes Laimer
  2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 01/10] rename VERIFY_SCHEDULE_SCHEMA to VERIFICATION_SCHEDULE_SCHEMA Hannes Laimer
@ 2020-10-20  9:10 ` Hannes Laimer
  2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 03/10] api2: add verification admin endpoint and do_verification_job function Hannes Laimer
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Hannes Laimer @ 2020-10-20  9:10 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/config.rs        |   2 +
 src/api2/config/verify.rs | 274 ++++++++++++++++++++++++++++++++++++++
 src/api2/types/mod.rs     |  10 ++
 src/config.rs             |   1 +
 src/config/verify.rs      | 189 ++++++++++++++++++++++++++
 5 files changed, 476 insertions(+)
 create mode 100644 src/api2/config/verify.rs
 create mode 100644 src/config/verify.rs

diff --git a/src/api2/config.rs b/src/api2/config.rs
index be7397c8..7a5129c7 100644
--- a/src/api2/config.rs
+++ b/src/api2/config.rs
@@ -4,11 +4,13 @@ use proxmox::list_subdirs_api_method;
 pub mod datastore;
 pub mod remote;
 pub mod sync;
+pub mod verify;
 
 const SUBDIRS: SubdirMap = &[
     ("datastore", &datastore::ROUTER),
     ("remote", &remote::ROUTER),
     ("sync", &sync::ROUTER),
+    ("verify", &verify::ROUTER)
 ];
 
 pub const ROUTER: Router = Router::new()
diff --git a/src/api2/config/verify.rs b/src/api2/config/verify.rs
new file mode 100644
index 00000000..efc33a5c
--- /dev/null
+++ b/src/api2/config/verify.rs
@@ -0,0 +1,274 @@
+use anyhow::{bail, Error};
+use serde_json::Value;
+use ::serde::{Deserialize, Serialize};
+
+use proxmox::api::{api, Router, RpcEnvironment};
+use proxmox::tools::fs::open_file_locked;
+
+use crate::api2::types::*;
+use crate::config::verify::{self, VerificationJobConfig};
+
+#[api(
+    input: {
+        properties: {},
+    },
+    returns: {
+        description: "List configured jobs.",
+        type: Array,
+        items: { type: verify::VerificationJobConfig },
+    },
+)]
+/// List all verification jobs
+pub fn list_verification_jobs(
+    _param: Value,
+    mut rpcenv: &mut dyn RpcEnvironment,
+) -> Result<Vec<VerificationJobConfig>, Error> {
+
+    let (config, digest) = verify::config()?;
+
+    let list = config.convert_to_typed_array("verification")?;
+
+    rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into();
+
+    Ok(list)
+}
+
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            id: {
+                schema: JOB_ID_SCHEMA,
+            },
+            store: {
+                schema: DATASTORE_SCHEMA,
+            },
+            "ignore-verified": {
+                optional: true,
+                schema: IGNORE_VERIFIED_BACKUPS_SCHEMA,
+            },
+            "outdated-after": {
+                optional: true,
+                schema: VERIFICATION_OUTDATED_AFTER_SCHEMA,
+            },
+            comment: {
+                optional: true,
+                schema: SINGLE_LINE_COMMENT_SCHEMA,
+            },
+            schedule: {
+                optional: true,
+                schema: VERIFICATION_SCHEDULE_SCHEMA,
+            },
+        }
+    }
+)]
+/// Create a new verification job.
+pub fn create_verification_job(param: Value) -> Result<(), Error> {
+
+    let _lock = open_file_locked(verify::VERIFICATION_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+
+    let verification_job: verify::VerificationJobConfig = serde_json::from_value(param.clone())?;
+
+    let (mut config, _digest) = verify::config()?;
+
+    if let Some(_) = config.sections.get(&verification_job.id) {
+        bail!("job '{}' already exists.", verification_job.id);
+    }
+
+    config.set_data(&verification_job.id, "verification", &verification_job)?;
+
+    verify::save_config(&config)?;
+
+    crate::config::jobstate::create_state_file("verificationjob", &verification_job.id)?;
+
+    Ok(())
+}
+
+#[api(
+   input: {
+        properties: {
+            id: {
+                schema: JOB_ID_SCHEMA,
+            },
+        },
+    },
+    returns: {
+        description: "The verification job configuration.",
+        type: verify::VerificationJobConfig,
+    },
+)]
+/// Read a verification job configuration.
+pub fn read_verification_job(
+    id: String,
+    mut rpcenv: &mut dyn RpcEnvironment,
+) -> Result<VerificationJobConfig, Error> {
+    let (config, digest) = verify::config()?;
+
+    let verification_job = config.lookup("verification", &id)?;
+    rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into();
+
+    Ok(verification_job)
+}
+
+#[api()]
+#[derive(Serialize, Deserialize)]
+#[serde(rename_all="kebab-case")]
+/// Deletable property name
+pub enum DeletableProperty {
+    /// Delete the ignore verified property.
+    IgnoreVerified,
+    /// Delete the comment property.
+    Comment,
+    /// Delete the job schedule.
+    Schedule,
+    /// Delete outdated after property.
+    OutdatedAfter
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            id: {
+                schema: JOB_ID_SCHEMA,
+            },
+            store: {
+                optional: true,
+                schema: DATASTORE_SCHEMA,
+            },
+            "ignore-verified": {
+                optional: true,
+                schema: IGNORE_VERIFIED_BACKUPS_SCHEMA,
+            },
+            "outdated-after": {
+                optional: true,
+                schema: VERIFICATION_OUTDATED_AFTER_SCHEMA,
+            },
+            comment: {
+                optional: true,
+                schema: SINGLE_LINE_COMMENT_SCHEMA,
+            },
+            schedule: {
+                optional: true,
+                schema: VERIFICATION_SCHEDULE_SCHEMA,
+            },
+            delete: {
+                description: "List of properties to delete.",
+                type: Array,
+                optional: true,
+                items: {
+                    type: DeletableProperty,
+                }
+            },
+            digest: {
+                optional: true,
+                schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
+            },
+        },
+    },
+)]
+/// Update verification job config.
+pub fn update_verification_job(
+    id: String,
+    store: Option<String>,
+    ignore_verified: Option<bool>,
+    outdated_after: Option<i64>,
+    comment: Option<String>,
+    schedule: Option<String>,
+    delete: Option<Vec<DeletableProperty>>,
+    digest: Option<String>,
+) -> Result<(), Error> {
+
+    let _lock = open_file_locked(verify::VERIFICATION_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+
+    // pass/compare digest
+    let (mut config, expected_digest) = verify::config()?;
+
+    if let Some(ref digest) = digest {
+        let digest = proxmox::tools::hex_to_digest(digest)?;
+        crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
+    }
+
+    let mut data: verify::VerificationJobConfig = config.lookup("verification", &id)?;
+
+     if let Some(delete) = delete {
+        for delete_prop in delete {
+            match delete_prop {
+                DeletableProperty::IgnoreVerified => { data.ignore_verified = None; },
+                DeletableProperty::OutdatedAfter => { data.outdated_after = None; },
+                DeletableProperty::Comment => { data.comment = None; },
+                DeletableProperty::Schedule => { data.schedule = None; },
+            }
+        }
+    }
+
+    if let Some(comment) = comment {
+        let comment = comment.trim().to_string();
+        if comment.is_empty() {
+            data.comment = None;
+        } else {
+            data.comment = Some(comment);
+        }
+    }
+
+    if let Some(store) = store { data.store = store; }
+
+    if ignore_verified.is_some() { data.ignore_verified = ignore_verified; }
+    if outdated_after.is_some() { data.outdated_after = outdated_after; }
+    if schedule.is_some() { data.schedule = schedule; }
+
+    config.set_data(&id, "verification", &data)?;
+
+    verify::save_config(&config)?;
+
+    Ok(())
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            id: {
+                schema: JOB_ID_SCHEMA,
+            },
+            digest: {
+                optional: true,
+                schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
+            },
+        },
+    },
+)]
+/// Remove a verification job configuration
+pub fn delete_verification_job(id: String, digest: Option<String>) -> Result<(), Error> {
+
+    let _lock = open_file_locked(verify::VERIFICATION_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+
+    let (mut config, expected_digest) = verify::config()?;
+
+    if let Some(ref digest) = digest {
+        let digest = proxmox::tools::hex_to_digest(digest)?;
+        crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
+    }
+
+    match config.sections.get(&id) {
+        Some(_) => { config.sections.remove(&id); },
+        None => bail!("job '{}' does not exist.", id),
+    }
+
+    verify::save_config(&config)?;
+
+    crate::config::jobstate::remove_state_file("verificationjob", &id)?;
+
+    Ok(())
+}
+
+const ITEM_ROUTER: Router = Router::new()
+    .get(&API_METHOD_READ_VERIFICATION_JOB)
+    .put(&API_METHOD_UPDATE_VERIFICATION_JOB)
+    .delete(&API_METHOD_DELETE_VERIFICATION_JOB);
+
+pub const ROUTER: Router = Router::new()
+    .get(&API_METHOD_LIST_VERIFICATION_JOBS)
+    .post(&API_METHOD_CREATE_VERIFICATION_JOB)
+    .match_all("id", &ITEM_ROUTER);
\ No newline at end of file
diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
index 5a30bb89..f97db557 100644
--- a/src/api2/types/mod.rs
+++ b/src/api2/types/mod.rs
@@ -324,6 +324,16 @@ pub const REMOVE_VANISHED_BACKUPS_SCHEMA: Schema = BooleanSchema::new(
     .default(true)
     .schema();
 
+pub const IGNORE_VERIFIED_BACKUPS_SCHEMA: Schema = BooleanSchema::new(
+    "Do not verify backups that are already verified if their verification is not outdated.")
+    .default(true)
+    .schema();
+
+pub const VERIFICATION_OUTDATED_AFTER_SCHEMA: Schema = IntegerSchema::new(
+    "Days after that a verification becomes outdated")
+    .minimum(1)
+    .schema();
+
 pub const SINGLE_LINE_COMMENT_SCHEMA: Schema = StringSchema::new("Comment (single line).")
     .format(&SINGLE_LINE_COMMENT_FORMAT)
     .schema();
diff --git a/src/config.rs b/src/config.rs
index c2ac6da1..ab7fc81a 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -23,6 +23,7 @@ pub mod network;
 pub mod remote;
 pub mod sync;
 pub mod user;
+pub mod verify;
 
 /// Check configuration directory permissions
 ///
diff --git a/src/config/verify.rs b/src/config/verify.rs
new file mode 100644
index 00000000..af3d20a7
--- /dev/null
+++ b/src/config/verify.rs
@@ -0,0 +1,189 @@
+use anyhow::{Error};
+use lazy_static::lazy_static;
+use std::collections::HashMap;
+use serde::{Serialize, Deserialize};
+
+use proxmox::api::{
+    api,
+    schema::*,
+    section_config::{
+        SectionConfig,
+        SectionConfigData,
+        SectionConfigPlugin,
+    }
+};
+
+use proxmox::tools::{fs::replace_file, fs::CreateOptions};
+
+use crate::api2::types::*;
+
+lazy_static! {
+    static ref CONFIG: SectionConfig = init();
+}
+
+
+#[api(
+    properties: {
+        id: {
+            schema: JOB_ID_SCHEMA,
+        },
+        store: {
+            schema: DATASTORE_SCHEMA,
+        },
+        "ignore-verified": {
+            optional: true,
+            schema: IGNORE_VERIFIED_BACKUPS_SCHEMA,
+        },
+        "outdated-after": {
+            optional: true,
+            schema: VERIFICATION_OUTDATED_AFTER_SCHEMA,
+        },
+        comment: {
+            optional: true,
+            schema: SINGLE_LINE_COMMENT_SCHEMA,
+        },
+        schedule: {
+            optional: true,
+            schema: VERIFICATION_SCHEDULE_SCHEMA,
+        },
+    }
+)]
+#[serde(rename_all="kebab-case")]
+#[derive(Serialize,Deserialize)]
+/// Verification Job
+pub struct VerificationJobConfig {
+    pub id: String,
+    pub store: String,
+    #[serde(skip_serializing_if="Option::is_none")]
+    pub ignore_verified: Option<bool>,
+    #[serde(skip_serializing_if="Option::is_none")]
+    pub outdated_after: Option<i64>,
+    #[serde(skip_serializing_if="Option::is_none")]
+    pub comment: Option<String>,
+    #[serde(skip_serializing_if="Option::is_none")]
+    pub schedule: Option<String>,
+}
+
+
+#[api(
+    properties: {
+        id: {
+            schema: JOB_ID_SCHEMA,
+        },
+        store: {
+            schema: DATASTORE_SCHEMA,
+        },
+        "ignore-verified": {
+            optional: true,
+            schema: IGNORE_VERIFIED_BACKUPS_SCHEMA,
+        },
+        "outdated-after": {
+            optional: true,
+            schema: VERIFICATION_OUTDATED_AFTER_SCHEMA,
+        },
+        comment: {
+            optional: true,
+            schema: SINGLE_LINE_COMMENT_SCHEMA,
+        },
+        schedule: {
+            optional: true,
+            schema: VERIFICATION_SCHEDULE_SCHEMA,
+        },
+        "next-run": {
+            description: "Estimated time of the next run (UNIX epoch).",
+            optional: true,
+            type: Integer,
+        },
+        "last-run-state": {
+            description: "Result of the last run.",
+            optional: true,
+            type: String,
+        },
+        "last-run-upid": {
+            description: "Task UPID of the last run.",
+            optional: true,
+            type: String,
+        },
+        "last-run-endtime": {
+            description: "Endtime of the last run.",
+            optional: true,
+            type: Integer,
+        },
+    }
+)]
+#[serde(rename_all="kebab-case")]
+#[derive(Serialize,Deserialize)]
+/// Status of Verification Job
+pub struct VerificationJobStatus {
+    pub id: String,
+    pub store: String,
+    #[serde(skip_serializing_if="Option::is_none")]
+    pub ignore_verified: Option<bool>,
+    #[serde(skip_serializing_if="Option::is_none")]
+    pub outdated_after: Option<i64>,
+    #[serde(skip_serializing_if="Option::is_none")]
+    pub comment: Option<String>,
+    #[serde(skip_serializing_if="Option::is_none")]
+    pub schedule: Option<String>,
+    #[serde(skip_serializing_if="Option::is_none")]
+    pub next_run: Option<i64>,
+    #[serde(skip_serializing_if="Option::is_none")]
+    pub last_run_state: Option<String>,
+    #[serde(skip_serializing_if="Option::is_none")]
+    pub last_run_upid: Option<String>,
+    #[serde(skip_serializing_if="Option::is_none")]
+    pub last_run_endtime: Option<i64>,
+}
+
+
+fn init() -> SectionConfig {
+    let obj_schema = match VerificationJobConfig::API_SCHEMA {
+        Schema::Object(ref obj_schema) => obj_schema,
+        _ => unreachable!(),
+    };
+
+    let plugin = SectionConfigPlugin::new("verification".to_string(), Some(String::from("id")), obj_schema);
+    let mut config = SectionConfig::new(&JOB_ID_SCHEMA);
+    config.register_plugin(plugin);
+
+    config
+}
+
+pub const VERIFICATION_CFG_FILENAME: &str = "/etc/proxmox-backup/verification.cfg";
+pub const VERIFICATION_CFG_LOCKFILE: &str = "/etc/proxmox-backup/.verification.lck";
+
+pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> {
+
+    let content = proxmox::tools::fs::file_read_optional_string(VERIFICATION_CFG_FILENAME)?;
+    let content = content.unwrap_or_else(String::new);
+
+    let digest = openssl::sha::sha256(content.as_bytes());
+    let data = CONFIG.parse(VERIFICATION_CFG_FILENAME, &content)?;
+    Ok((data, digest))
+}
+
+pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
+    let raw = CONFIG.write(VERIFICATION_CFG_FILENAME, &config)?;
+
+    let backup_user = crate::backup::backup_user()?;
+    let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640);
+    // set the correct owner/group/permissions while saving file
+    // owner(rw) = root, group(r)= backup
+
+    let options = CreateOptions::new()
+        .perm(mode)
+        .owner(nix::unistd::ROOT)
+        .group(backup_user.gid);
+
+    replace_file(VERIFICATION_CFG_FILENAME, raw.as_bytes(), options)?;
+
+    Ok(())
+}
+
+// shell completion helper
+pub fn complete_verification_job_id(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> {
+    match config() {
+        Ok((data, _digest)) => data.sections.iter().map(|(id, _)| id.to_string()).collect(),
+        Err(_) => return vec![],
+    }
+}
\ No newline at end of file
-- 
2.20.1





^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pbs-devel] [PATCH v4 proxmox-backup 03/10] api2: add verification admin endpoint and do_verification_job function
  2020-10-20  9:10 [pbs-devel] [PATCH v4 proxmox-backup 00/10] add job based verify scheduling Hannes Laimer
  2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 01/10] rename VERIFY_SCHEDULE_SCHEMA to VERIFICATION_SCHEDULE_SCHEMA Hannes Laimer
  2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 02/10] api2: add verification job config endpoint Hannes Laimer
@ 2020-10-20  9:10 ` Hannes Laimer
  2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 04/10] proxy: add scheduling for verification jobs Hannes Laimer
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Hannes Laimer @ 2020-10-20  9:10 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/admin.rs        |   4 +-
 src/api2/admin/verify.rs | 107 +++++++++++++++++++++++++++++++++++++++
 src/backup/verify.rs     |  96 +++++++++++++++++++++++++++++++++++
 3 files changed, 206 insertions(+), 1 deletion(-)
 create mode 100644 src/api2/admin/verify.rs

diff --git a/src/api2/admin.rs b/src/api2/admin.rs
index b927ce1e..79ce29f3 100644
--- a/src/api2/admin.rs
+++ b/src/api2/admin.rs
@@ -3,10 +3,12 @@ use proxmox::list_subdirs_api_method;
 
 pub mod datastore;
 pub mod sync;
+pub mod verify;
 
 const SUBDIRS: SubdirMap = &[
     ("datastore", &datastore::ROUTER),
-    ("sync", &sync::ROUTER)
+    ("sync", &sync::ROUTER),
+    ("verify", &verify::ROUTER)
 ];
 
 pub const ROUTER: Router = Router::new()
diff --git a/src/api2/admin/verify.rs b/src/api2/admin/verify.rs
new file mode 100644
index 00000000..f61373a0
--- /dev/null
+++ b/src/api2/admin/verify.rs
@@ -0,0 +1,107 @@
+use anyhow::{format_err, Error};
+
+use proxmox::api::router::SubdirMap;
+use proxmox::{list_subdirs_api_method, sortable};
+use proxmox::api::{api, ApiMethod, Router, RpcEnvironment};
+
+use crate::api2::types::*;
+use crate::backup::do_verification_job;
+use crate::config::jobstate::{Job, JobState};
+use crate::config::verify;
+use crate::config::verify::{VerificationJobConfig, VerificationJobStatus};
+use serde_json::Value;
+use crate::tools::systemd::time::{parse_calendar_event, compute_next_event};
+use crate::server::UPID;
+
+#[api(
+    input: {
+        properties: {},
+    },
+    returns: {
+        description: "List configured jobs and their status.",
+        type: Array,
+        items: { type: verify::VerificationJobStatus },
+    },
+)]
+/// List all verification jobs
+pub fn list_verification_jobs(
+    _param: Value,
+    mut rpcenv: &mut dyn RpcEnvironment,
+) -> Result<Vec<VerificationJobStatus>, Error> {
+
+    let (config, digest) = verify::config()?;
+
+    let mut list: Vec<VerificationJobStatus> = config.convert_to_typed_array("verification")?;
+
+    for job in &mut list {
+        let last_state = JobState::load("verificationjob", &job.id)
+            .map_err(|err| format_err!("could not open statefile for {}: {}", &job.id, err))?;
+
+        let (upid, endtime, state, starttime) = match last_state {
+            JobState::Created { time } => (None, None, None, time),
+            JobState::Started { upid } => {
+                let parsed_upid: UPID = upid.parse()?;
+                (Some(upid), None, None, parsed_upid.starttime)
+            },
+            JobState::Finished { upid, state } => {
+                let parsed_upid: UPID = upid.parse()?;
+                (Some(upid), Some(state.endtime()), Some(state.to_string()), parsed_upid.starttime)
+            },
+        };
+
+        job.last_run_upid = upid;
+        job.last_run_state = state;
+        job.last_run_endtime = endtime;
+
+        let last = job.last_run_endtime.unwrap_or_else(|| starttime);
+
+        job.next_run = (|| -> Option<i64> {
+            let schedule = job.schedule.as_ref()?;
+            let event = parse_calendar_event(&schedule).ok()?;
+            // ignore errors
+            compute_next_event(&event, last, false).unwrap_or_else(|_| None)
+        })();
+    }
+
+    rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into();
+
+    Ok(list)
+}
+
+#[api(
+    input: {
+        properties: {
+            id: {
+                schema: JOB_ID_SCHEMA,
+            }
+        }
+    }
+)]
+/// Runs a verification job manually.
+fn run_verification_job(
+    id: String,
+    _info: &ApiMethod,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<String, Error> {
+    let (config, _digest) = verify::config()?;
+    let verification_job: VerificationJobConfig = config.lookup("verification", &id)?;
+
+    let userid: Userid = rpcenv.get_user().unwrap().parse()?;
+
+    let job = Job::new("verificationjob", &id)?;
+
+    let upid_str = do_verification_job(job, verification_job, &userid, None)?;
+
+    Ok(upid_str)
+}
+
+#[sortable]
+const VERIFICATION_INFO_SUBDIRS: SubdirMap = &[("run", &Router::new().post(&API_METHOD_RUN_VERIFICATION_JOB))];
+
+const VERIFICATION_INFO_ROUTER: Router = Router::new()
+    .get(&list_subdirs_api_method!(VERIFICATION_INFO_SUBDIRS))
+    .subdirs(VERIFICATION_INFO_SUBDIRS);
+
+pub const ROUTER: Router = Router::new()
+    .get(&API_METHOD_LIST_VERIFICATION_JOBS)
+    .match_all("id", &VERIFICATION_INFO_ROUTER);
diff --git a/src/backup/verify.rs b/src/backup/verify.rs
index ea3fa760..1d14870a 100644
--- a/src/backup/verify.rs
+++ b/src/backup/verify.rs
@@ -6,7 +6,10 @@ use std::time::Instant;
 use anyhow::{bail, format_err, Error};
 
 use crate::{
+    server::WorkerTask,
     api2::types::*,
+    config::jobstate::Job,
+    config::verify::VerificationJobConfig,
     backup::{
         DataStore,
         DataBlob,
@@ -504,3 +507,96 @@ pub fn verify_all_backups(
 
     Ok(errors)
 }
+
+/// Runs a verification job.
+pub fn do_verification_job(
+    mut job: Job,
+    verification_job: VerificationJobConfig,
+    userid: &Userid,
+    schedule: Option<String>,
+) -> Result<String, Error> {
+    let datastore = DataStore::lookup_datastore(&verification_job.store)?;
+
+    let mut backups_to_verify = BackupInfo::list_backups(&datastore.base_path())?;
+    if verification_job.ignore_verified.unwrap_or(true) {
+        backups_to_verify.retain(|backup_info| {
+            let manifest = match datastore.load_manifest(&backup_info.backup_dir) {
+                Ok((manifest, _)) => manifest,
+                Err(_) => return false,
+            };
+
+            let raw_verify_state = manifest.unprotected["verify_state"].clone();
+            let last_state = match serde_json::from_value::<SnapshotVerifyState>(raw_verify_state) {
+                Ok(last_state) => last_state,
+                Err(_) => return true,
+            };
+
+            let now = proxmox::tools::time::epoch_i64();
+            let days_since_last_verify = (now - last_state.upid.starttime) / 86400;
+            verification_job.outdated_after.is_some()
+                && days_since_last_verify > verification_job.outdated_after.unwrap()
+        })
+    }
+
+    let job_id = job.jobname().to_string();
+    let worker_type = job.jobtype().to_string();
+    let upid_str = WorkerTask::new_thread(
+        &worker_type,
+        Some(job.jobname().to_string()),
+        userid.clone(),
+        false,
+        move |worker| {
+            job.start(&worker.upid().to_string())?;
+
+            task_log!(worker,"Starting datastore verify job '{}'", job_id);
+            task_log!(worker,"verifying {} backups", backups_to_verify.len());
+            if let Some(event_str) = schedule {
+                task_log!(worker,"task triggered by schedule '{}'", event_str);
+            }
+
+            let verified_chunks = Arc::new(Mutex::new(HashSet::with_capacity(1024 * 16)));
+            let corrupt_chunks = Arc::new(Mutex::new(HashSet::with_capacity(64)));
+            let result = proxmox::try_block!({
+                let mut failed_dirs: Vec<String> = Vec::new();
+
+                for backup_info in backups_to_verify {
+                    let verification_result = verify_backup_dir(
+                        datastore.clone(),
+                        &backup_info.backup_dir,
+                        verified_chunks.clone(),
+                        corrupt_chunks.clone(),
+                        worker.clone(),
+                        worker.upid().clone()
+                    );
+
+                    if let Ok(false) = verification_result {
+                        failed_dirs.push(backup_info.backup_dir.to_string());
+                    } // otherwise successful or aborted
+                }
+
+                if !failed_dirs.is_empty() {
+                    task_log!(worker,"Failed to verify following snapshots:",);
+                    for dir in failed_dirs {
+                        task_log!(worker, "\t{}", dir)
+                    }
+                    bail!("verification failed - please check the log for details");
+                }
+                Ok(())
+            });
+
+            let status = worker.create_state(&result);
+
+            match job.finish(status) {
+                Err(err) => eprintln!(
+                    "could not finish job state for {}: {}",
+                    job.jobtype().to_string(),
+                    err
+                ),
+                Ok(_) => (),
+            }
+
+            result
+        },
+    )?;
+    Ok(upid_str)
+}
-- 
2.20.1





^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pbs-devel] [PATCH v4 proxmox-backup 04/10] proxy: add scheduling for verification jobs
  2020-10-20  9:10 [pbs-devel] [PATCH v4 proxmox-backup 00/10] add job based verify scheduling Hannes Laimer
                   ` (2 preceding siblings ...)
  2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 03/10] api2: add verification admin endpoint and do_verification_job function Hannes Laimer
@ 2020-10-20  9:10 ` Hannes Laimer
  2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 05/10] set a different worker_type based on what is going to be verified(snapshot, group, ds) Hannes Laimer
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Hannes Laimer @ 2020-10-20  9:10 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/bin/proxmox-backup-proxy.rs | 62 +++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index c946c20c..398488fa 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -29,6 +29,7 @@ use proxmox_backup::tools::{
 };
 
 use proxmox_backup::api2::pull::do_sync_job;
+use proxmox_backup::backup::do_verification_job;
 
 fn main() -> Result<(), Error> {
     proxmox_backup::tools::setup_safe_path_env();
@@ -213,6 +214,7 @@ async fn schedule_tasks() -> Result<(), Error> {
     schedule_datastore_prune().await;
     schedule_datastore_verification().await;
     schedule_datastore_sync_jobs().await;
+    schedule_datastore_verify_jobs().await;
     schedule_task_log_rotate().await;
 
     Ok(())
@@ -672,6 +674,66 @@ async fn schedule_datastore_sync_jobs() {
     }
 }
 
+async fn schedule_datastore_verify_jobs() {
+    use proxmox_backup::{
+        config::{verify::{self, VerificationJobConfig}, jobstate::{self, Job}},
+        tools::systemd::time::{parse_calendar_event, compute_next_event},
+    };
+    let config = match verify::config() {
+        Err(err) => {
+            eprintln!("unable to read verification job config - {}", err);
+            return;
+        }
+        Ok((config, _digest)) => config,
+    };
+    for (job_id, (_, job_config)) in config.sections {
+        let job_config: VerificationJobConfig = match serde_json::from_value(job_config) {
+            Ok(c) => c,
+            Err(err) => {
+                eprintln!("verification job config from_value failed - {}", err);
+                continue;
+            }
+        };
+        let event_str = match job_config.schedule {
+            Some(ref event_str) => event_str.clone(),
+            None => continue,
+        };
+        let event = match parse_calendar_event(&event_str) {
+            Ok(event) => event,
+            Err(err) => {
+                eprintln!("unable to parse schedule '{}' - {}", event_str, err);
+                continue;
+            }
+        };
+        let worker_type = "verificationjob";
+        let last = match jobstate::last_run_time(worker_type, &job_id) {
+            Ok(time) => time,
+            Err(err) => {
+                eprintln!("could not get last run time of {} {}: {}", worker_type, job_id, err);
+                continue;
+            }
+        };
+        let next = match compute_next_event(&event, last, false) {
+            Ok(Some(next)) => next,
+            Ok(None) => continue,
+            Err(err) => {
+                eprintln!("compute_next_event for '{}' failed - {}", event_str, err);
+                continue;
+            }
+        };
+        let now = proxmox::tools::time::epoch_i64();
+        if next > now { continue; }
+        let job = match Job::new(worker_type, &job_id) {
+            Ok(job) => job,
+            Err(_) => continue, // could not get lock
+        };
+        let userid = Userid::backup_userid().clone();
+        if let Err(err) = do_verification_job(job, job_config, &userid, Some(event_str)) {
+            eprintln!("unable to start datastore verification job {} - {}", &job_id, err);
+        }
+    }
+}
+
 async fn schedule_task_log_rotate() {
     use proxmox_backup::{
         config::jobstate::{self, Job},
-- 
2.20.1





^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pbs-devel] [PATCH v4 proxmox-backup 05/10] set a different worker_type based on what is going to be verified(snapshot, group, ds)
  2020-10-20  9:10 [pbs-devel] [PATCH v4 proxmox-backup 00/10] add job based verify scheduling Hannes Laimer
                   ` (3 preceding siblings ...)
  2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 04/10] proxy: add scheduling for verification jobs Hannes Laimer
@ 2020-10-20  9:10 ` Hannes Laimer
  2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 06/10] ui: add verification job view Hannes Laimer
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Hannes Laimer @ 2020-10-20  9:10 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/admin/datastore.rs | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 75e6d32b..640e098a 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -485,17 +485,20 @@ pub fn verify(
 
     let mut backup_dir = None;
     let mut backup_group = None;
+    let mut worker_type = "verify";
 
     match (backup_type, backup_id, backup_time) {
         (Some(backup_type), Some(backup_id), Some(backup_time)) => {
             worker_id = format!("{}_{}_{}_{:08X}", store, backup_type, backup_id, backup_time);
             let dir = BackupDir::new(backup_type, backup_id, backup_time)?;
             backup_dir = Some(dir);
+            worker_type = "verify_snapshot";
         }
         (Some(backup_type), Some(backup_id), None) => {
             worker_id = format!("{}_{}_{}", store, backup_type, backup_id);
             let group = BackupGroup::new(backup_type, backup_id);
             backup_group = Some(group);
+            worker_type = "verify_group";
         }
         (None, None, None) => {
             worker_id = store.clone();
@@ -507,7 +510,7 @@ pub fn verify(
     let to_stdout = if rpcenv.env_type() == RpcEnvironmentType::CLI { true } else { false };
 
     let upid_str = WorkerTask::new_thread(
-        "verify",
+        worker_type,
         Some(worker_id.clone()),
         userid,
         to_stdout,
-- 
2.20.1





^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pbs-devel] [PATCH v4 proxmox-backup 06/10] ui: add verification job view
  2020-10-20  9:10 [pbs-devel] [PATCH v4 proxmox-backup 00/10] add job based verify scheduling Hannes Laimer
                   ` (4 preceding siblings ...)
  2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 05/10] set a different worker_type based on what is going to be verified(snapshot, group, ds) Hannes Laimer
@ 2020-10-20  9:10 ` Hannes Laimer
  2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 07/10] ui: add verification job edit window Hannes Laimer
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Hannes Laimer @ 2020-10-20  9:10 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 www/Makefile             |   1 +
 www/NavigationTree.js    |   6 +
 www/config/VerifyView.js | 280 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 287 insertions(+)
 create mode 100644 www/config/VerifyView.js

diff --git a/www/Makefile b/www/Makefile
index 8e1ebbfc..a6c46718 100644
--- a/www/Makefile
+++ b/www/Makefile
@@ -16,6 +16,7 @@ JSSRC=							\
 	config/RemoteView.js				\
 	config/ACLView.js				\
 	config/SyncView.js				\
+	config/VerifyView.js				\
 	config/DataStoreConfig.js			\
 	window/UserEdit.js				\
 	window/UserPassword.js				\
diff --git a/www/NavigationTree.js b/www/NavigationTree.js
index 3fa782e1..a2835cd2 100644
--- a/www/NavigationTree.js
+++ b/www/NavigationTree.js
@@ -42,6 +42,12 @@ Ext.define('PBS.store.NavigationStore', {
 			path: 'pbsSyncJobView',
 			leaf: true,
 		    },
+		    {
+			text: gettext('Verify Jobs'),
+			iconCls: 'fa fa-check-circle',
+			path: 'pbsVerifyJobView',
+			leaf: true,
+		    },
 		    {
 			text: gettext('Subscription'),
 			iconCls: 'fa fa-support',
diff --git a/www/config/VerifyView.js b/www/config/VerifyView.js
new file mode 100644
index 00000000..91ceec46
--- /dev/null
+++ b/www/config/VerifyView.js
@@ -0,0 +1,280 @@
+Ext.define('pbs-verify-jobs-status', {
+    extend: 'Ext.data.Model',
+    fields: [
+	'id', 'store', 'outdated-after', 'ignore-verified', 'schedule',
+	'next-run', 'last-run-upid', 'last-run-state', 'last-run-endtime',
+	{
+	    name: 'duration',
+	    calculate: function(data) {
+		let endtime = data['last-run-endtime'];
+		if (!endtime) return undefined;
+		let task = Proxmox.Utils.parse_task_upid(data['last-run-upid']);
+		return endtime - task.starttime;
+	    },
+	},
+    ],
+    idProperty: 'id',
+    proxy: {
+	type: 'proxmox',
+	url: '/api2/json/admin/verify',
+    },
+});
+
+Ext.define('PBS.config.VerifyJobView', {
+    extend: 'Ext.grid.GridPanel',
+    alias: 'widget.pbsVerifyJobView',
+
+    stateful: true,
+    stateId: 'grid-verify-jobs',
+
+    title: gettext('Verify Jobs'),
+
+    controller: {
+	xclass: 'Ext.app.ViewController',
+
+	addVerifyJob: function() {
+	    let me = this;
+	    Ext.create('PBS.window.VerifyJobEdit', {
+		listeners: {
+		    destroy: function() {
+			me.reload();
+		    },
+		},
+	    }).show();
+	},
+
+	editVerifyJob: function() {
+	    let me = this;
+	    let view = me.getView();
+	    let selection = view.getSelection();
+	    if (selection.length < 1) return;
+
+	    Ext.create('PBS.window.VerifyJobEdit', {
+		id: selection[0].data.id,
+		listeners: {
+		    destroy: function() {
+			me.reload();
+		    },
+		},
+	    }).show();
+	},
+
+	openTaskLog: function() {
+	    let me = this;
+	    let view = me.getView();
+	    let selection = view.getSelection();
+	    if (selection.length < 1) return;
+
+	    let upid = selection[0].data['last-run-upid'];
+	    if (!upid) return;
+
+	    Ext.create('Proxmox.window.TaskViewer', {
+		upid
+	    }).show();
+	},
+
+	runVerifyJob: function() {
+	    let me = this;
+	    let view = me.getView();
+	    let selection = view.getSelection();
+	    if (selection.length < 1) return;
+
+	    let id = selection[0].data.id;
+	    Proxmox.Utils.API2Request({
+		method: 'POST',
+		url: `/admin/verify/${id}/run`,
+		success: function(response, opt) {
+		    Ext.create('Proxmox.window.TaskViewer', {
+			upid: response.result.data,
+			taskDone: function(success) {
+			    me.reload();
+			},
+		    }).show();
+		},
+		failure: function(response, opt) {
+		    Ext.Msg.alert(gettext('Error'), response.htmlStatus);
+		},
+	    });
+	},
+
+	render_verify_status: function(value, metadata, record) {
+	    if (!record.data['last-run-upid']) {
+		return '-';
+	    }
+
+	    if (!record.data['last-run-endtime']) {
+		metadata.tdCls = 'x-grid-row-loading';
+		return '';
+	    }
+
+	    let parsed = Proxmox.Utils.parse_task_status(value);
+	    let text = value;
+	    let icon = '';
+	    switch (parsed) {
+		case 'unknown':
+		    icon = 'question faded';
+		    text = Proxmox.Utils.unknownText;
+		    break;
+		case 'error':
+		    icon = 'times critical';
+		    text = Proxmox.Utils.errorText + ': ' + value;
+		    break;
+		case 'warning':
+		    icon = 'exclamation warning';
+		    break;
+		case 'ok':
+		    icon = 'check good';
+		    text = gettext("OK");
+	    }
+
+	    return `<i class="fa fa-${icon}"></i> ${text}`;
+	},
+
+	render_next_run: function(value, metadat, record) {
+	    if (!value) return '-';
+
+	    let now = new Date();
+	    let next = new Date(value*1000);
+
+	    if (next < now) {
+		return gettext('pending');
+	    }
+	    return Proxmox.Utils.render_timestamp(value);
+	},
+
+	render_optional_timestamp: function(value, metadata, record) {
+	    if (!value) return '-';
+	    return Proxmox.Utils.render_timestamp(value);
+	},
+
+	reload: function() { this.getView().getStore().rstore.load(); },
+
+	init: function(view) {
+	    Proxmox.Utils.monStoreErrors(view, view.getStore().rstore);
+	},
+    },
+
+    listeners: {
+	activate: 'reload',
+	itemdblclick: 'editVerifyJob',
+    },
+
+    store: {
+	type: 'diff',
+	autoDestroy: true,
+	autoDestroyRstore: true,
+	sorters: 'id',
+	rstore: {
+	    type: 'update',
+	    storeid: 'pbs-verify-jobs-status',
+	    model: 'pbs-verify-jobs-status',
+	    autoStart: true,
+	    interval: 5000,
+	},
+    },
+
+    tbar: [
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Add'),
+	    handler: 'addVerifyJob',
+	    selModel: false,
+	},
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Edit'),
+	    handler: 'editVerifyJob',
+	    disabled: true,
+	},
+	{
+	    xtype: 'proxmoxStdRemoveButton',
+	    baseurl: '/config/verify/',
+	    callback: 'reload',
+	},
+	'-',
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Log'),
+	    handler: 'openTaskLog',
+	    enableFn: (rec) => !!rec.data['last-run-upid'],
+	    disabled: true,
+	},
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Run now'),
+	    handler: 'runVerifyJob',
+	    disabled: true,
+	},
+    ],
+
+    viewConfig: {
+	trackOver: false,
+    },
+
+    columns: [
+	{
+	    header: gettext('Verify Job'),
+	    width: 100,
+	    sortable: true,
+	    renderer: Ext.String.htmlEncode,
+	    dataIndex: 'id',
+	},
+	{
+	    header: gettext('Datastore'),
+	    width: 100,
+	    sortable: true,
+	    dataIndex: 'store',
+	},
+	{
+	    header: gettext('Days valid'),
+	    width: 125,
+	    sortable: true,
+	    dataIndex: 'outdated-after',
+	},
+	{
+	    header: gettext('Ignore verified'),
+	    width: 125,
+	    sortable: true,
+	    renderer: Proxmox.Utils.format_boolean,
+	    dataIndex: 'ignore-verified',
+	},
+	{
+	    header: gettext('Schedule'),
+	    sortable: true,
+	    dataIndex: 'schedule',
+	},
+	{
+	    header: gettext('Status'),
+	    dataIndex: 'last-run-state',
+	    flex: 1,
+	    renderer: 'render_verify_status',
+	},
+	{
+	    header: gettext('Last Verification'),
+	    sortable: true,
+	    minWidth: 200,
+	    renderer: 'render_optional_timestamp',
+	    dataIndex: 'last-run-endtime',
+	},
+	{
+	    text: gettext('Duration'),
+	    dataIndex: 'duration',
+	    width: 60,
+	    renderer: Proxmox.Utils.render_duration,
+	},
+	{
+	    header: gettext('Next Run'),
+	    sortable: true,
+	    minWidth: 200,
+	    renderer: 'render_next_run',
+	    dataIndex: 'next-run',
+	},
+	{
+	    header: gettext('Comment'),
+	    hidden: true,
+	    sortable: true,
+	    renderer: Ext.String.htmlEncode,
+	    dataIndex: 'comment',
+	},
+    ],
+});
-- 
2.20.1





^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pbs-devel] [PATCH v4 proxmox-backup 07/10] ui: add verification job edit window
  2020-10-20  9:10 [pbs-devel] [PATCH v4 proxmox-backup 00/10] add job based verify scheduling Hannes Laimer
                   ` (5 preceding siblings ...)
  2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 06/10] ui: add verification job view Hannes Laimer
@ 2020-10-20  9:10 ` Hannes Laimer
  2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 08/10] ui: add task descriptions for the different types of verification(job, snapshot, group, ds) Hannes Laimer
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Hannes Laimer @ 2020-10-20  9:10 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 www/Makefile                |  1 +
 www/window/VerifyJobEdit.js | 93 +++++++++++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+)
 create mode 100644 www/window/VerifyJobEdit.js

diff --git a/www/Makefile b/www/Makefile
index a6c46718..e04af930 100644
--- a/www/Makefile
+++ b/www/Makefile
@@ -20,6 +20,7 @@ JSSRC=							\
 	config/DataStoreConfig.js			\
 	window/UserEdit.js				\
 	window/UserPassword.js				\
+	window/VerifyJobEdit.js				\
 	window/RemoteEdit.js				\
 	window/SyncJobEdit.js				\
 	window/ACLEdit.js				\
diff --git a/www/window/VerifyJobEdit.js b/www/window/VerifyJobEdit.js
new file mode 100644
index 00000000..9d29eba7
--- /dev/null
+++ b/www/window/VerifyJobEdit.js
@@ -0,0 +1,93 @@
+Ext.define('PBS.window.VerifyJobEdit', {
+    extend: 'Proxmox.window.Edit',
+    alias: 'widget.pbsVerifyJobEdit',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    userid: undefined,
+
+    onlineHelp: 'verifyjobs',
+
+    isAdd: true,
+
+    subject: gettext('VerifyJob'),
+
+    fieldDefaults: { labelWidth: 120 },
+
+    cbindData: function(initialConfig) {
+	let me = this;
+
+	let baseurl = '/api2/extjs/config/verify';
+	let id = initialConfig.id;
+
+	me.isCreate = !id;
+	me.url = id ? `${baseurl}/${id}` : baseurl;
+	me.method = id ? 'PUT' : 'POST';
+	me.autoLoad = !!id;
+	return { };
+    },
+
+    items: {
+	xtype: 'inputpanel',
+	column1: [
+	    {
+		fieldLabel: gettext('Verify Job ID'),
+		xtype: 'pmxDisplayEditField',
+		name: 'id',
+		renderer: Ext.htmlEncode,
+		allowBlank: false,
+		minLength: 4,
+		cbind: {
+		    editable: '{isCreate}',
+		},
+	    },
+	    {
+		fieldLabel: gettext('Datastore'),
+		xtype: 'pbsDataStoreSelector',
+		allowBlank: false,
+		name: 'store',
+	    },
+	    {
+		xtype: 'proxmoxintegerfield',
+		fieldLabel: gettext('Days valid'),
+		minValue: 1,
+		value: '',
+		allowBlank: true,
+		name: 'outdated-after',
+		emptyText: gettext('no expiration'),
+		cbind: {
+		    deleteEmpty: '{!isCreate}',
+		},
+	    },
+	],
+
+	column2: [
+	    {
+		fieldLabel: gettext('Ignore verified'),
+		xtype: 'proxmoxcheckbox',
+		name: 'ignore-verified',
+		uncheckedValue: false,
+		value: true,
+	    },
+	    {
+		fieldLabel: gettext('Schedule'),
+		xtype: 'pbsCalendarEvent',
+		name: 'schedule',
+		emptyText: gettext('none'),
+		cbind: {
+		    deleteEmpty: '{!isCreate}',
+		},
+	    },
+	],
+
+	columnB: [
+	    {
+		fieldLabel: gettext('Comment'),
+		xtype: 'proxmoxtextfield',
+		name: 'comment',
+		cbind: {
+		    deleteEmpty: '{!isCreate}',
+		},
+	    },
+	],
+    },
+});
-- 
2.20.1





^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pbs-devel] [PATCH v4 proxmox-backup 08/10] ui: add task descriptions for the different types of verification(job, snapshot, group, ds)
  2020-10-20  9:10 [pbs-devel] [PATCH v4 proxmox-backup 00/10] add job based verify scheduling Hannes Laimer
                   ` (6 preceding siblings ...)
  2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 07/10] ui: add verification job edit window Hannes Laimer
@ 2020-10-20  9:10 ` Hannes Laimer
  2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 09/10] api proxy: remove old verification scheduling Hannes Laimer
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Hannes Laimer @ 2020-10-20  9:10 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 www/Utils.js | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/www/Utils.js b/www/Utils.js
index 8ad332eb..221a2f2b 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -91,9 +91,12 @@ Ext.define('PBS.Utils', {
 	Proxmox.Utils.override_task_descriptions({
 	    garbage_collection: ['Datastore', gettext('Garbage collect')],
 	    sync: ['Datastore', gettext('Remote Sync')],
+	    verify: ['Datastore', gettext('Verification')],
+	    verify_group: ['Group', gettext('Verification')],
+	    verify_snapshot: ['Snapshot', gettext('Verification')],
 	    syncjob: [gettext('Sync Job'), gettext('Remote Sync')],
+	    verifyjob: [gettext('Verify Job'), gettext('Scheduled Verification')],
 	    prune: (type, id) => PBS.Utils.render_datastore_worker_id(id, gettext('Prune')),
-	    verify: (type, id) => PBS.Utils.render_datastore_worker_id(id, gettext('Verify')),
 	    backup: (type, id) => PBS.Utils.render_datastore_worker_id(id, gettext('Backup')),
 	    reader: (type, id) => PBS.Utils.render_datastore_worker_id(id, gettext('Read objects')),
 	    logrotate: [gettext('Log'), gettext('Rotation')],
-- 
2.20.1





^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pbs-devel] [PATCH v4 proxmox-backup 09/10] api proxy: remove old verification scheduling
  2020-10-20  9:10 [pbs-devel] [PATCH v4 proxmox-backup 00/10] add job based verify scheduling Hannes Laimer
                   ` (7 preceding siblings ...)
  2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 08/10] ui: add task descriptions for the different types of verification(job, snapshot, group, ds) Hannes Laimer
@ 2020-10-20  9:10 ` Hannes Laimer
  2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 10/10] postinst: correct invalid old datastore configs Hannes Laimer
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Hannes Laimer @ 2020-10-20  9:10 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/config/datastore.rs    |  24 -------
 src/bin/proxmox-backup-proxy.rs | 116 --------------------------------
 src/config/datastore.rs         |   6 --
 www/config/DataStoreConfig.js   |   2 +-
 www/window/DataStoreEdit.js     |   9 ---
 5 files changed, 1 insertion(+), 156 deletions(-)

diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index ad5c03a6..4f9cb7c7 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -75,10 +75,6 @@ pub fn list_datastores(
                 optional: true,
                 schema: PRUNE_SCHEDULE_SCHEMA,
             },
-            "verify-schedule": {
-                optional: true,
-                schema: VERIFICATION_SCHEDULE_SCHEMA,
-            },
             "keep-last": {
                 optional: true,
                 schema: PRUNE_SCHEMA_KEEP_LAST,
@@ -133,7 +129,6 @@ pub fn create_datastore(param: Value) -> Result<(), Error> {
 
     crate::config::jobstate::create_state_file("prune", &datastore.name)?;
     crate::config::jobstate::create_state_file("garbage_collection", &datastore.name)?;
-    crate::config::jobstate::create_state_file("verify", &datastore.name)?;
 
     Ok(())
 }
@@ -179,8 +174,6 @@ pub enum DeletableProperty {
     gc_schedule,
     /// Delete the prune job schedule.
     prune_schedule,
-    /// Delete the verify schedule property
-    verify_schedule,
     /// Delete the keep-last property
     keep_last,
     /// Delete the keep-hourly property
@@ -214,10 +207,6 @@ pub enum DeletableProperty {
                 optional: true,
                 schema: PRUNE_SCHEDULE_SCHEMA,
             },
-            "verify-schedule": {
-                optional: true,
-                schema: VERIFICATION_SCHEDULE_SCHEMA,
-            },
             "keep-last": {
                 optional: true,
                 schema: PRUNE_SCHEMA_KEEP_LAST,
@@ -266,7 +255,6 @@ pub fn update_datastore(
     comment: Option<String>,
     gc_schedule: Option<String>,
     prune_schedule: Option<String>,
-    verify_schedule: Option<String>,
     keep_last: Option<u64>,
     keep_hourly: Option<u64>,
     keep_daily: Option<u64>,
@@ -295,7 +283,6 @@ pub fn update_datastore(
                 DeletableProperty::comment => { data.comment = None; },
                 DeletableProperty::gc_schedule => { data.gc_schedule = None; },
                 DeletableProperty::prune_schedule => { data.prune_schedule = None; },
-                DeletableProperty::verify_schedule => { data.verify_schedule = None; },
                 DeletableProperty::keep_last => { data.keep_last = None; },
                 DeletableProperty::keep_hourly => { data.keep_hourly = None; },
                 DeletableProperty::keep_daily => { data.keep_daily = None; },
@@ -327,12 +314,6 @@ pub fn update_datastore(
         data.prune_schedule = prune_schedule;
     }
 
-    let mut verify_schedule_changed = false;
-    if verify_schedule.is_some() {
-        verify_schedule_changed = data.verify_schedule != verify_schedule;
-        data.verify_schedule = verify_schedule;
-    }
-
     if keep_last.is_some() { data.keep_last = keep_last; }
     if keep_hourly.is_some() { data.keep_hourly = keep_hourly; }
     if keep_daily.is_some() { data.keep_daily = keep_daily; }
@@ -354,10 +335,6 @@ pub fn update_datastore(
         crate::config::jobstate::create_state_file("prune", &name)?;
     }
 
-    if verify_schedule_changed {
-        crate::config::jobstate::create_state_file("verify", &name)?;
-    }
-
     Ok(())
 }
 
@@ -400,7 +377,6 @@ pub fn delete_datastore(name: String, digest: Option<String>) -> Result<(), Erro
     // ignore errors
     let _ = crate::config::jobstate::remove_state_file("prune", &name);
     let _ = crate::config::jobstate::remove_state_file("garbage_collection", &name);
-    let _ = crate::config::jobstate::remove_state_file("verify", &name);
 
     Ok(())
 }
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index 398488fa..375ce078 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -212,7 +212,6 @@ async fn schedule_tasks() -> Result<(), Error> {
 
     schedule_datastore_garbage_collection().await;
     schedule_datastore_prune().await;
-    schedule_datastore_verification().await;
     schedule_datastore_sync_jobs().await;
     schedule_datastore_verify_jobs().await;
     schedule_task_log_rotate().await;
@@ -486,121 +485,6 @@ async fn schedule_datastore_prune() {
     }
 }
 
-async fn schedule_datastore_verification() {
-    use proxmox_backup::backup::{DataStore, verify_all_backups};
-    use proxmox_backup::server::{WorkerTask};
-    use proxmox_backup::config::{
-        jobstate::{self, Job},
-        datastore::{self, DataStoreConfig}
-    };
-    use proxmox_backup::tools::systemd::time::{
-        parse_calendar_event, compute_next_event};
-
-    let config = match datastore::config() {
-        Err(err) => {
-            eprintln!("unable to read datastore config - {}", err);
-            return;
-        }
-        Ok((config, _digest)) => config,
-    };
-
-    for (store, (_, store_config)) in config.sections {
-        let datastore = match DataStore::lookup_datastore(&store) {
-            Ok(datastore) => datastore,
-            Err(err) => {
-                eprintln!("lookup_datastore failed - {}", err);
-                continue;
-            }
-        };
-
-        let store_config: DataStoreConfig = match serde_json::from_value(store_config) {
-            Ok(c) => c,
-            Err(err) => {
-                eprintln!("datastore config from_value failed - {}", err);
-                continue;
-            }
-        };
-
-        let event_str = match store_config.verify_schedule {
-            Some(event_str) => event_str,
-            None => continue,
-        };
-
-        let event = match parse_calendar_event(&event_str) {
-            Ok(event) => event,
-            Err(err) => {
-                eprintln!("unable to parse schedule '{}' - {}", event_str, err);
-                continue;
-            }
-        };
-
-        let worker_type = "verify";
-
-        let last = match jobstate::last_run_time(worker_type, &store) {
-            Ok(time) => time,
-            Err(err) => {
-                eprintln!("could not get last run time of {} {}: {}", worker_type, store, err);
-                continue;
-            }
-        };
-
-        let next = match compute_next_event(&event, last, false) {
-            Ok(Some(next)) => next,
-            Ok(None) => continue,
-            Err(err) => {
-                eprintln!("compute_next_event for '{}' failed - {}", event_str, err);
-                continue;
-            }
-        };
-
-        let now = proxmox::tools::time::epoch_i64();
-
-        if next > now { continue; }
-
-        let mut job = match Job::new(worker_type, &store) {
-            Ok(job) => job,
-            Err(_) => continue, // could not get lock
-        };
-
-        let worker_id = store.clone();
-        let store2 = store.clone();
-        if let Err(err) = WorkerTask::new_thread(
-            worker_type,
-            Some(worker_id),
-            Userid::backup_userid().clone(),
-            false,
-            move |worker| {
-                job.start(&worker.upid().to_string())?;
-                worker.log(format!("starting verification on store {}", store2));
-                worker.log(format!("task triggered by schedule '{}'", event_str));
-                let result = try_block!({
-                    let failed_dirs =
-                        verify_all_backups(datastore, worker.clone(), worker.upid())?;
-                    if failed_dirs.len() > 0 {
-                        worker.log("Failed to verify following snapshots:");
-                        for dir in failed_dirs {
-                            worker.log(format!("\t{}", dir));
-                        }
-                        Err(format_err!("verification failed - please check the log for details"))
-                    } else {
-                        Ok(())
-                    }
-                });
-
-                let status = worker.create_state(&result);
-
-                if let Err(err) = job.finish(status) {
-                    eprintln!("could not finish job state for {}: {}", worker_type, err);
-                }
-
-                result
-            },
-        ) {
-            eprintln!("unable to start verification on store {} - {}", store, err);
-        }
-    }
-}
-
 async fn schedule_datastore_sync_jobs() {
 
     use proxmox_backup::{
diff --git a/src/config/datastore.rs b/src/config/datastore.rs
index 3284a63d..df81e1db 100644
--- a/src/config/datastore.rs
+++ b/src/config/datastore.rs
@@ -44,10 +44,6 @@ pub const DIR_NAME_SCHEMA: Schema = StringSchema::new("Directory name").schema()
             optional: true,
             schema: PRUNE_SCHEDULE_SCHEMA,
         },
-        "verify-schedule": {
-            optional: true,
-            schema: VERIFICATION_SCHEDULE_SCHEMA,
-        },
         "keep-last": {
             optional: true,
             schema: PRUNE_SCHEMA_KEEP_LAST,
@@ -87,8 +83,6 @@ pub struct DataStoreConfig {
     #[serde(skip_serializing_if="Option::is_none")]
     pub prune_schedule: Option<String>,
     #[serde(skip_serializing_if="Option::is_none")]
-    pub verify_schedule: Option<String>,
-    #[serde(skip_serializing_if="Option::is_none")]
     pub keep_last: Option<u64>,
     #[serde(skip_serializing_if="Option::is_none")]
     pub keep_hourly: Option<u64>,
diff --git a/www/config/DataStoreConfig.js b/www/config/DataStoreConfig.js
index b7e78e2f..440feea5 100644
--- a/www/config/DataStoreConfig.js
+++ b/www/config/DataStoreConfig.js
@@ -12,7 +12,7 @@ Ext.define('pbs-data-store-config', {
     extend: 'Ext.data.Model',
     fields: [
 	'name', 'path', 'comment', 'gc-schedule', 'prune-schedule',
-	'verify-schedule', 'keep-last', 'keep-hourly', 'keep-daily',
+	'keep-last', 'keep-hourly', 'keep-daily',
 	'keep-weekly', 'keep-monthly', 'keep-yearly',
     ],
     proxy: {
diff --git a/www/window/DataStoreEdit.js b/www/window/DataStoreEdit.js
index 5e7ccb78..ab2f3175 100644
--- a/www/window/DataStoreEdit.js
+++ b/www/window/DataStoreEdit.js
@@ -77,15 +77,6 @@ Ext.define('PBS.DataStoreEdit', {
 			    deleteEmpty: '{!isCreate}',
 			},
 		    },
-		{
-			xtype: 'pbsCalendarEvent',
-			name: 'verify-schedule',
-			fieldLabel: gettext("Verify Schedule"),
-			emptyText: gettext('none'),
-			cbind: {
-			    deleteEmpty: '{!isCreate}',
-			},
-		    },
 		],
 		columnB: [
 		    {
-- 
2.20.1





^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pbs-devel] [PATCH v4 proxmox-backup 10/10] postinst: correct invalid old datastore configs
  2020-10-20  9:10 [pbs-devel] [PATCH v4 proxmox-backup 00/10] add job based verify scheduling Hannes Laimer
                   ` (8 preceding siblings ...)
  2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 09/10] api proxy: remove old verification scheduling Hannes Laimer
@ 2020-10-20  9:10 ` Hannes Laimer
  2020-10-20 17:18 ` [pbs-devel] [PATCH v4 proxmox-backup 00/10] add job based verify scheduling Thomas Lamprecht
  2020-10-21 10:54 ` [pbs-devel] applied-series: " Thomas Lamprecht
  11 siblings, 0 replies; 13+ messages in thread
From: Hannes Laimer @ 2020-10-20  9:10 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---

 debian/postinst | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/debian/postinst b/debian/postinst
index adf49da6..2e6e145f 100644
--- a/debian/postinst
+++ b/debian/postinst
@@ -15,6 +15,8 @@ case "$1" in
 	fi
 	deb-systemd-invoke $_dh_action proxmox-backup.service proxmox-backup-proxy.service >/dev/null || true
 
+	flock -w 30 /etc/proxmox-backup/.datastore.lck sed -i '/^\s\+verify-schedule /d' /etc/proxmox-backup/datastore.cfg
+
 	# FIXME: Remove in future version once we're sure no broken entries remain in anyone's files
 	if grep -q -e ':termproxy::[^@]\+: ' /var/log/proxmox-backup/tasks/active; then
 	    echo "Fixing up termproxy user id in task log..."
-- 
2.20.1





^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pbs-devel] [PATCH v4 proxmox-backup 00/10] add job based verify scheduling
  2020-10-20  9:10 [pbs-devel] [PATCH v4 proxmox-backup 00/10] add job based verify scheduling Hannes Laimer
                   ` (9 preceding siblings ...)
  2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 10/10] postinst: correct invalid old datastore configs Hannes Laimer
@ 2020-10-20 17:18 ` Thomas Lamprecht
  2020-10-21 10:54 ` [pbs-devel] applied-series: " Thomas Lamprecht
  11 siblings, 0 replies; 13+ messages in thread
From: Thomas Lamprecht @ 2020-10-20 17:18 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Hannes Laimer

On 20.10.20 11:10, Hannes Laimer wrote:
> Replaces the first implementation of scheduled verification with a new
> job-based version with additional options that may be specified through
> the web ui.
> 
> Options available for verification jobs:
>  * schedule when to run the job
>  * set datastore on which the job should run
>  * set a number of days after which a verification becomes "outdated"
>     empty => verifications are valid forever
>  * specify if already successfuly verified snapshots should be verified
>     again even if they're not outdated(failed ones will always be done)
> 
> v4:
>  * squashed patches
>  * rebased
>  * no build-breaking patch
>  * correct old config files in postinst

much nicer split and organization of the patch series and each commit builds
and tests successfully, great!

How those schedules and datastore stuff is organized in the GUI is still open,
but that should not be a blocker for this series, can be done afterwards -
Dominik and I had already some discussion about this.
@Dominik, would be good if we could hatch a more specific plan for this tomorrow.

There are two things which I find a bit "weird" or not ideal with the verification
schedule settings.

1. Why a "Verify Job ID" (possibly also s/Verify/Verification/)?
   The user already has the comment fields for notes, and forcing them to give
   each job a name makes no sense, IMO. I know that an ID is required for working
   with each schedule, but that could be internal and automatically derived.
   Either by choosing some random UUID assigned on add, similar to what we do in
   PVE for backup jobs, or, alternatively join all relevant settings (datastore id,
   schedule, days valid, ignore verified) and use that as ID (e.g., encoded or
   md5sum'd) - this could even help to avoid that user specify the exact same job
   multiple times.
   However, the ID has to go IMO, this is not something a user will often specify
   when interfacing with the PBS, like the Datstore id is, but a schedule which one
   normally does not names. (naming is not only hard for Devs, it's also hard for
   users)

2. documentation is missing

3. there was something besides that, but it's late and I'm really have to stop
   working for today ^^

Anyway, it seems to work (from a few quick tests), I'd like to have a quick talk
with Dominik tomorrow (mostly about point 1. above) and then I'd apply this,
UX stuff can then be followed up.

thanks!





^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pbs-devel] applied-series: [PATCH v4 proxmox-backup 00/10] add job based verify scheduling
  2020-10-20  9:10 [pbs-devel] [PATCH v4 proxmox-backup 00/10] add job based verify scheduling Hannes Laimer
                   ` (10 preceding siblings ...)
  2020-10-20 17:18 ` [pbs-devel] [PATCH v4 proxmox-backup 00/10] add job based verify scheduling Thomas Lamprecht
@ 2020-10-21 10:54 ` Thomas Lamprecht
  11 siblings, 0 replies; 13+ messages in thread
From: Thomas Lamprecht @ 2020-10-21 10:54 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Hannes Laimer

On 20.10.20 11:10, Hannes Laimer wrote:
> Replaces the first implementation of scheduled verification with a new
> job-based version with additional options that may be specified through
> the web ui.
> 
> Options available for verification jobs:
>  * schedule when to run the job
>  * set datastore on which the job should run
>  * set a number of days after which a verification becomes "outdated"
>     empty => verifications are valid forever
>  * specify if already successfuly verified snapshots should be verified
>     again even if they're not outdated(failed ones will always be done)
> 


applied, thanks!

Note regarding my reply from yesterday:
Dominik is checking on a idea to move that stuff into each datastore content
and I'll try to make the IDs from this and sync jobs at least optional, or
remove them all together (the only real use for them is that we can include
them in the task log).





^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-10-21 10:54 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-20  9:10 [pbs-devel] [PATCH v4 proxmox-backup 00/10] add job based verify scheduling Hannes Laimer
2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 01/10] rename VERIFY_SCHEDULE_SCHEMA to VERIFICATION_SCHEDULE_SCHEMA Hannes Laimer
2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 02/10] api2: add verification job config endpoint Hannes Laimer
2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 03/10] api2: add verification admin endpoint and do_verification_job function Hannes Laimer
2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 04/10] proxy: add scheduling for verification jobs Hannes Laimer
2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 05/10] set a different worker_type based on what is going to be verified(snapshot, group, ds) Hannes Laimer
2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 06/10] ui: add verification job view Hannes Laimer
2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 07/10] ui: add verification job edit window Hannes Laimer
2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 08/10] ui: add task descriptions for the different types of verification(job, snapshot, group, ds) Hannes Laimer
2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 09/10] api proxy: remove old verification scheduling Hannes Laimer
2020-10-20  9:10 ` [pbs-devel] [PATCH v4 proxmox-backup 10/10] postinst: correct invalid old datastore configs Hannes Laimer
2020-10-20 17:18 ` [pbs-devel] [PATCH v4 proxmox-backup 00/10] add job based verify scheduling Thomas Lamprecht
2020-10-21 10:54 ` [pbs-devel] applied-series: " Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal