[PATCH proxmox-backup v3 05/10] fix #7904: api: implement "disable" for {sync,verify,tape,GC} jobs

[Jonas Theisen <j.theisen@proxmox.com>]

To allow users to disable jobs without having to remove the
schedule this patch introduces the "disable" parameter.

This can also be used to create jobs which are supposed to
be run only manually, since with the "disable" parameter
set to true, no schedule is required.

The create endpoint for the respective jobs also now raises
a paramter error if schedule is not set but disable is also
not set i.e. false.
Updates are still allowed to not prevent unrelated config updates.

The value is inserted into the job config and honored by
the scheduler by skipping disabled jobs altogether.
For GC jobs the value is inserted into the datastore config.

If the value is not present in a job config it is assumed the job
is to be enabled for backwards compatibility and to reduce
config-bloat.

Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=7904
Signed-off-by: Jonas Theisen <j.theisen@proxmox.com>
---
 src/api2/admin/datastore.rs        |  1 +
 src/api2/admin/gc.rs               | 12 +++++++++++-
 src/api2/admin/sync.rs             |  7 ++++++-
 src/api2/admin/verify.rs           |  7 ++++++-
 src/api2/config/datastore.rs       |  9 +++++++++
 src/api2/config/sync.rs            | 18 +++++++++++++++++-
 src/api2/config/tape_backup_job.rs |  9 ++++++++-
 src/api2/config/verify.rs          | 17 ++++++++++++++++-
 src/api2/tape/backup.rs            |  6 +++++-
 src/bin/proxmox-backup-proxy.rs    | 16 ++++++++++++++++
 10 files changed, 95 insertions(+), 7 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 3367fb2c1..543f3ecf4 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -1264,6 +1264,7 @@ pub(crate) fn garbage_collection_status_unchecked(
     let mut info = GarbageCollectionJobStatus {
         store: store.clone(),
         schedule: store_config.gc_schedule,
+        disable: store_config.gc_disable,
         ..Default::default()
     };
 
diff --git a/src/api2/admin/gc.rs b/src/api2/admin/gc.rs
index 3c8bc2946..db79a2581 100644
--- a/src/api2/admin/gc.rs
+++ b/src/api2/admin/gc.rs
@@ -58,7 +58,17 @@ pub fn list_all_gc_jobs(
             .collect::<Vec<_>>(),
     };
 
-    Ok(gc_info)
+    let mut list = Vec::new();
+
+    for mut job in gc_info {
+        if job.disable {
+            job.next_run = None;
+        }
+
+        list.push(job);
+    }
+
+    Ok(list)
 }
 
 const GC_ROUTER: Router = Router::new().get(&API_METHOD_LIST_ALL_GC_JOBS);
diff --git a/src/api2/admin/sync.rs b/src/api2/admin/sync.rs
index 1799f51c0..10649ab66 100644
--- a/src/api2/admin/sync.rs
+++ b/src/api2/admin/sync.rs
@@ -112,7 +112,12 @@ pub fn list_config_sync_jobs(
             continue;
         }
 
-        let status = compute_schedule_status("syncjob", &job.id, job.schedule.as_deref())?;
+        let mut status =
+            compute_schedule_status("syncjob", &job.id, job.schedule.as_deref())?;
+
+        if job.disable {
+            status.next_run = None;
+        }
 
         list.push(SyncJobStatus {
             config: job,
diff --git a/src/api2/admin/verify.rs b/src/api2/admin/verify.rs
index 80659ebe2..413ee79d4 100644
--- a/src/api2/admin/verify.rs
+++ b/src/api2/admin/verify.rs
@@ -73,7 +73,12 @@ pub fn list_verification_jobs(
     let mut list = Vec::new();
 
     for job in job_config_iter {
-        let status = compute_schedule_status("verificationjob", &job.id, job.schedule.as_deref())?;
+        let mut status =
+            compute_schedule_status("verificationjob", &job.id, job.schedule.as_deref())?;
+
+        if job.disable {
+            status.next_run = None;
+        }
 
         list.push(VerificationJobStatus {
             config: job,
diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index a8b003601..f3644149e 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -406,6 +406,8 @@ pub fn read_datastore(
 pub enum DeletableProperty {
     /// Delete the comment property.
     Comment,
+    /// Unset the disable flag.
+    GcDisable,
     /// Delete the garbage collection schedule.
     GcSchedule,
     /// Delete the gc-on-unmount property.
@@ -493,6 +495,9 @@ pub fn update_datastore(
                 DeletableProperty::Comment => {
                     data.comment = None;
                 }
+                DeletableProperty::GcDisable => {
+                    data.gc_disable = false;
+                }
                 DeletableProperty::GcSchedule => {
                     data.gc_schedule = None;
                 }
@@ -571,6 +576,10 @@ pub fn update_datastore(
         data.gc_on_unmount = update.gc_on_unmount;
     }
 
+    if let Some(value) = update.gc_disable {
+        data.gc_disable = value;
+    }
+
     macro_rules! prune_disabled {
         ($(($param:literal, $($member:tt)+)),+) => {
             $(
diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
index beed6e7e3..e5c23dcd0 100644
--- a/src/api2/config/sync.rs
+++ b/src/api2/config/sync.rs
@@ -1,5 +1,5 @@
 use ::serde::{Deserialize, Serialize};
-use anyhow::{Error, bail};
+use anyhow::{Error, bail, format_err};
 use pbs_api_types::SyncDirection;
 use serde_json::Value;
 
@@ -306,6 +306,13 @@ pub fn create_sync_job(
         }
     }
 
+    if !config.disable && config.schedule.is_none() {
+        param_bail!(
+            "schedule",
+            format_err!("no schedule provided but job also not disabled")
+        );
+    }
+
     let (mut section_config, _digest) = sync::config()?;
 
     if section_config.sections.contains_key(&config.id) {
@@ -383,6 +390,8 @@ pub enum DeletableProperty {
     Owner,
     /// Delete the comment property.
     Comment,
+    /// Unset the disable flag.
+    Disable,
     /// Delete the job schedule.
     Schedule,
     /// Delete the remove-vanished flag.
@@ -499,6 +508,9 @@ pub fn update_sync_job(
                 DeletableProperty::Comment => {
                     data.comment = None;
                 }
+                DeletableProperty::Disable => {
+                    data.disable = false;
+                }
                 DeletableProperty::Schedule => {
                     data.schedule = None;
                 }
@@ -702,6 +714,9 @@ pub fn update_sync_job(
     if update.schedule.is_some() {
         data.schedule = update.schedule;
     }
+    if let Some(value) = update.disable {
+        data.disable = value;
+    }
     if update.remove_vanished.is_some() {
         data.remove_vanished = update.remove_vanished;
     }
@@ -847,6 +862,7 @@ acl:1:/remote/remote1/remotestore1:write@pbs:RemoteSyncOperator
         max_depth: None,
         group_filter: None,
         schedule: None,
+        disable: false,
         limit: pbs_api_types::RateLimitConfig::default(), // no limit
         transfer_last: None,
         encrypted_only: None,
diff --git a/src/api2/config/tape_backup_job.rs b/src/api2/config/tape_backup_job.rs
index 4bfc7f871..bb77abec5 100644
--- a/src/api2/config/tape_backup_job.rs
+++ b/src/api2/config/tape_backup_job.rs
@@ -1,5 +1,5 @@
 use ::serde::{Deserialize, Serialize};
-use anyhow::Error;
+use anyhow::{Error, format_err};
 use serde_json::Value;
 
 use proxmox_router::{Permission, Router, RpcEnvironment, http_bail};
@@ -79,6 +79,13 @@ pub fn create_tape_backup_job(
 
     check_tape_backup_permission(&auth_id, &job.setup)?;
 
+    if !job.disable && job.schedule.is_none() {
+        param_bail!(
+            "schedule",
+            format_err!("no schedule provided but job also not disabled")
+        );
+    }
+
     let _lock = pbs_config::tape_job::lock()?;
 
     let (mut config, _digest) = pbs_config::tape_job::config()?;
diff --git a/src/api2/config/verify.rs b/src/api2/config/verify.rs
index 35be9b175..afd0c3c85 100644
--- a/src/api2/config/verify.rs
+++ b/src/api2/config/verify.rs
@@ -1,5 +1,5 @@
 use ::serde::{Deserialize, Serialize};
-use anyhow::Error;
+use anyhow::{Error, format_err};
 use serde_json::Value;
 
 use proxmox_router::{Permission, Router, RpcEnvironment, http_bail};
@@ -80,6 +80,13 @@ pub fn create_verification_job(
 
     user_info.check_privs(&auth_id, &config.acl_path(), PRIV_DATASTORE_VERIFY, false)?;
 
+    if !config.disable && config.schedule.is_none() {
+        param_bail!(
+            "schedule",
+            format_err!("no schedule provided but job also not disabled")
+        );
+    }
+
     let _lock = verify::lock_config()?;
 
     let (mut section_config, _digest) = verify::config()?;
@@ -140,6 +147,8 @@ pub enum DeletableProperty {
     IgnoreVerified,
     /// Delete the comment property.
     Comment,
+    /// Unset the disable flag.
+    Disable,
     /// Delete the job schedule.
     Schedule,
     /// Delete outdated after property.
@@ -220,6 +229,9 @@ pub fn update_verification_job(
                 DeletableProperty::Comment => {
                     data.comment = None;
                 }
+                DeletableProperty::Disable => {
+                    data.disable = false;
+                }
                 DeletableProperty::Schedule => {
                     data.schedule = None;
                 }
@@ -262,6 +274,9 @@ pub fn update_verification_job(
     if update.schedule.is_some() {
         data.schedule = update.schedule;
     }
+    if let Some(value) = update.disable {
+        data.disable = value;
+    }
     if let Some(ns) = update.ns {
         if !ns.is_root() {
             data.ns = Some(ns);
diff --git a/src/api2/tape/backup.rs b/src/api2/tape/backup.rs
index c0a4ca65b..f86ab2e47 100644
--- a/src/api2/tape/backup.rs
+++ b/src/api2/tape/backup.rs
@@ -83,7 +83,11 @@ pub fn list_tape_backup_jobs(
 
         let status = compute_schedule_status("tape-backup-job", &job.id, job.schedule.as_deref())?;
 
-        let next_run = status.next_run.unwrap_or(current_time);
+        let next_run = if job.disable {
+            0
+        } else {
+            status.next_run.unwrap_or(current_time)
+        };
 
         let mut next_media_label = None;
 
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index c66cffeb4..ce2629d0e 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -537,6 +537,10 @@ async fn schedule_datastore_garbage_collection() {
             }
         };
 
+        if store_config.gc_disable {
+            continue;
+        }
+
         let event_str = match store_config.gc_schedule {
             Some(event_str) => event_str,
             None => continue,
@@ -689,6 +693,10 @@ async fn schedule_datastore_sync_jobs() {
             None => continue,
         };
 
+        if job_config.disable {
+            continue;
+        }
+
         let worker_type = "syncjob";
         if check_schedule(worker_type, &event_str, &job_id) {
             let job = match Job::new(worker_type, &job_id) {
@@ -725,6 +733,10 @@ async fn schedule_datastore_verify_jobs() {
             None => continue,
         };
 
+        if job_config.disable {
+            continue;
+        }
+
         let worker_type = "verificationjob";
         let auth_id = Authid::root_auth_id().clone();
         if check_schedule(worker_type, &event_str, &job_id) {
@@ -761,6 +773,10 @@ async fn schedule_tape_backup_jobs() {
             None => continue,
         };
 
+        if job_config.disable {
+            continue;
+        }
+
         let worker_type = "tape-backup-job";
         let auth_id = Authid::root_auth_id().clone();
         if check_schedule(worker_type, &event_str, &job_id) {
-- 
2.47.3