public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH v2 proxmox-backup 00/15] add job based verify scheduling
@ 2020-10-07  9:03 Hannes Laimer
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 01/15] add two new schemas for verify jobs Hannes Laimer
                   ` (14 more replies)
  0 siblings, 15 replies; 20+ messages in thread
From: Hannes Laimer @ 2020-10-07  9:03 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)

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 (15):
  add two new schemas for verify jobs
  add verify job config
  api2: add verify job config endpoint
  add do_verification_job function to verify.rs
  api2: add verify job admin endpoint
  add scheduling for verify jobs
  set a diffrent worker_type based on what is going to be
    verified(snapshot, group, ds)
  ui: add verify job view
  ui: add verify job edit window
  ui: add task descriptions for the different types of verify(job,
    snapshot, group, ds)
  remove verify_schedule field from DatastoreConfig
  remove verify_schedule field from datastore config endpoint
  remove verify-schedule field from DataStoreEdit and DataStoreConfig
  remove old verification scheduling from proxmox-backup-proxy.rs
  verify job: log number of planned verifications

 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       | 275 +++++++++++++++++++++++++++++++
 src/api2/types/mod.rs           |  10 ++
 src/backup/verify.rs            |  72 ++++++++
 src/bin/proxmox-backup-proxy.rs | 115 ++++---------
 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     |  89 ++++++++++
 19 files changed, 1076 insertions(+), 127 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] 20+ messages in thread

* [pbs-devel] [PATCH v2 proxmox-backup 01/15] add two new schemas for verify jobs
  2020-10-07  9:03 [pbs-devel] [PATCH v2 proxmox-backup 00/15] add job based verify scheduling Hannes Laimer
@ 2020-10-07  9:03 ` Hannes Laimer
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 02/15] add verify job config Hannes Laimer
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Hannes Laimer @ 2020-10-07  9:03 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/types/mod.rs | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
index 348d3bf4..eabeab92 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();
-- 
2.20.1





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

* [pbs-devel] [PATCH v2 proxmox-backup 02/15] add verify job config
  2020-10-07  9:03 [pbs-devel] [PATCH v2 proxmox-backup 00/15] add job based verify scheduling Hannes Laimer
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 01/15] add two new schemas for verify jobs Hannes Laimer
@ 2020-10-07  9:03 ` Hannes Laimer
  2020-10-08  7:50   ` Wolfgang Bumiller
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 03/15] api2: add verify job config endpoint Hannes Laimer
                   ` (12 subsequent siblings)
  14 siblings, 1 reply; 20+ messages in thread
From: Hannes Laimer @ 2020-10-07  9:03 UTC (permalink / raw)
  To: pbs-devel

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

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..7c92a16e
--- /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: VERIFY_SCHEDULE_SCHEMA,
+        },
+    }
+)]
+#[serde(rename_all="kebab-case")]
+#[derive(Serialize,Deserialize)]
+/// Verify Job
+pub struct VerifyJobConfig {
+    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>,
+}
+
+// FIXME: generate duplicate schemas/structs from one listing?
+#[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: VERIFY_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 Verify Job
+pub struct VerifyJobStatus {
+    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 VerifyJobConfig::API_SCHEMA {
+        Schema::Object(ref obj_schema) => obj_schema,
+        _ => unreachable!(),
+    };
+
+    let plugin = SectionConfigPlugin::new("verify".to_string(), Some(String::from("id")), obj_schema);
+    let mut config = SectionConfig::new(&JOB_ID_SCHEMA);
+    config.register_plugin(plugin);
+
+    config
+}
+
+pub const VERIFY_CFG_FILENAME: &str = "/etc/proxmox-backup/verify.cfg";
+pub const VERIFY_CFG_LOCKFILE: &str = "/etc/proxmox-backup/.verify.lck";
+
+pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> {
+
+    let content = proxmox::tools::fs::file_read_optional_string(VERIFY_CFG_FILENAME)?;
+    let content = content.unwrap_or(String::from(""));
+
+    let digest = openssl::sha::sha256(content.as_bytes());
+    let data = CONFIG.parse(VERIFY_CFG_FILENAME, &content)?;
+    Ok((data, digest))
+}
+
+pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
+    let raw = CONFIG.write(VERIFY_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(VERIFY_CFG_FILENAME, raw.as_bytes(), options)?;
+
+    Ok(())
+}
+
+// shell completion helper
+pub fn complete_verify_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] 20+ messages in thread

* [pbs-devel] [PATCH v2 proxmox-backup 03/15] api2: add verify job config endpoint
  2020-10-07  9:03 [pbs-devel] [PATCH v2 proxmox-backup 00/15] add job based verify scheduling Hannes Laimer
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 01/15] add two new schemas for verify jobs Hannes Laimer
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 02/15] add verify job config Hannes Laimer
@ 2020-10-07  9:03 ` Hannes Laimer
  2020-10-08  8:22   ` Wolfgang Bumiller
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 04/15] add do_verification_job function to verify.rs Hannes Laimer
                   ` (11 subsequent siblings)
  14 siblings, 1 reply; 20+ messages in thread
