all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox-backup] reuse-datastore: avoid creating another default prune job
@ 2024-11-22 11:11 Gabriel Goller
  2024-11-22 11:18 ` Christian Ebner
  0 siblings, 1 reply; 3+ messages in thread
From: Gabriel Goller @ 2024-11-22 11:11 UTC (permalink / raw)
  To: pbs-devel

If a datastore with a default 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.

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

This is a bit janky, because we rely on the default datastore being
named `default-{datastore}`, but that shouldn't be an issue.

 src/api2/config/datastore.rs | 41 ++++++++++++++++++++----------------
 src/api2/config/prune.rs     | 15 +++++++++++++
 2 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index 37d1528c70fb..cbe67cfc6ac5 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -23,7 +23,9 @@ use pbs_datastore::chunk_store::ChunkStore;
 use crate::api2::admin::{
     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::{
+    default_prune_job_existing, delete_prune_job, do_create_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;
@@ -150,23 +152,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 !default_prune_job_existing(&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..dafb97e2f1e5 100644
--- a/src/api2/config/prune.rs
+++ b/src/api2/config/prune.rs
@@ -77,6 +77,21 @@ pub fn do_create_prune_job(config: PruneJobConfig) -> Result<(), Error> {
     Ok(())
 }
 
+pub fn default_prune_job_existing(datastore: &str) -> Result<bool, Error> {
+    let (section_config, _digest) = prune::config()?;
+    if section_config
+        .sections
+        .keys()
+        .filter(|s| s.starts_with(&format!("default-{datastore}")))
+        .count()
+        > 0
+    {
+        Ok(true)
+    } else {
+        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] 3+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup] reuse-datastore: avoid creating another default prune job
  2024-11-22 11:11 [pbs-devel] [PATCH proxmox-backup] reuse-datastore: avoid creating another default prune job Gabriel Goller
@ 2024-11-22 11:18 ` Christian Ebner
  2024-11-25  8:58   ` Gabriel Goller
  0 siblings, 1 reply; 3+ messages in thread
From: Christian Ebner @ 2024-11-22 11:18 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Gabriel Goller

On 11/22/24 12:11, Gabriel Goller wrote:
> If a datastore with a default 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.
> 
> Reported-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
> 
> This is a bit janky, because we rely on the default datastore being
> named `default-{datastore}`, but that shouldn't be an issue.
> 
>   src/api2/config/datastore.rs | 41 ++++++++++++++++++++----------------
>   src/api2/config/prune.rs     | 15 +++++++++++++
>   2 files changed, 38 insertions(+), 18 deletions(-)
> 
> diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
> index 37d1528c70fb..cbe67cfc6ac5 100644
> --- a/src/api2/config/datastore.rs
> +++ b/src/api2/config/datastore.rs
> @@ -23,7 +23,9 @@ use pbs_datastore::chunk_store::ChunkStore;
>   use crate::api2::admin::{
>       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::{
> +    default_prune_job_existing, delete_prune_job, do_create_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;
> @@ -150,23 +152,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 !default_prune_job_existing(&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..dafb97e2f1e5 100644
> --- a/src/api2/config/prune.rs
> +++ b/src/api2/config/prune.rs
> @@ -77,6 +77,21 @@ pub fn do_create_prune_job(config: PruneJobConfig) -> Result<(), Error> {
>       Ok(())
>   }
>   
> +pub fn default_prune_job_existing(datastore: &str) -> Result<bool, Error> {
> +    let (section_config, _digest) = prune::config()?;
> +    if section_config
> +        .sections
> +        .keys()
> +        .filter(|s| s.starts_with(&format!("default-{datastore}")))
> +        .count()
> +        > 0
> +    {
> +        Ok(true)
> +    } else {
> +        Ok(false)
> +    }

could be more compact? no need for the if block:

let has_default = section_config.sections.keys().filter(...).count() >
0;
Ok(has_default)

> +}
> +
>   #[api(
>       protected: true,
>       input: {



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

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

* Re: [pbs-devel] [PATCH proxmox-backup] reuse-datastore: avoid creating another default prune job
  2024-11-22 11:18 ` Christian Ebner
@ 2024-11-25  8:58   ` Gabriel Goller
  0 siblings, 0 replies; 3+ messages in thread
From: Gabriel Goller @ 2024-11-25  8:58 UTC (permalink / raw)
  To: Christian Ebner; +Cc: Proxmox Backup Server development discussion

On 22.11.2024 12:18, Christian Ebner wrote:
>On 11/22/24 12:11, Gabriel Goller wrote:
>>diff --git a/src/api2/config/prune.rs b/src/api2/config/prune.rs
>>index ce7b8ce565ce..dafb97e2f1e5 100644
>>--- a/src/api2/config/prune.rs
>>+++ b/src/api2/config/prune.rs
>>@@ -77,6 +77,21 @@ pub fn do_create_prune_job(config: PruneJobConfig) -> Result<(), Error> {
>>      Ok(())
>>  }
>>+pub fn default_prune_job_existing(datastore: &str) -> Result<bool, Error> {
>>+    let (section_config, _digest) = prune::config()?;
>>+    if section_config
>>+        .sections
>>+        .keys()
>>+        .filter(|s| s.starts_with(&format!("default-{datastore}")))
>>+        .count()
>>+        > 0
>>+    {
>>+        Ok(true)
>>+    } else {
>>+        Ok(false)
>>+    }
>
>could be more compact? no need for the if block:
>
>let has_default = section_config.sections.keys().filter(...).count() >
>0;
>Ok(has_default)

Yep, corrected it in v2!
Thanks for looking at it!

>>+}
>>+
>>  #[api(
>>      protected: true,
>>      input: {
>


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


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

end of thread, other threads:[~2024-11-25  8:58 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-11-22 11:11 [pbs-devel] [PATCH proxmox-backup] reuse-datastore: avoid creating another default prune job Gabriel Goller
2024-11-22 11:18 ` Christian Ebner
2024-11-25  8:58   ` Gabriel Goller

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