[pbs-devel] [RFC proxmox-backup] PruneJobConfig: make "schedule" optional

[Dietmar Maurer <dietmar@proxmox.com>]

The API macro already mark this property as optional:

  schedule: {
            schema: PRUNE_SCHEDULE_SCHEMA,
            optional: true,
  },

So the correct representation is Option<String> (not String).

This is now consistent with other job types.
---

Honestly, I have no idea if it was indented to make schedule non-optional?

Also, why have prune jobs a disable property, while other jobs types
do not have it?

Also, update_to_prune_jobs_config() looks wrong now because it remove jobs
without a schedule. Was this really indendet?


 pbs-api-types/src/jobs.rs               |  3 ++-
 src/api2/admin/prune.rs                 |  2 +-
 src/api2/config/prune.rs                | 12 ++++++++----
 src/bin/proxmox-backup-proxy.rs         |  9 +++++++--
 src/bin/proxmox_backup_manager/prune.rs |  2 +-
 5 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/pbs-api-types/src/jobs.rs b/pbs-api-types/src/jobs.rs
index f3e64ee4..1fb74381 100644
--- a/pbs-api-types/src/jobs.rs
+++ b/pbs-api-types/src/jobs.rs
@@ -712,7 +712,8 @@ pub struct PruneJobConfig {
     #[updater(serde(skip_serializing_if = "Option::is_none"))]
     pub disable: bool,
 
-    pub schedule: String,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub schedule: Option<String>,
 
     #[serde(skip_serializing_if = "Option::is_none")]
     pub comment: Option<String>,
diff --git a/src/api2/admin/prune.rs b/src/api2/admin/prune.rs
index a5ebf297..85dab54f 100644
--- a/src/api2/admin/prune.rs
+++ b/src/api2/admin/prune.rs
@@ -76,7 +76,7 @@ pub fn list_prune_jobs(
         let last_state = JobState::load("prunejob", &job.id)
             .map_err(|err| format_err!("could not open statefile for {}: {}", &job.id, err))?;
 
-        let mut status = compute_schedule_status(&last_state, Some(&job.schedule))?;
+        let mut status = compute_schedule_status(&last_state, job.schedule.as_deref())?;
         if job.disable {
             status.next_run = None;
         }
diff --git a/src/api2/config/prune.rs b/src/api2/config/prune.rs
index 6f391722..4679431d 100644
--- a/src/api2/config/prune.rs
+++ b/src/api2/config/prune.rs
@@ -157,6 +157,8 @@ pub enum DeletableProperty {
     KeepMonthly,
     /// Delete number of yearly backups to keep.
     KeepYearly,
+    /// Delete the job schedule.
+    Schedule,
 }
 
 #[api(
@@ -248,6 +250,9 @@ pub fn update_prune_job(
                 DeletableProperty::KeepYearly => {
                     data.options.keep.keep_yearly = None;
                 }
+                DeletableProperty::Schedule => {
+                    data.schedule = None;
+                }
             }
         }
     }
@@ -268,10 +273,9 @@ pub fn update_prune_job(
         user_info.check_privs(&auth_id, &data.acl_path(), PRIV_DATASTORE_MODIFY, true)?;
     }
 
-    let mut schedule_changed = false;
-    if let Some(schedule) = update.schedule {
-        schedule_changed = data.schedule != schedule;
-        data.schedule = schedule;
+    let schedule_changed = data.schedule != update.schedule;
+    if update.schedule.is_some() {
+        data.schedule = update.schedule;
     }
 
     if let Some(max_depth) = update.options.max_depth {
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index f38a02bd..e7a124bc 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -556,9 +556,14 @@ async fn schedule_datastore_prune_jobs() {
             continue; // no 'keep' values set, keep all
         }
 
+        let event_str = match job_config.schedule {
+            Some(ref event_str) => event_str.clone(),
+            None => continue,
+        };
+
         let worker_type = "prunejob";
         let auth_id = Authid::root_auth_id().clone();
-        if check_schedule(worker_type, &job_config.schedule, &job_id) {
+        if check_schedule(worker_type, &event_str, &job_id) {
             let job = match Job::new(worker_type, &job_id) {
                 Ok(job) => job,
                 Err(_) => continue, // could not get lock
@@ -568,7 +573,7 @@ async fn schedule_datastore_prune_jobs() {
                 job_config.options,
                 job_config.store,
                 &auth_id,
-                Some(job_config.schedule),
+                Some(event_str),
             ) {
                 eprintln!("unable to start datastore prune job {job_id} - {err}");
             }
diff --git a/src/bin/proxmox_backup_manager/prune.rs b/src/bin/proxmox_backup_manager/prune.rs
index 923eb6f5..7738f661 100644
--- a/src/bin/proxmox_backup_manager/prune.rs
+++ b/src/bin/proxmox_backup_manager/prune.rs
@@ -245,7 +245,7 @@ pub(crate) fn update_to_prune_jobs_config() -> Result<(), Error> {
             store: store.clone(),
             disable: false,
             comment: None,
-            schedule,
+            schedule: Some(schedule),
             options,
         };
 
-- 
2.39.2