From: Hannes Laimer @ 2020-10-07  9:03 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 | 275 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 277 insertions(+)
 create mode 100644 src/api2/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..9897aaf9
--- /dev/null
+++ b/src/api2/config/verify.rs
@@ -0,0 +1,275 @@
+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, VerifyJobConfig};
+
+#[api(
+    input: {
+        properties: {},
+    },
+    returns: {
+        description: "List configured jobs.",
+        type: Array,
+        items: { type: verify::VerifyJobConfig },
+    },
+)]
+/// List all verify jobs
+pub fn list_verify_jobs(
+    _param: Value,
+    mut rpcenv: &mut dyn RpcEnvironment,
+) -> Result<Vec<VerifyJobConfig>, Error> {
+
+    let (config, digest) = verify::config()?;
+
+    let list = config.convert_to_typed_array("verify")?;
+
+    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: VERIFY_SCHEDULE_SCHEMA,
+            },
+        }
+    }
+)]
+/// Create a new verify job.
+pub fn create_verify_job(param: Value) -> Result<(), Error> {
+
+    let _lock = open_file_locked(verify::VERIFY_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+
+    let verify_job: verify::VerifyJobConfig = serde_json::from_value(param.clone())?;
+
+    let (mut config, _digest) = verify::config()?;
+
+    if let Some(_) = config.sections.get(&verify_job.id) {
+        bail!("job '{}' already exists.", verify_job.id);
+    }
+
+    config.set_data(&verify_job.id, "verify", &verify_job)?;
+
+    verify::save_config(&config)?;
+
+    crate::config::jobstate::create_state_file("verifyjob", &verify_job.id)?;
+
+    Ok(())
+}
+
+#[api(
+   input: {
+        properties: {
+            id: {
+                schema: JOB_ID_SCHEMA,
+            },
+        },
+    },
+    returns: {
+        description: "The verify job configuration.",
+        type: verify::VerifyJobConfig,
+    },
+)]
+/// Read a verify job configuration.
+pub fn read_verify_job(
+    id: String,
+    mut rpcenv: &mut dyn RpcEnvironment,
+) -> Result<VerifyJobConfig, Error> {
+    let (config, digest) = verify::config()?;
+
+    let verify_job = config.lookup("verify", &id)?;
+    rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into();
+
+    Ok(verify_job)
+}
+
+#[api()]
+#[derive(Serialize, Deserialize)]
+#[serde(rename_all="kebab-case")]
+#[allow(non_camel_case_types)]
+/// Deletable property name
+pub enum DeletableProperty {
+    /// Delete ignore verified
+    ignore_verified,
+    /// Delete the comment property.
+    comment,
+    /// Delete the job schedule.
+    schedule,
+    /// Delete outdated_after.
+    outdated_after
+}
+
+#[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: VERIFY_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 verify job config.
+pub fn update_verify_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::VERIFY_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::VerifyJobConfig = config.lookup("verify", &id)?;
+
+     if let Some(delete) = delete {
+        for delete_prop in delete {
+            match delete_prop {
+                DeletableProperty::ignore_verified => { data.ignore_verified = None; },
+                DeletableProperty::outdated_after => { 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, "verify", &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 verify job configuration
+pub fn delete_verify_job(id: String, digest: Option<String>) -> Result<(), Error> {
+
+    let _lock = open_file_locked(verify::VERIFY_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("verifyjob", &id)?;
+
+    Ok(())
+}
+
+const ITEM_ROUTER: Router = Router::new()
+    .get(&API_METHOD_READ_VERIFY_JOB)
+    .put(&API_METHOD_UPDATE_VERIFY_JOB)
+    .delete(&API_METHOD_DELETE_VERIFY_JOB);
+
+pub const ROUTER: Router = Router::new()
+    .get(&API_METHOD_LIST_VERIFY_JOBS)
+    .post(&API_METHOD_CREATE_VERIFY_JOB)
+    .match_all("id", &ITEM_ROUTER);
\ No newline at end of file
-- 
2.20.1





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

* [pbs-devel] [PATCH v2 proxmox-backup 04/15] add do_verification_job function to verify.rs
  2020-10-07  9:03 [pbs-devel] [PATCH v2 proxmox-backup 00/15] add job based verify scheduling Hannes Laimer
                   ` (2 preceding siblings ...)
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 03/15] api2: add verify job config endpoint Hannes Laimer
@ 2020-10-07  9:03 ` Hannes Laimer
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 05/15] api2: add verify job admin endpoint Hannes Laimer
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Hannes Laimer @ 2020-10-07  9:03 UTC (permalink / raw)
  To: pbs-devel

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

diff --git a/src/backup/verify.rs b/src/backup/verify.rs
index 0c55305f..517ecbb2 100644
--- a/src/backup/verify.rs
+++ b/src/backup/verify.rs
@@ -8,6 +8,8 @@ use anyhow::{bail, format_err, Error};
 use crate::{
     server::WorkerTask,
     api2::types::*,
+    config::jobstate::Job,
+    config::verify::VerifyJobConfig,
     tools::ParallelHandler,
     backup::{
         DataStore,
@@ -419,3 +421,72 @@ pub fn verify_all_backups(datastore: Arc<DataStore>, worker: Arc<WorkerTask>) ->
 
     Ok(errors)
 }
+
+/// Runs a verification job.
+pub fn do_verification_job(
+    mut job: Job,
+    verify_job: VerifyJobConfig,
+    userid: &Userid,
+    schedule: Option<String>,
+) -> Result<String, Error> {
+    let datastore = DataStore::lookup_datastore(&verify_job.store)?;
+    let mut backups_to_verify = BackupInfo::list_backups(&datastore.base_path())?;
+    if verify_job.ignore_verified.unwrap_or(true) {
+        backups_to_verify.retain(|backup_info| {
+            if let Ok((manifest, _)) = datastore.load_manifest(&backup_info.backup_dir) {
+                let verify = manifest.unprotected["verify_state"].clone();
+                if let Ok(verify) = serde_json::from_value::<SnapshotVerifyState>(verify) {
+                    let days_since_last_verify =
+                        (proxmox::tools::time::epoch_i64() - verify.upid.starttime) / 86400;
+                    // if outdated_after is None, verifications do not become outdated
+                    verify_job.outdated_after.is_some() && days_since_last_verify > verify_job.outdated_after.unwrap()
+                } else { true } // was never verified, therefore we always want to verify
+            } else { false } // manifest could not be loaded, do not verify in that case
+        })
+    }
+    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())?;
+            let verified_chunks = Arc::new(Mutex::new(HashSet::with_capacity(1024 * 16)));
+            let corrupt_chunks = Arc::new(Mutex::new(HashSet::with_capacity(64)));
+            worker.log(format!("Starting datastore verify job '{}'", job_id));
+            if let Some(event_str) = schedule {
+                worker.log(format!("task triggered by schedule '{}'", event_str));
+            }
+            let result = proxmox::try_block!({
+                let mut failed_dirs: Vec<String> = Vec::new();
+                for backup_info in backups_to_verify {
+                    if let Ok(false) = verify_backup_dir(
+                        datastore.clone(),
+                        &backup_info.backup_dir,
+                        verified_chunks.clone(),
+                        corrupt_chunks.clone(),
+                        worker.clone(),
+                    ) { failed_dirs.push(backup_info.backup_dir.to_string()); }
+                }
+                if !failed_dirs.is_empty() {
+                    worker.log("Failed to verify following snapshots:");
+                    for dir in failed_dirs {
+                        worker.log(format!("\t{}", dir));
+                    }
+                    bail!("verification failed - please check the log for details");
+                }
+                Ok(())
+            });
+
+            let status = worker.create_state(&result);
+
+            if let Err(err) = job.finish(status) {
+                eprintln!("could not finish job state for {}: {}", job.jobtype().to_string(), err);
+            }
+
+            result
+        })?;
+    Ok(upid_str)
+}
-- 
2.20.1





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

* [pbs-devel] [PATCH v2 proxmox-backup 05/15] api2: add verify job admin endpoint
  2020-10-07  9:03 [pbs-devel] [PATCH v2 proxmox-backup 00/15] add job based verify scheduling Hannes Laimer
                   ` (3 preceding siblings ...)
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 04/15] add do_verification_job function to verify.rs Hannes Laimer
@ 2020-10-07  9:03 ` Hannes Laimer
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 06/15] add scheduling for verify jobs Hannes Laimer
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Hannes Laimer @ 2020-10-07  9:03 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 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 110 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..06262efb
--- /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::{VerifyJobConfig, VerifyJobStatus};
+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::VerifyJobStatus },
+    },
+)]
+/// List all verify jobs
+pub fn list_verify_jobs(
+    _param: Value,
+    mut rpcenv: &mut dyn RpcEnvironment,
+) -> Result<Vec<VerifyJobStatus>, Error> {
+
+    let (config, digest) = verify::config()?;
+
+    let mut list: Vec<VerifyJobStatus> = config.convert_to_typed_array("verify")?;
+
+    for job in &mut list {
+        let last_state = JobState::load("verifyjob", &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 verify job manually.
+fn run_verify_job(
+    id: String,
+    _info: &ApiMethod,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<String, Error> {
+    let (config, _digest) = verify::config()?;
+    let verify_job: VerifyJobConfig = config.lookup("verify", &id)?;
+
+    let userid: Userid = rpcenv.get_user().unwrap().parse()?;
+
+    let job = Job::new("verifyjob", &id)?;
+
+    let upid_str = do_verification_job(job, verify_job, &userid, None)?;
+
+    Ok(upid_str)
+}
+
+#[sortable]
+const VERIFY_INFO_SUBDIRS: SubdirMap = &[("run", &Router::new().post(&API_METHOD_RUN_VERIFY_JOB))];
+
+const VERIFY_INFO_ROUTER: Router = Router::new()
+    .get(&list_subdirs_api_method!(VERIFY_INFO_SUBDIRS))
+    .subdirs(VERIFY_INFO_SUBDIRS);
+
+pub const ROUTER: Router = Router::new()
+    .get(&API_METHOD_LIST_VERIFY_JOBS)
+    .match_all("id", &VERIFY_INFO_ROUTER);
-- 
2.20.1





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

* [pbs-devel] [PATCH v2 proxmox-backup 06/15] add scheduling for verify jobs
  2020-10-07  9:03 [pbs-devel] [PATCH v2 proxmox-backup 00/15] add job based verify scheduling Hannes Laimer
                   ` (4 preceding siblings ...)
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 05/15] api2: add verify job admin endpoint Hannes Laimer
@ 2020-10-07  9:03 ` Hannes Laimer
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 07/15] set a diffrent worker_type based on what is going to be verified(snapshot, group, ds) Hannes Laimer
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Hannes Laimer @ 2020-10-07  9:03 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 67fbc541..79d5fe50 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -19,6 +19,7 @@ use proxmox_backup::auth_helpers::*;
 use proxmox_backup::tools::disks::{ DiskManage, zfs_pool_stats };
 
 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();
@@ -198,6 +199,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(())
@@ -656,6 +658,66 @@ async fn schedule_datastore_sync_jobs() {
     }
 }
 
+async fn schedule_datastore_verify_jobs() {
+    use proxmox_backup::{
+        config::{ verify::{self, VerifyJobConfig}, jobstate::{self, Job} },
+        tools::systemd::time::{ parse_calendar_event, compute_next_event },
+    };
+    let config = match verify::config() {
+        Err(err) => {
+            eprintln!("unable to read verify job config - {}", err);
+            return;
+        }
+        Ok((config, _digest)) => config,
+    };
+    for (job_id, (_, job_config)) in config.sections {
+        let job_config: VerifyJobConfig = match serde_json::from_value(job_config) {
+            Ok(c) => c,
+            Err(err) => {
+                eprintln!("verify 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 = "verifyjob";
+        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 verify 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] 20+ messages in thread

* [pbs-devel] [PATCH v2 proxmox-backup 07/15] set a diffrent worker_type based on what is going to be verified(snapshot, group, ds)
  2020-10-07  9:03 [pbs-devel] [PATCH v2 proxmox-backup 00/15] add job based verify scheduling Hannes Laimer
                   ` (5 preceding siblings ...)
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 06/15] add scheduling for verify jobs Hannes Laimer
@ 2020-10-07  9:03 ` Hannes Laimer
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 08/15] ui: add verify job view Hannes Laimer
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Hannes Laimer @ 2020-10-07  9:03 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 af3af0ad..6ae606d9 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -486,17 +486,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();
@@ -508,7 +511,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] 20+ messages in thread

* [pbs-devel] [PATCH v2 proxmox-backup 08/15] ui: add verify job view
  2020-10-07  9:03 [pbs-devel] [PATCH v2 proxmox-backup 00/15] add job based verify scheduling Hannes Laimer
                   ` (6 preceding siblings ...)
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 07/15] set a diffrent worker_type based on what is going to be verified(snapshot, group, ds) Hannes Laimer
@ 2020-10-07  9:03 ` Hannes Laimer
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 09/15] ui: add verify job edit window Hannes Laimer
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Hannes Laimer @ 2020-10-07  9:03 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] 20+ messages in thread

* [pbs-devel] [PATCH v2 proxmox-backup 09/15] ui: add verify job edit window
  2020-10-07  9:03 [pbs-devel] [PATCH v2 proxmox-backup 00/15] add job based verify scheduling Hannes Laimer
                   ` (7 preceding siblings ...)
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 08/15] ui: add verify job view Hannes Laimer
@ 2020-10-07  9:03 ` Hannes Laimer
  2020-10-08  9:58   ` Dominik Csapak
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 10/15] ui: add task descriptions for the different types of verify(job, snapshot, group, ds) Hannes Laimer
                   ` (5 subsequent siblings)
  14 siblings, 1 reply; 20+ messages in thread
From: Hannes Laimer @ 2020-10-07  9:03 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 www/Makefile                |  1 +
 www/window/VerifyJobEdit.js | 89 +++++++++++++++++++++++++++++++++++++
 2 files changed, 90 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..985d2ef6
--- /dev/null
+++ b/www/window/VerifyJobEdit.js
@@ -0,0 +1,89 @@
+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'
+	    },
+	],
+
+	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] 20+ messages in thread

