Re: [pve-devel] [PATCH v2 proxmox-backup 06/10] sync: add group filtering

[Dominik Csapak <d.csapak@proxmox.com>]

On 9/15/21 3:41 PM, Fabian Grünbichler wrote:
> like for manual pulls, but persisted in the sync job config and visible
> in the relevant GUI parts.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> 
> Notes:
>      GUI is read-only for now (and defaults to no filtering on creation), as this is
>      a rather advanced feature that requires a complex GUI to be user-friendly
>      (regex-freeform, type-combobox, remote group scanning + selector with
>      additional freeform input).
>      
>      I did test the API manually though to see whether it works as expected, and
>      updating the filter list by overwriting with a new one passed in as multiple
>      parameters works as expected.
>      
>      if we want to make this configurable over the GUI, we probably want to switch
>      the job edit window to a tabpanel and add a second grid tab for selecting
>      the groups.
> 
>   pbs-api-types/src/jobs.rs |  6 ++++++
>   src/api2/config/sync.rs   |  5 +++++
>   src/api2/pull.rs          |  4 +++-
>   www/config/SyncView.js    | 13 ++++++++++++-
>   www/window/SyncJobEdit.js | 12 ++++++++++++
>   5 files changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/pbs-api-types/src/jobs.rs b/pbs-api-types/src/jobs.rs
> index 94638fe5..06a12131 100644
> --- a/pbs-api-types/src/jobs.rs
> +++ b/pbs-api-types/src/jobs.rs
> @@ -439,6 +439,10 @@ impl GroupFilterList {
>               optional: true,
>               schema: SYNC_SCHEDULE_SCHEMA,
>           },
> +        groups: {
> +            type: GroupFilterList,
> +            optional: true,
> +        },
>       }
>   )]
>   #[derive(Serialize,Deserialize,Clone,Updater)]
> @@ -458,6 +462,8 @@ pub struct SyncJobConfig {
>       pub comment: Option<String>,
>       #[serde(skip_serializing_if="Option::is_none")]
>       pub schedule: Option<String>,
> +    #[serde(skip_serializing_if="Option::is_none")]
> +    pub groups: Option<GroupFilterList>,
>   }
>   
>   #[api(
> diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
> index 3c2bfd78..cbd5b192 100644
> --- a/src/api2/config/sync.rs
> +++ b/src/api2/config/sync.rs
> @@ -191,6 +191,8 @@ pub enum DeletableProperty {
>       schedule,
>       /// Delete the remove-vanished flag.
>       remove_vanished,
> +    /// Delete the groups property.
> +    groups,
>   }
>   
>   #[api(
> @@ -253,6 +255,7 @@ pub fn update_sync_job(
>                   DeletableProperty::comment => { data.comment = None; },
>                   DeletableProperty::schedule => { data.schedule = None; },
>                   DeletableProperty::remove_vanished => { data.remove_vanished = None; },
> +                DeletableProperty::groups => { data.groups = None; },
>               }
>           }
>       }
> @@ -270,6 +273,7 @@ pub fn update_sync_job(
>       if let Some(remote) = update.remote { data.remote = remote; }
>       if let Some(remote_store) = update.remote_store { data.remote_store = remote_store; }
>       if let Some(owner) = update.owner { data.owner = Some(owner); }
> +    if let Some(groups) = update.groups { data.groups = Some(groups); }
>   
>       let schedule_changed = data.schedule != update.schedule;
>       if update.schedule.is_some() { data.schedule = update.schedule; }
> @@ -389,6 +393,7 @@ acl:1:/remote/remote1/remotestore1:write@pbs:RemoteSyncOperator
>           owner: Some(write_auth_id.clone()),
>           comment: None,
>           remove_vanished: None,
> +        groups: None,
>           schedule: None,
>       };
>   
> diff --git a/src/api2/pull.rs b/src/api2/pull.rs
> index dfa6a938..86b618ed 100644
> --- a/src/api2/pull.rs
> +++ b/src/api2/pull.rs
> @@ -102,7 +102,9 @@ pub fn do_sync_job(
>                   worker.log(format!("Sync datastore '{}' from '{}/{}'",
>                           sync_job.store, sync_job.remote, sync_job.remote_store));
>   
> -                pull_store(&worker, &client, &src_repo, tgt_store.clone(), delete, sync_owner, None).await?;
> +                let sync_group_filter = sync_job.groups.map(GroupFilterList::filters);
> +
> +                pull_store(&worker, &client, &src_repo, tgt_store.clone(), delete, sync_owner, sync_group_filter).await?;

imho at this point, i'd look into giving pull_store the syncJobConfig 
directly and creating one for the 'pull' api... (just a struct in memory
to pass it to pull_store)

or, if the config contains too much, create a smaller struct and use that.

>   
>                   worker.log(format!("sync job '{}' end", &job_id));
>   
> diff --git a/www/config/SyncView.js b/www/config/SyncView.js
> index 7d7e751c..d2a3954f 100644
> --- a/www/config/SyncView.js
> +++ b/www/config/SyncView.js
> @@ -1,7 +1,7 @@
>   Ext.define('pbs-sync-jobs-status', {
>       extend: 'Ext.data.Model',
>       fields: [
> -	'id', 'owner', 'remote', 'remote-store', 'store', 'schedule',
> +	'id', 'owner', 'remote', 'remote-store', 'store', 'schedule', 'groups',
>   	'next-run', 'last-run-upid', 'last-run-state', 'last-run-endtime',
>   	{
>   	    name: 'duration',
> @@ -106,6 +106,11 @@ Ext.define('PBS.config.SyncJobView', {
>   	    return Ext.String.htmlEncode(value, metadata, record);
>   	},
>   
> +	render_optional_groups: function(value, metadata, record) {
> +	    if (!value) return gettext('All');
> +	    return Ext.String.htmlEncode(value, metadata, record);
> +	},
> +
>   	startStore: function() { this.getView().getStore().rstore.startUpdate(); },
>   	stopStore: function() { this.getView().getStore().rstore.stopUpdate(); },
>   
> @@ -214,6 +219,12 @@ Ext.define('PBS.config.SyncJobView', {
>   	    flex: 2,
>   	    sortable: true,
>   	},
> +	{
> +	    header: gettext('Backup Groups'),
> +	    dataIndex: 'groups',
> +	    renderer: 'render_optional_groups',
> +	    width: 80,
> +	},
>   	{
>   	    header: gettext('Schedule'),
>   	    dataIndex: 'schedule',
> diff --git a/www/window/SyncJobEdit.js b/www/window/SyncJobEdit.js
> index 47e65ae3..2399f11f 100644
> --- a/www/window/SyncJobEdit.js
> +++ b/www/window/SyncJobEdit.js
> @@ -199,6 +199,18 @@ Ext.define('PBS.window.SyncJobEdit', {
>   	],
>   
>   	columnB: [
> +	    {
> +		fieldLabel: gettext('Backup Groups'),
> +		xtype: 'displayfield',
> +		name: 'groups',
> +		renderer: function(value, metadata, record) {
> +		    if (!value) return gettext('All');
> +		    return Ext.String.htmlEncode(value, metadata, record);
> +		},
> +		cbind: {
> +		    hidden: '{isCreate}',
> +		},
> +	    },
>   	    {
>   		fieldLabel: gettext('Comment'),
>   		xtype: 'proxmoxtextfield',
>