public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox-backup v3] reuse-datastore: avoid creating another prune job
@ 2024-11-26  9:54 Gabriel Goller
  2024-11-26 10:38 ` [pbs-devel] applied: " Fabian Grünbichler
  0 siblings, 1 reply; 2+ messages in thread
From: Gabriel Goller @ 2024-11-26  9:54 UTC (permalink / raw)
  To: pbs-devel

If a datastore with a prune job is removed, the prune job is preserverd
as it is stored in /etc/proxmox-backup/prune.cfg. We also create a
default prune job for every datastore – this means that when reusing a
datastore that previously existed, you end up with duplicate prune jobs.
To avoid this we check if a prune job already exists, and when it does,
we refrain from creating the default one. (We also check if specific
keep-options have been added, if yes, then we create the job
nevertheless.)

Reported-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---

v3, thanks @Christian and @Fabian:
 - don't rely on default-prune-jobs but check all
 - check if specific keep-options have been added

v2, thanks @Christian:
 - convert if-statement to inline condition

 src/api2/config/datastore.rs | 39 +++++++++++++++++++-----------------
 src/api2/config/prune.rs     | 11 ++++++++++
 2 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index 9f2dac4b22ee..121222c40396 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -24,7 +24,7 @@ use crate::api2::admin::{
     datastore::do_mount_device, prune::list_prune_jobs, sync::list_config_sync_jobs,
     verify::list_verification_jobs,
 };
-use crate::api2::config::prune::{delete_prune_job, do_create_prune_job};
+use crate::api2::config::prune::{delete_prune_job, do_create_prune_job, has_prune_job};
 use crate::api2::config::sync::delete_sync_job;
 use crate::api2::config::tape_backup_job::{delete_tape_backup_job, list_tape_backup_jobs};
 use crate::api2::config::verify::delete_verification_job;
@@ -204,23 +204,26 @@ pub fn create_datastore(
     let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
     let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
 
-    let prune_job_config = config.prune_schedule.as_ref().map(|schedule| {
-        let mut id = format!("default-{}-{}", config.name, Uuid::generate());
-        id.truncate(32);
-
-        PruneJobConfig {
-            id,
-            store: config.name.clone(),
-            comment: None,
-            disable: false,
-            schedule: schedule.clone(),
-            options: PruneJobOptions {
-                keep: config.keep.clone(),
-                max_depth: None,
-                ns: None,
-            },
-        }
-    });
+    let mut prune_job_config = None;
+    if config.keep.keeps_something() || !has_prune_job(&config.name)? {
+        prune_job_config = config.prune_schedule.as_ref().map(|schedule| {
+            let mut id = format!("default-{}-{}", config.name, Uuid::generate());
+            id.truncate(32);
+
+            PruneJobConfig {
+                id,
+                store: config.name.clone(),
+                comment: None,
+                disable: false,
+                schedule: schedule.clone(),
+                options: PruneJobOptions {
+                    keep: config.keep.clone(),
+                    max_depth: None,
+                    ns: None,
+                },
+            }
+        });
+    }
 
     // clearing prune settings in the datastore config, as they are now handled by prune jobs
     let config = DataStoreConfig {
diff --git a/src/api2/config/prune.rs b/src/api2/config/prune.rs
index ce7b8ce565ce..b433c248ac5a 100644
--- a/src/api2/config/prune.rs
+++ b/src/api2/config/prune.rs
@@ -77,6 +77,17 @@ pub fn do_create_prune_job(config: PruneJobConfig) -> Result<(), Error> {
     Ok(())
 }
 
+pub fn has_prune_job(datastore: &str) -> Result<bool, Error> {
+    let (section_config, _digest) = prune::config()?;
+    for (_, (_, job_config)) in section_config.sections {
+        let job_config: PruneJobConfig = serde_json::from_value(job_config)?;
+        if job_config.store == datastore {
+            return Ok(true);
+        }
+    }
+    Ok(false)
+}
+
 #[api(
     protected: true,
     input: {
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

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

* [pbs-devel] applied: [PATCH proxmox-backup v3] reuse-datastore: avoid creating another prune job
  2024-11-26  9:54 [pbs-devel] [PATCH proxmox-backup v3] reuse-datastore: avoid creating another prune job Gabriel Goller
@ 2024-11-26 10:38 ` Fabian Grünbichler
  0 siblings, 0 replies; 2+ messages in thread
From: Fabian Grünbichler @ 2024-11-26 10:38 UTC (permalink / raw)
  To: Gabriel Goller, pbs-devel