* [pbs-devel] [PATCH v2 proxmox-backup 10/15] ui: add task descriptions for the different types of verify(job, snapshot, group, ds)
  2020-10-07  9:03 [pbs-devel] [PATCH v2 proxmox-backup 00/15] add job based verify scheduling Hannes Laimer
                   ` (8 preceding siblings ...)
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 09/15] ui: add verify job edit window Hannes Laimer
@ 2020-10-07  9:03 ` Hannes Laimer
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 11/15] remove verify_schedule field from DatastoreConfig Hannes Laimer
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Hannes Laimer @ 2020-10-07  9:03 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] 20+ messages in thread

* [pbs-devel] [PATCH v2 proxmox-backup 11/15] remove verify_schedule field from DatastoreConfig
  2020-10-07  9:03 [pbs-devel] [PATCH v2 proxmox-backup 00/15] add job based verify scheduling Hannes Laimer
                   ` (9 preceding siblings ...)
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 10/15] ui: add task descriptions for the different types of verify(job, snapshot, group, ds) Hannes Laimer
@ 2020-10-07  9:03 ` Hannes Laimer
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 12/15] remove verify_schedule field from datastore config endpoint Hannes Laimer
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Hannes Laimer @ 2020-10-07  9:03 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/config/datastore.rs | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/src/config/datastore.rs b/src/config/datastore.rs
index 1f5b4f84..7e2ae573 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: VERIFY_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>,
-- 
2.20.1





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

* [pbs-devel] [PATCH v2 proxmox-backup 12/15] remove verify_schedule field from datastore config endpoint
  2020-10-07  9:03 [pbs-devel] [PATCH v2 proxmox-backup 00/15] add job based verify scheduling Hannes Laimer
                   ` (10 preceding siblings ...)
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 11/15] remove verify_schedule field from DatastoreConfig Hannes Laimer
@ 2020-10-07  9:03 ` Hannes Laimer
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 13/15] remove verify-schedule field from DataStoreEdit and DataStoreConfig Hannes Laimer
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Hannes Laimer @ 2020-10-07  9:03 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/config/datastore.rs | 24 ------------------------
 1 file changed, 24 deletions(-)

diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index 07ca4ab8..767db1ea 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: VERIFY_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: VERIFY_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(())
 }
-- 
2.20.1





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

* [pbs-devel] [PATCH v2 proxmox-backup 13/15] remove verify-schedule field from DataStoreEdit and DataStoreConfig
  2020-10-07  9:03 [pbs-devel] [PATCH v2 proxmox-backup 00/15] add job based verify scheduling Hannes Laimer
                   ` (11 preceding siblings ...)
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 12/15] remove verify_schedule field from datastore config endpoint Hannes Laimer
@ 2020-10-07  9:03 ` Hannes Laimer
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 14/15] remove old verification scheduling from proxmox-backup-proxy.rs Hannes Laimer
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 15/15] verify job: log number of planned verifications Hannes Laimer
  14 siblings, 0 replies; 20+ messages in thread
