Re: [pbs-devel] [PATCH proxmox-backup] Fix 3335: Allow removing datastore contents on delete

* Re: [pbs-devel] [PATCH proxmox-backup] Fix 3335: Allow removing datastore contents on delete
@ 2022-01-25 17:44 Dylan Whyte
  0 siblings, 0 replies; 3+ messages in thread
From: Dylan Whyte @ 2022-01-25 17:44 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion

Please ignore. Immediately after sending, I realised a last minute change (which i thought i tested) broke the build. Sending a new version..

Dylan
> On 25.01.2022 18:33 Dylan Whyte <d.whyte@proxmox.com> wrote:
> 
>  
> This adds an option to 'datastore remove', to additionally remove the
> datatore's underlying contents.
> 
> In addition, the task is now carried out in a worker, in order to
> prevent the GUI from blocking or timing out during a removal (GUI patch
> to follow).
> 
> Signed-off-by: Dylan Whyte <d.whyte@proxmox.com>
> ---
>  src/api2/config/datastore.rs                | 85 ++++++++++++++++++---
>  src/bin/proxmox_backup_manager/datastore.rs | 55 ++++++++++++-
>  2 files changed, 126 insertions(+), 14 deletions(-)
> 
> diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
> index 60bc3c0e..2605cab1 100644
> --- a/src/api2/config/datastore.rs
> +++ b/src/api2/config/datastore.rs
> @@ -8,7 +8,7 @@ use hex::FromHex;
>  use proxmox_router::{Router, RpcEnvironment, RpcEnvironmentType, Permission};
>  use proxmox_schema::{api, ApiType};
>  use proxmox_section_config::SectionConfigData;
> -use proxmox_sys::WorkerTaskContext;
> +use proxmox_sys::{WorkerTaskContext, task_log};
>  
>  use pbs_datastore::chunk_store::ChunkStore;
>  use pbs_config::BackupLockGuard;
> @@ -16,7 +16,7 @@ use pbs_api_types::{
>      Authid, DatastoreNotify,
>      DATASTORE_SCHEMA, PROXMOX_CONFIG_DIGEST_SCHEMA,
>      PRIV_DATASTORE_ALLOCATE, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_MODIFY,
> -    DataStoreConfig, DataStoreConfigUpdater,
> +    PRIV_SYS_MODIFY, DataStoreConfig, DataStoreConfigUpdater,
>  };
>  
>  use crate::api2::config::sync::delete_sync_job;
> @@ -329,6 +329,12 @@ pub fn update_datastore(
>                  optional: true,
>                  schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
>              },
> +            "destroy-data": {
> +                description: "Delete the datastore's underlying contents",
> +                optional: true,
> +                type: bool,
> +                default: false,
> +            },
>          },
>      },
>      access: {
> @@ -340,23 +346,19 @@ pub async fn delete_datastore(
>      name: String,
>      keep_job_configs: bool,
>      digest: Option<String>,
> +    destroy_data: Option<bool>,
>      rpcenv: &mut dyn RpcEnvironment,
> -) -> Result<(), Error> {
> +) -> Result<String, Error> {
>  
> -    let _lock = pbs_config::datastore::lock_config()?;
> +    let lock = pbs_config::datastore::lock_config()?;
>  
> -    let (mut config, expected_digest) = pbs_config::datastore::config()?;
> +    let (config, expected_digest) = pbs_config::datastore::config()?;
>  
>      if let Some(ref digest) = digest {
>          let digest = <[u8; 32]>::from_hex(digest)?;
>          crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
>      }
>  
> -    match config.sections.get(&name) {
> -        Some(_) => { config.sections.remove(&name); },
> -        None => bail!("datastore '{}' does not exist.", name),
> -    }
> -
>      if !keep_job_configs {
>          for job in list_verification_jobs(Some(name.clone()), Value::Null, rpcenv)? {
>              delete_verification_job(job.config.id, None, rpcenv)?
> @@ -371,7 +373,23 @@ pub async fn delete_datastore(
>          }
>      }
>  
> -    pbs_config::datastore::save_config(&config)?;
> +    let datastore_config: DataStoreConfig = config.lookup("datastore", &name)?;
> +
> +    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> +    let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
> +
> +    let upid = WorkerTask::new_thread(
> +        "delete-datastore",
> +        Some(datastore_config.name.to_string()),
> +        auth_id.to_string(),
> +        to_stdout,
> +        move |worker| do_delete_datastore(
> +            lock,
> +            config,
> +            datastore_config,
> +            destroy_data,
> +            &worker),
> +    )?;
>  
>      // ignore errors
>      let _ = jobstate::remove_state_file("prune", &name);
> @@ -379,6 +397,51 @@ pub async fn delete_datastore(
>  
>      crate::server::notify_datastore_removed().await?;
>  
> +    Ok(upid)
> +}
> +
> +fn do_delete_datastore(
> +    _lock: BackupLockGuard,
> +    mut config: SectionConfigData,
> +    datastore: DataStoreConfig,
> +    destroy_data: Option<bool>,
> +    worker: &dyn WorkerTaskContext,
> +) -> Result<(), Error> {
> +    task_log!(worker, "Removing datastore: {}...", datastore.name);
> +    match config.sections.get(&datastore.name) {
> +        Some(_) => {
> +            if let Some(_destroy_data) = destroy_data {
> +                let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> +                let user_info = CachedUserInfo::new()?;
> +                user_info.check_privs(&auth_id, &["datastore", &datastore_config.name], PRIV_SYS_MODIFY, true)?;
> +
> +                task_log!(worker, "Destroying datastore data...");
> +                match std::fs::read_dir(&datastore.path) {
> +                    Ok(dir) => {
> +                        for entry in dir {
> +                            if let Ok(entry) = entry {
> +                                let path = entry.path();
> +                                task_log!(worker, "Removing {}...", path.to_str().unwrap());
> +                                if path.is_dir() {
> +                                    std::fs::remove_dir_all(path)?;
> +                                } else {
> +                                    std::fs::remove_file(path)?;
> +                                }
> +                            };
> +                        }
> +                        task_log!(worker, "Finished destroying data...");
> +                    },
> +                    Err(err) => bail!("Failed to read {}: {}", &datastore.path, err),
> +                }
> +            }
> +            task_log!(worker, "Removing datastore from config...");
> +            config.sections.remove(&datastore.name);
> +        },
> +        None => bail!("datastore '{}' does not exist.", datastore.name),
> +    }
> +
> +    pbs_config::datastore::save_config(&config)?;
> +
>      Ok(())
>  }
>  
> diff --git a/src/bin/proxmox_backup_manager/datastore.rs b/src/bin/proxmox_backup_manager/datastore.rs
> index a35e7bf5..98a0b1b2 100644
> --- a/src/bin/proxmox_backup_manager/datastore.rs
> +++ b/src/bin/proxmox_backup_manager/datastore.rs
> @@ -1,11 +1,13 @@
>  use anyhow::Error;
>  use serde_json::Value;
>  
> -use proxmox_router::{cli::*, ApiHandler, RpcEnvironment};
> +use proxmox_router::{cli::*, ApiHandler, RpcEnvironment, Permission};
>  use proxmox_schema::api;
>  
>  use pbs_client::view_task_result;
> -use pbs_api_types::{DataStoreConfig, DATASTORE_SCHEMA};
> +use pbs_api_types::{DataStoreConfig, DATASTORE_SCHEMA, PROXMOX_CONFIG_DIGEST_SCHEMA,
> +    PRIV_DATASTORE_ALLOCATE
> +};
>  
>  use proxmox_backup::api2;
>  use proxmox_backup::client_helpers::connect_to_localhost;
> @@ -100,6 +102,53 @@ async fn create_datastore(mut param: Value) -> Result<Value, Error> {
>      Ok(Value::Null)
>  }
>  
> +#[api(
> +    protected: true,
> +    input: {
> +        properties: {
> +            name: {
> +                schema: DATASTORE_SCHEMA,
> +            },
> +            "keep-job-configs": {
> +                description: "If enabled, the job configurations related to this datastore will be kept.",
> +                type: bool,
> +                optional: true,
> +                default: false,
> +            },
> +            digest: {
> +                optional: true,
> +                schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
> +            },
> +            "destroy-data": {
> +                description: "Delete the datastore's underlying contents",
> +                optional: true,
> +                type: bool,
> +                default: false,
> +            }
> +        },
> +    },
> +    access: {
> +        permission: &Permission::Privilege(&["datastore", "{name}"], PRIV_DATASTORE_ALLOCATE, false),
> +    },
> +)]
> +/// Remove a datastore configuration.
> +async fn delete_datastore(
> +    mut param: Value,
> +    rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<Value, Error> {
> +    param["node"] = "localhost".into();
> +
> +    let info = &api2::config::datastore::API_METHOD_DELETE_DATASTORE;
> +    let result = match info.handler {
> +        ApiHandler::Async(handler) => (handler)(param, info, rpcenv).await?,
> +        _ => unreachable!(),
> +    };
> +
> +    crate::wait_for_local_worker(result.as_str().unwrap()).await?;
> +    Ok(Value::Null)
> +
> +}
> +
>  pub fn datastore_commands() -> CommandLineInterface {
>  
>      let cmd_def = CliCommandMap::new()
> @@ -121,7 +170,7 @@ pub fn datastore_commands() -> CommandLineInterface {
>                  .completion_cb("prune-schedule", pbs_config::datastore::complete_calendar_event)
>          )
>          .insert("remove",
> -                CliCommand::new(&api2::config::datastore::API_METHOD_DELETE_DATASTORE)
> +                CliCommand::new(&API_METHOD_DELETE_DATASTORE)
>                  .arg_param(&["name"])
>                  .completion_cb("name", pbs_config::datastore::complete_datastore_name)
>          );
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> 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