[pbs-devel] [PATCH-SERIES proxmox/proxmox-backup v3 0/7] trigger sync jobs on mount

[pbs-devel] [PATCH-SERIES proxmox/proxmox-backup v3 0/7] trigger sync jobs on mount

[Hannes Laimer]

Sync jobs now have a run-on-mount flag, that, if set, runs the job whenever
a relevant removable datastore is mounted.

changes since v3, thanks @Chris:
- call run_sync_job endpoint so jobs run on the proxy process,
- use the UPID to keep track of job status
- log on the mount task that we'll run sync jobs
- changed some small things mentioned by @Chris 


proxmox:
Hannes Laimer (1):
  pbs-api-types: add run-on-mount flag to SyncJobConfig

 pbs-api-types/src/jobs.rs | 8 ++++++++
 1 file changed, 8 insertions(+)

proxmox-backup:
Hannes Laimer (6):
  api: config: sync: update run-on-mount correctly
  api: admin: run configured sync jobs when a datastore is mounted
  api: admin: trigger sync jobs only on datastore mount
  bin: manager: run uuid_mount/mount tasks on the proxy
  ui: add run-on-mount checkbox to SyncJob form
  ui: add task title for triggering sync jobs

 docs/storage.rst                                   |   4 +-
 pbs-datastore/Cargo.toml                           |   2 -
 pbs-datastore/src/backup_info.rs                   |  55 +++-----
 pbs-datastore/src/datastore.rs                     |  11 --
 .../src/proxmox_restore_daemon/disk.rs             |   2 +-
 src/api2/admin/datastore.rs                        | 138 ++++++++++++++++++---
 src/api2/config/sync.rs                            |   9 ++
 src/bin/proxmox_backup_manager/datastore.rs        |  41 ++++--
 www/Utils.js                                       |   1 +
 www/window/SyncJobEdit.js                          |  13 +-
 10 files changed, 195 insertions(+), 81 deletions(-)

-- 
2.39.5


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

[pbs-devel] [PATCH proxmox v3 1/7] pbs-api-types: add run-on-mount flag to SyncJobConfig

[Hannes Laimer]

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 pbs-api-types/src/jobs.rs | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/pbs-api-types/src/jobs.rs b/pbs-api-types/src/jobs.rs
index 6ef13dc2..3eb61cde 100644
--- a/pbs-api-types/src/jobs.rs
+++ b/pbs-api-types/src/jobs.rs
@@ -536,6 +536,8 @@ pub const SYNC_ENCRYPTED_ONLY_SCHEMA: Schema =
     BooleanSchema::new("Only synchronize encrypted backup snapshots, exclude others.").schema();
 pub const SYNC_VERIFIED_ONLY_SCHEMA: Schema =
     BooleanSchema::new("Only synchronize verified backup snapshots, exclude others.").schema();
+pub const RUN_SYNC_ON_MOUNT_SCHEMA: Schema =
+    BooleanSchema::new("Run this job when a relevant datastore is mounted.").schema();
 
 #[api(
     properties: {
@@ -603,6 +605,10 @@ pub const SYNC_VERIFIED_ONLY_SCHEMA: Schema =
             schema: SYNC_VERIFIED_ONLY_SCHEMA,
             optional: true,
         },
+        "run-on-mount": {
+            schema: RUN_SYNC_ON_MOUNT_SCHEMA,
+            optional: true,
+        },
         "sync-direction": {
             type: SyncDirection,
             optional: true,
@@ -647,6 +653,8 @@ pub struct SyncJobConfig {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub verified_only: Option<bool>,
     #[serde(skip_serializing_if = "Option::is_none")]
+    pub run_on_mount: Option<bool>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub sync_direction: Option<SyncDirection>,
 }
 
-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup v3 2/7] api: config: sync: update run-on-mount correctly

[Hannes Laimer]

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/config/sync.rs | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
index 6194d865..358409b5 100644
--- a/src/api2/config/sync.rs
+++ b/src/api2/config/sync.rs
@@ -339,6 +339,8 @@ pub enum DeletableProperty {
     EncryptedOnly,
     /// Delete the verified_only property,
     VerifiedOnly,
+    /// Delete the run_on_mount property,
+    RunOnMount,
     /// Delete the sync_direction property,
     SyncDirection,
 }