From: Hannes Laimer @ 2020-10-07  9:03 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 www/config/DataStoreConfig.js | 2 +-
 www/window/DataStoreEdit.js   | 9 ---------
 2 files changed, 1 insertion(+), 10 deletions(-)

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 f565cee5..3cf218f1 100644
--- a/www/window/DataStoreEdit.js
+++ b/www/window/DataStoreEdit.js
@@ -74,15 +74,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] 20+ messages in thread

* [pbs-devel] [PATCH v2 proxmox-backup 14/15] remove old verification scheduling from proxmox-backup-proxy.rs
  2020-10-07  9:03 [pbs-devel] [PATCH v2 proxmox-backup 00/15] add job based verify scheduling Hannes Laimer
                   ` (12 preceding siblings ...)
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 13/15] remove verify-schedule field from DataStoreEdit and DataStoreConfig Hannes Laimer
@ 2020-10-07  9:03 ` Hannes Laimer
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 15/15] verify job: log number of planned verifications Hannes Laimer
  14 siblings, 0 replies; 20+ messages in thread
From: Hannes Laimer @ 2020-10-07  9:03 UTC (permalink / raw)
  To: pbs-devel

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

diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index 79d5fe50..1c0d3857 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -197,7 +197,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;
@@ -471,120 +470,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())?;
-                    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::{
-- 
2.20.1





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

* [pbs-devel] [PATCH v2 proxmox-backup 15/15] verify job: log number of planned verifications
  2020-10-07  9:03 [pbs-devel] [PATCH v2 proxmox-backup 00/15] add job based verify scheduling Hannes Laimer
                   ` (13 preceding siblings ...)
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 14/15] remove old verification scheduling from proxmox-backup-proxy.rs Hannes Laimer
@ 2020-10-07  9:03 ` Hannes Laimer
  14 siblings, 0 replies; 20+ messages in thread
From: Hannes Laimer @ 2020-10-07  9:03 UTC (permalink / raw)
  To: pbs-devel

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

diff --git a/src/backup/verify.rs b/src/backup/verify.rs
index 517ecbb2..fac2677e 100644
--- a/src/backup/verify.rs
+++ b/src/backup/verify.rs
@@ -459,6 +459,7 @@ pub fn do_verification_job(
             if let Some(event_str) = schedule {
                 worker.log(format!("task triggered by schedule '{}'", event_str));
             }
+            worker.log(format!("verifying {} backups", backups_to_verify.len()));
             let result = proxmox::try_block!({
                 let mut failed_dirs: Vec<String> = Vec::new();
                 for backup_info in backups_to_verify {
-- 
2.20.1





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

* Re: [pbs-devel] [PATCH v2 proxmox-backup 02/15] add verify job config
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 02/15] add verify job config Hannes Laimer
@ 2020-10-08  7:50   ` Wolfgang Bumiller
  2020-10-08  8:15     ` Dominik Csapak
  0 siblings, 1 reply; 20+ messages in thread
From: Wolfgang Bumiller @ 2020-10-08  7:50 UTC (permalink / raw)
  To: Hannes Laimer; +Cc: pbs-devel

On Wed, Oct 07, 2020 at 11:03:11AM +0200, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  src/config.rs        |   1 +
>  src/config/verify.rs | 189 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 190 insertions(+)
>  create mode 100644 src/config/verify.rs
> 
> 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..7c92a16e
> --- /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: VERIFY_SCHEDULE_SCHEMA,
> +        },
> +    }
> +)]
> +#[serde(rename_all="kebab-case")]
> +#[derive(Serialize,Deserialize)]
> +/// Verify Job
> +pub struct VerifyJobConfig {

First things first - can we name this `VerificationJobConfig`? I'm not a
fan of using a verb here...

> +    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>,
> +}
> +
> +// FIXME: generate duplicate schemas/structs from one listing?

