Re: [pbs-devel] [PATCH proxmox-backup v8 06/45] datastore: allow to get the backend for a datastore

[Christian Ebner <c.ebner@proxmox.com>]

On 7/18/25 9:52 AM, Lukas Wagner wrote:
> With my feedback addressed:
> 
> Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
> 
> 
> On  2025-07-15 14:52, Christian Ebner wrote:
>> Implements an enum with variants Filesystem and S3 to distinguish
>> between available backends. Filesystem will be used as default, if no
>> backend is configured in the datastores configuration. If the
>> datastore has an s3 backend configured, the backend method will
>> instantiate and s3 client and return it with the S3 variant.
>>
>> This allows to instantiate the client once, keeping and reusing the
>> same open connection to the api for the lifetime of task or job, e.g.
>> in the backup writer/readers runtime environment.
>>
>> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
>> ---
>> changes since version 7:
>> - no changes
>>
>>   pbs-datastore/src/datastore.rs | 52 ++++++++++++++++++++++++++++++++--
>>   pbs-datastore/src/lib.rs       |  1 +
>>   2 files changed, 51 insertions(+), 2 deletions(-)
>>
>> diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
>> index 924d8cf9c..90ab80005 100644
>> --- a/pbs-datastore/src/datastore.rs
>> +++ b/pbs-datastore/src/datastore.rs
>> @@ -12,6 +12,7 @@ use pbs_tools::lru_cache::LruCache;
>>   use tracing::{info, warn};
>>   
>>   use proxmox_human_byte::HumanByte;
>> +use proxmox_s3_client::{S3Client, S3ClientConfig, S3ClientOptions, S3ClientSecretsConfig};
>>   use proxmox_schema::ApiType;
>>   
>>   use proxmox_sys::error::SysError;
>> @@ -23,8 +24,8 @@ use proxmox_worker_task::WorkerTaskContext;
>>   
>>   use pbs_api_types::{
>>       ArchiveType, Authid, BackupGroupDeleteStats, BackupNamespace, BackupType, ChunkOrder,
>> -    DataStoreConfig, DatastoreFSyncLevel, DatastoreTuning, GarbageCollectionStatus,
>> -    MaintenanceMode, MaintenanceType, Operation, UPID,
>> +    DataStoreConfig, DatastoreBackendConfig, DatastoreBackendType, DatastoreFSyncLevel,
>> +    DatastoreTuning, GarbageCollectionStatus, MaintenanceMode, MaintenanceType, Operation, UPID,
>>   };
>>   use pbs_config::BackupLockGuard;
>>   
>> @@ -127,6 +128,7 @@ pub struct DataStoreImpl {
>>       chunk_order: ChunkOrder,
>>       last_digest: Option<[u8; 32]>,
>>       sync_level: DatastoreFSyncLevel,
>> +    backend_config: DatastoreBackendConfig,
>>   }
>>   
>>   impl DataStoreImpl {
>> @@ -141,6 +143,7 @@ impl DataStoreImpl {
>>               chunk_order: Default::default(),
>>               last_digest: None,
>>               sync_level: Default::default(),
>> +            backend_config: Default::default(),
>>           })
>>       }
>>   }
>> @@ -196,6 +199,12 @@ impl Drop for DataStore {
>>       }
>>   }
>>   
>> +#[derive(Clone)]
>> +pub enum DatastoreBackend {
>> +    Filesystem,
>> +    S3(Arc<S3Client>),
>> +}
>> +
> 
> Missing doc comments for this public enum

Added docs for both, the enum and the individual variants.

>>   impl DataStore {
>>       // This one just panics on everything
>>       #[doc(hidden)]
>> @@ -206,6 +215,39 @@ impl DataStore {
>>           })
>>       }
>>   
>> +    /// Get the backend for this datastore based on it's configuration
>> +    pub fn backend(&self) -> Result<DatastoreBackend, Error> {
>> +        let backend_type = match self.inner.backend_config.ty.unwrap_or_default() {
>> +            DatastoreBackendType::Filesystem => DatastoreBackend::Filesystem,
>> +            DatastoreBackendType::S3 => {
>> +                let s3_client_id = self
>> +                    .inner
>> +                    .backend_config
>> +                    .client
>> +                    .as_ref()
>> +                    .ok_or_else(|| format_err!("missing client for s3 backend"))?;
>> +                let bucket = self
>> +                    .inner
>> +                    .backend_config
>> +                    .bucket
>> +                    .clone()
>> +                    .ok_or_else(|| format_err!("missing bucket for s3 backend"))?;
>> +
>> +                let (config, _config_digest) = pbs_config::s3::config()?;
>> +                let (secrets, _secrets_digest) = pbs_config::s3::secrets_config()?;
>> +                let config: S3ClientConfig = config.lookup("s3client", s3_client_id)?;
>> +                let secrets: S3ClientSecretsConfig = secrets.lookup("s3secrets", s3_client_id)?;
> 
> Same thing here with regards to the hard-coded section type names.

Adapted as well, thx for pointing them out!

> 
>> +
>> +                let options =
>> +                    S3ClientOptions::from_config(config, secrets, bucket, self.name().to_owned());
>> +                let s3_client = S3Client::new(options)?;
>> +                DatastoreBackend::S3(Arc::new(s3_client))
>> +            }
>> +        };
>> +
>> +        Ok(backend_type)
>> +    }
>> +
>>       pub fn lookup_datastore(
>>           name: &str,
>>           operation: Option<Operation>,
>> @@ -383,6 +425,11 @@ impl DataStore {
>>                   .parse_property_string(config.tuning.as_deref().unwrap_or(""))?,
>>           )?;
>>   
>> +        let backend_config: DatastoreBackendConfig = serde_json::from_value(
>> +            DatastoreBackendConfig::API_SCHEMA
>> +                .parse_property_string(config.backend.as_deref().unwrap_or(""))?,
>> +        )?;
>> +
>>           Ok(DataStoreImpl {
>>               chunk_store,
>>               gc_mutex: Mutex::new(()),
>> @@ -391,6 +438,7 @@ impl DataStore {
>>               chunk_order: tuning.chunk_order.unwrap_or_default(),
>>               last_digest,
>>               sync_level: tuning.sync_level.unwrap_or_default(),
>> +            backend_config,
>>           })
>>       }
>>   
>> diff --git a/pbs-datastore/src/lib.rs b/pbs-datastore/src/lib.rs
>> index ffd0d91b2..ca6fdb7d8 100644
>> --- a/pbs-datastore/src/lib.rs
>> +++ b/pbs-datastore/src/lib.rs
>> @@ -204,6 +204,7 @@ pub use store_progress::StoreProgress;
>>   mod datastore;
>>   pub use datastore::{
>>       check_backup_owner, ensure_datastore_is_mounted, get_datastore_mount_status, DataStore,
>> +    DatastoreBackend,
>>   };
>>   
>>   mod hierarchy;
> 



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