@@ -458,6 +460,9 @@ pub fn update_sync_job(
                 DeletableProperty::VerifiedOnly => {
                     data.verified_only = None;
                 }
+                DeletableProperty::RunOnMount => {
+                    data.run_on_mount = None;
+                }
                 DeletableProperty::SyncDirection => {
                     data.sync_direction = None;
                 }
@@ -507,6 +512,9 @@ pub fn update_sync_job(
     if let Some(verified_only) = update.verified_only {
         data.verified_only = Some(verified_only);
     }
+    if let Some(run_on_mount) = update.run_on_mount {
+        data.run_on_mount = Some(run_on_mount);
+    }
     if let Some(sync_direction) = update.sync_direction {
         data.sync_direction = Some(sync_direction);
     }
@@ -683,6 +691,7 @@ acl:1:/remote/remote1/remotestore1:write@pbs:RemoteSyncOperator
         transfer_last: None,
         encrypted_only: None,
         verified_only: None,
+        run_on_mount: None,
         sync_direction: None, // use default
     };
 
-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup v3 3/7] api: admin: run configured sync jobs when a datastore is mounted

[Hannes Laimer]

When a datastore is mounted, spawn a new task to run all sync jobs
marked with `run-on-mount`. These jobs run sequentially and include
any job for which the mounted datastore is:

- The source or target in a local push/pull job
- The source in a push job to a remote datastore
- The target in a pull job from a remote datastore

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/admin/datastore.rs | 106 ++++++++++++++++++++++++++++++++++--
 1 file changed, 100 insertions(+), 6 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 39249448..68bb2a1f 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -42,8 +42,8 @@ use pbs_api_types::{
     DataStoreConfig, DataStoreListItem, DataStoreMountStatus, DataStoreStatus,
     GarbageCollectionJobStatus, GroupListItem, JobScheduleStatus, KeepOptions, MaintenanceMode,
     MaintenanceType, Operation, PruneJobOptions, SnapshotListItem, SnapshotVerifyState,
-    BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA, BACKUP_TIME_SCHEMA,
-    BACKUP_TYPE_SCHEMA, CATALOG_NAME, CLIENT_LOG_BLOB_NAME, DATASTORE_SCHEMA,
+    SyncJobConfig, BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA,
+    BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA, CATALOG_NAME, CLIENT_LOG_BLOB_NAME, DATASTORE_SCHEMA,
     IGNORE_VERIFIED_BACKUPS_SCHEMA, MANIFEST_BLOB_NAME, MAX_NAMESPACE_DEPTH, NS_MAX_DEPTH_SCHEMA,
     PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE,
     PRIV_DATASTORE_READ, PRIV_DATASTORE_VERIFY, PRIV_SYS_MODIFY, UPID, UPID_SCHEMA,
@@ -66,7 +66,7 @@ use pbs_datastore::{
     DataStore, LocalChunkReader, StoreProgress,
 };
 use pbs_tools::json::required_string_param;
-use proxmox_rest_server::{formatter, WorkerTask};
+use proxmox_rest_server::{formatter, worker_is_active, WorkerTask};
 
 use crate::api2::backup::optional_ns_param;
 use crate::api2::node::rrd::create_value_from_rrd;
@@ -2510,6 +2510,63 @@ pub fn do_mount_device(datastore: DataStoreConfig) -> Result<(), Error> {
     Ok(())
 }
 
+async fn do_sync_jobs(
+    jobs_to_run: Vec<SyncJobConfig>,
+    worker: Arc<WorkerTask>,
+) -> Result<(), Error> {
+    let count = jobs_to_run.len();
+    info!(
+        "will run {} sync jobs: {}",
+        count,
+        jobs_to_run
+            .iter()
+            .map(|job| job.id.as_str())
+            .collect::<Vec<&str>>()
+            .join(", ")
+    );
+
+    for (i, job_config) in jobs_to_run.into_iter().enumerate() {
+        if worker.abort_requested() {
+            bail!("aborted due to user request");
+        }
+        let job_id = &job_config.id;
+        let client = crate::client_helpers::connect_to_localhost()?;
+        let Ok(result) = client
+            .post(format!("api2/json/admin/sync/{job_id}/run").as_str(), None)
+            .await
+        else {
+            warn!("unable to start sync job {job_id}");
+            continue;
+        };
+        info!("[{}/{count}] starting '{job_id}'...", i + 1);
+        let Some(upid_str) = &result["data"].as_str() else {
+            warn!(
+                "could not recieve UPID of started job (may be runnig, just can't track it here)"
+            );
+            continue;
+        };
+        let upid: UPID = upid_str.parse()?;
+
+        let sleep_duration = core::time::Duration::new(1, 0);
+        let mut status_retries = 1;
+        loop {
+            if worker.abort_requested() {
+                bail!("aborted due to user request, already started job will finish");
+            }
+            match worker_is_active(&upid).await {
+                Ok(true) => tokio::time::sleep(sleep_duration).await,
+                Ok(false) => break,
+                Err(_) if status_retries > 3 => break,
+                Err(err) => {
+                    warn!("could not get job status: {err} ({}/3)", status_retries);
+                    status_retries += 1;
+                }
+            }
+        }
+    }
+    Ok(())
+}
+
 #[api(
     protected: true,
     input: {
@@ -2541,12 +2598,49 @@ pub fn mount(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Er
     let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
     let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
 
-    let upid = WorkerTask::new_thread(
+    let upid = WorkerTask::spawn(
         "mount-device",
-        Some(store),
+        Some(store.clone()),
         auth_id.to_string(),
         to_stdout,
-        move |_worker| do_mount_device(datastore),
+        move |_worker| async move {
+            do_mount_device(datastore.clone())?;
+            let Ok((sync_config, _digest)) = pbs_config::sync::config() else {
+                warn!("unable to read sync job config, won't run any sync jobs");
+                return Ok(());
+            };
+            let Ok(list) = sync_config.convert_to_typed_array("sync") else {
+                warn!("unable to parse sync job config, won't run any sync jobs");
+                return Ok(());
+            };
+            let jobs_to_run: Vec<SyncJobConfig> = list
+                .into_iter()
+                .filter(|job: &SyncJobConfig| {
+                    // add job iff (running on mount is enabled and) any of these apply
+                    //   - the jobs is local and we are source or target
+                    //   - we are the source of a push to a remote
+                    //   - we are the target of a pull from a remote
+                    //
+                    // `job.store == datastore.name` iff we are the target for pull from remote or we
+                    // are the source for push to remote, therefore we don't have to check for the
+                    // direction of the job.
+                    job.run_on_mount.unwrap_or(false)
+                        && (job.remote.is_none() && job.remote_store == datastore.name
+                            || job.store == datastore.name)
+                })
+                .collect();
+            if !jobs_to_run.is_empty() {
+                info!("starting {} sync jobs", jobs_to_run.len());
+                let _ = WorkerTask::spawn(
+                    "mount-sync-jobs",
+                    Some(store),
+                    auth_id.to_string(),
+                    false,
+                    move |worker| async move { do_sync_jobs(jobs_to_run, worker).await },
+                );
+            }
+            Ok(())
+        },
     )?;
 
     Ok(json!(upid))
-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup v3 4/7] api: admin: trigger sync jobs only on datastore mount

[Hannes Laimer]

Ensure sync jobs are triggered only when the datastore is actually
mounted. If the datastore is already mounted, we don't fail,
but sync jobs should not be re-triggered unnecessarily. This change
prevents redundant sync job execution.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/admin/datastore.rs | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 68bb2a1f..3f4957c0 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -2455,14 +2455,14 @@ fn setup_mounted_device(datastore: &DataStoreConfig, tmp_mount_path: &str) -> Re
 /// The reason for the randomized device mounting paths is to avoid two tasks trying to mount to
 /// the same path, this is *very* unlikely since the device is only mounted really shortly, but
 /// technically possible.
-pub fn do_mount_device(datastore: DataStoreConfig) -> Result<(), Error> {
+pub fn do_mount_device(datastore: DataStoreConfig) -> Result<bool, Error> {
     if let Some(uuid) = datastore.backing_device.as_ref() {
         if pbs_datastore::get_datastore_mount_status(&datastore) == Some(true) {
             info!(
                 "device is already mounted at '{}'",
                 datastore.absolute_path()
             );
-            return Ok(());
+            return Ok(false);
         }
         let tmp_mount_path = format!(
             "{}/{:x}",
@@ -2507,7 +2507,7 @@ pub fn do_mount_device(datastore: DataStoreConfig) -> Result<(), Error> {
             datastore.name
         )
     }
-    Ok(())
+    Ok(true)
 }
 
 async fn do_sync_jobs(
@@ -2604,7 +2604,9 @@ pub fn mount(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Er
         auth_id.to_string(),
         to_stdout,
         move |_worker| async move {
-            do_mount_device(datastore.clone())?;
+            if !do_mount_device(datastore.clone())? {
+                return Ok(());
+            }
             let Ok((sync_config, _digest)) = pbs_config::sync::config() else {
                 warn!("unable to read sync job config, won't run any sync jobs");
                 return Ok(());
-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup v3 5/7] bin: manager: run uuid_mount/mount tasks on the proxy

[Hannes Laimer]

Use the API instead of running uuid_mount/mount directly in the CLI binary.
This ensures that all triggered tasks are handled by the proxy process.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/bin/proxmox_backup_manager/datastore.rs | 41 ++++++++++++++++-----
 1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/src/bin/proxmox_backup_manager/datastore.rs b/src/bin/proxmox_backup_manager/datastore.rs
index 1922a55a..3fbb5fe5 100644
--- a/src/bin/proxmox_backup_manager/datastore.rs
+++ b/src/bin/proxmox_backup_manager/datastore.rs
@@ -49,6 +49,10 @@ fn list_datastores(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Valu
             store: {
                 schema: DATASTORE_SCHEMA,
             },
+            "output-format": {
+                schema: OUTPUT_FORMAT,
+                optional: true,
+            },
             digest: {
                 optional: true,
                 schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
@@ -57,16 +61,23 @@ fn list_datastores(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Valu
     },
 )]
 /// Mount a removable datastore.
-async fn mount_datastore(mut param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
-    param["node"] = "localhost".into();
+async fn mount_datastore(
+    store: String,
+    mut param: Value,
+    _rpcenv: &mut dyn RpcEnvironment,
+) -> Result<(), Error> {
+    let output_format = extract_output_format(&mut param);
 
-    let info = &api2::admin::datastore::API_METHOD_MOUNT;
-    let result = match info.handler {
-        ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
-        _ => unreachable!(),
-    };
+    let client = connect_to_localhost()?;
+    let result = client
+        .post(
+            format!("api2/json/admin/datastore/{store}/mount").as_str(),
+            None,
+        )
+        .await?;
+
+    view_task_result(&client, result, &output_format).await?;
 
-    crate::wait_for_local_worker(result.as_str().unwrap()).await?;
     Ok(())
 }
 
@@ -260,7 +271,8 @@ async fn update_datastore(name: String, mut param: Value) -> Result<(), Error> {
     },
 )]
 /// Try mounting a removable datastore given the UUID.
-async fn uuid_mount(param: Value, _rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
+async fn uuid_mount(mut param: Value, _rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
+    let output_format = extract_output_format(&mut param);
     let uuid = param["uuid"]
         .as_str()
         .ok_or_else(|| format_err!("uuid has to be specified"))?;
@@ -282,7 +294,16 @@ async fn uuid_mount(param: Value, _rpcenv: &mut dyn RpcEnvironment) -> Result<Va
     }
 
     if let Some(store) = matching_stores.first() {
-        api2::admin::datastore::do_mount_device(store.clone())?;
+        let client = connect_to_localhost()?;
+        let result = client
+            .post(
+                format!("api2/json/admin/datastore/{}/mount", store.name).as_str(),
+                None,
+            )
+            .await?;
+
+        view_task_result(&client, result, &output_format).await?;
+        return Ok(Value::Null);
     }
 
     // we don't want to fail for UUIDs that are not associated with datastores, as that produces
-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup v3 6/7] ui: add run-on-mount checkbox to SyncJob form

[Hannes Laimer]

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 www/window/SyncJobEdit.js | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/www/window/SyncJobEdit.js b/www/window/SyncJobEdit.js
index 4cef9a1d..73a489f1 100644
--- a/www/window/SyncJobEdit.js
+++ b/www/window/SyncJobEdit.js
@@ -195,7 +195,7 @@ Ext.define('PBS.window.SyncJobEdit', {
 			xtype: 'pbsCalendarEvent',
 			name: 'schedule',
 			fieldLabel: gettext('Sync Schedule'),
-			emptyText: gettext('none (disabled)'),
+			emptyText: gettext('none'),
 			cbind: {
 			    deleteEmpty: '{!isCreate}',
 			    value: '{scheduleValue}',
@@ -451,6 +451,17 @@ Ext.define('PBS.window.SyncJobEdit', {
 			uncheckedValue: false,
 			value: false,
 		    },
+		    {
+			xtype: 'proxmoxcheckbox',
+			name: 'run-on-mount',
+			fieldLabel: gettext('Run when mounted'),
+			autoEl: {
+			    tag: 'div',
+			    'data-qtip': gettext('Run this job when a relevant removable datastore is mounted.'),
+			},
+			uncheckedValue: false,
+			value: false,
+		    },
 		],
 	    },
 	    {
-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup v3 7/7] ui: add task title for triggering sync jobs

[Hannes Laimer]

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 www/Utils.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/www/Utils.js b/www/Utils.js
index 9dcde694..eb494301 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -422,6 +422,7 @@ Ext.define('PBS.Utils', {
 	    prunejob: (type, id) => PBS.Utils.render_prune_job_worker_id(id, gettext('Prune Job')),
 	    reader: (type, id) => PBS.Utils.render_datastore_worker_id(id, gettext('Read Objects')),
 	    'rewind-media': [gettext('Drive'), gettext('Rewind Media')],
+	    'mount-sync-jobs': [gettext('Datastore'), gettext('trigger sync jobs')],
 	    sync: ['Datastore', gettext('Remote Sync')],
 	    syncjob: [gettext('Sync Job'), gettext('Remote Sync')],
 	    'tape-backup': (type, id) => PBS.Utils.render_tape_backup_id(id, gettext('Tape Backup')),
-- 
2.39.5



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