Why though?

So my question there is, what is this supposed to actually represent?

It is a separate struct and it contains the keys from the other struct,
but how do they relate to each other? (And I mean apart from "we do this
in PVE, too", I do not consider that a good argument...)

Do you really need all of these fields in there or really just the id?

And if you want/need all the info as part of the state, what's stopping
you from simply using an actual struct member of type `VerifyJobConfig`
instead of flattening it out?

> +#[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: VERIFY_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 Verify Job
> +pub struct VerifyJobStatus {
> +    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 VerifyJobConfig::API_SCHEMA {
> +        Schema::Object(ref obj_schema) => obj_schema,
> +        _ => unreachable!(),
> +    };
> +
> +    let plugin = SectionConfigPlugin::new("verify".to_string(), Some(String::from("id")), obj_schema);
> +    let mut config = SectionConfig::new(&JOB_ID_SCHEMA);
> +    config.register_plugin(plugin);
> +
> +    config
> +}
> +
> +pub const VERIFY_CFG_FILENAME: &str = "/etc/proxmox-backup/verify.cfg";
> +pub const VERIFY_CFG_LOCKFILE: &str = "/etc/proxmox-backup/.verify.lck";
> +
> +pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> {
> +
> +    let content = proxmox::tools::fs::file_read_optional_string(VERIFY_CFG_FILENAME)?;
> +    let content = content.unwrap_or(String::from(""));

Use `unwrap_or_else(String::new)`.

> +
> +    let digest = openssl::sha::sha256(content.as_bytes());
> +    let data = CONFIG.parse(VERIFY_CFG_FILENAME, &content)?;
> +    Ok((data, digest))
> +}
> +
> +pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
> +    let raw = CONFIG.write(VERIFY_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(VERIFY_CFG_FILENAME, raw.as_bytes(), options)?;
> +
> +    Ok(())
> +}
> +
> +// shell completion helper
> +pub fn complete_verify_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] 20+ messages in thread

* Re: [pbs-devel] [PATCH v2 proxmox-backup 02/15] add verify job config
  2020-10-08  7:50   ` Wolfgang Bumiller
@ 2020-10-08  8:15     ` Dominik Csapak
  0 siblings, 0 replies; 20+ messages in thread
From: Dominik Csapak @ 2020-10-08  8:15 UTC (permalink / raw)
  To: pbs-devel

