all lists on 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal