From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dietmar@druiddev.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits))
(No client certificate requested)
by lists.proxmox.com (Postfix) with ESMTPS id E0A1799833
for <pbs-devel@lists.proxmox.com>; Thu, 16 Nov 2023 14:19:39 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id C211D13CA4
for <pbs-devel@lists.proxmox.com>; Thu, 16 Nov 2023 14:19:09 +0100 (CET)
Received: from druiddev.proxmox.com (unknown [94.136.29.99])
by firstgate.proxmox.com (Proxmox) with ESMTP
for <pbs-devel@lists.proxmox.com>; Thu, 16 Nov 2023 14:19:09 +0100 (CET)
Received: by druiddev.proxmox.com (Postfix, from userid 1000)
id E234187988; Thu, 16 Nov 2023 14:19:08 +0100 (CET)
From: Dietmar Maurer <dietmar@proxmox.com>
To: pbs-devel@lists.proxmox.com
Date: Thu, 16 Nov 2023 14:19:04 +0100
Message-Id: <20231116131904.2415371-1-dietmar@proxmox.com>
X-Mailer: git-send-email 2.39.2
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-SPAM-LEVEL: Spam detection results: 0
AWL -0.524 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery
methods
RDNS_NONE 0.793 Delivered to internal network by a host with no rDNS
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_NONE 0.001 SPF: sender does not publish an SPF Record
T_SCC_BODY_TEXT_LINE -0.01 -
Subject: [pbs-devel] [RFC proxmox-backup] PruneJobConfig: make "schedule"
optional
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
<pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Thu, 16 Nov 2023 13:19:39 -0000
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