On 10/8/20 9:50 AM, Wolfgang Bumiller wrote:
> On Wed, Oct 07, 2020 at 11:03:11AM +0200, Hannes Laimer wrote:
>> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
>> ---
>>   src/config.rs        |   1 +
>>   src/config/verify.rs | 189 +++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 190 insertions(+)
>>   create mode 100644 src/config/verify.rs
>>
>> 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..7c92a16e
>> --- /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: VERIFY_SCHEDULE_SCHEMA,
>> +        },
>> +    }
>> +)]
>> +#[serde(rename_all="kebab-case")]
>> +#[derive(Serialize,Deserialize)]
>> +/// Verify Job
>> +pub struct VerifyJobConfig {
> 
> First things first - can we name this `VerificationJobConfig`? I'm not a
> fan of using a verb here...
> 
>> +    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>,
>> +}
>> +
>> +// FIXME: generate duplicate schemas/structs from one listing?
> 
> Why though?
> 
> So my question there is, what is this supposed to actually represent?
> 
> It is a separate struct and it contains the keys from the other struct,
> but how do they relate to each other? (And I mean apart from "we do this
> in PVE, too", I do not consider that a good argument...)
> 
> Do you really need all of these fields in there or really just the id?
> 
> And if you want/need all the info as part of the state, what's stopping
> you from simply using an actual struct member of type `VerifyJobConfig`
> instead of flattening it out?
> 

fyi, hannes probably copied it from our sync jobs

the rationale here is that we need a list of all those properties below
in a flat array (so no nested object) for easy use in the gui
and we wanted to have a strict type (Vec<VerifyJobStatus>) instead of
Vec<Value> (or simply Value) to get some compile time guarantees

but the additional fields do not belong in the config so we needed
a second struct

>> +#[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: VERIFY_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 Verify Job
>> +pub struct VerifyJobStatus {
>> +    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 VerifyJobConfig::API_SCHEMA {
>> +        Schema::Object(ref obj_schema) => obj_schema,
>> +        _ => unreachable!(),
>> +    };
>> +
>> +    let plugin = SectionConfigPlugin::new("verify".to_string(), Some(String::from("id")), obj_schema);
>> +    let mut config = SectionConfig::new(&JOB_ID_SCHEMA);
>> +    config.register_plugin(plugin);
>> +
>> +    config
>> +}
>> +
>> +pub const VERIFY_CFG_FILENAME: &str = "/etc/proxmox-backup/verify.cfg";
>> +pub const VERIFY_CFG_LOCKFILE: &str = "/etc/proxmox-backup/.verify.lck";
>> +
>> +pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> {
>> +
>> +    let content = proxmox::tools::fs::file_read_optional_string(VERIFY_CFG_FILENAME)?;
>> +    let content = content.unwrap_or(String::from(""));
> 
> Use `unwrap_or_else(String::new)`.
> 
>> +
>> +    let digest = openssl::sha::sha256(content.as_bytes());
>> +    let data = CONFIG.parse(VERIFY_CFG_FILENAME, &content)?;
>> +    Ok((data, digest))
>> +}
>> +
>> +pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
>> +    let raw = CONFIG.write(VERIFY_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(VERIFY_CFG_FILENAME, raw.as_bytes(), options)?;
>> +
>> +    Ok(())
>> +}
>> +
>> +// shell completion helper
>> +pub fn complete_verify_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
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 





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

* Re: [pbs-devel] [PATCH v2 proxmox-backup 03/15] api2: add verify job config endpoint
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 03/15] api2: add verify job config endpoint Hannes Laimer
@ 2020-10-08  8:22   ` Wolfgang Bumiller
  0 siblings, 0 replies; 20+ messages in thread
From: Wolfgang Bumiller @ 2020-10-08  8:22 UTC (permalink / raw)
  To: Hannes Laimer; +Cc: pbs-devel

On Wed, Oct 07, 2020 at 11:03:12AM +0200, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  src/api2/config.rs        |   2 +
>  src/api2/config/verify.rs | 275 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 277 insertions(+)
>  create mode 100644 src/api2/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..9897aaf9
> --- /dev/null
> +++ b/src/api2/config/verify.rs
> @@ -0,0 +1,275 @@
> +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, VerifyJobConfig};
> +
> +#[api(
> +    input: {
> +        properties: {},
> +    },
> +    returns: {
> +        description: "List configured jobs.",
> +        type: Array,
> +        items: { type: verify::VerifyJobConfig },
> +    },
> +)]
> +/// List all verify jobs
> +pub fn list_verify_jobs(
> +    _param: Value,
> +    mut rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<Vec<VerifyJobConfig>, Error> {
> +
> +    let (config, digest) = verify::config()?;
> +
> +    let list = config.convert_to_typed_array("verify")?;
> +
> +    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: VERIFY_SCHEDULE_SCHEMA,
> +            },
> +        }
> +    }
> +)]
> +/// Create a new verify job.
> +pub fn create_verify_job(param: Value) -> Result<(), Error> {
> +
> +    let _lock = open_file_locked(verify::VERIFY_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> +
> +    let verify_job: verify::VerifyJobConfig = serde_json::from_value(param.clone())?;
> +
> +    let (mut config, _digest) = verify::config()?;
> +
> +    if let Some(_) = config.sections.get(&verify_job.id) {
> +        bail!("job '{}' already exists.", verify_job.id);
> +    }
> +
> +    config.set_data(&verify_job.id, "verify", &verify_job)?;
> +
> +    verify::save_config(&config)?;
> +
> +    crate::config::jobstate::create_state_file("verifyjob", &verify_job.id)?;
> +
> +    Ok(())
> +}
> +
> +#[api(
> +   input: {
> +        properties: {
> +            id: {
> +                schema: JOB_ID_SCHEMA,
> +            },
> +        },
> +    },
> +    returns: {
> +        description: "The verify job configuration.",
> +        type: verify::VerifyJobConfig,
> +    },
> +)]
> +/// Read a verify job configuration.
> +pub fn read_verify_job(
> +    id: String,
> +    mut rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<VerifyJobConfig, Error> {
> +    let (config, digest) = verify::config()?;
> +
> +    let verify_job = config.lookup("verify", &id)?;
> +    rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into();
> +
> +    Ok(verify_job)
> +}
> +
> +#[api()]
> +#[derive(Serialize, Deserialize)]
> +#[serde(rename_all="kebab-case")]
> +#[allow(non_camel_case_types)]

You already `rename-all`, please drop the non-camel-case-types and just
use standard style.

> +/// Deletable property name
> +pub enum DeletableProperty {
> +    /// Delete ignore verified
> +    ignore_verified,
> +    /// Delete the comment property.
> +    comment,
> +    /// Delete the job schedule.
> +    schedule,
> +    /// Delete outdated_after.
> +    outdated_after
> +}
> +
> +#[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: VERIFY_SCHEDULE_SCHEMA,
> +            },

^ This is the part that needs actual fixing in the schema, because this
is where all the properties become optional. (That wouldn't striclty be
necessary, though, I mean most of the time we have the complete data
available already anyway before doing the call...)
But before this can be fixed in the `#[api]` macro it needs to be fixed
in the schema code itself.

