* [pbs-devel] [PATCH proxmox-backup v7 1/4] fix #4315: jobs: modify GroupFilter so include/exclude is tracked
2024-01-02 11:06 [pbs-devel] [PATCH proxmox-backup v7 0/4] fix #4315: datastore: Exclude entries from sync Philipp Hufnagl
@ 2024-01-02 11:06 ` Philipp Hufnagl
2024-01-02 11:06 ` [pbs-devel] [PATCH proxmox-backup v7 2/4] ui: Show if Filter includes or excludes Philipp Hufnagl
` (4 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Philipp Hufnagl @ 2024-01-02 11:06 UTC (permalink / raw)
To: pbs-devel
After some discussion I canged the include/exclude behavior to first run
all include filter and after that all exclude filter (rather then
allowing to alternate inbetween). This is done by splitting them into 2
lists, running include first.
A lot of discussion happened how edge cases should be handled and we
came to following conclusion:
no include filter + no exclude filter => include all
some include filter + no exclude filter => filter as always
no include filter + some exclude filter => include all then exclude
Since a GroupFilter now also features an behavior, the Struct has been
renamed To GroupType (since simply type is a keyword). The new
GroupFilter now has a behaviour as a flag 'is_exclude'.
I considered calling it 'is_include' but a reader later then might not
know what the opposite of 'include' is (do not include? deactivate?). I
also considered making a new enum 'behaviour' but since there are only 2
values I considered it over engeneered.
Signed-off-by: Philipp Hufnagl <p.hufnagl@proxmox.com>
---
pbs-api-types/src/datastore.rs | 36 ++++++++++++++++++------
pbs-api-types/src/jobs.rs | 51 +++++++++++++++++++++++++---------
src/api2/tape/backup.rs | 39 +++++++++++---------------
src/server/pull.rs | 46 +++++++++++++-----------------
4 files changed, 100 insertions(+), 72 deletions(-)
diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index 74f610d1..cce9888b 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -10,9 +10,9 @@ use proxmox_schema::{
};
use crate::{
- Authid, CryptMode, Fingerprint, MaintenanceMode, Userid, DATASTORE_NOTIFY_STRING_SCHEMA,
- GC_SCHEDULE_SCHEMA, PROXMOX_SAFE_ID_FORMAT, PRUNE_SCHEDULE_SCHEMA, SHA256_HEX_REGEX,
- SINGLE_LINE_COMMENT_SCHEMA, UPID,
+ Authid, CryptMode, Fingerprint, GroupFilter, MaintenanceMode, Userid,
+ DATASTORE_NOTIFY_STRING_SCHEMA, GC_SCHEDULE_SCHEMA, PROXMOX_SAFE_ID_FORMAT,
+ PRUNE_SCHEDULE_SCHEMA, SHA256_HEX_REGEX, SINGLE_LINE_COMMENT_SCHEMA, UPID,
};
const_regex! {
@@ -843,19 +843,37 @@ impl BackupGroup {
}
pub fn matches(&self, filter: &crate::GroupFilter) -> bool {
- use crate::GroupFilter;
-
- match filter {
- GroupFilter::Group(backup_group) => {
+ use crate::FilterType;
+ match &filter.filter_type {
+ FilterType::Group(backup_group) => {
match backup_group.parse::<BackupGroup>() {
Ok(group) => *self == group,
Err(_) => false, // shouldn't happen if value is schema-checked
}
}
- GroupFilter::BackupType(ty) => self.ty == *ty,
- GroupFilter::Regex(regex) => regex.is_match(&self.to_string()),
+ FilterType::BackupType(ty) => self.ty == *ty,
+ FilterType::Regex(regex) => regex.is_match(&self.to_string()),
}
}
+
+ pub fn apply_filters(&self, filters: &[GroupFilter]) -> bool {
+ // since there will only be view filter in the list, an extra iteration to get the umber of
+ // include filter should not be an issue
+ let is_included = if filters.iter().filter(|f| !f.is_exclude).count() == 0 {
+ true
+ } else {
+ filters
+ .iter()
+ .filter(|f| !f.is_exclude)
+ .any(|filter| self.matches(filter))
+ };
+
+ is_included
+ && !filters
+ .iter()
+ .filter(|f| f.is_exclude)
+ .any(|filter| self.matches(filter))
+ }
}
impl AsRef<BackupGroup> for BackupGroup {
diff --git a/pbs-api-types/src/jobs.rs b/pbs-api-types/src/jobs.rs
index 1f5b3cf1..607451ff 100644
--- a/pbs-api-types/src/jobs.rs
+++ b/pbs-api-types/src/jobs.rs
@@ -388,7 +388,7 @@ pub struct TapeBackupJobStatus {
#[derive(Clone, Debug)]
/// Filter for matching `BackupGroup`s, for use with `BackupGroup::filter`.
-pub enum GroupFilter {
+pub enum FilterType {
/// BackupGroup type - either `vm`, `ct`, or `host`.
BackupType(BackupType),
/// Full identifier of BackupGroup, including type
@@ -397,7 +397,7 @@ pub enum GroupFilter {
Regex(Regex),
}
-impl PartialEq for GroupFilter {
+impl PartialEq for FilterType {
fn eq(&self, other: &Self) -> bool {
match (self, other) {
(Self::BackupType(a), Self::BackupType(b)) => a == b,
@@ -408,27 +408,52 @@ impl PartialEq for GroupFilter {
}
}
+#[derive(Clone, Debug)]
+pub struct GroupFilter {
+ pub is_exclude: bool,
+ pub filter_type: FilterType,
+}
+
+impl PartialEq for GroupFilter {
+ fn eq(&self, other: &Self) -> bool {
+ self.filter_type == other.filter_type && self.is_exclude == other.is_exclude
+ }
+}
+
+impl Eq for GroupFilter {}
+
impl std::str::FromStr for GroupFilter {
type Err = anyhow::Error;
fn from_str(s: &str) -> Result<Self, Self::Err> {
- match s.split_once(':') {
- Some(("group", value)) => BACKUP_GROUP_SCHEMA.parse_simple_value(value).map(|_| GroupFilter::Group(value.to_string())),
- Some(("type", value)) => Ok(GroupFilter::BackupType(value.parse()?)),
- Some(("regex", value)) => Ok(GroupFilter::Regex(Regex::new(value)?)),
+ let (is_exclude, type_str) = match s.split_once(':') {
+ Some(("include", value)) => (false, value),
+ Some(("exclude", value)) => (true, value),
+ _ => (false, s),
+ };
+
+ let filter_type = match type_str.split_once(':') {
+ Some(("group", value)) => BACKUP_GROUP_SCHEMA.parse_simple_value(value).map(|_| FilterType::Group(value.to_string())),
+ Some(("type", value)) => Ok(FilterType::BackupType(value.parse()?)),
+ Some(("regex", value)) => Ok(FilterType::Regex(Regex::new(value)?)),
Some((ty, _value)) => Err(format_err!("expected 'group', 'type' or 'regex' prefix, got '{}'", ty)),
None => Err(format_err!("input doesn't match expected format '<group:GROUP||type:<vm|ct|host>|regex:REGEX>'")),
- }.map_err(|err| format_err!("'{}' - {}", s, err))
+ }?;
+ Ok(GroupFilter {
+ is_exclude,
+ filter_type,
+ })
}
}
// used for serializing below, caution!
impl std::fmt::Display for GroupFilter {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
- match self {
- GroupFilter::BackupType(backup_type) => write!(f, "type:{}", backup_type),
- GroupFilter::Group(backup_group) => write!(f, "group:{}", backup_group),
- GroupFilter::Regex(regex) => write!(f, "regex:{}", regex.as_str()),
+ let exclude = if self.is_exclude { "exclude:" } else { "" };
+ match &self.filter_type {
+ FilterType::BackupType(backup_type) => write!(f, "{}type:{}", exclude, backup_type),
+ FilterType::Group(backup_group) => write!(f, "{}group:{}", exclude, backup_group),
+ FilterType::Regex(regex) => write!(f, "{}regex:{}", exclude, regex.as_str()),
}
}
}
@@ -441,9 +466,9 @@ fn verify_group_filter(input: &str) -> Result<(), anyhow::Error> {
}
pub const GROUP_FILTER_SCHEMA: Schema = StringSchema::new(
- "Group filter based on group identifier ('group:GROUP'), group type ('type:<vm|ct|host>'), or regex ('regex:RE').")
+ "Group filter based on group identifier ('group:GROUP'), group type ('type:<vm|ct|host>'), or regex ('regex:RE'). Can be inverted by adding 'exclude:' before.")
.format(&ApiStringFormat::VerifyFn(verify_group_filter))
- .type_text("<type:<vm|ct|host>|group:GROUP|regex:RE>")
+ .type_text("[<exclude:|include:>]<type:<vm|ct|host>|group:GROUP|regex:RE>")
.schema();
pub const GROUP_FILTER_LIST_SCHEMA: Schema =
diff --git a/src/api2/tape/backup.rs b/src/api2/tape/backup.rs
index 2f9385a7..28d7e720 100644
--- a/src/api2/tape/backup.rs
+++ b/src/api2/tape/backup.rs
@@ -9,13 +9,13 @@ use proxmox_schema::api;
use proxmox_sys::{task_log, task_warn, WorkerTaskContext};
use pbs_api_types::{
- print_ns_and_snapshot, print_store_and_ns, Authid, GroupFilter, MediaPoolConfig, Operation,
+ print_ns_and_snapshot, print_store_and_ns, Authid, MediaPoolConfig, Operation,
TapeBackupJobConfig, TapeBackupJobSetup, TapeBackupJobStatus, Userid, JOB_ID_SCHEMA,
PRIV_DATASTORE_READ, PRIV_TAPE_AUDIT, PRIV_TAPE_WRITE, UPID_SCHEMA,
};
use pbs_config::CachedUserInfo;
-use pbs_datastore::backup_info::{BackupDir, BackupGroup, BackupInfo};
+use pbs_datastore::backup_info::{BackupDir, BackupInfo};
use pbs_datastore::{DataStore, StoreProgress};
use proxmox_rest_server::WorkerTask;
@@ -411,31 +411,24 @@ fn backup_worker(
group_list.sort_unstable_by(|a, b| a.group().cmp(b.group()));
- let (group_list, group_count) = if let Some(group_filters) = &setup.group_filter {
- let filter_fn = |group: &BackupGroup, group_filters: &[GroupFilter]| {
- group_filters.iter().any(|filter| group.matches(filter))
- };
+ let group_count_full = group_list.len();
- let group_count_full = group_list.len();
- let list: Vec<BackupGroup> = group_list
+ let group_list = match &setup.group_filter {
+ Some(f) => group_list
.into_iter()
- .filter(|group| filter_fn(group, group_filters))
- .collect();
- let group_count = list.len();
- task_log!(
- worker,
- "found {} groups (out of {} total)",
- group_count,
- group_count_full
- );
- (list, group_count)
- } else {
- let group_count = group_list.len();
- task_log!(worker, "found {} groups", group_count);
- (group_list, group_count)
+ .filter(|group| group.group().apply_filters(f))
+ .collect(),
+ None => group_list,
};
- let mut progress = StoreProgress::new(group_count as u64);
+ task_log!(
+ worker,
+ "found {} groups (out of {} total)",
+ group_list.len(),
+ group_count_full
+ );
+
+ let mut progress = StoreProgress::new(group_list.len() as u64);
let latest_only = setup.latest_only.unwrap_or(false);
diff --git a/src/server/pull.rs b/src/server/pull.rs
index 3b71c156..b60f3a1e 100644
--- a/src/server/pull.rs
+++ b/src/server/pull.rs
@@ -486,7 +486,7 @@ pub(crate) struct PullParameters {
/// How many levels of sub-namespaces to pull (0 == no recursion, None == maximum recursion)
max_depth: Option<usize>,
/// Filters for reducing the pull scope
- group_filter: Option<Vec<GroupFilter>>,
+ group_filter: Vec<GroupFilter>,
/// How many snapshots should be transferred at most (taking the newest N snapshots)
transfer_last: Option<usize>,
}
@@ -539,6 +539,11 @@ impl PullParameters {
ns,
};
+ let group_filter = match group_filter {
+ Some(f) => f,
+ None => Vec::<GroupFilter>::new(),
+ };
+
Ok(Self {
source,
target,
@@ -1358,7 +1363,6 @@ pub(crate) async fn pull_ns(
) -> Result<(StoreProgress, bool), Error> {
let mut list: Vec<BackupGroup> = params.source.list_groups(namespace, ¶ms.owner).await?;
- let total_count = list.len();
list.sort_unstable_by(|a, b| {
let type_order = a.ty.cmp(&b.ty);
if type_order == std::cmp::Ordering::Equal {
@@ -1368,27 +1372,17 @@ pub(crate) async fn pull_ns(
}
});
- let apply_filters = |group: &BackupGroup, filters: &[GroupFilter]| -> bool {
- filters.iter().any(|filter| group.matches(filter))
- };
-
- let list = if let Some(ref group_filter) = ¶ms.group_filter {
- let unfiltered_count = list.len();
- let list: Vec<BackupGroup> = list
- .into_iter()
- .filter(|group| apply_filters(group, group_filter))
- .collect();
- task_log!(
- worker,
- "found {} groups to sync (out of {} total)",
- list.len(),
- unfiltered_count
- );
- list
- } else {
- task_log!(worker, "found {} groups to sync", total_count);
- list
- };
+ let unfiltered_count = list.len();
+ let list: Vec<BackupGroup> = list
+ .into_iter()
+ .filter(|group| group.apply_filters(¶ms.group_filter))
+ .collect();
+ task_log!(
+ worker,
+ "found {} groups to sync (out of {} total)",
+ list.len(),
+ unfiltered_count
+ );
let mut errors = false;
@@ -1457,10 +1451,8 @@ pub(crate) async fn pull_ns(
if check_backup_owner(&owner, ¶ms.owner).is_err() {
continue;
}
- if let Some(ref group_filter) = ¶ms.group_filter {
- if !apply_filters(local_group, group_filter) {
- continue;
- }
+ if !local_group.apply_filters(¶ms.group_filter) {
+ continue;
}
task_log!(worker, "delete vanished group '{local_group}'",);
match params
--
2.39.2
^ permalink raw reply [flat|nested] 8+ messages in thread* [pbs-devel] [PATCH proxmox-backup v7 2/4] ui: Show if Filter includes or excludes
2024-01-02 11:06 [pbs-devel] [PATCH proxmox-backup v7 0/4] fix #4315: datastore: Exclude entries from sync Philipp Hufnagl
2024-01-02 11:06 ` [pbs-devel] [PATCH proxmox-backup v7 1/4] fix #4315: jobs: modify GroupFilter so include/exclude is tracked Philipp Hufnagl
@ 2024-01-02 11:06 ` Philipp Hufnagl
2024-01-02 11:06 ` [pbs-devel] [PATCH proxmox-backup v7 3/4] docs: document new include/exclude paramenter Philipp Hufnagl
` (3 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Philipp Hufnagl @ 2024-01-02 11:06 UTC (permalink / raw)
To: pbs-devel
To make the UI compatible, the Group Filter dialogue has been extended
by a second list, so it now features a list for all include filter and
one for all exclude filters.
Internally, all include as well as exclude filter are managed into one
list. The 2 list view is just for a cleaner representation in the UI.
Signed-off-by: Philipp Hufnagl <p.hufnagl@proxmox.com>
---
www/form/GroupFilter.js | 233 +++++++++++++++++++++++++++++-----------
1 file changed, 169 insertions(+), 64 deletions(-)
diff --git a/www/form/GroupFilter.js b/www/form/GroupFilter.js
index 708811c0..c9c2d913 100644
--- a/www/form/GroupFilter.js
+++ b/www/form/GroupFilter.js
@@ -35,13 +35,36 @@ Ext.define('PBS.form.GroupFilter', {
// break cyclic reference
me.removeReferences(record);
- me.lookup('grid').getStore().remove(record);
+ me.lookup('grid-include').getStore().remove(record);
+ me.lookup('grid-exclude').getStore().remove(record);
me.updateRealField();
},
- addFilter: function() {
+ addIncludeFilter: function() {
let me = this;
- me.lookup('grid').getStore().add({});
+ me.lookup('grid-include').getStore().add({ behavior: 'include' });
+ me.updateRealField();
+ },
+
+ addExcludeFilter: function() {
+ let me = this;
+ me.lookup('grid-exclude').getStore().add({ behavior: 'exclude' });
+ me.updateRealField();
+ },
+
+
+ onBehaviorChange: function(field, value) {
+ let me = this;
+ let record = field.getWidgetRecord();
+ if (record === undefined) {
+ return;
+ }
+
+ record.set('behavior', value);
+ record.commit();
+ if (record.widgets) {
+ me.setInputValue(record.widgets, record);
+ }
me.updateRealField();
},
@@ -77,8 +100,12 @@ Ext.define('PBS.form.GroupFilter', {
},
parseGroupFilter: function(filter) {
- let [, type, input] = filter.match(/^(type|group|regex):(.*)$/);
+ let [, behavior, type, input] = filter.match(/^(?:(exclude|include):)?(type|group|regex):(.*)$/);
+ if (behavior === undefined) {
+ behavior = "include";
+ }
return {
+ behavior,
type,
input,
};
@@ -86,13 +113,16 @@ Ext.define('PBS.form.GroupFilter', {
onValueChange: function(field, values) {
let me = this;
- let grid = me.lookup('grid');
+ let grid_include = me.lookup('grid-include');
+ let grid_exclude = me.lookup('grid-exclude');
if (!values || values.length === 0) {
- grid.getStore().removeAll();
+ grid_include.getStore().removeAll();
+ grid_exclude.getStore().removeAll();
return;
}
let records = values.map((filter) => me.parseGroupFilter(filter));
- grid.getStore().setData(records);
+ grid_include.getStore().setData(records);
+ grid_exclude.getStore().setData(records);
},
setInputValue: function(widgets, rec) {
@@ -162,9 +192,18 @@ Ext.define('PBS.form.GroupFilter', {
let me = this;
let filter = [];
- me.lookup('grid').getStore().each((rec) => {
- if (rec.data.type && rec.data.input) {
- filter.push(`${rec.data.type}:${rec.data.input}`);
+ me.lookup('grid-include').getStore().each((rec) => {
+ if (rec.data.type && rec.data.input) {
+ filter.push(`${rec.data.type}:${rec.data.input}`);
+ }
+ });
+ me.lookup('grid-exclude').getStore().each((rec) => {
+ if (rec.data.type && rec.data.input && rec.data.behavior) {
+ let behavior_string = '';
+ if (rec.data.behavior === 'exclude') {
+ behavior_string = 'exclude:';
+ }
+ filter.push(`${behavior_string}${rec.data.type}:${rec.data.input}`);
}
});
@@ -175,6 +214,9 @@ Ext.define('PBS.form.GroupFilter', {
},
control: {
+ 'grid pbsGroupBehaviorSelector': {
+ change: 'onBehaviorChange',
+ },
'grid pbsGroupFilterTypeSelector': {
change: 'onTypeChange',
},
@@ -264,72 +306,59 @@ Ext.define('PBS.form.GroupFilter', {
items: [
{
- xtype: 'grid',
- reference: 'grid',
+ xtype: 'pbsGroupFilterGrid',
+ title: 'Include filters',
margin: '0 0 5 0',
- scrollable: true,
- height: 300,
+ reference: 'grid-include',
store: {
- fields: ['type', 'input'],
- },
+ filters: [
+ function(item) {
+ return item.data.behavior === "include";
+ },
+ ],
+ },
emptyText: gettext('Include all groups'),
viewConfig: {
deferEmptyText: false,
},
- columns: [
+ },
+ {
+ xtype: 'container',
+ layout: {
+ type: 'hbox',
+ },
+ items: [
{
- text: gettext('Filter Type'),
- xtype: 'widgetcolumn',
- dataIndex: 'type',
- flex: 1,
- widget: {
- xtype: 'pbsGroupFilterTypeSelector',
- isFormField: false,
- },
+ xtype: 'button',
+ text: gettext('Add include'),
+ iconCls: 'fa fa-plus-circle',
+ handler: 'addIncludeFilter',
},
{
- text: gettext('Filter Value'),
- xtype: 'widgetcolumn',
+ xtype: 'box',
flex: 1,
- onWidgetAttach: 'newInputColumn',
- widget: {
- padding: 0,
- bodyPadding: 0,
- xtype: 'fieldcontainer',
- layout: 'fit',
- defaults: {
- margin: 0,
- },
- items: [
- {
- hidden: true,
- xtype: 'pbsGroupTypeSelector',
- isFormField: false,
- },
- {
- hidden: true,
- xtype: 'textfield',
- type: 'regex',
- isFormField: false,
- },
- {
- hidden: true,
- xtype: 'pbsGroupSelector',
- isFormField: false,
- },
- ],
- },
},
{
- xtype: 'widgetcolumn',
- width: 40,
- widget: {
- xtype: 'button',
- iconCls: 'fa fa-trash-o',
- },
+ xtype: 'box',
+ style: 'margin: 3px 0px;',
+ html: `<span class="pmx-hint">${gettext('Note')}</span>: `
+ + gettext('Filters are additive'),
},
],
},
+ {
+ xtype: 'pbsGroupFilterGrid',
+ title: 'Exclude filters',
+ margin: '10 0 5 0',
+ reference: 'grid-exclude',
+ store: {
+ filters: [
+ function(item) {
+ return item.data.behavior === "exclude";
+ },
+ ],
+ },
+ },
{
xtype: 'hiddenfield',
reference: 'realfield',
@@ -356,9 +385,9 @@ Ext.define('PBS.form.GroupFilter', {
items: [
{
xtype: 'button',
- text: gettext('Add'),
+ text: gettext('Add exclude'),
iconCls: 'fa fa-plus-circle',
- handler: 'addFilter',
+ handler: 'addExcludeFilter',
},
{
xtype: 'box',
@@ -368,7 +397,7 @@ Ext.define('PBS.form.GroupFilter', {
xtype: 'box',
style: 'margin: 3px 0px;',
html: `<span class="pmx-hint">${gettext('Note')}</span>: `
- + gettext('Filters are additive (OR-like)'),
+ + gettext('Exclude filters will be applied after include filters'),
},
],
},
@@ -384,6 +413,82 @@ Ext.define('PBS.form.GroupFilter', {
},
});
+Ext.define('PBS.form.pbsGroupBehaviorSelector', {
+ extend: 'Proxmox.form.KVComboBox',
+ alias: 'widget.pbsGroupBehaviorSelector',
+
+ allowBlank: false,
+
+ comboItems: [
+ ['include', gettext('Include')],
+ ['exclude', gettext('Exclude')],
+ ],
+});
+Ext.define('PBS.form.GroupFilterGrid', {
+ extend: 'Ext.grid.Panel',
+ alias: 'widget.pbsGroupFilterGrid',
+
+ scrollable: true,
+ height: 200,
+ store: {
+ fields: ['type', 'input'],
+ },
+ columns: [
+ {
+ text: gettext('Filter Type'),
+ xtype: 'widgetcolumn',
+ dataIndex: 'type',
+ flex: 1,
+ widget: {
+ xtype: 'pbsGroupFilterTypeSelector',
+ isFormField: false,
+ },
+ },
+ {
+ text: gettext('Filter Value'),
+ xtype: 'widgetcolumn',
+ flex: 1,
+ onWidgetAttach: 'newInputColumn',
+ widget: {
+ padding: 0,
+ bodyPadding: 0,
+ xtype: 'fieldcontainer',
+ layout: 'fit',
+ defaults:
+ {
+ margin: 0,
+ },
+ items: [
+ {
+ hidden: true,
+ xtype: 'pbsGroupTypeSelector',
+ isFormField: false,
+ },
+ {
+ hidden: true,
+ xtype: 'textfield',
+ type: 'regex',
+ isFormField: false,
+ },
+ {
+ hidden: true,
+ xtype: 'pbsGroupSelector',
+ isFormField: false,
+ },
+ ],
+ },
+ },
+ {
+ xtype: 'widgetcolumn',
+ width: 40,
+ widget: {
+ xtype: 'button',
+ iconCls: 'fa fa-trash-o',
+ },
+ },
+ ],
+});
+
Ext.define('PBS.form.GroupFilterTypeSelector', {
extend: 'Proxmox.form.KVComboBox',
alias: 'widget.pbsGroupFilterTypeSelector',
--
2.39.2
^ permalink raw reply [flat|nested] 8+ messages in thread* [pbs-devel] [PATCH proxmox-backup v7 3/4] docs: document new include/exclude paramenter
2024-01-02 11:06 [pbs-devel] [PATCH proxmox-backup v7 0/4] fix #4315: datastore: Exclude entries from sync Philipp Hufnagl
2024-01-02 11:06 ` [pbs-devel] [PATCH proxmox-backup v7 1/4] fix #4315: jobs: modify GroupFilter so include/exclude is tracked Philipp Hufnagl
2024-01-02 11:06 ` [pbs-devel] [PATCH proxmox-backup v7 2/4] ui: Show if Filter includes or excludes Philipp Hufnagl
@ 2024-01-02 11:06 ` Philipp Hufnagl
2024-01-22 15:13 ` Thomas Lamprecht
2024-01-02 11:06 ` [pbs-devel] [PATCH proxmox-backup v7 4/4] tests: check if include/exclude behavior works correctly Philipp Hufnagl
` (2 subsequent siblings)
5 siblings, 1 reply; 8+ messages in thread
From: Philipp Hufnagl @ 2024-01-02 11:06 UTC (permalink / raw)
To: pbs-devel
Adding the newly introduced optional include/exclude parameter to the
PBS documentation.
Signed-off-by: Philipp Hufnagl <p.hufnagl@proxmox.com>
---
docs/managing-remotes.rst | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/docs/managing-remotes.rst b/docs/managing-remotes.rst
index f8dcff6d..10ca1084 100644
--- a/docs/managing-remotes.rst
+++ b/docs/managing-remotes.rst
@@ -116,6 +116,19 @@ of the specified criteria are synced. The available criteria are:
The same filter is applied to local groups, for handling of the
``remove-vanished`` option.
+A ``group-filter`` can be inverted by adding ``exclude:`` to its beginning.
+
+* Regular expression example, excluding the match:
+ .. code-block:: console
+
+ # proxmox-backup-manager sync-job update ID --group-filter exclude:regex:'^vm/1\d{2,3}$'
+
+For mixing include and exclude filter, following rules apply:
+
+ - no filters: all backup groups
+ - include: only those matching the include filters
+ - exclude: all but those matching the exclude filters
+ - both: those matching the include filters, but without those matching the exclude filters
.. note:: The ``protected`` flag of remote backup snapshots will not be synced.
Namespace Support
--
2.39.2
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v7 3/4] docs: document new include/exclude paramenter
2024-01-02 11:06 ` [pbs-devel] [PATCH proxmox-backup v7 3/4] docs: document new include/exclude paramenter Philipp Hufnagl
@ 2024-01-22 15:13 ` Thomas Lamprecht
0 siblings, 0 replies; 8+ messages in thread
From: Thomas Lamprecht @ 2024-01-22 15:13 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Philipp Hufnagl
Am 02/01/2024 um 12:06 schrieb Philipp Hufnagl:
> Adding the newly introduced optional include/exclude parameter to the
> PBS documentation.
>
> Signed-off-by: Philipp Hufnagl <p.hufnagl@proxmox.com>
> ---
> docs/managing-remotes.rst | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/docs/managing-remotes.rst b/docs/managing-remotes.rst
> index f8dcff6d..10ca1084 100644
> --- a/docs/managing-remotes.rst
> +++ b/docs/managing-remotes.rst
> @@ -116,6 +116,19 @@ of the specified criteria are synced. The available criteria are:
> The same filter is applied to local groups, for handling of the
> ``remove-vanished`` option.
>
> +A ``group-filter`` can be inverted by adding ``exclude:`` to its beginning.
> +
> +* Regular expression example, excluding the match:
> + .. code-block:: console
> +
> + # proxmox-backup-manager sync-job update ID --group-filter exclude:regex:'^vm/1\d{2,3}$'
> +
> +For mixing include and exclude filter, following rules apply:
> +
> + - no filters: all backup groups
> + - include: only those matching the include filters
> + - exclude: all but those matching the exclude filters
> + - both: those matching the include filters, but without those matching the exclude filters
FYI: missing a newline here, that broke the admonition node below as sphinx/rst
interprets this as continuation of the list.
Fixed already in git with a follow up, just noting to avoid repetition of
this tiny mistake, restructured text is (sadly) like python really whitespace
sensitive..
> .. note:: The ``protected`` flag of remote backup snapshots will not be synced.
>
> Namespace Support
^ permalink raw reply [flat|nested] 8+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v7 4/4] tests: check if include/exclude behavior works correctly
2024-01-02 11:06 [pbs-devel] [PATCH proxmox-backup v7 0/4] fix #4315: datastore: Exclude entries from sync Philipp Hufnagl
` (2 preceding siblings ...)
2024-01-02 11:06 ` [pbs-devel] [PATCH proxmox-backup v7 3/4] docs: document new include/exclude paramenter Philipp Hufnagl
@ 2024-01-02 11:06 ` Philipp Hufnagl
2024-01-02 13:33 ` [pbs-devel] [PATCH proxmox-backup v7 0/4] fix #4315: datastore: Exclude entries from sync Lukas Wagner
2024-01-10 9:14 ` [pbs-devel] applied-series: " Wolfgang Bumiller
5 siblings, 0 replies; 8+ messages in thread
From: Philipp Hufnagl @ 2024-01-02 11:06 UTC (permalink / raw)
To: pbs-devel
This checks if including and excluding works as expected. That the
filter are added out of order is on purpose since it sould make no
difference.
Signed-off-by: Philipp Hufnagl <p.hufnagl@proxmox.com>
---
tests/sync_jobs.rs | 76 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 76 insertions(+)
create mode 100644 tests/sync_jobs.rs
diff --git a/tests/sync_jobs.rs b/tests/sync_jobs.rs
new file mode 100644
index 00000000..45b46f14
--- /dev/null
+++ b/tests/sync_jobs.rs
@@ -0,0 +1,76 @@
+use pbs_api_types::{BackupGroup, BackupType, GroupFilter};
+use std::str::FromStr;
+
+#[test]
+fn test_no_filters() {
+ let group_filters = vec![];
+
+ let do_backup = vec![
+ "vm/101", "vm/102", "vm/103", "vm/104", "vm/105", "vm/106", "vm/107", "vm/108", "vm/109",
+ ];
+
+ for id in do_backup {
+ assert!(BackupGroup::new(BackupType::Vm, id).apply_filters(&group_filters));
+ }
+}
+
+#[test]
+fn test_include_filters() {
+ let group_filters = vec![GroupFilter::from_str("regex:.*10[2-8]").unwrap()];
+
+ let do_backup = vec![
+ "vm/102", "vm/103", "vm/104", "vm/105", "vm/106", "vm/107", "vm/108",
+ ];
+
+ let dont_backup = vec!["vm/101", "vm/109"];
+
+ for id in do_backup {
+ assert!(BackupGroup::new(BackupType::Vm, id).apply_filters(&group_filters));
+ }
+
+ for id in dont_backup {
+ assert!(!BackupGroup::new(BackupType::Vm, id).apply_filters(&group_filters));
+ }
+}
+
+#[test]
+fn test_exclude_filters() {
+ let group_filters = vec![
+ GroupFilter::from_str("exclude:regex:.*10[1-3]").unwrap(),
+ GroupFilter::from_str("exclude:regex:.*10[5-7]").unwrap(),
+ ];
+
+ let do_backup = vec!["vm/104", "vm/108", "vm/109"];
+
+ let dont_backup = vec!["vm/101", "vm/102", "vm/103", "vm/105", "vm/106", "vm/107"];
+
+ for id in do_backup {
+ assert!(BackupGroup::new(BackupType::Vm, id).apply_filters(&group_filters));
+ }
+ for id in dont_backup {
+ assert!(!BackupGroup::new(BackupType::Vm, id).apply_filters(&group_filters));
+ }
+}
+
+#[test]
+fn test_include_and_exclude_filters() {
+ let group_filters = vec![
+ GroupFilter::from_str("exclude:regex:.*10[1-3]").unwrap(),
+ GroupFilter::from_str("regex:.*10[2-8]").unwrap(),
+ GroupFilter::from_str("exclude:regex:.*10[5-7]").unwrap(),
+ ];
+
+ let do_backup = vec!["vm/104", "vm/108"];
+
+ let dont_backup = vec![
+ "vm/101", "vm/102", "vm/103", "vm/105", "vm/106", "vm/107", "vm/109",
+ ];
+
+ for id in do_backup {
+ assert!(BackupGroup::new(BackupType::Vm, id).apply_filters(&group_filters));
+ }
+
+ for id in dont_backup {
+ assert!(!BackupGroup::new(BackupType::Vm, id).apply_filters(&group_filters));
+ }
+}
--
2.39.2
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v7 0/4] fix #4315: datastore: Exclude entries from sync
2024-01-02 11:06 [pbs-devel] [PATCH proxmox-backup v7 0/4] fix #4315: datastore: Exclude entries from sync Philipp Hufnagl
` (3 preceding siblings ...)
2024-01-02 11:06 ` [pbs-devel] [PATCH proxmox-backup v7 4/4] tests: check if include/exclude behavior works correctly Philipp Hufnagl
@ 2024-01-02 13:33 ` Lukas Wagner
2024-01-10 9:14 ` [pbs-devel] applied-series: " Wolfgang Bumiller
5 siblings, 0 replies; 8+ messages in thread
From: Lukas Wagner @ 2024-01-02 13:33 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Philipp Hufnagl
On 1/2/24 12:06, Philipp Hufnagl wrote:
> This allows to use Group Filter for sync jobs so matches can not just be
> included but also excluded. For this the "group-filter" configuration
> syntax has been extended with an optional "behaviour" parameter.
> this can be "include" or "exclude". First, all include filter will be
> applied, then all exclude filter. If no include filters exist, all will
> be considered included.
>
Looks good to me! In my opinion this should be ready to be merged now.
Tested-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
--
- Lukas
^ permalink raw reply [flat|nested] 8+ messages in thread* [pbs-devel] applied-series: [PATCH proxmox-backup v7 0/4] fix #4315: datastore: Exclude entries from sync
2024-01-02 11:06 [pbs-devel] [PATCH proxmox-backup v7 0/4] fix #4315: datastore: Exclude entries from sync Philipp Hufnagl
` (4 preceding siblings ...)
2024-01-02 13:33 ` [pbs-devel] [PATCH proxmox-backup v7 0/4] fix #4315: datastore: Exclude entries from sync Lukas Wagner
@ 2024-01-10 9:14 ` Wolfgang Bumiller
5 siblings, 0 replies; 8+ messages in thread
From: Wolfgang Bumiller @ 2024-01-10 9:14 UTC (permalink / raw)
To: Philipp Hufnagl; +Cc: pbs-devel
applied series, thanks
^ permalink raw reply [flat|nested] 8+ messages in thread