thanks!

On November 26, 2024 10:54 am, Gabriel Goller wrote:
> If a datastore with a prune job is removed, the prune job is preserverd
> as it is stored in /etc/proxmox-backup/prune.cfg. We also create a
> default prune job for every datastore – this means that when reusing a
> datastore that previously existed, you end up with duplicate prune jobs.
> To avoid this we check if a prune job already exists, and when it does,
> we refrain from creating the default one. (We also check if specific
> keep-options have been added, if yes, then we create the job
> nevertheless.)
> 
> Reported-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
> 
> v3, thanks @Christian and @Fabian:
>  - don't rely on default-prune-jobs but check all
>  - check if specific keep-options have been added
> 
> v2, thanks @Christian:
>  - convert if-statement to inline condition
> 
>  src/api2/config/datastore.rs | 39 +++++++++++++++++++-----------------
>  src/api2/config/prune.rs     | 11 ++++++++++
>  2 files changed, 32 insertions(+), 18 deletions(-)
> 
> diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
> index 9f2dac4b22ee..121222c40396 100644
> --- a/src/api2/config/datastore.rs
> +++ b/src/api2/config/datastore.rs
> @@ -24,7 +24,7 @@ use crate::api2::admin::{
>      datastore::do_mount_device, prune::list_prune_jobs, sync::list_config_sync_jobs,
>      verify::list_verification_jobs,
>  };
> -use crate::api2::config::prune::{delete_prune_job, do_create_prune_job};
> +use crate::api2::config::prune::{delete_prune_job, do_create_prune_job, has_prune_job};
>  use crate::api2::config::sync::delete_sync_job;
>  use crate::api2::config::tape_backup_job::{delete_tape_backup_job, list_tape_backup_jobs};
>  use crate::api2::config::verify::delete_verification_job;
> @@ -204,23 +204,26 @@ pub fn create_datastore(
>      let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
>      let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
>  
> -    let prune_job_config = config.prune_schedule.as_ref().map(|schedule| {
> -        let mut id = format!("default-{}-{}", config.name, Uuid::generate());
> -        id.truncate(32);
> -
> -        PruneJobConfig {
> -            id,
> -            store: config.name.clone(),
> -            comment: None,
> -            disable: false,
> -            schedule: schedule.clone(),
> -            options: PruneJobOptions {
> -                keep: config.keep.clone(),
> -                max_depth: None,
> -                ns: None,
> -            },
> -        }
> -    });
> +    let mut prune_job_config = None;
> +    if config.keep.keeps_something() || !has_prune_job(&config.name)? {
> +        prune_job_config = config.prune_schedule.as_ref().map(|schedule| {
> +            let mut id = format!("default-{}-{}", config.name, Uuid::generate());
> +            id.truncate(32);
> +
> +            PruneJobConfig {
> +                id,
> +                store: config.name.clone(),
> +                comment: None,
> +                disable: false,
> +                schedule: schedule.clone(),
> +                options: PruneJobOptions {
> +                    keep: config.keep.clone(),
> +                    max_depth: None,
> +                    ns: None,
> +                },
> +            }
> +        });
> +    }
>  
>      // clearing prune settings in the datastore config, as they are now handled by prune jobs
>      let config = DataStoreConfig {
> diff --git a/src/api2/config/prune.rs b/src/api2/config/prune.rs
> index ce7b8ce565ce..b433c248ac5a 100644
> --- a/src/api2/config/prune.rs
> +++ b/src/api2/config/prune.rs
> @@ -77,6 +77,17 @@ pub fn do_create_prune_job(config: PruneJobConfig) -> Result<(), Error> {
>      Ok(())
>  }
>  
> +pub fn has_prune_job(datastore: &str) -> Result<bool, Error> {
> +    let (section_config, _digest) = prune::config()?;
> +    for (_, (_, job_config)) in section_config.sections {
> +        let job_config: PruneJobConfig = serde_json::from_value(job_config)?;
> +        if job_config.store == datastore {
> +            return Ok(true);
> +        }
> +    }
> +    Ok(false)
> +}
> +
>  #[api(
>      protected: true,
>      input: {
> -- 
> 2.39.5
> 
> 


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

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

end of thread, other threads:[~2024-11-26 10:38 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-11-26  9:54 [pbs-devel] [PATCH proxmox-backup v3] reuse-datastore: avoid creating another prune job Gabriel Goller
2024-11-26 10:38 ` [pbs-devel] applied: " Fabian Grünbichler

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