> +            delete: {
> +                description: "List of properties to delete.",
> +                type: Array,
> +                optional: true,
> +                items: {
> +                    type: DeletableProperty,
> +                }
> +            },
> +            digest: {
> +                optional: true,
> +                schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
> +            },
> +        },
> +    },
> +)]
> +/// Update verify job config.
> +pub fn update_verify_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::VERIFY_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::VerifyJobConfig = config.lookup("verify", &id)?;
> +
> +     if let Some(delete) = delete {
> +        for delete_prop in delete {
> +            match delete_prop {
> +                DeletableProperty::ignore_verified => { data.ignore_verified = None; },
> +                DeletableProperty::outdated_after => { 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, "verify", &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 verify job configuration
> +pub fn delete_verify_job(id: String, digest: Option<String>) -> Result<(), Error> {
> +
> +    let _lock = open_file_locked(verify::VERIFY_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("verifyjob", &id)?;
> +
> +    Ok(())
> +}
> +
> +const ITEM_ROUTER: Router = Router::new()
> +    .get(&API_METHOD_READ_VERIFY_JOB)
> +    .put(&API_METHOD_UPDATE_VERIFY_JOB)
> +    .delete(&API_METHOD_DELETE_VERIFY_JOB);
> +
> +pub const ROUTER: Router = Router::new()
> +    .get(&API_METHOD_LIST_VERIFY_JOBS)
> +    .post(&API_METHOD_CREATE_VERIFY_JOB)
> +    .match_all("id", &ITEM_ROUTER);
> \ No newline at end of file
> -- 
> 2.20.1




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

* Re: [pbs-devel] [PATCH v2 proxmox-backup 09/15] ui: add verify job edit window
  2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 09/15] ui: add verify job edit window Hannes Laimer
@ 2020-10-08  9:58   ` Dominik Csapak
  0 siblings, 0 replies; 20+ messages in thread
From: Dominik Csapak @ 2020-10-08  9:58 UTC (permalink / raw)
  To: pbs-devel

one comment inline

On 10/7/20 11:03 AM, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>   www/Makefile                |  1 +
>   www/window/VerifyJobEdit.js | 89 +++++++++++++++++++++++++++++++++++++
>   2 files changed, 90 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..985d2ef6
> --- /dev/null
> +++ b/www/window/VerifyJobEdit.js
> @@ -0,0 +1,89 @@
> +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'

i would also add a

cbind: {
     deleteEmpty: '{!isCreate}'
}

here (basically every field that is optional and can be empty should 
have that)

> +	    },
> +	],
> +
> +	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}',
> +		},
> +	    },
> +	],
> +    },
> +});
> 





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

end of thread, other threads:[~2020-10-08  9:59 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-07  9:03 [pbs-devel] [PATCH v2 proxmox-backup 00/15] add job based verify scheduling Hannes Laimer
2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 01/15] add two new schemas for verify jobs Hannes Laimer
2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 02/15] add verify job config Hannes Laimer
2020-10-08  7:50   ` Wolfgang Bumiller
2020-10-08  8:15     ` Dominik Csapak
2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 03/15] api2: add verify job config endpoint Hannes Laimer
2020-10-08  8:22   ` Wolfgang Bumiller
2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 04/15] add do_verification_job function to verify.rs Hannes Laimer
2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 05/15] api2: add verify job admin endpoint Hannes Laimer
2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 06/15] add scheduling for verify jobs Hannes Laimer
2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 07/15] set a diffrent worker_type based on what is going to be verified(snapshot, group, ds) Hannes Laimer
2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 08/15] ui: add verify job view Hannes Laimer
2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 09/15] ui: add verify job edit window Hannes Laimer
2020-10-08  9:58   ` Dominik Csapak
2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 10/15] ui: add task descriptions for the different types of verify(job, snapshot, group, ds) Hannes Laimer
2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 11/15] remove verify_schedule field from DatastoreConfig Hannes Laimer
2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 12/15] remove verify_schedule field from datastore config endpoint Hannes Laimer
2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 13/15] remove verify-schedule field from DataStoreEdit and DataStoreConfig Hannes Laimer
2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 14/15] remove old verification scheduling from proxmox-backup-proxy.rs Hannes Laimer
2020-10-07  9:03 ` [pbs-devel] [PATCH v2 proxmox-backup 15/15] verify job: log number of planned verifications Hannes Laimer

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